Skip to content

Commit b4c9f27

Browse files
conradoplgCopilotjvff
authored
fix(consensus): classify wrapped state commit duplicate errors as duplicate requests (#10916)
* fix(consensus): classify wrapped state commit duplicate errors as duplicate requests * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Update CHANGELOG.md Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com> * address comments --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
1 parent 078286e commit b4c9f27

7 files changed

Lines changed: 88 additions & 9 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).
1212
- Don't disconnect from peers that return empty `FindBlocks` or `FindHeaders`
1313
responses when the local node is at or near the chain tip
1414
([#10732](https://github.com/ZcashFoundation/zebra/pull/10732))
15+
- Fix syncer restarts due to incorrect error downcasting.
1516

1617
## [Zebra 6.0.0-rc.0](https://github.com/ZcashFoundation/zebra/releases/tag/v6.0.0-rc.0) - 2026-07-02
1718

zebra-consensus/src/block.rs

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,25 @@ impl VerifyBlockError {
119119
}
120120
}
121121

122+
/// Converts an error from a `CommitSemanticallyVerifiedBlock` state request
123+
/// into a [`VerifyBlockError`].
124+
///
125+
/// The state boxes commit errors as [`zs::CommitSemanticallyVerifiedError`], a
126+
/// newtype around [`zs::CommitBlockError`], so the wrapper must be unwrapped
127+
/// here for `is_duplicate_request()` and `misbehavior_score()` to classify
128+
/// duplicate blocks as benign.
129+
fn map_commit_error(source: BoxError, hash: block::Hash) -> VerifyBlockError {
130+
if let Some(commit_err) = source
131+
.downcast_ref::<zs::CommitSemanticallyVerifiedError>()
132+
.map(zs::CommitSemanticallyVerifiedError::inner)
133+
.or_else(|| source.downcast_ref::<zs::CommitBlockError>())
134+
{
135+
return VerifyBlockError::Commit(commit_err.clone());
136+
}
137+
138+
VerifyBlockError::StateService { source, hash }
139+
}
140+
122141
/// The maximum number of transparent signature operations allowed in a block.
123142
///
124143
/// # Consensus
@@ -380,13 +399,7 @@ where
380399
Ok(hash)
381400
}
382401

383-
Err(source) => {
384-
if let Some(commit_err) = source.downcast_ref::<zs::CommitBlockError>() {
385-
return Err(VerifyBlockError::Commit(commit_err.clone()));
386-
}
387-
388-
Err(VerifyBlockError::StateService { source, hash })
389-
}
402+
Err(source) => Err(map_commit_error(source, hash)),
390403

391404
_ => unreachable!("wrong response for CommitSemanticallyVerifiedBlock"),
392405
}

zebra-consensus/src/block/tests.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -844,3 +844,28 @@ fn verify_block_error_misbehavior_scores() {
844844
};
845845
assert_eq!(VerifyBlockError::Commit(dup_err).misbehavior_score(), 0);
846846
}
847+
848+
/// Duplicate block errors must stay classified as duplicate requests after the
849+
/// state wraps them, so they don't restart the syncer or turn `submitblock`
850+
/// duplicates into rejections.
851+
#[test]
852+
fn state_commit_duplicate_errors_are_duplicate_requests() {
853+
let duplicate = zs::CommitBlockError::Duplicate {
854+
hash_or_height: None,
855+
location: zs::KnownBlock::BestChain,
856+
};
857+
858+
// Box the error the same way the state's `CommitSemanticallyVerifiedBlock`
859+
// handler does. This mirrors the wrapping manually, so it won't fail
860+
// automatically if the state changes its error type — keep it in sync by hand.
861+
let source: BoxError = Box::new(zs::CommitSemanticallyVerifiedError::from(duplicate));
862+
863+
let err = map_commit_error(source, block::Hash([0; 32]));
864+
865+
assert!(
866+
matches!(err, VerifyBlockError::Commit(_)),
867+
"state commit errors must be unwrapped into VerifyBlockError::Commit, got: {err:?}"
868+
);
869+
assert!(err.is_duplicate_request());
870+
assert_eq!(err.misbehavior_score(), 0);
871+
}

