stellar / js-stellar-sdk

Main Stellar client library for the JavaScript language.
https://stellar.github.io/js-stellar-sdk/
Apache License 2.0
615 stars 296 forks source link

set timeout after simulation and before sign #951

Closed BlaineHeffron closed 2 months ago

BlaineHeffron commented 2 months ago

Timeout for a transaction is now set to start after the simulation returns and before it is signed and sent. This makes sure its more in line with the time used in the retry logic. We also increased the default value to 60 from 10 seconds so that people using freighter have time to review and sign the transaction. This gives enough time to avoid the txTooLate error in https://github.com/stellar/js-stellar-sdk/issues/950

chadoh commented 2 months ago

Currently marked as draft because TransactionBuilder throws an exception when you try to simulate the transaction without time bounds set:

  Error {
    message: 'TimeBounds has to be set or you must call setTimeout(TimeoutInfinite).',
  }

  › TransactionBuilder.build (node_modules/@stellar/stellar-base/lib/transaction_builder.js:544:15)

Also you can't set the timeout after the max time is already set so cloning the transaction and then calling setTimeout doesn't work:

message: 'TimeBounds.max_time has been already set - setting timeout would overwrite it.',

You get the same error if you use setTimebounds.

I've tried working around it by doing this:

const timeoutInSeconds =
  this.assembled.options.timeoutInSeconds ?? DEFAULT_TIMEOUT;
const timeoutTimestamp =
  Math.floor(Date.now() / 1000) + timeoutInSeconds + 1;
this.assembled.built = TransactionBuilder.cloneFrom(this.assembled.built!, {
  fee: this.assembled.built!.fee,
  timebounds: {
    minTime: 0,
    maxTime: timeoutTimestamp,
  },
}).build();

But this gets me a txMalformed RPC error without much extra info.

Shaptic commented 2 months ago

@chadoh @BlaineHeffron you probably need to inject the SorobanTransactionData from the original transaction as this is not cloned by cloneFrom. I think your code can be fixed with something like:


const timeoutInSeconds = this.assembled.options.timeoutInSeconds ?? DEFAULT_TIMEOUT;
this.assembled.built = TransactionBuilder.cloneFrom(this.assembled.built!)
  .setTimeout(timeoutInSeconds)
  .setSorobanData(new SorobanDataBuilder(this.assembled.built!.[dig and get the data structure out]).build())
  .build();
BlaineHeffron commented 2 months ago

With the following changes all e2e tests are passing.

  1. Add back in the setTimeout call in assembled_transaction.ts
  2. Set the proper time bounds before signing by using cloneFrom to clone the transaction from this.assembled.built while manually passing in the new bounds, and making sure to add the sorobanData which doesn't get copied via cloneFrom.
BlaineHeffron commented 2 months ago

rebased and added CL entry sorry about the obnoxiously long commit history...