From f1c3be018e99dcf4cb9da98e9639ae652f9b547a Mon Sep 17 00:00:00 2001 From: bdchatham Date: Thu, 14 May 2026 10:15:50 -0700 Subject: [PATCH 1/4] refactor(sei-tendermint): collapse ConsensusPolicy to ShouldSwallow(ErrorKind) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the per-failure Skip*Validation surface with a single ShouldSwallow(kind ErrorKind) bool method on ConsensusPolicy, consulted by every halting check in the block-validation hot path. - types/consensus_policy.go: ErrorKind type + 13 constants (one per audit-row swallow-eligible site) and AllSwallowEligibleErrorKinds(). - consensus_policy_default.go: ShouldSwallow always false; build constraint widened to !mock_block_validation && !mock_chain_validation. - consensus_policy_mock_block_validation.go: ShouldSwallow returns true for AppHash and DataHash (preserving the tag's user-visible chain-progresses semantics), false for the other 11 kinds. Drops the Skip*Validation short-circuit — the comparison now runs and the divergence is logged before being swallowed. Tests assert the variant matrix on both build tags and pin the 13-row audit invariant. This is step A of the two-commit M1.1 rewrite for sei-chain#3427. The call-site refactor in internal/state/validation.go and types/block.go follows in the next commit and is what restores the build. --- sei-tendermint/types/consensus_policy.go | 174 +++++++++++++++++- .../types/consensus_policy_default.go | 11 +- .../consensus_policy_mock_block_validation.go | 27 ++- ...ensus_policy_mock_block_validation_test.go | 47 +++++ sei-tendermint/types/consensus_policy_test.go | 35 ++++ 5 files changed, 282 insertions(+), 12 deletions(-) create mode 100644 sei-tendermint/types/consensus_policy_mock_block_validation_test.go create mode 100644 sei-tendermint/types/consensus_policy_test.go diff --git a/sei-tendermint/types/consensus_policy.go b/sei-tendermint/types/consensus_policy.go index c7c848ecb6..dc41f61970 100644 --- a/sei-tendermint/types/consensus_policy.go +++ b/sei-tendermint/types/consensus_policy.go @@ -1,7 +1,175 @@ +// Package types — ConsensusPolicy is the build-tag-selected gating surface +// for sei-tendermint's halting validation checks. +// +// # Shape +// +// ConsensusPolicy is a zero-sized struct with a single method: +// +// func (ConsensusPolicy) ShouldSwallow(kind ErrorKind) bool +// +// Each call site that performs a halting check consults the policy. When +// ShouldSwallow returns true, the site logs the divergence (expected/got), +// increments the sei_unsafe_validation_skipped_total counter, and continues +// past the failure. When it returns false, the original error-return +// behavior is preserved. Sites always run the comparison authentically; +// the policy only governs the failure-handling decision. +// +// # Variant matrix +// +// The struct's method is declared in exactly one of three per-variant files, +// selected by build tag (same-receiver redeclaration is forbidden in Go, so +// each variant is its own compilation unit): +// +// consensus_policy_default.go // !mock_block_validation && !mock_chain_validation +// consensus_policy_mock_block_validation.go // mock_block_validation +// consensus_policy_mock_chain_validation.go // mock_chain_validation (M2 deliverable) +// +// +------------------------+-----------------------------+ +// | build | ShouldSwallow behavior | +// +------------------------+-----------------------------+ +// | default (production) | always false | +// | mock_block_validation | true for AppHash, DataHash; | +// | | false otherwise | +// | mock_chain_validation | true for every audit-row | +// | | swallow-eligible ErrorKind | +// +------------------------+-----------------------------+ +// +// # Design — single-method collapse +// +// An earlier draft exposed thirteen Swallow*Failure() bool methods on the +// policy (one per swallow-eligible audit row). Review feedback collapsed +// the surface to a single ShouldSwallow(kind) method consulting an internal +// set: O(1) lookup, one method to maintain per variant instead of 13, and +// the policy file's switch on ErrorKind doubles as a registry of which +// failures the variant tolerates. +// +// # Design — no Skip*Validation short-circuit +// +// An earlier shape distinguished "skip" (short-circuit before the +// comparison) from "swallow" (run the comparison, then conditionally +// continue). The current design retains only swallow semantics: every +// check computes its comparison, then the policy decides whether to halt +// on failure. This keeps the build-tag variants on the same code path as +// production (closer to authentic prod behavior) at the cost of +// mock_block_validation losing its Data.Hash(false) compute fast-path — +// an explicitly accepted trade. +// +// # Carve-out — LastResultsHash atomic +// +// One Skip*-style early-return guard is preserved: the +// tmtypes.SkipLastResultsHashValidation atomic.Bool consulted at +// internal/state/validation.go for the LastResultsHash check. That flag +// is set unconditionally by Giga at app init and is load-bearing for +// Giga's production halt-resistance. Migrating Giga to a build-tagged +// ConsensusPolicy variant is its own future workstream. The asymmetry +// is documented inline at the consulting site. package types // DefaultConsensusPolicy returns the zero-value policy. Behavior is -// build-tag-dependent: production builds enforce every check; mock_block_validation -// builds bypass every gated check (see consensus_policy_default.go and -// consensus_policy_mock_block_validation.go). +// build-tag-dependent: production builds enforce every check; +// mock_block_validation builds swallow AppHash and DataHash mismatches; +// mock_chain_validation (M2) swallows every audit-row swallow-eligible +// failure. See package documentation above. func DefaultConsensusPolicy() ConsensusPolicy { return ConsensusPolicy{} } + +// ErrorKind names a swallow-eligible validation failure. Each constant +// corresponds to a row in the M1.0 audit table (docs/designs/ +// mock-chain-validation-m1-audit.md). The string value is the metric label +// emitted on sei_unsafe_validation_skipped_total{kind=...} when the failure +// is swallowed. +type ErrorKind string + +const ( + // ErrorKindAppHash — internal/state/validation.go AppHash check. + // Audit row 1. block.AppHash vs state.AppHash. Forked-boot block-1 + // blocker; the export's AppHash will not match what the new state + // produces. + ErrorKindAppHash ErrorKind = "app_hash" + + // ErrorKindDataHash — types/block.go Data.Hash() check in + // Block.ValidateBasic. Audit row 2. Forked-boot block-1 blocker. + ErrorKindDataHash ErrorKind = "data_hash" + + // ErrorKindLastResultsHash — internal/state/validation.go + // LastResultsHash check. Audit row 8. Already gated by the + // SkipLastResultsHashValidation atomic for Giga; this policy entry + // allows mock_chain_validation to participate without depending on + // Giga's runtime flip. + ErrorKindLastResultsHash ErrorKind = "last_results_hash" + + // ErrorKindLastBlockID — internal/state/validation.go LastBlockID + // continuity check. Audit row 6. Hash check (BlockID is hash + + // part-set-header); forked-boot diverges here at block-1. + ErrorKindLastBlockID ErrorKind = "last_block_id" + + // ErrorKindConsensusHash — internal/state/validation.go + // ConsensusHash check. Audit row 7. ConsensusParams.Hash mismatch + // fires when the exported chain had different consensus params than + // the new genesis. + ErrorKindConsensusHash ErrorKind = "consensus_hash" + + // ErrorKindValidatorsHash — internal/state/validation.go + // ValidatorsHash check. Audit row 9. Primary block-1 blocker for + // forked-boot with a new validator set. + ErrorKindValidatorsHash ErrorKind = "validators_hash" + + // ErrorKindNextValidatorsHash — internal/state/validation.go + // NextValidatorsHash check. Audit row 10. Sibling to ValidatorsHash + // for next-height validator set. + ErrorKindNextValidatorsHash ErrorKind = "next_validators_hash" + + // ErrorKindLastCommitVerify — internal/state/validation.go + // state.LastValidators.VerifyCommit. Audit row 12. Load-bearing + // block-1 blocker for forked-boot — the new LastCommit is signed by + // the new validators, which will not verify against the exported set. + ErrorKindLastCommitVerify ErrorKind = "last_commit_verify" + + // ErrorKindProposerNotInValidatorSet — internal/state/validation.go + // state.Validators.HasAddress(ProposerAddress) check. Audit row 13. + // Forked-boot with a new validator set: the proposer is in the new + // set, not the exported one. + ErrorKindProposerNotInValidatorSet ErrorKind = "proposer_not_in_validator_set" + + // ErrorKindEvidenceOverflow — internal/state/validation.go + // Evidence.ByteSize > MaxBytes check. Audit row 17. Possible block-1 + // blocker if the export carries evidence near the limit. + ErrorKindEvidenceOverflow ErrorKind = "evidence_overflow" + + // ErrorKindLastCommitHash — types/block.go LastCommitHash check in + // Block.ValidateBasic (with legacy-6.4 fallback). Audit row 18. + // Forked-boot block-1: new LastCommit's hash will not match the + // export's expected LastCommitHash. + ErrorKindLastCommitHash ErrorKind = "last_commit_hash" + + // ErrorKindEvidenceHash — types/block.go EvidenceHash check in + // Block.ValidateBasic. Audit row 19. Hash check. + ErrorKindEvidenceHash ErrorKind = "evidence_hash" + + // ErrorKindPerEvidenceValidateBasic — types/block.go per-evidence + // ValidateBasic loop in Block.ValidateBasic. Audit row 20. + // Cryptographic well-formedness per item; swallowing lets builds + // keep moving past malformed export-carried evidence. + ErrorKindPerEvidenceValidateBasic ErrorKind = "per_evidence_validate_basic" +) + +// AllSwallowEligibleErrorKinds returns the 13 ErrorKind constants in the +// audit's swallow-eligible set. Useful for tests asserting the variant +// matrix (every test should iterate this list and check ShouldSwallow's +// return for each). +func AllSwallowEligibleErrorKinds() []ErrorKind { + return []ErrorKind{ + ErrorKindAppHash, + ErrorKindDataHash, + ErrorKindLastResultsHash, + ErrorKindLastBlockID, + ErrorKindConsensusHash, + ErrorKindValidatorsHash, + ErrorKindNextValidatorsHash, + ErrorKindLastCommitVerify, + ErrorKindProposerNotInValidatorSet, + ErrorKindEvidenceOverflow, + ErrorKindLastCommitHash, + ErrorKindEvidenceHash, + ErrorKindPerEvidenceValidateBasic, + } +} diff --git a/sei-tendermint/types/consensus_policy_default.go b/sei-tendermint/types/consensus_policy_default.go index 783892743d..51cecd2a20 100644 --- a/sei-tendermint/types/consensus_policy_default.go +++ b/sei-tendermint/types/consensus_policy_default.go @@ -1,10 +1,11 @@ -//go:build !mock_block_validation +//go:build !mock_block_validation && !mock_chain_validation package types -// ConsensusPolicy is empty in production builds. Its methods return -// constant false so the compiler DCEs the bypass branches. +// ConsensusPolicy in production builds: never swallows. Every call site +// returns the original error on failure. type ConsensusPolicy struct{} -func (ConsensusPolicy) SkipAppHashValidation() bool { return false } -func (ConsensusPolicy) SkipDataHashValidation() bool { return false } +// ShouldSwallow always returns false in production builds. The compiler +// inlines and DCEs the swallow branch at every call site. +func (ConsensusPolicy) ShouldSwallow(_ ErrorKind) bool { return false } diff --git a/sei-tendermint/types/consensus_policy_mock_block_validation.go b/sei-tendermint/types/consensus_policy_mock_block_validation.go index 65f8d7fdf9..c560fb1d10 100644 --- a/sei-tendermint/types/consensus_policy_mock_block_validation.go +++ b/sei-tendermint/types/consensus_policy_mock_block_validation.go @@ -2,9 +2,28 @@ package types -// ConsensusPolicy is empty in mock_block_validation builds. Each Skip* -// method returns true unconditionally — running this binary IS the bypass. +// ConsensusPolicy in mock_block_validation builds: swallows AppHash and +// DataHash mismatches. The two checks compute their comparison +// authentically (no more pre-comparison short-circuit), and the divergence +// is logged before continuing. All other audit-row checks halt as in +// production. +// +// Behavior change relative to the prior Skip*Validation shape: +// - The Data.Hash(false) compute fast-path is gone (explicitly accepted). +// - AppHash / DataHash mismatches are now LOGGED in addition to being +// bypassed (previously they were silently skipped). +// +// User-visible outcome is preserved: the chain still progresses past +// AppHash and DataHash mismatches under this tag. type ConsensusPolicy struct{} -func (ConsensusPolicy) SkipAppHashValidation() bool { return true } -func (ConsensusPolicy) SkipDataHashValidation() bool { return true } +// ShouldSwallow returns true for the two CometBFT hash checks that the +// mock_block_validation tag has always relaxed. All other ErrorKinds halt. +func (ConsensusPolicy) ShouldSwallow(kind ErrorKind) bool { + switch kind { + case ErrorKindAppHash, ErrorKindDataHash: + return true + default: + return false + } +} diff --git a/sei-tendermint/types/consensus_policy_mock_block_validation_test.go b/sei-tendermint/types/consensus_policy_mock_block_validation_test.go new file mode 100644 index 0000000000..8171821d72 --- /dev/null +++ b/sei-tendermint/types/consensus_policy_mock_block_validation_test.go @@ -0,0 +1,47 @@ +//go:build mock_block_validation + +package types + +import "testing" + +// TestConsensusPolicy_MockBlockValidation_Matrix asserts the variant's +// swallow matrix: AppHash and DataHash swallow; the other 11 audit-row +// kinds halt as in production. +func TestConsensusPolicy_MockBlockValidation_Matrix(t *testing.T) { + policy := DefaultConsensusPolicy() + swallowExpected := map[ErrorKind]bool{ + ErrorKindAppHash: true, + ErrorKindDataHash: true, + ErrorKindLastResultsHash: false, + ErrorKindLastBlockID: false, + ErrorKindConsensusHash: false, + ErrorKindValidatorsHash: false, + ErrorKindNextValidatorsHash: false, + ErrorKindLastCommitVerify: false, + ErrorKindProposerNotInValidatorSet: false, + ErrorKindEvidenceOverflow: false, + ErrorKindLastCommitHash: false, + ErrorKindEvidenceHash: false, + ErrorKindPerEvidenceValidateBasic: false, + } + for _, kind := range AllSwallowEligibleErrorKinds() { + want, ok := swallowExpected[kind] + if !ok { + t.Errorf("test matrix missing entry for ErrorKind %q — audit added a new row?", kind) + continue + } + if got := policy.ShouldSwallow(kind); got != want { + t.Errorf("mock_block_validation ConsensusPolicy.ShouldSwallow(%q) = %v, want %v", + kind, got, want) + } + } +} + +// TestConsensusPolicy_MockBlockValidation_UnknownKindHalts ensures +// previously unknown kinds halt by default (closed-set semantics). +func TestConsensusPolicy_MockBlockValidation_UnknownKindHalts(t *testing.T) { + policy := DefaultConsensusPolicy() + if policy.ShouldSwallow(ErrorKind("not_a_real_kind")) { + t.Errorf("mock_block_validation ConsensusPolicy.ShouldSwallow(unknown) = true, want false") + } +} diff --git a/sei-tendermint/types/consensus_policy_test.go b/sei-tendermint/types/consensus_policy_test.go new file mode 100644 index 0000000000..20d1f51856 --- /dev/null +++ b/sei-tendermint/types/consensus_policy_test.go @@ -0,0 +1,35 @@ +//go:build !mock_block_validation && !mock_chain_validation + +package types + +import "testing" + +// TestConsensusPolicy_Default_AllKindsHalt asserts the production variant +// never swallows. Every swallow-eligible ErrorKind must return false. +func TestConsensusPolicy_Default_AllKindsHalt(t *testing.T) { + policy := DefaultConsensusPolicy() + for _, kind := range AllSwallowEligibleErrorKinds() { + if policy.ShouldSwallow(kind) { + t.Errorf("default ConsensusPolicy.ShouldSwallow(%q) = true, want false", kind) + } + } +} + +// TestConsensusPolicy_Default_UnknownKindHalts asserts unrecognized kinds +// also halt (defensive). +func TestConsensusPolicy_Default_UnknownKindHalts(t *testing.T) { + policy := DefaultConsensusPolicy() + if policy.ShouldSwallow(ErrorKind("not_a_real_kind")) { + t.Errorf("default ConsensusPolicy.ShouldSwallow(unknown) = true, want false") + } +} + +// TestAllSwallowEligibleErrorKinds_Count asserts the audit's 13-row +// invariant. If this number changes the audit (docs/designs/ +// mock-chain-validation-m1-audit.md) must be revisited. +func TestAllSwallowEligibleErrorKinds_Count(t *testing.T) { + got := len(AllSwallowEligibleErrorKinds()) + if got != 13 { + t.Errorf("AllSwallowEligibleErrorKinds() returned %d kinds, want 13 (per M1.0 audit)", got) + } +} From 1eecec778f1b819941ecaa5930faa5f8646a63f8 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Thu, 14 May 2026 10:56:18 -0700 Subject: [PATCH 2/4] refactor(sei-tendermint): route swallow-eligible checks through SwallowOrErr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidate per-site swallow boilerplate into a single types.SwallowOrErr helper and refactor every audit-row swallow-eligible site to use it. Every check now computes its comparison authentically; the ConsensusPolicy governs only the failure-handling decision. - types/swallow.go: SwallowOrErr(policy, kind, logger, site, height, expected, got, format, args...) — returns nil to swallow (logs + increments sei_unsafe_validation_skipped_total{site, kind}) or a formatted error to halt. LogSwallowedFailure produces the structured divergence record; counter is auto-registered via promauto. - internal/state/validation.go: 9 audit-row sites routed through SwallowOrErr (AppHash, LastBlockID, ConsensusHash, LastResultsHash, ValidatorsHash, NextValidatorsHash, LastCommitVerify, ProposerNotInValidatorSet, EvidenceOverflow). The pre-existing tmtypes.SkipLastResultsHashValidation atomic.Bool early-return guard is preserved as the sole carve-out — load-bearing for Giga's production halt-resistance; migration to a build-tagged variant is a future Giga workstream documented inline. - types/block.go: 4 audit-row sites routed through SwallowOrErr (LastCommitHash, DataHash, PerEvidenceValidateBasic, EvidenceHash). The Skip*Validation short-circuit on DataHash is gone — the comparison now runs unconditionally; only the failure-handling differs per ConsensusPolicy. Trades the Data.Hash(false) compute fast-path for authentic prod-code-path execution under all build tags (explicitly accepted in review). Behavior preservation under mock_block_validation: - AppHash and DataHash mismatches are still bypassed (chain continues) but are now LOGGED in addition to being swallowed. - Three pre-existing test failures keep failing in the same shape: TestBlockValidateBasic (Tampered_Data, Tampered_DataHash subtests) and TestValidateBlockHeader (DataHash subcase) — all assert an error on tampered data and now observe the swallowed nil. TestBlockProtoBuf, which previously failed because SkipDataHashValidation suppressed the side-effect populate of Data.hash, now passes — a positive consequence of the explicitly-accepted fast-path trade. Step B of two for sei-chain#3427. --- sei-tendermint/internal/state/validation.go | 119 ++++++++++++++------ sei-tendermint/types/block.go | 38 +++++-- sei-tendermint/types/swallow.go | 102 +++++++++++++++++ 3 files changed, 218 insertions(+), 41 deletions(-) create mode 100644 sei-tendermint/types/swallow.go diff --git a/sei-tendermint/internal/state/validation.go b/sei-tendermint/internal/state/validation.go index e04ac712dd..4e705ee50e 100644 --- a/sei-tendermint/internal/state/validation.go +++ b/sei-tendermint/internal/state/validation.go @@ -40,45 +40,78 @@ func validateBlock(state State, block *types.Block, policy types.ConsensusPolicy block.Height, ) } - // Validate prev block info. + // Validate prev block info — audit row 6 (LastBlockID, swallow-eligible). if !block.LastBlockID.Equals(state.LastBlockID) { - return fmt.Errorf("wrong Block.Header.LastBlockID. Expected %v, got %v", - state.LastBlockID, - block.LastBlockID, - ) + if err := types.SwallowOrErr(policy, types.ErrorKindLastBlockID, logger, + "internal/state/validation.go:LastBlockID", block.Height, + state.LastBlockID, block.LastBlockID, + "wrong Block.Header.LastBlockID. Expected %v, got %v", + state.LastBlockID, block.LastBlockID); err != nil { + return err + } } - // Validate app info - if !policy.SkipAppHashValidation() && !bytes.Equal(block.AppHash, state.AppHash) { - return fmt.Errorf("wrong Block.Header.AppHash. Expected %X, got %v", - state.AppHash, - block.AppHash, - ) + // Validate app info — audit row 1 (AppHash, swallow-eligible). + if !bytes.Equal(block.AppHash, state.AppHash) { + if err := types.SwallowOrErr(policy, types.ErrorKindAppHash, logger, + "internal/state/validation.go:AppHash", block.Height, + state.AppHash, block.AppHash, + "wrong Block.Header.AppHash. Expected %X, got %v", + state.AppHash, block.AppHash); err != nil { + return err + } } + // Audit row 7 (ConsensusHash, swallow-eligible). hashCP := state.ConsensusParams.HashConsensusParams() if !bytes.Equal(block.ConsensusHash, hashCP) { - return fmt.Errorf("wrong Block.Header.ConsensusHash. Expected %X, got %v", - hashCP, - block.ConsensusHash, - ) + if err := types.SwallowOrErr(policy, types.ErrorKindConsensusHash, logger, + "internal/state/validation.go:ConsensusHash", block.Height, + hashCP, block.ConsensusHash, + "wrong Block.Header.ConsensusHash. Expected %X, got %v", + hashCP, block.ConsensusHash); err != nil { + return err + } } - if !types.SkipLastResultsHashValidation.Load() && !bytes.Equal(block.LastResultsHash, state.LastResultsHash) { - return fmt.Errorf("wrong Block.Header.LastResultsHash. Expected %X, got %v", - state.LastResultsHash, - block.LastResultsHash, - ) + // Audit row 8 (LastResultsHash, swallow-eligible). + // + // Giga production escape hatch — the pre-existing + // tmtypes.SkipLastResultsHashValidation atomic.Bool is set + // unconditionally by Giga at app init (app.go:749) and is load-bearing + // for Giga's production halt-resistance on LastResultsHash. Migrating + // Giga onto a build-tagged ConsensusPolicy variant is its own future + // workstream. Until then this is the only Skip*-style early-return + // guard preserved in the codebase; every other swallow-eligible site + // computes its comparison and consults SwallowOrErr. + if !types.SkipLastResultsHashValidation.Load() { + if !bytes.Equal(block.LastResultsHash, state.LastResultsHash) { + if err := types.SwallowOrErr(policy, types.ErrorKindLastResultsHash, logger, + "internal/state/validation.go:LastResultsHash", block.Height, + state.LastResultsHash, block.LastResultsHash, + "wrong Block.Header.LastResultsHash. Expected %X, got %v", + state.LastResultsHash, block.LastResultsHash); err != nil { + return err + } + } } + // Audit row 9 (ValidatorsHash, swallow-eligible). if !bytes.Equal(block.ValidatorsHash, state.Validators.Hash()) { - return fmt.Errorf("wrong Block.Header.ValidatorsHash. Expected %X, got %v", - state.Validators.Hash(), - block.ValidatorsHash, - ) + if err := types.SwallowOrErr(policy, types.ErrorKindValidatorsHash, logger, + "internal/state/validation.go:ValidatorsHash", block.Height, + state.Validators.Hash(), block.ValidatorsHash, + "wrong Block.Header.ValidatorsHash. Expected %X, got %v", + state.Validators.Hash(), block.ValidatorsHash); err != nil { + return err + } } + // Audit row 10 (NextValidatorsHash, swallow-eligible). if !bytes.Equal(block.NextValidatorsHash, state.NextValidators.Hash()) { - return fmt.Errorf("wrong Block.Header.NextValidatorsHash. Expected %X, got %v", - state.NextValidators.Hash(), - block.NextValidatorsHash, - ) + if err := types.SwallowOrErr(policy, types.ErrorKindNextValidatorsHash, logger, + "internal/state/validation.go:NextValidatorsHash", block.Height, + state.NextValidators.Hash(), block.NextValidatorsHash, + "wrong Block.Header.NextValidatorsHash. Expected %X, got %v", + state.NextValidators.Hash(), block.NextValidatorsHash); err != nil { + return err + } } // Validate block LastCommit. @@ -87,10 +120,16 @@ func validateBlock(state State, block *types.Block, policy types.ConsensusPolicy return errors.New("initial block can't have LastCommit signatures") } } else { + // Audit row 12 (LastCommit signature verification, swallow-eligible). // LastCommit.Signatures length is checked in VerifyCommit. if err := state.LastValidators.VerifyCommit( state.ChainID, state.LastBlockID, block.Height-1, block.LastCommit); err != nil { - return fmt.Errorf("VerifyCommit(): %w", err) + if swErr := types.SwallowOrErr(policy, types.ErrorKindLastCommitVerify, logger, + "internal/state/validation.go:LastCommitVerify", block.Height, + state.LastBlockID, err.Error(), + "VerifyCommit(): %w", err); swErr != nil { + return swErr + } } } @@ -98,10 +137,16 @@ func validateBlock(state State, block *types.Block, policy types.ConsensusPolicy // know what round the block was first proposed. So just check that it's // a legit address and a known validator. // The length is checked in ValidateBasic above. + // + // Audit row 13 (ProposerNotInValidatorSet, swallow-eligible). if !state.Validators.HasAddress(block.ProposerAddress) { - return fmt.Errorf("block.Header.ProposerAddress %X is not a validator", - block.ProposerAddress, - ) + if err := types.SwallowOrErr(policy, types.ErrorKindProposerNotInValidatorSet, logger, + "internal/state/validation.go:ProposerNotInValidatorSet", block.Height, + "", block.ProposerAddress, + "block.Header.ProposerAddress %X is not a validator", + block.ProposerAddress); err != nil { + return err + } } // Validate block Time @@ -128,9 +173,15 @@ func validateBlock(state State, block *types.Block, policy types.ConsensusPolicy block.Height, state.InitialHeight) } - // Check evidence doesn't exceed the limit amount of bytes. + // Check evidence doesn't exceed the limit amount of bytes — audit + // row 17 (EvidenceOverflow, swallow-eligible). if max, got := state.ConsensusParams.Evidence.MaxBytes, block.Evidence.ByteSize(); got > max { - return types.NewErrEvidenceOverflow(max, got) + if err := types.SwallowOrErr(policy, types.ErrorKindEvidenceOverflow, logger, + "internal/state/validation.go:EvidenceOverflow", block.Height, + max, got, + "%w", types.NewErrEvidenceOverflow(max, got)); err != nil { + return err + } } return nil diff --git a/sei-tendermint/types/block.go b/sei-tendermint/types/block.go index 836abaea31..eb511eb9af 100644 --- a/sei-tendermint/types/block.go +++ b/sei-tendermint/types/block.go @@ -88,29 +88,53 @@ func (b *Block) ValidateBasic(policy ConsensusPolicy) error { return fmt.Errorf("wrong LastCommit: %w", err) } + // Audit row 18 (LastCommitHash, swallow-eligible). if w, g := b.LastCommit.Hash(), b.LastCommitHash; !bytes.Equal(w, g) { // Fall back to legacy hash calculation pre-6.4. if wLegacy := b.LastCommit.legacyHash(); !bytes.Equal(wLegacy, g) { - return fmt.Errorf("wrong Header.LastCommitHash. Expected %X, got %X", w, g) + if err := SwallowOrErr(policy, ErrorKindLastCommitHash, logger, + "types/block.go:LastCommitHash", b.Height, + w, g, + "wrong Header.LastCommitHash. Expected %X, got %X", w, g); err != nil { + return err + } } } - if !policy.SkipDataHashValidation() { - // NOTE: b.Data.Txs may be nil, but b.Data.Hash() still works fine. - if w, g := b.Data.Hash(false), b.DataHash; !bytes.Equal(w, g) { - return fmt.Errorf("wrong Header.DataHash. Expected %X, got %X. Len of txs %d", w, g, len(b.Txs)) + // Audit row 2 (DataHash, swallow-eligible). The Skip*-style + // short-circuit is gone — the comparison runs unconditionally; only + // the failure-handling is policy-driven. + // NOTE: b.Data.Txs may be nil, but b.Data.Hash() still works fine. + if w, g := b.Data.Hash(false), b.DataHash; !bytes.Equal(w, g) { + if err := SwallowOrErr(policy, ErrorKindDataHash, logger, + "types/block.go:DataHash", b.Height, + w, g, + "wrong Header.DataHash. Expected %X, got %X. Len of txs %d", w, g, len(b.Txs)); err != nil { + return err } } + // Audit row 20 (PerEvidenceValidateBasic, swallow-eligible). // NOTE: b.Evidence may be nil, but we're just looping. for i, ev := range b.Evidence { if err := ev.ValidateBasic(); err != nil { - return fmt.Errorf("invalid evidence (#%d): %v", i, err) + if swErr := SwallowOrErr(policy, ErrorKindPerEvidenceValidateBasic, logger, + "types/block.go:PerEvidenceValidateBasic", b.Height, + i, err.Error(), + "invalid evidence (#%d): %v", i, err); swErr != nil { + return swErr + } } } + // Audit row 19 (EvidenceHash, swallow-eligible). if w, g := b.Evidence.Hash(), b.EvidenceHash; !bytes.Equal(w, g) { - return fmt.Errorf("wrong Header.EvidenceHash. Expected %X, got %X", w, g) + if err := SwallowOrErr(policy, ErrorKindEvidenceHash, logger, + "types/block.go:EvidenceHash", b.Height, + w, g, + "wrong Header.EvidenceHash. Expected %X, got %X", w, g); err != nil { + return err + } } return nil diff --git a/sei-tendermint/types/swallow.go b/sei-tendermint/types/swallow.go new file mode 100644 index 0000000000..ea4120c203 --- /dev/null +++ b/sei-tendermint/types/swallow.go @@ -0,0 +1,102 @@ +package types + +import ( + "fmt" + "log/slog" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" +) + +// unsafeValidationSkippedTotal counts every halting validation failure +// that ConsensusPolicy chose to swallow. Labeled by site (the calling +// file:line, hand-coded at each call) and kind (the ErrorKind constant +// string). Lives at the default Prometheus registry — production builds +// never increment it because ShouldSwallow returns false on all kinds. +// +// Metric name follows the audit's specification: +// sei_unsafe_validation_skipped_total{site, kind}. +var unsafeValidationSkippedTotal = promauto.NewCounterVec( + prometheus.CounterOpts{ + Name: "sei_unsafe_validation_skipped_total", + Help: "Halting validation failures swallowed by a non-default ConsensusPolicy (mock_block_validation, mock_chain_validation). Always zero in production builds.", + }, + []string{"site", "kind"}, +) + +// LogSwallowedFailure emits the structured log line that accompanies a +// swallowed validation failure. Every call site uses this so the operator +// sees a uniform record of (kind, site, height, expected, got). The +// counter increment is the responsibility of SwallowOrErr — this function +// only logs. +// +// If the logger is nil it falls back to the types package logger +// (declared in part_set.go) so call sites that don't carry one (such as +// Block.ValidateBasic) can still produce a record. +func LogSwallowedFailure( + lg *slog.Logger, + kind ErrorKind, + site string, + height int64, + expected, got interface{}, +) { + if lg == nil { + lg = logger + } + lg.Warn("swallowed validation failure (unsafe ConsensusPolicy)", + "kind", string(kind), + "site", site, + "height", height, + "expected", fmt.Sprintf("%X", expected), + "got", fmt.Sprintf("%X", got), + ) +} + +// SwallowOrErr applies the ConsensusPolicy failure-handling decision at a +// halting validation site. Call it from the failure branch (i.e. when +// the comparison has already failed); it returns nil to swallow (logging +// + counter) or a formatted error to halt. +// +// Typical use: +// +// if !bytes.Equal(block.AppHash, state.AppHash) { +// if err := types.SwallowOrErr(policy, types.ErrorKindAppHash, logger, site, +// block.Height, state.AppHash, block.AppHash, +// "wrong Block.Header.AppHash. Expected %X, got %v", +// state.AppHash, block.AppHash); err != nil { +// return err +// } +// } +// +// Skip-before-compute is intentionally no longer supported — every check +// computes its comparison authentically; only the failure-handling +// differs per ConsensusPolicy. +// +// Parameters: +// - policy: the per-binary ConsensusPolicy (build-tag-selected). +// - kind: the audit-row ErrorKind being checked. +// - lg: *slog.Logger to receive the divergence record. nil falls +// back to the types package logger. +// - site: file:line of the calling check (helps trace which audit row +// fired in metrics + logs). +// - height: block height of the failing block. +// - expected, got: the two values being compared. Logged hex-formatted. +// - format, args: the original fmt.Errorf format the call site would +// have used to halt. Returned on the halt path. +func SwallowOrErr( + policy ConsensusPolicy, + kind ErrorKind, + lg *slog.Logger, + site string, + height int64, + expected, got interface{}, + format string, + args ...interface{}, +) error { + if policy.ShouldSwallow(kind) { + LogSwallowedFailure(lg, kind, site, height, expected, got) + unsafeValidationSkippedTotal.WithLabelValues(site, string(kind)).Inc() + return nil + } + return fmt.Errorf(format, args...) +} From 0d6a8f0c4d7066564701e1f18ff1b3559fb834eb Mon Sep 17 00:00:00 2001 From: bdchatham Date: Thu, 14 May 2026 12:44:28 -0700 Subject: [PATCH 3/4] chore(sei-tendermint): trim comment density on swallow-eligible refactor Comment-only sweep on the M1 PR. Cuts ~175 lines of comments that restated what the code obviously does (per-constant ErrorKind docstrings, per-site audit-row tags, duplicated build-tag matrices, parameter-restating docstrings on SwallowOrErr). Keeps the load-bearing ones: the SkipLastResultsHashValidation Giga escape-hatch carve-out, a 10-line package doc naming the build-tag variant pattern, the SwallowOrErr nil/error contract, the mock_block_validation tradeoff (AppHash+DataHash only preserves prior tag outcomes), and the audit-row count invariant test. --- sei-tendermint/internal/state/validation.go | 32 ++-- sei-tendermint/types/block.go | 6 - sei-tendermint/types/consensus_policy.go | 181 ++++-------------- .../types/consensus_policy_default.go | 4 - .../consensus_policy_mock_block_validation.go | 19 +- ...ensus_policy_mock_block_validation_test.go | 5 - sei-tendermint/types/consensus_policy_test.go | 9 +- sei-tendermint/types/swallow.go | 59 ++---- 8 files changed, 64 insertions(+), 251 deletions(-) diff --git a/sei-tendermint/internal/state/validation.go b/sei-tendermint/internal/state/validation.go index 4e705ee50e..854858a75b 100644 --- a/sei-tendermint/internal/state/validation.go +++ b/sei-tendermint/internal/state/validation.go @@ -11,6 +11,9 @@ import ( //----------------------------------------------------- // Validate block +// Swallow-eligible failure sites in this function consult +// types.ConsensusPolicy via types.SwallowOrErr; the audit-row ErrorKind +// passed at each site is the cross-reference (see types.ErrorKind*). func validateBlock(state State, block *types.Block, policy types.ConsensusPolicy) error { // Validate internal consistency. if err := block.ValidateBasic(policy); err != nil { @@ -40,7 +43,7 @@ func validateBlock(state State, block *types.Block, policy types.ConsensusPolicy block.Height, ) } - // Validate prev block info — audit row 6 (LastBlockID, swallow-eligible). + // Validate prev block info. if !block.LastBlockID.Equals(state.LastBlockID) { if err := types.SwallowOrErr(policy, types.ErrorKindLastBlockID, logger, "internal/state/validation.go:LastBlockID", block.Height, @@ -51,7 +54,7 @@ func validateBlock(state State, block *types.Block, policy types.ConsensusPolicy } } - // Validate app info — audit row 1 (AppHash, swallow-eligible). + // Validate app info. if !bytes.Equal(block.AppHash, state.AppHash) { if err := types.SwallowOrErr(policy, types.ErrorKindAppHash, logger, "internal/state/validation.go:AppHash", block.Height, @@ -61,7 +64,6 @@ func validateBlock(state State, block *types.Block, policy types.ConsensusPolicy return err } } - // Audit row 7 (ConsensusHash, swallow-eligible). hashCP := state.ConsensusParams.HashConsensusParams() if !bytes.Equal(block.ConsensusHash, hashCP) { if err := types.SwallowOrErr(policy, types.ErrorKindConsensusHash, logger, @@ -72,16 +74,12 @@ func validateBlock(state State, block *types.Block, policy types.ConsensusPolicy return err } } - // Audit row 8 (LastResultsHash, swallow-eligible). - // - // Giga production escape hatch — the pre-existing - // tmtypes.SkipLastResultsHashValidation atomic.Bool is set - // unconditionally by Giga at app init (app.go:749) and is load-bearing - // for Giga's production halt-resistance on LastResultsHash. Migrating - // Giga onto a build-tagged ConsensusPolicy variant is its own future - // workstream. Until then this is the only Skip*-style early-return - // guard preserved in the codebase; every other swallow-eligible site - // computes its comparison and consults SwallowOrErr. + // Giga production escape hatch: tmtypes.SkipLastResultsHashValidation + // is set unconditionally by Giga at app init (app.go:749) and is + // load-bearing for Giga's production halt-resistance on + // LastResultsHash. This is the only Skip*-style early-return preserved + // in the codebase; migrating Giga onto a build-tagged ConsensusPolicy + // variant is its own future workstream. if !types.SkipLastResultsHashValidation.Load() { if !bytes.Equal(block.LastResultsHash, state.LastResultsHash) { if err := types.SwallowOrErr(policy, types.ErrorKindLastResultsHash, logger, @@ -93,7 +91,6 @@ func validateBlock(state State, block *types.Block, policy types.ConsensusPolicy } } } - // Audit row 9 (ValidatorsHash, swallow-eligible). if !bytes.Equal(block.ValidatorsHash, state.Validators.Hash()) { if err := types.SwallowOrErr(policy, types.ErrorKindValidatorsHash, logger, "internal/state/validation.go:ValidatorsHash", block.Height, @@ -103,7 +100,6 @@ func validateBlock(state State, block *types.Block, policy types.ConsensusPolicy return err } } - // Audit row 10 (NextValidatorsHash, swallow-eligible). if !bytes.Equal(block.NextValidatorsHash, state.NextValidators.Hash()) { if err := types.SwallowOrErr(policy, types.ErrorKindNextValidatorsHash, logger, "internal/state/validation.go:NextValidatorsHash", block.Height, @@ -120,7 +116,6 @@ func validateBlock(state State, block *types.Block, policy types.ConsensusPolicy return errors.New("initial block can't have LastCommit signatures") } } else { - // Audit row 12 (LastCommit signature verification, swallow-eligible). // LastCommit.Signatures length is checked in VerifyCommit. if err := state.LastValidators.VerifyCommit( state.ChainID, state.LastBlockID, block.Height-1, block.LastCommit); err != nil { @@ -137,8 +132,6 @@ func validateBlock(state State, block *types.Block, policy types.ConsensusPolicy // know what round the block was first proposed. So just check that it's // a legit address and a known validator. // The length is checked in ValidateBasic above. - // - // Audit row 13 (ProposerNotInValidatorSet, swallow-eligible). if !state.Validators.HasAddress(block.ProposerAddress) { if err := types.SwallowOrErr(policy, types.ErrorKindProposerNotInValidatorSet, logger, "internal/state/validation.go:ProposerNotInValidatorSet", block.Height, @@ -173,8 +166,7 @@ func validateBlock(state State, block *types.Block, policy types.ConsensusPolicy block.Height, state.InitialHeight) } - // Check evidence doesn't exceed the limit amount of bytes — audit - // row 17 (EvidenceOverflow, swallow-eligible). + // Check evidence doesn't exceed the limit amount of bytes. if max, got := state.ConsensusParams.Evidence.MaxBytes, block.Evidence.ByteSize(); got > max { if err := types.SwallowOrErr(policy, types.ErrorKindEvidenceOverflow, logger, "internal/state/validation.go:EvidenceOverflow", block.Height, diff --git a/sei-tendermint/types/block.go b/sei-tendermint/types/block.go index eb511eb9af..cddadcfd1d 100644 --- a/sei-tendermint/types/block.go +++ b/sei-tendermint/types/block.go @@ -88,7 +88,6 @@ func (b *Block) ValidateBasic(policy ConsensusPolicy) error { return fmt.Errorf("wrong LastCommit: %w", err) } - // Audit row 18 (LastCommitHash, swallow-eligible). if w, g := b.LastCommit.Hash(), b.LastCommitHash; !bytes.Equal(w, g) { // Fall back to legacy hash calculation pre-6.4. if wLegacy := b.LastCommit.legacyHash(); !bytes.Equal(wLegacy, g) { @@ -101,9 +100,6 @@ func (b *Block) ValidateBasic(policy ConsensusPolicy) error { } } - // Audit row 2 (DataHash, swallow-eligible). The Skip*-style - // short-circuit is gone — the comparison runs unconditionally; only - // the failure-handling is policy-driven. // NOTE: b.Data.Txs may be nil, but b.Data.Hash() still works fine. if w, g := b.Data.Hash(false), b.DataHash; !bytes.Equal(w, g) { if err := SwallowOrErr(policy, ErrorKindDataHash, logger, @@ -114,7 +110,6 @@ func (b *Block) ValidateBasic(policy ConsensusPolicy) error { } } - // Audit row 20 (PerEvidenceValidateBasic, swallow-eligible). // NOTE: b.Evidence may be nil, but we're just looping. for i, ev := range b.Evidence { if err := ev.ValidateBasic(); err != nil { @@ -127,7 +122,6 @@ func (b *Block) ValidateBasic(policy ConsensusPolicy) error { } } - // Audit row 19 (EvidenceHash, swallow-eligible). if w, g := b.Evidence.Hash(), b.EvidenceHash; !bytes.Equal(w, g) { if err := SwallowOrErr(policy, ErrorKindEvidenceHash, logger, "types/block.go:EvidenceHash", b.Height, diff --git a/sei-tendermint/types/consensus_policy.go b/sei-tendermint/types/consensus_policy.go index dc41f61970..67c79001e1 100644 --- a/sei-tendermint/types/consensus_policy.go +++ b/sei-tendermint/types/consensus_policy.go @@ -1,161 +1,46 @@ -// Package types — ConsensusPolicy is the build-tag-selected gating surface -// for sei-tendermint's halting validation checks. -// -// # Shape -// -// ConsensusPolicy is a zero-sized struct with a single method: -// -// func (ConsensusPolicy) ShouldSwallow(kind ErrorKind) bool -// -// Each call site that performs a halting check consults the policy. When -// ShouldSwallow returns true, the site logs the divergence (expected/got), -// increments the sei_unsafe_validation_skipped_total counter, and continues -// past the failure. When it returns false, the original error-return -// behavior is preserved. Sites always run the comparison authentically; -// the policy only governs the failure-handling decision. -// -// # Variant matrix -// -// The struct's method is declared in exactly one of three per-variant files, -// selected by build tag (same-receiver redeclaration is forbidden in Go, so -// each variant is its own compilation unit): -// -// consensus_policy_default.go // !mock_block_validation && !mock_chain_validation -// consensus_policy_mock_block_validation.go // mock_block_validation -// consensus_policy_mock_chain_validation.go // mock_chain_validation (M2 deliverable) -// -// +------------------------+-----------------------------+ -// | build | ShouldSwallow behavior | -// +------------------------+-----------------------------+ -// | default (production) | always false | -// | mock_block_validation | true for AppHash, DataHash; | -// | | false otherwise | -// | mock_chain_validation | true for every audit-row | -// | | swallow-eligible ErrorKind | -// +------------------------+-----------------------------+ -// -// # Design — single-method collapse -// -// An earlier draft exposed thirteen Swallow*Failure() bool methods on the -// policy (one per swallow-eligible audit row). Review feedback collapsed -// the surface to a single ShouldSwallow(kind) method consulting an internal -// set: O(1) lookup, one method to maintain per variant instead of 13, and -// the policy file's switch on ErrorKind doubles as a registry of which -// failures the variant tolerates. -// -// # Design — no Skip*Validation short-circuit -// -// An earlier shape distinguished "skip" (short-circuit before the -// comparison) from "swallow" (run the comparison, then conditionally -// continue). The current design retains only swallow semantics: every -// check computes its comparison, then the policy decides whether to halt -// on failure. This keeps the build-tag variants on the same code path as -// production (closer to authentic prod behavior) at the cost of -// mock_block_validation losing its Data.Hash(false) compute fast-path — -// an explicitly accepted trade. -// -// # Carve-out — LastResultsHash atomic -// -// One Skip*-style early-return guard is preserved: the -// tmtypes.SkipLastResultsHashValidation atomic.Bool consulted at -// internal/state/validation.go for the LastResultsHash check. That flag -// is set unconditionally by Giga at app init and is load-bearing for -// Giga's production halt-resistance. Migrating Giga to a build-tagged -// ConsensusPolicy variant is its own future workstream. The asymmetry -// is documented inline at the consulting site. +// Package types — ConsensusPolicy is a zero-sized, build-tag-selected gate +// that decides, per ErrorKind, whether a halting validation failure halts +// (default) or is swallowed (logged + counter, then continued). The single +// method ShouldSwallow(kind) is declared in exactly one of three per-tag +// files, so each binary compiles in one fixed policy with no runtime branch: +// +// default (production) → always false; the swallow branch is DCE'd +// mock_block_validation → true for AppHash and DataHash; preserves the +// long-standing behavior of that tag +// mock_chain_validation → true for every swallow-eligible audit-row kind +// (M2 deliverable) +// +// One Skip*-style early-return is preserved alongside the policy: +// tmtypes.SkipLastResultsHashValidation; see validation.go for context. package types -// DefaultConsensusPolicy returns the zero-value policy. Behavior is -// build-tag-dependent: production builds enforce every check; -// mock_block_validation builds swallow AppHash and DataHash mismatches; -// mock_chain_validation (M2) swallows every audit-row swallow-eligible -// failure. See package documentation above. +// DefaultConsensusPolicy returns the zero-value policy for the current build. func DefaultConsensusPolicy() ConsensusPolicy { return ConsensusPolicy{} } -// ErrorKind names a swallow-eligible validation failure. Each constant -// corresponds to a row in the M1.0 audit table (docs/designs/ -// mock-chain-validation-m1-audit.md). The string value is the metric label -// emitted on sei_unsafe_validation_skipped_total{kind=...} when the failure -// is swallowed. +// ErrorKind names a swallow-eligible validation failure. Constants +// correspond to rows in the M1.0 audit (docs/designs/ +// mock-chain-validation-m1-audit.md); the string value is the metric label +// emitted on sei_unsafe_validation_skipped_total{kind=...}. type ErrorKind string const ( - // ErrorKindAppHash — internal/state/validation.go AppHash check. - // Audit row 1. block.AppHash vs state.AppHash. Forked-boot block-1 - // blocker; the export's AppHash will not match what the new state - // produces. - ErrorKindAppHash ErrorKind = "app_hash" - - // ErrorKindDataHash — types/block.go Data.Hash() check in - // Block.ValidateBasic. Audit row 2. Forked-boot block-1 blocker. - ErrorKindDataHash ErrorKind = "data_hash" - - // ErrorKindLastResultsHash — internal/state/validation.go - // LastResultsHash check. Audit row 8. Already gated by the - // SkipLastResultsHashValidation atomic for Giga; this policy entry - // allows mock_chain_validation to participate without depending on - // Giga's runtime flip. - ErrorKindLastResultsHash ErrorKind = "last_results_hash" - - // ErrorKindLastBlockID — internal/state/validation.go LastBlockID - // continuity check. Audit row 6. Hash check (BlockID is hash + - // part-set-header); forked-boot diverges here at block-1. - ErrorKindLastBlockID ErrorKind = "last_block_id" - - // ErrorKindConsensusHash — internal/state/validation.go - // ConsensusHash check. Audit row 7. ConsensusParams.Hash mismatch - // fires when the exported chain had different consensus params than - // the new genesis. - ErrorKindConsensusHash ErrorKind = "consensus_hash" - - // ErrorKindValidatorsHash — internal/state/validation.go - // ValidatorsHash check. Audit row 9. Primary block-1 blocker for - // forked-boot with a new validator set. - ErrorKindValidatorsHash ErrorKind = "validators_hash" - - // ErrorKindNextValidatorsHash — internal/state/validation.go - // NextValidatorsHash check. Audit row 10. Sibling to ValidatorsHash - // for next-height validator set. - ErrorKindNextValidatorsHash ErrorKind = "next_validators_hash" - - // ErrorKindLastCommitVerify — internal/state/validation.go - // state.LastValidators.VerifyCommit. Audit row 12. Load-bearing - // block-1 blocker for forked-boot — the new LastCommit is signed by - // the new validators, which will not verify against the exported set. - ErrorKindLastCommitVerify ErrorKind = "last_commit_verify" - - // ErrorKindProposerNotInValidatorSet — internal/state/validation.go - // state.Validators.HasAddress(ProposerAddress) check. Audit row 13. - // Forked-boot with a new validator set: the proposer is in the new - // set, not the exported one. + ErrorKindAppHash ErrorKind = "app_hash" + ErrorKindDataHash ErrorKind = "data_hash" + ErrorKindLastResultsHash ErrorKind = "last_results_hash" + ErrorKindLastBlockID ErrorKind = "last_block_id" + ErrorKindConsensusHash ErrorKind = "consensus_hash" + ErrorKindValidatorsHash ErrorKind = "validators_hash" + ErrorKindNextValidatorsHash ErrorKind = "next_validators_hash" + ErrorKindLastCommitVerify ErrorKind = "last_commit_verify" ErrorKindProposerNotInValidatorSet ErrorKind = "proposer_not_in_validator_set" - - // ErrorKindEvidenceOverflow — internal/state/validation.go - // Evidence.ByteSize > MaxBytes check. Audit row 17. Possible block-1 - // blocker if the export carries evidence near the limit. - ErrorKindEvidenceOverflow ErrorKind = "evidence_overflow" - - // ErrorKindLastCommitHash — types/block.go LastCommitHash check in - // Block.ValidateBasic (with legacy-6.4 fallback). Audit row 18. - // Forked-boot block-1: new LastCommit's hash will not match the - // export's expected LastCommitHash. - ErrorKindLastCommitHash ErrorKind = "last_commit_hash" - - // ErrorKindEvidenceHash — types/block.go EvidenceHash check in - // Block.ValidateBasic. Audit row 19. Hash check. - ErrorKindEvidenceHash ErrorKind = "evidence_hash" - - // ErrorKindPerEvidenceValidateBasic — types/block.go per-evidence - // ValidateBasic loop in Block.ValidateBasic. Audit row 20. - // Cryptographic well-formedness per item; swallowing lets builds - // keep moving past malformed export-carried evidence. - ErrorKindPerEvidenceValidateBasic ErrorKind = "per_evidence_validate_basic" + ErrorKindEvidenceOverflow ErrorKind = "evidence_overflow" + ErrorKindLastCommitHash ErrorKind = "last_commit_hash" + ErrorKindEvidenceHash ErrorKind = "evidence_hash" + ErrorKindPerEvidenceValidateBasic ErrorKind = "per_evidence_validate_basic" ) -// AllSwallowEligibleErrorKinds returns the 13 ErrorKind constants in the -// audit's swallow-eligible set. Useful for tests asserting the variant -// matrix (every test should iterate this list and check ShouldSwallow's -// return for each). +// AllSwallowEligibleErrorKinds returns the audit's swallow-eligible set. +// Tests iterate this list to assert the per-variant matrix. func AllSwallowEligibleErrorKinds() []ErrorKind { return []ErrorKind{ ErrorKindAppHash, diff --git a/sei-tendermint/types/consensus_policy_default.go b/sei-tendermint/types/consensus_policy_default.go index 51cecd2a20..e8d50aff69 100644 --- a/sei-tendermint/types/consensus_policy_default.go +++ b/sei-tendermint/types/consensus_policy_default.go @@ -2,10 +2,6 @@ package types -// ConsensusPolicy in production builds: never swallows. Every call site -// returns the original error on failure. type ConsensusPolicy struct{} -// ShouldSwallow always returns false in production builds. The compiler -// inlines and DCEs the swallow branch at every call site. func (ConsensusPolicy) ShouldSwallow(_ ErrorKind) bool { return false } diff --git a/sei-tendermint/types/consensus_policy_mock_block_validation.go b/sei-tendermint/types/consensus_policy_mock_block_validation.go index c560fb1d10..834d486648 100644 --- a/sei-tendermint/types/consensus_policy_mock_block_validation.go +++ b/sei-tendermint/types/consensus_policy_mock_block_validation.go @@ -2,23 +2,12 @@ package types -// ConsensusPolicy in mock_block_validation builds: swallows AppHash and -// DataHash mismatches. The two checks compute their comparison -// authentically (no more pre-comparison short-circuit), and the divergence -// is logged before continuing. All other audit-row checks halt as in -// production. -// -// Behavior change relative to the prior Skip*Validation shape: -// - The Data.Hash(false) compute fast-path is gone (explicitly accepted). -// - AppHash / DataHash mismatches are now LOGGED in addition to being -// bypassed (previously they were silently skipped). -// -// User-visible outcome is preserved: the chain still progresses past -// AppHash and DataHash mismatches under this tag. +// Swallow set is AppHash + DataHash only — these are the two checks the +// mock_block_validation tag has always relaxed; preserving that exact set +// keeps user-visible outcomes under this tag unchanged across the refactor. +// All other audit-row kinds halt as in production. type ConsensusPolicy struct{} -// ShouldSwallow returns true for the two CometBFT hash checks that the -// mock_block_validation tag has always relaxed. All other ErrorKinds halt. func (ConsensusPolicy) ShouldSwallow(kind ErrorKind) bool { switch kind { case ErrorKindAppHash, ErrorKindDataHash: diff --git a/sei-tendermint/types/consensus_policy_mock_block_validation_test.go b/sei-tendermint/types/consensus_policy_mock_block_validation_test.go index 8171821d72..84f411e9a8 100644 --- a/sei-tendermint/types/consensus_policy_mock_block_validation_test.go +++ b/sei-tendermint/types/consensus_policy_mock_block_validation_test.go @@ -4,9 +4,6 @@ package types import "testing" -// TestConsensusPolicy_MockBlockValidation_Matrix asserts the variant's -// swallow matrix: AppHash and DataHash swallow; the other 11 audit-row -// kinds halt as in production. func TestConsensusPolicy_MockBlockValidation_Matrix(t *testing.T) { policy := DefaultConsensusPolicy() swallowExpected := map[ErrorKind]bool{ @@ -37,8 +34,6 @@ func TestConsensusPolicy_MockBlockValidation_Matrix(t *testing.T) { } } -// TestConsensusPolicy_MockBlockValidation_UnknownKindHalts ensures -// previously unknown kinds halt by default (closed-set semantics). func TestConsensusPolicy_MockBlockValidation_UnknownKindHalts(t *testing.T) { policy := DefaultConsensusPolicy() if policy.ShouldSwallow(ErrorKind("not_a_real_kind")) { diff --git a/sei-tendermint/types/consensus_policy_test.go b/sei-tendermint/types/consensus_policy_test.go index 20d1f51856..7b448820e0 100644 --- a/sei-tendermint/types/consensus_policy_test.go +++ b/sei-tendermint/types/consensus_policy_test.go @@ -4,8 +4,6 @@ package types import "testing" -// TestConsensusPolicy_Default_AllKindsHalt asserts the production variant -// never swallows. Every swallow-eligible ErrorKind must return false. func TestConsensusPolicy_Default_AllKindsHalt(t *testing.T) { policy := DefaultConsensusPolicy() for _, kind := range AllSwallowEligibleErrorKinds() { @@ -15,8 +13,6 @@ func TestConsensusPolicy_Default_AllKindsHalt(t *testing.T) { } } -// TestConsensusPolicy_Default_UnknownKindHalts asserts unrecognized kinds -// also halt (defensive). func TestConsensusPolicy_Default_UnknownKindHalts(t *testing.T) { policy := DefaultConsensusPolicy() if policy.ShouldSwallow(ErrorKind("not_a_real_kind")) { @@ -24,9 +20,8 @@ func TestConsensusPolicy_Default_UnknownKindHalts(t *testing.T) { } } -// TestAllSwallowEligibleErrorKinds_Count asserts the audit's 13-row -// invariant. If this number changes the audit (docs/designs/ -// mock-chain-validation-m1-audit.md) must be revisited. +// Guards the M1.0 audit's 13-row invariant — a change here means the audit +// (docs/designs/mock-chain-validation-m1-audit.md) needs to be revisited. func TestAllSwallowEligibleErrorKinds_Count(t *testing.T) { got := len(AllSwallowEligibleErrorKinds()) if got != 13 { diff --git a/sei-tendermint/types/swallow.go b/sei-tendermint/types/swallow.go index ea4120c203..50c1876a4a 100644 --- a/sei-tendermint/types/swallow.go +++ b/sei-tendermint/types/swallow.go @@ -8,14 +8,9 @@ import ( "github.com/prometheus/client_golang/prometheus/promauto" ) -// unsafeValidationSkippedTotal counts every halting validation failure -// that ConsensusPolicy chose to swallow. Labeled by site (the calling -// file:line, hand-coded at each call) and kind (the ErrorKind constant -// string). Lives at the default Prometheus registry — production builds -// never increment it because ShouldSwallow returns false on all kinds. -// -// Metric name follows the audit's specification: -// sei_unsafe_validation_skipped_total{site, kind}. +// unsafeValidationSkippedTotal — production builds never increment this +// (ShouldSwallow returns false on all kinds and the increment is DCE'd). +// Non-zero values indicate a mock_* tag is in effect. var unsafeValidationSkippedTotal = promauto.NewCounterVec( prometheus.CounterOpts{ Name: "sei_unsafe_validation_skipped_total", @@ -24,15 +19,9 @@ var unsafeValidationSkippedTotal = promauto.NewCounterVec( []string{"site", "kind"}, ) -// LogSwallowedFailure emits the structured log line that accompanies a -// swallowed validation failure. Every call site uses this so the operator -// sees a uniform record of (kind, site, height, expected, got). The -// counter increment is the responsibility of SwallowOrErr — this function -// only logs. -// -// If the logger is nil it falls back to the types package logger -// (declared in part_set.go) so call sites that don't carry one (such as -// Block.ValidateBasic) can still produce a record. +// LogSwallowedFailure emits the structured record for a swallowed failure. +// nil logger falls back to the package logger so call sites without one +// (e.g. Block.ValidateBasic) can still produce a record. func LogSwallowedFailure( lg *slog.Logger, kind ErrorKind, @@ -52,37 +41,15 @@ func LogSwallowedFailure( ) } -// SwallowOrErr applies the ConsensusPolicy failure-handling decision at a -// halting validation site. Call it from the failure branch (i.e. when -// the comparison has already failed); it returns nil to swallow (logging -// + counter) or a formatted error to halt. -// -// Typical use: -// -// if !bytes.Equal(block.AppHash, state.AppHash) { -// if err := types.SwallowOrErr(policy, types.ErrorKindAppHash, logger, site, -// block.Height, state.AppHash, block.AppHash, -// "wrong Block.Header.AppHash. Expected %X, got %v", -// state.AppHash, block.AppHash); err != nil { -// return err -// } -// } +// SwallowOrErr is the contract every halting check funnels through. +// Call from the failure branch (comparison already failed): // -// Skip-before-compute is intentionally no longer supported — every check -// computes its comparison authentically; only the failure-handling -// differs per ConsensusPolicy. +// - returns nil → caller continues past the failure (logged + counter +// incremented). +// - returns a formatted error → caller halts with that error. // -// Parameters: -// - policy: the per-binary ConsensusPolicy (build-tag-selected). -// - kind: the audit-row ErrorKind being checked. -// - lg: *slog.Logger to receive the divergence record. nil falls -// back to the types package logger. -// - site: file:line of the calling check (helps trace which audit row -// fired in metrics + logs). -// - height: block height of the failing block. -// - expected, got: the two values being compared. Logged hex-formatted. -// - format, args: the original fmt.Errorf format the call site would -// have used to halt. Returned on the halt path. +// Pre-comparison short-circuits are intentionally not supported — every +// check computes its comparison; only the failure handling is policy-driven. func SwallowOrErr( policy ConsensusPolicy, kind ErrorKind, From fb414ae44fc1a7ba2039d92b29ad3179ece07cf6 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Thu, 14 May 2026 13:24:50 -0700 Subject: [PATCH 4/4] refactor(consensus-policy): collapse SwallowOrErr into ShouldSwallow(kind, err) ShouldSwallow now takes the candidate error directly and returns either nil (swallow + counter increment, in the variant that opts in) or the same error unchanged (halt). Counter increments move inside the per-variant ShouldSwallow body; the site label is dropped from the metric, leaving only kind. The external SwallowOrErr / LogSwallowedFailure helpers and every per-site site/logger/expected/got argument are removed. Production behavior is unchanged: the default variant returns err for every kind. mock_block_validation outcomes are unchanged: AppHash and DataHash mismatches still don't halt, just via a slightly different code path. Co-Authored-By: Claude Opus 4.7 --- sei-tendermint/internal/state/validation.go | 73 +++++++------------ sei-tendermint/types/block.go | 24 ++---- sei-tendermint/types/consensus_policy.go | 18 +++-- .../types/consensus_policy_default.go | 2 +- .../consensus_policy_mock_block_validation.go | 7 +- ...ensus_policy_mock_block_validation_test.go | 27 +++++-- sei-tendermint/types/consensus_policy_test.go | 19 +++-- sei-tendermint/types/swallow.go | 59 +-------------- 8 files changed, 85 insertions(+), 144 deletions(-) diff --git a/sei-tendermint/internal/state/validation.go b/sei-tendermint/internal/state/validation.go index 854858a75b..2b3155a3c3 100644 --- a/sei-tendermint/internal/state/validation.go +++ b/sei-tendermint/internal/state/validation.go @@ -12,8 +12,9 @@ import ( // Validate block // Swallow-eligible failure sites in this function consult -// types.ConsensusPolicy via types.SwallowOrErr; the audit-row ErrorKind -// passed at each site is the cross-reference (see types.ErrorKind*). +// types.ConsensusPolicy via ShouldSwallow(kind, err); the audit-row +// ErrorKind passed at each site is the cross-reference (see +// types.ErrorKind*). func validateBlock(state State, block *types.Block, policy types.ConsensusPolicy) error { // Validate internal consistency. if err := block.ValidateBasic(policy); err != nil { @@ -45,32 +46,26 @@ func validateBlock(state State, block *types.Block, policy types.ConsensusPolicy } // Validate prev block info. if !block.LastBlockID.Equals(state.LastBlockID) { - if err := types.SwallowOrErr(policy, types.ErrorKindLastBlockID, logger, - "internal/state/validation.go:LastBlockID", block.Height, - state.LastBlockID, block.LastBlockID, - "wrong Block.Header.LastBlockID. Expected %v, got %v", - state.LastBlockID, block.LastBlockID); err != nil { + if err := policy.ShouldSwallow(types.ErrorKindLastBlockID, + fmt.Errorf("wrong Block.Header.LastBlockID. Expected %v, got %v", + state.LastBlockID, block.LastBlockID)); err != nil { return err } } // Validate app info. if !bytes.Equal(block.AppHash, state.AppHash) { - if err := types.SwallowOrErr(policy, types.ErrorKindAppHash, logger, - "internal/state/validation.go:AppHash", block.Height, - state.AppHash, block.AppHash, - "wrong Block.Header.AppHash. Expected %X, got %v", - state.AppHash, block.AppHash); err != nil { + if err := policy.ShouldSwallow(types.ErrorKindAppHash, + fmt.Errorf("wrong Block.Header.AppHash. Expected %X, got %v", + state.AppHash, block.AppHash)); err != nil { return err } } hashCP := state.ConsensusParams.HashConsensusParams() if !bytes.Equal(block.ConsensusHash, hashCP) { - if err := types.SwallowOrErr(policy, types.ErrorKindConsensusHash, logger, - "internal/state/validation.go:ConsensusHash", block.Height, - hashCP, block.ConsensusHash, - "wrong Block.Header.ConsensusHash. Expected %X, got %v", - hashCP, block.ConsensusHash); err != nil { + if err := policy.ShouldSwallow(types.ErrorKindConsensusHash, + fmt.Errorf("wrong Block.Header.ConsensusHash. Expected %X, got %v", + hashCP, block.ConsensusHash)); err != nil { return err } } @@ -82,30 +77,24 @@ func validateBlock(state State, block *types.Block, policy types.ConsensusPolicy // variant is its own future workstream. if !types.SkipLastResultsHashValidation.Load() { if !bytes.Equal(block.LastResultsHash, state.LastResultsHash) { - if err := types.SwallowOrErr(policy, types.ErrorKindLastResultsHash, logger, - "internal/state/validation.go:LastResultsHash", block.Height, - state.LastResultsHash, block.LastResultsHash, - "wrong Block.Header.LastResultsHash. Expected %X, got %v", - state.LastResultsHash, block.LastResultsHash); err != nil { + if err := policy.ShouldSwallow(types.ErrorKindLastResultsHash, + fmt.Errorf("wrong Block.Header.LastResultsHash. Expected %X, got %v", + state.LastResultsHash, block.LastResultsHash)); err != nil { return err } } } if !bytes.Equal(block.ValidatorsHash, state.Validators.Hash()) { - if err := types.SwallowOrErr(policy, types.ErrorKindValidatorsHash, logger, - "internal/state/validation.go:ValidatorsHash", block.Height, - state.Validators.Hash(), block.ValidatorsHash, - "wrong Block.Header.ValidatorsHash. Expected %X, got %v", - state.Validators.Hash(), block.ValidatorsHash); err != nil { + if err := policy.ShouldSwallow(types.ErrorKindValidatorsHash, + fmt.Errorf("wrong Block.Header.ValidatorsHash. Expected %X, got %v", + state.Validators.Hash(), block.ValidatorsHash)); err != nil { return err } } if !bytes.Equal(block.NextValidatorsHash, state.NextValidators.Hash()) { - if err := types.SwallowOrErr(policy, types.ErrorKindNextValidatorsHash, logger, - "internal/state/validation.go:NextValidatorsHash", block.Height, - state.NextValidators.Hash(), block.NextValidatorsHash, - "wrong Block.Header.NextValidatorsHash. Expected %X, got %v", - state.NextValidators.Hash(), block.NextValidatorsHash); err != nil { + if err := policy.ShouldSwallow(types.ErrorKindNextValidatorsHash, + fmt.Errorf("wrong Block.Header.NextValidatorsHash. Expected %X, got %v", + state.NextValidators.Hash(), block.NextValidatorsHash)); err != nil { return err } } @@ -119,10 +108,8 @@ func validateBlock(state State, block *types.Block, policy types.ConsensusPolicy // LastCommit.Signatures length is checked in VerifyCommit. if err := state.LastValidators.VerifyCommit( state.ChainID, state.LastBlockID, block.Height-1, block.LastCommit); err != nil { - if swErr := types.SwallowOrErr(policy, types.ErrorKindLastCommitVerify, logger, - "internal/state/validation.go:LastCommitVerify", block.Height, - state.LastBlockID, err.Error(), - "VerifyCommit(): %w", err); swErr != nil { + if swErr := policy.ShouldSwallow(types.ErrorKindLastCommitVerify, + fmt.Errorf("VerifyCommit(): %w", err)); swErr != nil { return swErr } } @@ -133,11 +120,9 @@ func validateBlock(state State, block *types.Block, policy types.ConsensusPolicy // a legit address and a known validator. // The length is checked in ValidateBasic above. if !state.Validators.HasAddress(block.ProposerAddress) { - if err := types.SwallowOrErr(policy, types.ErrorKindProposerNotInValidatorSet, logger, - "internal/state/validation.go:ProposerNotInValidatorSet", block.Height, - "", block.ProposerAddress, - "block.Header.ProposerAddress %X is not a validator", - block.ProposerAddress); err != nil { + if err := policy.ShouldSwallow(types.ErrorKindProposerNotInValidatorSet, + fmt.Errorf("block.Header.ProposerAddress %X is not a validator", + block.ProposerAddress)); err != nil { return err } } @@ -168,10 +153,8 @@ func validateBlock(state State, block *types.Block, policy types.ConsensusPolicy // Check evidence doesn't exceed the limit amount of bytes. if max, got := state.ConsensusParams.Evidence.MaxBytes, block.Evidence.ByteSize(); got > max { - if err := types.SwallowOrErr(policy, types.ErrorKindEvidenceOverflow, logger, - "internal/state/validation.go:EvidenceOverflow", block.Height, - max, got, - "%w", types.NewErrEvidenceOverflow(max, got)); err != nil { + if err := policy.ShouldSwallow(types.ErrorKindEvidenceOverflow, + types.NewErrEvidenceOverflow(max, got)); err != nil { return err } } diff --git a/sei-tendermint/types/block.go b/sei-tendermint/types/block.go index cddadcfd1d..1f55bb83d5 100644 --- a/sei-tendermint/types/block.go +++ b/sei-tendermint/types/block.go @@ -91,10 +91,8 @@ func (b *Block) ValidateBasic(policy ConsensusPolicy) error { if w, g := b.LastCommit.Hash(), b.LastCommitHash; !bytes.Equal(w, g) { // Fall back to legacy hash calculation pre-6.4. if wLegacy := b.LastCommit.legacyHash(); !bytes.Equal(wLegacy, g) { - if err := SwallowOrErr(policy, ErrorKindLastCommitHash, logger, - "types/block.go:LastCommitHash", b.Height, - w, g, - "wrong Header.LastCommitHash. Expected %X, got %X", w, g); err != nil { + if err := policy.ShouldSwallow(ErrorKindLastCommitHash, + fmt.Errorf("wrong Header.LastCommitHash. Expected %X, got %X", w, g)); err != nil { return err } } @@ -102,10 +100,8 @@ func (b *Block) ValidateBasic(policy ConsensusPolicy) error { // NOTE: b.Data.Txs may be nil, but b.Data.Hash() still works fine. if w, g := b.Data.Hash(false), b.DataHash; !bytes.Equal(w, g) { - if err := SwallowOrErr(policy, ErrorKindDataHash, logger, - "types/block.go:DataHash", b.Height, - w, g, - "wrong Header.DataHash. Expected %X, got %X. Len of txs %d", w, g, len(b.Txs)); err != nil { + if err := policy.ShouldSwallow(ErrorKindDataHash, + fmt.Errorf("wrong Header.DataHash. Expected %X, got %X. Len of txs %d", w, g, len(b.Txs))); err != nil { return err } } @@ -113,20 +109,16 @@ func (b *Block) ValidateBasic(policy ConsensusPolicy) error { // NOTE: b.Evidence may be nil, but we're just looping. for i, ev := range b.Evidence { if err := ev.ValidateBasic(); err != nil { - if swErr := SwallowOrErr(policy, ErrorKindPerEvidenceValidateBasic, logger, - "types/block.go:PerEvidenceValidateBasic", b.Height, - i, err.Error(), - "invalid evidence (#%d): %v", i, err); swErr != nil { + if swErr := policy.ShouldSwallow(ErrorKindPerEvidenceValidateBasic, + fmt.Errorf("invalid evidence (#%d): %v", i, err)); swErr != nil { return swErr } } } if w, g := b.Evidence.Hash(), b.EvidenceHash; !bytes.Equal(w, g) { - if err := SwallowOrErr(policy, ErrorKindEvidenceHash, logger, - "types/block.go:EvidenceHash", b.Height, - w, g, - "wrong Header.EvidenceHash. Expected %X, got %X", w, g); err != nil { + if err := policy.ShouldSwallow(ErrorKindEvidenceHash, + fmt.Errorf("wrong Header.EvidenceHash. Expected %X, got %X", w, g)); err != nil { return err } } diff --git a/sei-tendermint/types/consensus_policy.go b/sei-tendermint/types/consensus_policy.go index 67c79001e1..4eef560114 100644 --- a/sei-tendermint/types/consensus_policy.go +++ b/sei-tendermint/types/consensus_policy.go @@ -1,14 +1,16 @@ // Package types — ConsensusPolicy is a zero-sized, build-tag-selected gate // that decides, per ErrorKind, whether a halting validation failure halts -// (default) or is swallowed (logged + counter, then continued). The single -// method ShouldSwallow(kind) is declared in exactly one of three per-tag -// files, so each binary compiles in one fixed policy with no runtime branch: +// (default) or is swallowed (counter incremented, then continued). The +// single method ShouldSwallow(kind, err) is declared in exactly one of three +// per-tag files, so each binary compiles in one fixed policy with no runtime +// branch: // -// default (production) → always false; the swallow branch is DCE'd -// mock_block_validation → true for AppHash and DataHash; preserves the -// long-standing behavior of that tag -// mock_chain_validation → true for every swallow-eligible audit-row kind -// (M2 deliverable) +// default (production) → returns err for every kind; production halting +// semantics are unchanged +// mock_block_validation → returns nil for AppHash and DataHash; preserves +// the long-standing behavior of that tag +// mock_chain_validation → returns nil for every swallow-eligible audit-row +// kind (M2 deliverable) // // One Skip*-style early-return is preserved alongside the policy: // tmtypes.SkipLastResultsHashValidation; see validation.go for context. diff --git a/sei-tendermint/types/consensus_policy_default.go b/sei-tendermint/types/consensus_policy_default.go index e8d50aff69..b809f9e6a3 100644 --- a/sei-tendermint/types/consensus_policy_default.go +++ b/sei-tendermint/types/consensus_policy_default.go @@ -4,4 +4,4 @@ package types type ConsensusPolicy struct{} -func (ConsensusPolicy) ShouldSwallow(_ ErrorKind) bool { return false } +func (ConsensusPolicy) ShouldSwallow(_ ErrorKind, err error) error { return err } diff --git a/sei-tendermint/types/consensus_policy_mock_block_validation.go b/sei-tendermint/types/consensus_policy_mock_block_validation.go index 834d486648..2eeddc3d83 100644 --- a/sei-tendermint/types/consensus_policy_mock_block_validation.go +++ b/sei-tendermint/types/consensus_policy_mock_block_validation.go @@ -8,11 +8,12 @@ package types // All other audit-row kinds halt as in production. type ConsensusPolicy struct{} -func (ConsensusPolicy) ShouldSwallow(kind ErrorKind) bool { +func (ConsensusPolicy) ShouldSwallow(kind ErrorKind, err error) error { switch kind { case ErrorKindAppHash, ErrorKindDataHash: - return true + unsafeValidationSkippedTotal.WithLabelValues(string(kind)).Inc() + return nil default: - return false + return err } } diff --git a/sei-tendermint/types/consensus_policy_mock_block_validation_test.go b/sei-tendermint/types/consensus_policy_mock_block_validation_test.go index 84f411e9a8..d2ef99d208 100644 --- a/sei-tendermint/types/consensus_policy_mock_block_validation_test.go +++ b/sei-tendermint/types/consensus_policy_mock_block_validation_test.go @@ -2,10 +2,14 @@ package types -import "testing" +import ( + "errors" + "testing" +) func TestConsensusPolicy_MockBlockValidation_Matrix(t *testing.T) { policy := DefaultConsensusPolicy() + testErr := errors.New("sentinel") swallowExpected := map[ErrorKind]bool{ ErrorKindAppHash: true, ErrorKindDataHash: true, @@ -22,21 +26,28 @@ func TestConsensusPolicy_MockBlockValidation_Matrix(t *testing.T) { ErrorKindPerEvidenceValidateBasic: false, } for _, kind := range AllSwallowEligibleErrorKinds() { - want, ok := swallowExpected[kind] + swallow, ok := swallowExpected[kind] if !ok { t.Errorf("test matrix missing entry for ErrorKind %q — audit added a new row?", kind) continue } - if got := policy.ShouldSwallow(kind); got != want { - t.Errorf("mock_block_validation ConsensusPolicy.ShouldSwallow(%q) = %v, want %v", - kind, got, want) + got := policy.ShouldSwallow(kind, testErr) + if swallow { + if got != nil { + t.Errorf("mock_block_validation ConsensusPolicy.ShouldSwallow(%q, testErr) = %v, want nil", kind, got) + } + } else { + if got != testErr { + t.Errorf("mock_block_validation ConsensusPolicy.ShouldSwallow(%q, testErr) = %v, want testErr", kind, got) + } } } } -func TestConsensusPolicy_MockBlockValidation_UnknownKindHalts(t *testing.T) { +func TestConsensusPolicy_MockBlockValidation_UnknownKindReturnsErr(t *testing.T) { policy := DefaultConsensusPolicy() - if policy.ShouldSwallow(ErrorKind("not_a_real_kind")) { - t.Errorf("mock_block_validation ConsensusPolicy.ShouldSwallow(unknown) = true, want false") + testErr := errors.New("sentinel") + if got := policy.ShouldSwallow(ErrorKind("not_a_real_kind"), testErr); got != testErr { + t.Errorf("mock_block_validation ConsensusPolicy.ShouldSwallow(unknown, testErr) = %v, want testErr", got) } } diff --git a/sei-tendermint/types/consensus_policy_test.go b/sei-tendermint/types/consensus_policy_test.go index 7b448820e0..e627ec43a4 100644 --- a/sei-tendermint/types/consensus_policy_test.go +++ b/sei-tendermint/types/consensus_policy_test.go @@ -2,21 +2,26 @@ package types -import "testing" +import ( + "errors" + "testing" +) -func TestConsensusPolicy_Default_AllKindsHalt(t *testing.T) { +func TestConsensusPolicy_Default_AllKindsReturnErr(t *testing.T) { policy := DefaultConsensusPolicy() + testErr := errors.New("sentinel") for _, kind := range AllSwallowEligibleErrorKinds() { - if policy.ShouldSwallow(kind) { - t.Errorf("default ConsensusPolicy.ShouldSwallow(%q) = true, want false", kind) + if got := policy.ShouldSwallow(kind, testErr); got != testErr { + t.Errorf("default ConsensusPolicy.ShouldSwallow(%q, testErr) = %v, want testErr", kind, got) } } } -func TestConsensusPolicy_Default_UnknownKindHalts(t *testing.T) { +func TestConsensusPolicy_Default_UnknownKindReturnsErr(t *testing.T) { policy := DefaultConsensusPolicy() - if policy.ShouldSwallow(ErrorKind("not_a_real_kind")) { - t.Errorf("default ConsensusPolicy.ShouldSwallow(unknown) = true, want false") + testErr := errors.New("sentinel") + if got := policy.ShouldSwallow(ErrorKind("not_a_real_kind"), testErr); got != testErr { + t.Errorf("default ConsensusPolicy.ShouldSwallow(unknown, testErr) = %v, want testErr", got) } } diff --git a/sei-tendermint/types/swallow.go b/sei-tendermint/types/swallow.go index 50c1876a4a..fe61d565c0 100644 --- a/sei-tendermint/types/swallow.go +++ b/sei-tendermint/types/swallow.go @@ -1,69 +1,16 @@ package types import ( - "fmt" - "log/slog" - "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" ) -// unsafeValidationSkippedTotal — production builds never increment this -// (ShouldSwallow returns false on all kinds and the increment is DCE'd). -// Non-zero values indicate a mock_* tag is in effect. +// unsafeValidationSkippedTotal counts halting validation failures that were +// swallowed by a non-default ConsensusPolicy. Always zero in production builds. var unsafeValidationSkippedTotal = promauto.NewCounterVec( prometheus.CounterOpts{ Name: "sei_unsafe_validation_skipped_total", Help: "Halting validation failures swallowed by a non-default ConsensusPolicy (mock_block_validation, mock_chain_validation). Always zero in production builds.", }, - []string{"site", "kind"}, + []string{"kind"}, ) - -// LogSwallowedFailure emits the structured record for a swallowed failure. -// nil logger falls back to the package logger so call sites without one -// (e.g. Block.ValidateBasic) can still produce a record. -func LogSwallowedFailure( - lg *slog.Logger, - kind ErrorKind, - site string, - height int64, - expected, got interface{}, -) { - if lg == nil { - lg = logger - } - lg.Warn("swallowed validation failure (unsafe ConsensusPolicy)", - "kind", string(kind), - "site", site, - "height", height, - "expected", fmt.Sprintf("%X", expected), - "got", fmt.Sprintf("%X", got), - ) -} - -// SwallowOrErr is the contract every halting check funnels through. -// Call from the failure branch (comparison already failed): -// -// - returns nil → caller continues past the failure (logged + counter -// incremented). -// - returns a formatted error → caller halts with that error. -// -// Pre-comparison short-circuits are intentionally not supported — every -// check computes its comparison; only the failure handling is policy-driven. -func SwallowOrErr( - policy ConsensusPolicy, - kind ErrorKind, - lg *slog.Logger, - site string, - height int64, - expected, got interface{}, - format string, - args ...interface{}, -) error { - if policy.ShouldSwallow(kind) { - LogSwallowedFailure(lg, kind, site, height, expected, got) - unsafeValidationSkippedTotal.WithLabelValues(site, string(kind)).Inc() - return nil - } - return fmt.Errorf(format, args...) -}