zebra-consensus/src/checkpoint.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,6 +1025,12 @@ impl VerifyCheckpointError {
10251025
// TODO: make this duplicate-incomplete
10261026
VerifyCheckpointError::NewerRequest { .. } => true,
10271027
VerifyCheckpointError::VerifyBlock(block_error) => block_error.is_duplicate_request(),
1028+
// The state boxes commit errors as `zs::CommitCheckpointVerifiedError`,
1029+
// a newtype around `zs::CommitBlockError`, so the wrapper must be
1030+
// unwrapped to classify duplicate blocks as benign.
1031+
VerifyCheckpointError::CommitCheckpointVerified(source) => source
1032+
.downcast_ref::<zs::CommitCheckpointVerifiedError>()
1033+
.is_some_and(|commit_err| commit_err.inner().is_duplicate_request()),
10281034
_ => false,
10291035
}
10301036
}

zebra-consensus/src/checkpoint/tests.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -814,3 +814,23 @@ async fn hard_coded_mainnet() -> Result<(), Report> {
814814

815815
Ok(())
816816
}
817+
818+
/// Duplicate block errors must stay classified as duplicate requests after the
819+
/// state wraps them, so they don't restart the syncer during checkpoint sync.
820+
#[test]
821+
fn state_commit_duplicate_errors_are_duplicate_requests() {
822+
let duplicate = zs::CommitBlockError::Duplicate {
823+
hash_or_height: None,
824+
location: zs::KnownBlock::Finalized,
825+
};
826+
827+
// Box the error the same way the state's `CommitCheckpointVerifiedBlock`
828+
// handler does. This mirrors the wrapping manually, so it won't fail
829+
// automatically if the state changes its error type — keep it in sync by hand.
830+
let source: BoxError = Box::new(zs::CommitCheckpointVerifiedError::from(duplicate));
831+
832+
let err = VerifyCheckpointError::CommitCheckpointVerified(source);
833+
834+
assert!(err.is_duplicate_request());
835+
assert_eq!(err.misbehavior_score(), 0);
836+
}

zebra-state/src/error.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,13 @@ impl CommitBlockError {
136136
#[error("could not commit semantically-verified block")]
137137
pub struct CommitSemanticallyVerifiedError(#[from] CommitBlockError);
138138

139+
impl CommitSemanticallyVerifiedError {
140+
/// Returns the [`CommitBlockError`] describing why the commit failed.
141+
pub fn inner(&self) -> &CommitBlockError {
142+
&self.0
143+
}
144+
}
145+
139146
impl From<ValidateContextError> for CommitSemanticallyVerifiedError {
140147
fn from(value: ValidateContextError) -> Self {
141148
Self(CommitBlockError::ValidateContextError(Box::new(value)))
@@ -164,6 +171,13 @@ impl<E: std::error::Error + 'static> From<BoxError> for LayeredStateError<E> {
164171
#[error("could not commit checkpoint-verified block")]
165172
pub struct CommitCheckpointVerifiedError(#[from] CommitBlockError);
166173

174+
impl CommitCheckpointVerifiedError {
175+
/// Returns the [`CommitBlockError`] describing why the commit failed.
176+
pub fn inner(&self) -> &CommitBlockError {
177+
&self.0
178+
}
179+
}
180+
167181
impl From<ValidateContextError> for CommitCheckpointVerifiedError {
168182
fn from(value: ValidateContextError) -> Self {
169183
Self(CommitBlockError::ValidateContextError(Box::new(value)))

zebra-state/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ pub use constants::{
4343
state_database_format_version_in_code, MAX_BLOCK_REORG_HEIGHT, MAX_NON_FINALIZED_CHAIN_FORKS,
4444
};
4545
pub use error::{
46-
BoxError, CloneError, CommitBlockError, CommitSemanticallyVerifiedError,
47-
DuplicateNullifierError, StateInitError, ValidateContextError,
46+
BoxError, CloneError, CommitBlockError, CommitCheckpointVerifiedError,
47+
CommitSemanticallyVerifiedError, DuplicateNullifierError, StateInitError, ValidateContextError,
4848
};
4949
pub use request::{
5050
CheckpointVerifiedBlock, CommitSemanticallyVerifiedBlockRequest, HashOrHeight, MappedRequest,

0 commit comments

Comments
 (0)