Repository navigation
txnprovider/txpool: verify blob KZG proofs outside the pool lock - #25570
Conversation
Blob sidecar checks move from validateBlobTxn into prepareTxns, which runs them after the locked precheck and before the locked insert.
There was a problem hiding this comment.
🟡 Changes recommended
Unwind admission bypasses blob-sidecar validation, allowing a mined transaction without sidecars to trigger a panic or a remote-queue transaction with an unverified sidecar to enter the pool.
1 open issue
What changed in this PR
This PR reduces transaction-pool lock contention by moving blob KZG proof verification into the preparation phase, outside the pool lock.
Changes:
- Splits locked blob admission checks from sidecar verification.
- Extends transaction preparation to verify blobs and preserve per-peer short-circuiting after invalid proofs.
- Updates tests for preparation behavior, invalid-proof rejection, and locked validation.
| File | Description |
|---|---|
txnprovider/txpool/pool.go |
Separates blob sidecar verification from locked admission and preserves checks during database loading. |
txnprovider/txpool/pool_blob_kzg_shortcircuit_test.go |
Updates short-circuit tests and adds coverage for the validation split. |
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Sidecar verification now overlaps concurrent imports and block updates, but focused race-detector validation of these interleavings cannot be run in this review environment.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Remote batches without a peer ID continue verifying blob proofs after the first KZG failure despite later being discarded, wasting verification work and regressing the previous short-circuit behavior.
1 open issue
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Human assessment is needed because an unpublished finding indicates the regression test would not detect pool-lock contention returning during sidecar verification.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
# Conflicts: # txnprovider/txpool/pool.go # txnprovider/txpool/pool_blob_kzg_shortcircuit_test.go
# Conflicts: # txnprovider/txpool/pool.go # txnprovider/txpool/pool_blob_kzg_shortcircuit_test.go
| } | ||
| } | ||
|
|
||
| func TestBlobKZGVerifiedOutsidePoolLock(t *testing.T) { |
There was a problem hiding this comment.
Both subtests still pass if prepareTxns verifies sidecars under p.lock, so the lock fix has no guard.
There was a problem hiding this comment.
Agreed, neither subtest would catch the check moving back under p.lock. A test can only see that from inside the KZG call, so a deterministic guard needs a test hook in production code (txpool has none so far). For now it is structural: in prepareTxns the validateBlobSidecar loop runs after withLockedState returns. If you want the hook-based guard anyway, I will add it in a follow-up.
Bottleneck:
VerifyCellProofBatch/VerifyBlobKZGProofBatchran underp.lock, blocking p2p parsing (IdHashKnown),GetRlp, block building (best()) andOnNewBlock. Fix: verify blob sidecars inprepareTxnsbetween the locked precheck and the locked insert, like SetCode authorizations.