Skip to content

Improve VerifyBlockError::Commit typing, so we don't accidentally break syncer error handling #2908

Open

Description

Motivation

VerifyBlockError::Commit is a wrapper over a BoxError. However, in #2890 a check was added in the should_restart_sync function to ignore Commit errors corresponding to "block is already committed to the state" when deciding whether to reset the sync procedure. That check is fragile since it will break if the string changes.

Change Commit to wrap a specific error type enumeration, and create a specific item for that particular error to be used for matching and filtering it in should_restart_sync.

Currently, this check is implemented by the is_duplicate_request() method, which can't use Commit because it is a BoxError.

There's also a BlockDownloadVerifyError::Invalid error that comes from the chain verifier and a BlockDownloadVerifyError::DownloadFailed from the network service.

We should downcast all BoxErrors in the syncer's BlockDownloadVerifyError to concrete types and add them to the match statement in should_restart_sync():

fn should_restart_sync(e: &BlockDownloadVerifyError) -> bool {

// TODO: add a proper test and remove this
// https://github.com/ZcashFoundation/zebra/issues/2909
let err_str = format!("{e:?}");
if err_str.contains("AlreadyVerified")
|| err_str.contains("AlreadyInState")
|| err_str.contains("block is already committed to the state")
|| err_str.contains("block has already been sent to be committed to the state")
|| err_str.contains("NotFound")
{

Specifications

Designs

Related Work

Follow up to #2890

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    A-networkArea: Network protocol updates or fixesC-cleanupCategory: This is a cleanupC-enhancementCategory: This is an improvementC-tech-debtCategory: Code maintainability issues

    Type

    No type

    Projects

    • Status

      New

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions