xrpl.js icon indicating copy to clipboard operation
xrpl.js copied to clipboard

suggestions for Client.submitAndWait

Open rikublock opened this issue 1 year ago • 3 comments

Problem: If a transaction is unsuccessful or is rejected (accepted: false, with tec code), the behavior of the submitAndWait function is quite restricted. In certain cases, we would want to abort early and not wait for the sequence number timeout (can take quite some time). Furthermore, the function also doesn't provide a lot of details about the failure reason.

Possible solutions:

  • add an opts to fail immediately, if transaction is rejected
  • return both a SubmitResponse (like Client.submit) and Promise<TxResponse<T>>, depending on the result one can choose not to resolve the promise
  • expose/export the waitForFinalTransactionOutcome function to allow developers to easily implement their own version of submitAndWait

rikublock avatar Sep 07 '23 14:09 rikublock

It may also be worth considering that the default lastLedgerSequence when autofilling could be too generous. We don't want it to fail accidentally, but as it is now it takes a long time for finality

JST5000 avatar Sep 07 '23 21:09 JST5000

add an opts to fail immediately, if transaction is rejected

Why not just use submit in that case?

return both a SubmitResponse (like Client.submit) and Promise<TxResponse<T>>, depending on the result one can choose not to resolve the promise

This seems quite complicated to use if you just want the happy path or if you're more of a beginner dev (it took me a couple of minutes to understand how you could use it in the manner described).

expose/export the waitForFinalTransactionOutcome function to allow developers to easily implement their own version of submitAndWait

IMO this is probably the best solution - so that if you want the custom functionality in your other proposed solutions, you can just write it yourself.

It may also be worth considering that the default lastLedgerSequence when autofilling could be too generous. We don't want it to fail accidentally, but as it is now it takes a long time for finality

Do we have any opt for the LastLedgerSequence? This could just be configurable instead of reducing the default.

mvadari avatar Sep 08 '23 10:09 mvadari

add an opts to fail immediately, if transaction is rejected

Why not just use submit in that case?

I would still want the ability to easily check/wait for finalization. submit alone won't do that.

expose/export the waitForFinalTransactionOutcome function to allow developers to easily implement their own version of submitAndWait

IMO this is probably the best solution - so that if you want the custom functionality in your other proposed solutions, you can just write it yourself.

Agreed. Here a suggestion of how it could look like.

Slightly changing the interface of waitForFinalTransactionOutcome could make it a very easy to use function. All the required information is already contained in a SubmitResponse - just pass that instead:

  • SubmitResponse.result.engine_result
  • SubmitResponse.result.tx_json.hash
  • SubmitResponse.result.tx_json.LastLedgerSequence

For easy access, possibly make waitForFinalTransactionOutcome a member function of Client.

Old:

async function waitForFinalTransactionOutcome<
  T extends BaseTransaction = Transaction,
>(
  client: Client,
  txHash: string,
  lastLedger: number,
  submissionResult: string,
): Promise<TxResponse<T>>

New:

async function waitForFinalTransactionOutcome<
  T extends BaseTransaction = Transaction,
>(
  client: Client,
  submitResponse: SubmitResponse<T>,
): Promise<TxResponse<T>>

Usage:

const submitResponse = await client.submit(...);
const txResponse = await client.waitForFinalTransactionOutcome(submitResponse);

rikublock avatar Sep 11 '23 05:09 rikublock