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

On startup, check that the finalized state blocks match the checkpoint hashes #5912

Closed
teor2345 opened this issue Jan 4, 2023 · 3 comments · Fixed by #6163
Closed

On startup, check that the finalized state blocks match the checkpoint hashes #5912

teor2345 opened this issue Jan 4, 2023 · 3 comments · Fixed by #6163
Assignees
Labels
A-concurrency Area: Async code, needs extra work to make it work properly. A-consensus Area: Consensus rule updates C-security Category: Security issues I-consensus Zebra breaks a Zcash consensus rule

Comments

@teor2345
Copy link
Contributor

teor2345 commented Jan 4, 2023

Motivation

Currently, Zebra doesn't check if its checkpoints match the blocks in its cached state. This means that Zebra can follow a chain fork for a very long time, even after restarts.

This has become more of an issue recently, because Zebra will soon be able to:

  • mine its own blocks, and
  • generate its own checkpoints.

Specifications

zcashd checks for a few well-known block hashes on startup, which are typically network upgrade activations.

Complex Code or Requirements

To avoid startup delays, Zebra can do these checks concurrently with the rest of its startup and sync tasks. This can happen in a similar way to the Zcash parameter downloads. (But it can be async, because the state service is async.)

let groth16_download_handle = spawn_blocking(move || {
span.in_scope(|| {
if !debug_skip_parameter_preload {
// The lazy static initializer does the download, if needed,
// and the file hash checks.
lazy_static::initialize(&crate::groth16::GROTH16_PARAMETERS);
tracing::info!("Groth16 pre-download and check task finished");
}
})
});

Testing

We could manually insert a bad block hash into the database, then run this check and make sure it fails.

The success case will be covered by existing tests.

Related Work

This is part of a larger issue for the Zcash network - we don't have good monitoring for chain forks or other serious network events.

We also want to track the recent chain forks that an individual Zebra node has discovered in our metrics:

@teor2345 teor2345 added A-consensus Area: Consensus rule updates S-needs-triage Status: A bug report needs triage P-Low ❄️ C-security Category: Security issues I-consensus Zebra breaks a Zcash consensus rule A-concurrency Area: Async code, needs extra work to make it work properly. labels Jan 4, 2023
@mpguerra
Copy link
Contributor

tentatively scheduling this for after the GBT work

@teor2345
Copy link
Contributor Author

I had a bit of trouble deciding the priority for this ticket. I think it might be medium priority, because it is an easy fix for a consensus rule check we should be doing.

@mpguerra mpguerra moved this to 🛑 Won't Fix in Zebra Jan 19, 2023
@mpguerra mpguerra added this to Zebra Jan 19, 2023
@mpguerra mpguerra moved this from 🛑 Won't Fix to 🆕 New in Zebra Jan 19, 2023
@mpguerra mpguerra moved this from 🆕 New to 📋 Sprint Backlog in Zebra Jan 19, 2023
@mpguerra
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-concurrency Area: Async code, needs extra work to make it work properly. A-consensus Area: Consensus rule updates C-security Category: Security issues I-consensus Zebra breaks a Zcash consensus rule
Projects
Archived in project
2 participants