Skip to content

subtreevalidation: local store decode fault in the conflict check strikes or drops the announcing peer #2072

Open

Description

@icellan

When this node can't read or decode its own UTXO record during the conflict check, block validation blames the peer that sent the block.

What happens

checkCounterConflictingOnCurrentChainBatch wraps its store read failures as ProcessingError (services/subtreevalidation/check_counter_conflicting_batch.go ~300, ~355, ~462). The unbatched decorate path wraps the same GetMeta failure as StorageError (processTxMetaUsingStore.go ~397). So one fault gets two codes.

If the aerospike store reads a record but can't decode it, classifyRecordError returns TX_INVALID. That gives ProcessingError(TX_INVALID), which settledError then wraps as CONTEXT_CANCELED (the ErrTxInvalid arm of keepsCancellation, added in #2066).

In ValidateBlock (services/blockvalidation/BlockValidation.go ~2368) the error enters the ErrTxInvalid branch. The infrastructure guard there checks for context.Canceled by text and for ErrStorageError / ErrServiceError by code. None of them match:

  • HTTP route: NewBlockCorruptError + penalizeCorruptBlockPeer. The honest announcing peer gets a strike, and gets it again on every Kafka retry until the corrupt-attempt cap.
  • Legacy route: NewBlockInvalidError, which parkCommitFailure maps to parkDispositionBlockInvalid. The block hash is marked failed and the peer is dropped, all for our own store fault.

proven_conflict_classification_test.go (~319, corrupt: true) pins the current behaviour.

Fix

  • Wrap the conflict check's store read failures as StorageError. The walk limit, frozen output and out-of-range input stay ProcessingError. The existing ErrStorageError handling then covers it in ValidateBlock's guard, in isRecoverableBlockMessageError and in the catch-up classifier, which labels it local_storage_fault.
  • Drop the ErrTxInvalid arm from keepsCancellation. Flip the test case to corrupt: false and add a legacy-route case.
  • As a related change, give the missing-record case (ErrTxNotFound / missingRecordsError) a local code too, instead of relying on the CONTEXT_CANCELED wrap. Otherwise logs and catch-up report pruned records as cancellations.
  • Optionally, add errors.Is(err, errors.ErrContextCanceled) to the ValidateBlock guard as a backstop.

Found while reviewing #2066. It predates that PR.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions