Skip to content

node/cn: verify a blob tx sidecar only once - #1012

Open
hyunsooda wants to merge 2 commits into
kaiachain:devfrom
hyunsooda:fix/skip-verified-blob-sidecar-replays
Open

node/cn: verify a blob tx sidecar only once#1012
hyunsooda wants to merge 2 commits into
kaiachain:devfrom
hyunsooda:fix/skip-verified-blob-sidecar-replays

Conversation

@hyunsooda

@hyunsooda hyunsooda commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Proposed changes

A replayed blob transaction is decoded into a fresh sidecar object, so the sidecar's own validation cache never hits and the handler verified its KZG proofs again on every copy. The handler now skips verification for blob tx hashes it already verified; the tx pool still verifies the sidecar independently before admitting the transaction.

Types of changes

  • 🐛 Bug fix
  • ✨ Non-hardfork changes (node upgrade not required)
  • 💥 Hardfork / consensus-breaking changes
  • 🧪 Test improvements
  • 🧰 CI / build tool
  • ♻️ Chore / Refactor / Non-functional changes

Checklist

  • 📖 I have read the CONTRIBUTING GUIDELINES doc
  • 📝 I have signed in the PR comment I have read the CLA Document and I hereby sign the CLA in first time contribute after having read CLA
  • 🟢 Lint and unit tests pass locally with my changes ($ make test)

Related issues

Further comments

A replayed blob transaction is decoded into a fresh sidecar object, so the
sidecar's own validation cache never hits and the handler verified its KZG
proofs again on every copy. One single-blob v1 sidecar measures ~15.5ms, and a
12MiB TxMsg fits about 90 of them, so a captured transaction could be resent
indefinitely at no cost to the sender. The handler now keeps a bounded set of
blob tx hashes it already verified and skips the verification for those.

Constraint: the tx pool verifies the sidecar independently before admitting a transaction, so skipping the early check cannot let a bad sidecar through
Rejected: reject duplicate hashes within one message | subsumed, the set already skips the repeats inside a single batch
Rejected: global KZG semaphore or per-peer token buckets | the repeated verification is what made a small message expensive, and bounding concurrency would also delay legitimate propagation
Confidence: high
Scope-risk: narrow
Not-tested: a rotating set of distinct captured blob txs still costs one verification each, which is bandwidth-bound

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@hyunsooda hyunsooda self-assigned this Aug 4, 2026
@ian0371
ian0371 requested a review from 2dvorak August 7, 2026 08:41
Comment thread node/cn/handler_msg_test.go Outdated

// The tx hash does not cover the sidecar, so the replay carries a broken proof
// under the same hash and passes only because the verification is skipped.
blobTx.BlobTxSidecar().Proofs[0][0] ^= 0xFF

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cache key doesn't cover the sidecar, so this happens. Is this ok?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to keccak(blobHashes ‖ Version ‖ Blobs ‖ Proofs)

@2dvorak 2dvorak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. tx.Hash() as the key is bypassable at zero cost. The expensive work is per-sidecar, but the key is per-transaction, and the attacker controls that mapping: one precomputed sidecar attaches to unlimited distinct transactions by bumping a nonce — identical sidecar bytes, different tx hashes — so every one of them misses the set and pays a full verification.
  2. Reject blob txs with zero blob hashes. ValidateWithBlobHashes passes vacuously when there are no blobs so a 113-byte blob tx with an empty sidecar validates. The pool rejects these unconditionally (tx_pool.go:1071), so the handler accepts and forwards a transaction that can never be valid and never disconnects the peer for it. It also mints a cache entry per tx, cheap enough to flush the whole set for ~113 KiB — though that part goes away once the key changes per (1).

The set was keyed by transaction hash, which does not cover the sidecar. A sender
could bump the nonce to replay one sidecar under unlimited distinct hashes, so every
copy missed the set and paid a full verification, and a sidecar swapped under an
already verified hash hit the set and skipped verification entirely.

Key the set by what the verification consumes instead: the blob hashes, the sidecar
version, its blobs and its proofs. Commitments are excluded because each blob hash is
the sha256 of one, so the hashes already pin them.

Also reject a blob transaction that declares no blob hashes. Every length check in
the verification compares against the declared hashes, so it passes vacuously, while
the pool rejects such a transaction unconditionally - the handler forwarded something
that could never be valid and left the sender connected.

Constraint: the key has to cover the blobs, not only the proofs, since the proofs are
  verified against them
Rejected: keep the transaction hash and add a sidecar fingerprint | any key that omits
  the blobs is bypassable the same way
Confidence: high
Scope-risk: narrow
Not-tested: a rotating set of distinct captured blob txs still costs one verification
  each, which is bandwidth-bound

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@hyunsooda

Copy link
Copy Markdown
Contributor Author

@2dvorak The key now covers what the verification consumes, and rejects blobless tx earlier before the veficiation runs.

Comment thread node/cn/handler.go
)

// blobSidecarKey identifies what ValidateWithBlobHashes consumes. Commitments are left
// out because each hash is the sha256 of one, so the hashes already pin them.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commitments are excluded from the key on the grounds that the blob hashes already pin them — but the check that enforces that binding, ValidateBlobCommitmentHashes, is exactly what a cache hit skips. So an attacker can swap the commitment of an already-verified sidecar: the key still matches, validation is skipped, and the tx reaches the pool with the peer left connected. No CPU is burnt — the skip is the cheap path — but the misbehavior signal is silenced, where dev today disconnects on any malformed sidecar.

In addition, prefixing the element counts would make the key encoding unambiguous.

What do you think?

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.

3 participants