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

[Feature]: Non-sequential x/auth sequence numbers #20617

Closed
BrendanChou opened this issue Jun 10, 2024 · 6 comments
Closed

[Feature]: Non-sequential x/auth sequence numbers #20617

BrendanChou opened this issue Jun 10, 2024 · 6 comments

Comments

@BrendanChou
Copy link

BrendanChou commented Jun 10, 2024

Summary

Allow for non-sequential x/auth sequence numbers. That is, allow sequence numbers to skip values.

Problem Definition

The main issue right now is that sending transactions is a stateful endeavor where you have to query the chain for your current sequence number, and this number may not even end up being correct if you have other transactions in-flight. This is annoying for developers to implement in the client, adds a round-trip time to the node for any transaction sending, and requires clients to attempt to remember how many transactions are in-flight.

There have been other issues (#8308, #13621) over the years pointing out the poor UX around this feature.

This would also make writing tests much easier. Other devs have asked things such as "does anyone have nice code snippet of an object to broadcast txs and update account sequence? Use case: send many transactions in tests".

Proposed Feature

Standard practice can be using the current unix time (in milliseconds) as the sequence number which is a stateless way to go about things (does not require querying the chain or keeping track of in-flight messages, assuming it takes at least 1 millisecond between signing new transactions).

The x/auth/Params can have three new fields:

  • bool allow_sequence_skips
    • if true, allow sequence numbers to be skipped
    • default: false
  • uint64 num_stored_sequences
    • Store the highest n sequence numbers instead of the single highest sequence number
    • This allows the transactions to hit the chain/nodes in a different order than the order in which they were sent
    • default: 1
  • uint64 sequence_buffer_milliseconds
    • if non-zero, then any sequence numbers greater than ctx.BlockTime().UnixMilli() + sequence_buffer_ms are considered invalid
    • This prevents users from accidentally “locking” their account by using math.MaxUint64 as the sequence number
    • This prevents dapps from DOSing other dapps by using a too-high sequence number value
    • default: 86,400,000 (1 day in milliseconds)

Additionally, the uint64 sequence field on the BaseAccount would be replaced with a repeated uint64 highest_sequences field that is at-most num_stored_sequences in length.


Another solution is to avoid overloading the current sequence field and instead add repeated uint64 nonces field to BaseAccount. Any tx.sequence values greater-than 2^40 will be assumed to be nonces. This allows dapps to use either traditional sequence numbers or the newer skippable nonce values without interfering with each other.

@johnletey
Copy link
Contributor

@BrendanChou Have you seen ADR-070 on unordered transactions?

@BrendanChou
Copy link
Author

Yes it looks pretty good except for the good-til-block semantics which don't solve the issue of having to query chain state (to get the current block number) before sending a transaction.

I like the simplicity of the solution proposed above, but would be fine to use ADR-070 if it used good-til-time instead of good-til-block.

@tac0turtle
Copy link
Member

we can look into time instead of height. we would like to move into a direction in which auth is deprecated and accounts is the way forward. this would allow you to write your own accounts with custom replay protection.

@webmaster128
Copy link
Member

Strong support for the problem description. I had to write a big funds distribution system with different distibutor accounts in order to implement a faucet just to be able to use it at higher frequency than once per ~ 2 blocks.

I would stay away from using the term "nonce" as long as we don't support arbitrary unique values, which can be decreasing. Those values can come from an RNG and are used in various cryprtographic algorithms. I doubt we want to store all noces an account ever used.

@tac0turtle
Copy link
Member

in the design proposed how is ordering of transactions kept? is this pushed to the client level, broadcast and wait for the inclusion?

we are moving unordered tx to use time instead of blocks this should help fix the issue you conveyed. It will also make it simpler for users.

@tac0turtle
Copy link
Member

completed with unordered txs

@github-project-automation github-project-automation bot moved this from 📋 Backlog to 🥳 Done in Cosmos-SDK Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🥳 Done
Development

No branches or pull requests

4 participants