Skip to content

fix(core): validate StorageProof structure in CheckTx#316

Open
raymondjacobson wants to merge 1 commit into
mainfrom
fix/checktx-validate-storage-proof
Open

fix(core): validate StorageProof structure in CheckTx#316
raymondjacobson wants to merge 1 commit into
mainfrom
fix/checktx-validate-storage-proof

Conversation

@raymondjacobson
Copy link
Copy Markdown
Contributor

Summary

`CheckTx` previously accepted any transaction that parsed as valid protobuf — no per-type validation. This meant a malformed `StorageProof` (e.g. with empty `ProverAddresses`) could enter the mempool and get gossiped across the network, even though it would later be rejected at `ProcessProposal` (#315).

This PR adds a `checkTxStructural` step to `CheckTx` that performs lightweight, context-free validation before a transaction enters the mempool. For `StorageProof` txs it checks:

  • `ProverAddresses` is non-empty
  • `Address` (prover comet address) is set
  • `Height` is non-zero

These checks are purely structural — no DB lookups, no block height needed — so they add negligible latency to `CheckTx`.

The `checkTxStructural` method uses a type switch and is easy to extend with structural checks for other tx types in the future.

Defense in depth (full picture after #313, #315, this PR)

Layer Guard Prevents
Submission (`sendPoSChallengeToStorage`) Early-return on empty proverAddresses (#315) Bad tx from being created locally
CheckTx (`checkTxStructural`) Reject empty ProverAddresses (this PR) Bad tx from entering mempool / gossip
ProcessProposal (`isValidStorageProofTx`) Reject empty ProverAddresses (#315) Bad tx from being included in a block
FinalizeBlock (`finalizeStorageProof`) Coerce nil → `[]string{}` (#313) NULL constraint violation if all else fails

Test plan

  • CI passes
  • Submit a `StorageProof` tx with empty `ProverAddresses` via `SendTransaction` RPC — should get rejected with code 1 (not enter mempool)
  • Normal `StorageProof` txs with valid fields still pass `CheckTx`

🤖 Generated with Claude Code

CheckTx previously only verified that a transaction was valid protobuf,
accepting any structurally malformed tx into the mempool. Add a
checkTxStructural step that rejects transactions with obviously invalid
fields before they enter the mempool and get gossiped to peers.

For StorageProof txs, reject if ProverAddresses is empty, Address is
missing, or Height is zero. These are cheap checks that don't need
block context or DB lookups.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant