-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
feat: [ADR-070] Unordered Transactions (2/2) #18739
feat: [ADR-070] Unordered Transactions (2/2) #18739
Conversation
@alexanderbez your pull request is missing a changelog! |
Marking this ready to review to show the basic idea. Granted, there are docs that need to be written and this needs to be tested more thoroughly. In general, the design is so so to me personally. The main disadvantage here is that an external manager needs to be managed and we also write and read from disk. If a node for whatever reason fails to read/write to disk, then it's technically not functional and the node has to resync. This should be less of a concern with store v2, but still somewhat brittle IMO. The alternative to something like this is to manage the state in-protocol, i.e. a completely different design like a 2D nonce lane, e.g. https://github.com/ethereum/RIPs/blob/e8af8009251f24e67311e0d955e7a951284717fc/RIPS/rip-7560.md#non-sequential-nonce-support |
mu sync.RWMutex | ||
// txHashes defines a map from tx hash -> TTL value, which is used for duplicate | ||
// checking and replay protection, as well as purging the map when the TTL is | ||
// expired. | ||
txHashes map[TxHash]uint64 | ||
} | ||
|
||
func NewManager() *Manager { | ||
func NewManager(dataDir string) *Manager { | ||
path := filepath.Join(dataDir, dirName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whats the difference with putting the file creation from app.go here vs in app.go?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea, like WASMVM, is to have the "component" be responsible or "own" the directory/location of where data is written. Also, less work for the app to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does that mean this https://github.com/cosmos/cosmos-sdk/pull/18739/files#r1430642896 would get moved into here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, which file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some minor suggestions.
Addressed comments @yihuang :) |
453e270
into
bez/feature/unordered-txs
Description
#18641 addresses the core implementation of unordered txs. However, a crucial component to safely support unordered txs is the ability to ensure the map that stores them is durable and consistent[1] across all nodes.
Specifically, when a node restarts, the unordered tx manager (map), needs to be populated with all NON-STALE unordered txs that have been committed so far, i.e. unordered transactions whose TTL value is yet to expire.
We propose to do this by simply writing to a file. However, we also need to address the case of where a new node joins a network via state sync. As it will naturally not have this file, we will also implement a snapshotter extension that will allow the manager (map) to be populated.
Ref:
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
in the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...