search

XRPLF / xrpl.js

A JavaScript/TypeScript API for interacting with the XRP Ledger in Node.js and the browser
https://xrpl.org/
1.2k stars 510 forks source link

Auto-filled sequence should be based on 'current' ledger #999

Closed intelliot closed 3 years ago

intelliot commented 5 years ago

The 'validated' ledger is obsolete if you are submitting more than 1 tx per ledger.

Since it is always necessary to asynchronously confirm whether a tx has actually succeeded, there should be no harm in using the account's sequence based on the 'current' ledger.

intelliot commented 5 years ago

Making this change should be trivial, but we should come up with a way to make it a non-breaking change. Though unlikely to cause harm, we should not change the behavior of existing integrations. An option (on RippleAPI?) should be used to enable the new behavior.

intelliot commented 3 years ago

Here's what happens if you use a stale sequence number:

{ resultCode: 'tefPAST_SEQ',
  resultMessage: 'This sequence number has already passed.',
  engine_result: 'tefPAST_SEQ',
  engine_result_code: -190,
  engine_result_message: 'This sequence number has already passed.',
  tx_blob: '...',
  tx_json:
   { ... } }

You'll experience a race condition that occurs when multiple transactions are prepared at about the same time.

You can fix this with a semaphore or lock, which is good if you don't need to process more than 1 tx per ledger.

Based on https://xrpl.org/basic-data-types.html#specifying-ledgers , the default is 'current' for a normal rippled node. But Reporting Mode servers default to 'validated', which makes it more likely that you'll experience a problem.