Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up transaction timebounds API #160

Open
8 tasks
morleyzhi opened this issue Feb 12, 2019 · 1 comment
Open
8 tasks

Clean up transaction timebounds API #160

morleyzhi opened this issue Feb 12, 2019 · 1 comment

Comments

@morleyzhi
Copy link
Contributor

morleyzhi commented Feb 12, 2019

As of 0.11.0, this is how a user can set timebounds on transactions:

  • You can pass a timebounds property on the opts object passed to TransactionBuilder. This is an object with shape { minTime: UnixTimestamp }. (Note that after building, the timebounds property will also have a maxTime property, but TransactionBuilder won't use the maxTime property you pass to the constructor if you later call setTimeout.)
  • You must call TransactionBuilder.prototype.setTimeout, which takes a positive integer or zero. At this point, it sets the actual timebounds property used in the built transaction.
  • If you provide a positive timeout, It sets the minTime to zero if you didn't instantiate with timebounds, or timebounds.minTime if you did. The maxTime is the user's local time + the timeout.
  • If you provide a zero timeout, it sets the timebounds to null (if not instantiated) or whatever you provided (if instantiated).
  • The resulting transaction object has a timeBounds property, with a capital B.
  • There's no obvious way of setting a specific minTime and maxTime combo, which is required if we're going to allow "presigning.".
  • Because the official timestamp timebounds are set when setTimeout is called, and not when the transaction is signed, transactions can inadvertently time out if you have a complicated way of signing (for example, on StellarX, if you have to enter your password or confirm on your Ledger to complete signing.)

Here's my proposal for making this simpler and more internally consistent.

  • TransactionBuilder can be instantiated with a timeBounds property, and that's how it's spelled internally. Instantiating with timebounds will save to timeBounds.
  • Instantiation timeBounds accepts an object { minTime, maxTime }. Everything is optional.
  • If not instantiated with a value, minTime is set to zero.
  • If not instantiated with a value, maxTime is set to the current time, plus 60 seconds.
  • If timeBounds is not provided, then the transaction won't time out; it'll be valid forever.
  • TransactionBuilder will have functions setMinTimestamp, setMaxTimestamp, setTimeout, and cancelTimeout to manipulate timeBounds.
  • Make it insanely clear that the "timeout clock" starts when the transaction is built, so signing had better happen during the timeout!
  • During build, if minTime >= maxTime, throw.
@ire-and-curses
Copy link
Member

I did something like this in the Go SDK Timebounds, it's similar but not quite the same as your proposal. I actually did it three different ways before settling on this.

In the Go SDK:

  1. Timebounds are required to be added to the TX, before TX build time. If you absolutely can't have time limits, you can use NewInfiniteTimeout to generate unlimited bounds, but this is an explicit choice. We did this to encourage the setting of some kind of bounds (infinite timeout is more likely to "just work" and users might not stop to understand the value of bounds).
  2. Timebounds can be generated from a timeout, or from the server time, or by setting MinTime and MaxTime. If you set a limit explicitly, you have to set both (it's trivial to set minTime to 0 or current time if not required). This covers all the main use cases:
    2.1 Normal transaction (use some short timeout)
    2.2 Multi-sign transaction (pick a longer timeout)
    2.3 Smart contract (set bounds explicitly)
    2.4 Testing / TX status not important (set infinite timeout)

It was a deliberate decision to not set any defaults. It makes it less magic, and since we require the user to always provide a Timebounds object anyway, they can provide some kind of limits without any extra overhead.

The key simplification for the Go SDK was disallowing the setting of just one bound. For that reason, you must use one of the helpers to produce this object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants