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

Make sure the mandatory checkpoint includes the Canopy activation block #2131

Closed
3 of 4 tasks
mpguerra opened this issue May 11, 2021 · 1 comment · Fixed by #2235
Closed
3 of 4 tasks

Make sure the mandatory checkpoint includes the Canopy activation block #2131

mpguerra opened this issue May 11, 2021 · 1 comment · Fixed by #2235
Assignees
Labels
C-enhancement Category: This is an improvement NU-5 Network Upgrade: NU5 specific tasks

Comments

@mpguerra
Copy link
Contributor

mpguerra commented May 11, 2021

Is your feature request related to a problem? Please describe.

In general, the chain history for each block commits to all the blocks between:

  • the previous network upgrade activation + 1 and
  • the previous block

So the Canopy activation block commits to all the blocks between Heartwood activation + 1 and Canopy activation - 1.

If we change the mandatory checkpoint to Canopy activation + 1, we will be able to skip calculating all the heartwood chain history.

The ChainVerifier checkpoints all blocks up to and including the Canopy activation maximum height. Zebra chooses the next checkpoint at or after the mandatory height.

So the actual checkpoint does not need to change on either network:

Network Canopy Zebra Checkpoint
Mainnet 1028500 1028849
Testnet 1046400 1046400

But we do need to update the finalized state assertion, the documentation, and the checkpoint tests.

Describe the solution you'd like

  • assert that the canopy activation block is a finalized checkpoint block
  • clarify the checkpoint test docs - they're already checking that canopy activation is a checkpoint block
  • update Zebra's checkpoint docs to explicitly include Canopy activation
  • if the mandatory checkpoint appears in multiple places, turn it into a named constant (optional)

The hard-coded checkpoint lists extend a long way past Canopy, so they don't need to be updated.

Additional context

See #2091 for more details

@mpguerra mpguerra added C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage labels May 11, 2021
@mpguerra mpguerra added NU-5 Network Upgrade: NU5 specific tasks P-Medium and removed S-needs-triage Status: A bug report needs triage labels May 11, 2021
@mpguerra mpguerra added this to the 2021 Sprint 10 milestone May 13, 2021
@teor2345
Copy link
Contributor

teor2345 commented Jun 2, 2021

Zebra chooses the next checkpoint at or after the mandatory Canopy activation height. And the ChainVerifier checkpoints all blocks up to and including the checkpoint.

So we don't need to change the actual checkpoint - the first non-finalized block is Canopy activation + 1 block.

We should fix the state assertion, the test, the checkpoint documentation though.

@teor2345 teor2345 changed the title Change the mandatory checkpoint to Canopy activation +1 Make sure the mandatory checkpoint includes the Canopy activation block Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: This is an improvement NU-5 Network Upgrade: NU5 specific tasks
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants