-
Notifications
You must be signed in to change notification settings - Fork 983
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
Prune Merkle tree of bridge pool #2206
Conversation
let nonce = Uint::try_from_slice(&bytes) | ||
.map_err(Error::BorshCodingError)?; | ||
if nonce < current_nonce { | ||
break; |
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.
Isn't this returning at the first epoch with an invalid nonce instead of the last one matching the current nonce?
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 nonce was incremented to the current one during the epoch though the epoch has the invalid nonce at the starting height. The Merkle tree at the epoch starting would be required to rebuild one at a height (after the nonce was incremented) during the epoch. So, the Merkle tree in the epoch should have a valid nonce.
There is a corner case; the nonce was incremented at the same height when the epoch was updated. However, the Merkle tree snapshot would be pruned eventually.
storage.write(&bp_nonce_key, bytes).unwrap(); | ||
|
||
let bytes = types::encode(&Uint::default()); | ||
storage.write(&bp_nonce_key, bytes).unwrap(); |
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.
Do we need this second write since we've just written the same key?
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.
thanks, it's not needed
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.
lgtm, thanks!
* origin/yuji/prune-bp-merkle-tree: remove unneeded write add test and changelog prune merkle trees of bridge pool
Describe your changes
closes #2110
Indicate on which release or other PRs this topic is based on
0.27.0
Checklist before merging to
draft