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

ZIP-200: Test that mempool storage is cleared when a network upgrade activates #2374

Closed
3 tasks done
Tracked by #2309
teor2345 opened this issue Jun 23, 2021 · 2 comments · Fixed by #2897
Closed
3 tasks done
Tracked by #2309

ZIP-200: Test that mempool storage is cleared when a network upgrade activates #2374

teor2345 opened this issue Jun 23, 2021 · 2 comments · Fixed by #2897
Assignees
Labels
A-docs Area: Documentation A-network Area: Network protocol updates or fixes A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement NU-0 Overwinter Network Upgrade: Overwinter specific tasks (Sprout after Overwinter)

Comments

@teor2345
Copy link
Contributor

teor2345 commented Jun 23, 2021

Motivation

When a network upgrade activates, the mempool needs to redo validation with the new consensus rules.

Specifications

While the current chain tip height is below ACTIVATION_HEIGHT, nodes SHOULD NOT accept transactions that will only be valid on the post-upgrade consensus branch.

When the current chain tip height reaches ACTIVATION_HEIGHT, the node's local transaction memory pool SHOULD be cleared of transactions that will never be valid on the post-upgrade consensus branch.

https://zips.z.cash/zip-0200#memory-pool

It is possible for a reorganization to occur that rolls back from after the activation height, to before that height. This can handled in the same way as any regular chain orphaning or reorganization, as long as the new chain is valid.

https://zips.z.cash/zip-0200#chain-reorganization

Designs

Tasks can await network upgrade activation using ChainTipChange::tip_change and TipAction::Reset:

pub async fn tip_change(&mut self) -> Result<TipAction, watch::error::RecvError> {

Solution

Make sure the implementation satisfies the consensus rules:

@teor2345 teor2345 added A-docs Area: Documentation A-rust Area: Updates to Rust code NU-0 Overwinter Network Upgrade: Overwinter specific tasks (Sprout after Overwinter) C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage P-Medium A-network Area: Network protocol updates or fixes labels Jun 23, 2021
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Jun 25, 2021
@mpguerra mpguerra mentioned this issue Aug 11, 2021
59 tasks
@teor2345 teor2345 changed the title ZIP-200: Network Upgrade Mechanism: Memory pool ZIP-200: Make network upgrade activation clear mempool storage Aug 17, 2021
@teor2345 teor2345 changed the title ZIP-200: Make network upgrade activation clear mempool storage ZIP-200: Clear mempool storage when a network upgrade activates Aug 17, 2021
@mpguerra mpguerra added this to the 2021 Sprint 18 milestone Aug 23, 2021
@upbqdn upbqdn self-assigned this Sep 2, 2021
@teor2345
Copy link
Contributor Author

teor2345 commented Sep 30, 2021

* Test rollback and multiple activations of the same network upgrade

@upbqdn we're running the same reset code for network upgrade activation and rollbacks.
Do our existing tests cover multiple resets?

If we don't need any extra tests, we can just close this ticket.

@teor2345 teor2345 changed the title ZIP-200: Clear mempool storage when a network upgrade activates ZIP-200: Test that mempool storage is cleared when a network upgrade activates Oct 5, 2021
@teor2345
Copy link
Contributor Author

teor2345 commented Oct 5, 2021

I renamed this ticket to match the remaining work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation A-network Area: Network protocol updates or fixes A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement NU-0 Overwinter Network Upgrade: Overwinter specific tasks (Sprout after Overwinter)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants