Skip to content

Commit 8fc0fe4

Browse files
committed
fix(blockchain): report a bad proposer signature as an invalid block proof
The proposer signature moved out of the attestation aggregate but stayed inside `BlockProof`, so a failure there is still the spec's INVALID_BLOCK_PROOF rejection, not INVALID_SIGNATURE. Reusing the attestation-signature variants classified it as the latter, which the `test_corrupt_proof_rejected` and `test_proof_reused_under_different_message_rejected` fixtures reject now that the runner asserts the reason (#547). Separate variants keep the gossip attestation path on INVALID_SIGNATURE.
1 parent d598499 commit 8fc0fe4

2 files changed

Lines changed: 21 additions & 5 deletions

File tree

crates/blockchain/src/spec_test_runner.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,14 @@ pub fn rejection_reason(err: &StoreError) -> Option<RejectionReason> {
8383
StoreError::AttestationTooFarInFuture { .. } => RejectionReason::AttestationTooFarInFuture,
8484
StoreError::AggregateVerificationFailed(_) => RejectionReason::InvalidSignature,
8585
StoreError::BlockProofVerificationFailed(_) => RejectionReason::InvalidBlockProof,
86-
// An attestation-free block carries no aggregate, so stray proof bytes
87-
// are a malformed block proof rather than a distinct spec reason.
88-
StoreError::UnexpectedAttestationProof => RejectionReason::InvalidBlockProof,
86+
// The proposer signature and the attestation aggregate are both
87+
// components of the block proof, so a failure in either is the same
88+
// spec rejection even though we carry them as separate wire fields. An
89+
// attestation-free block carries no aggregate at all, so stray proof
90+
// bytes are a malformed block proof rather than a distinct reason.
91+
StoreError::ProposerSignatureDecodingFailed
92+
| StoreError::ProposerSignatureVerificationFailed
93+
| StoreError::UnexpectedAttestationProof => RejectionReason::InvalidBlockProof,
8994
StoreError::EmptyAggregationBits => RejectionReason::EmptyAggregationBits,
9095
StoreError::NotProposer { .. } => RejectionReason::WrongProposer,
9196
StoreError::DuplicateAttestationData { .. } => RejectionReason::DuplicateAttestationData,

crates/blockchain/src/store.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,6 +1013,17 @@ pub enum StoreError {
10131013
#[error("Validator signature verification failed")]
10141014
SignatureVerificationFailed,
10151015

1016+
/// Kept apart from [`Self::SignatureDecodingFailed`] because the proposer
1017+
/// signature is a component of the block proof, so the spec reports its
1018+
/// failure as an invalid block proof rather than an invalid signature.
1019+
#[error("Block proposer signature could not be decoded")]
1020+
ProposerSignatureDecodingFailed,
1021+
1022+
/// See [`Self::ProposerSignatureDecodingFailed`] for why this is distinct
1023+
/// from [`Self::SignatureVerificationFailed`].
1024+
#[error("Block proposer signature verification failed")]
1025+
ProposerSignatureVerificationFailed,
1026+
10161027
#[error("Block carries an attestation proof but has no attestations")]
10171028
UnexpectedAttestationProof,
10181029

@@ -1193,9 +1204,9 @@ pub fn verify_block_signatures(
11931204
let proposer_pubkey = ValidatorPublicKey::from_bytes(&proposer_validator.proposal_pubkey)
11941205
.map_err(|_| StoreError::PubkeyDecodingFailed(block.proposer_index))?;
11951206
let proposer_signature = ValidatorSignature::from_bytes(&signed_block.proof.proposer_signature)
1196-
.map_err(|_| StoreError::SignatureDecodingFailed)?;
1207+
.map_err(|_| StoreError::ProposerSignatureDecodingFailed)?;
11971208
if !proposer_signature.is_valid(&proposer_pubkey, block_slot_u32, &block_root) {
1198-
return Err(StoreError::SignatureVerificationFailed);
1209+
return Err(StoreError::ProposerSignatureVerificationFailed);
11991210
}
12001211

12011212
// 2. Verify the attestation aggregate (Type-2 over the body attestations

0 commit comments

Comments
 (0)