Skip to content

Preserve frame RPC fields and respect sender mempool concurrency - #306

Merged
pk910 merged 2 commits into
masterfrom
fix/bogota-frame-rpc-recovery
Oct 20, 2026
Merged

pk910 merged 2 commits into
masterfrom
fix/bogota-frame-rpc-recovery

Conversation

@qu0b

@qu0b qu0b commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

Frame RPC decoding still loses execution/state gas when clients use executionGas/stateGas, and Geth's empty signer: "0x" makes the decoder silently fall back to UnknownTx. Accept the client field spellings and preserve empty signer bytes so the reconstructed transaction hashes match the chain. Signers must remain empty or 20 bytes; other lengths return a decoder error instead of being silently dropped during JSON re-encoding.

The frametx scenario also pipelines several transactions from the same sender before confirmation, conflicting with EIP-8141 public mempool acceptance rule 7. Hold a per-wallet slot until completion. This changes the valid-transaction sender's scheduling; it does not relax transaction validation, rewrite fees/nonces in response to client errors, exclude clients, or change frametx-fuzz.

Tests include captured responses for the same transaction from Geth, Ethrex, Nethermind and Reth, nonzero gas aliases, explicit-zero nonce precedence, and wallet-slot cancellation/serialization. The fixtures also cover the nonce decoding fixed independently in #302, which this branch includes from current master.

Validation: go test ./txtypes ./spamoor ./scenario ./scenarios/frametx, targeted race tests, targeted go vet, and the core import check pass. The rebased decoder and scenario tests pass. The full CI checks, including go test -race -vet=off ./..., pass. A separate three-minute canary against all existing bogota-devnet-1 RPCs (100 wallets, target 100 tx/block) verified 1,330 frame receipts with zero mismatches, zero missing receipts and no submission warnings; pending remained at or below 51 in the per-block samples. A local Geth/Nimbus test at target 1,000 tx/block and 500 wallets verified 9,475 receipts with zero mismatches or missing receipts. Nimbus rejects non-blob frames as invalid blobs; those client errors remain visible with this patch.

Deployed the tested image to bogota-devnet-1 via ethpandaops/bogota-devnets#10. The first five minutes confirmed 11,815 transactions, with the last 20 blocks averaging 488.4 tx/block and sampled pending peaking at 464 (500 wallets). No nonce-too-high or nonce-mismatch errors occurred. There were 41 replacement-underpriced warnings; these remain an open observation, with no fee/error/client-specific workaround added. This is a short recovery observation, not a long soak.

Follow-up observation (2026-10-20): the deployed frametx scenario ran from 15:07 to 22:18 UTC on October 19 before being paused. It verified 1,075,916 frame receipts with zero mismatches and zero missing receipts, and its final 100 logged blocks averaged 500 confirmed tx/block. The pod has not restarted since deployment. The active workload is now the separate pq-fors-tx scenario; its zero-valued scenario counters do not reflect its on-chain traffic (sampled block 33843 contains 10 successful frame transactions from its deployed FORS accounts).

RedpandaBot follow-up (dabf895): reproduced the malformed-signer finding with failing regression tests, then restored length validation while retaining empty-signer support. Tests reject 1-, 19-, 21- and 32-byte signers and verify byte/hash preservation for empty and 20-byte signers across JSON round-trips. go test -race ./txtypes ./scenarios/frametx and targeted go vet pass; CI is rerunning for this head. The live deployment remains e9b66be.

@qu0b
qu0b force-pushed the fix/bogota-frame-rpc-recovery branch from a32edd3 to e9b66be Compare October 19, 2026 14:45
@qu0b qu0b changed the title Fix frame confirmation tracking and per-sender concurrency Preserve frame RPC fields and respect sender mempool concurrency Oct 19, 2026
@qu0b
qu0b marked this pull request as ready for review October 19, 2026 14:59

@redpandabot redpandabot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review of head e9b66be4c7ddc9a0979780e91c80444f0edc62a5. Review scope is incomplete; these findings do not constitute a clean verdict.

Evidence basis: source inspection at pinned revisions.

concern: Non-20-byte decoded signer silently dropped on re-encode

txtypes/frame.go:1253

jsonFrameSignature.Signer is now *hexutil.Bytes, so DecodeJSONTx accepts and stores a signer of any byte length, but EncodeJSONTx only re-emits the signer when len(sig.Signer)==20 and otherwise writes signer:null, so round-tripping drops the value that base rejected at json.Unmarshal.

Trigger: A JSON-RPC frame transaction whose signatures entry carries "signer" as a hex string of length other than 0 or 20 bytes, e.g. "0x000102030405060708090a0b0c0d0e0f1011" (21 bytes). json.Unmarshal into *hexutil.Bytes succeeds and entry.Signer is set to those 21 bytes.

Impact: Decode->encode of such a transaction produces signer:null, so the re-encoded JSON transaction (and any hash/receipt comparison derived from it) no longer reproduces what the client reported; the mismatch is no longer surfaced as an error. Base's *common.Address made the same input fail json.Unmarshal, so DecodeJSONTx returned an error instead of silently discarding the signer.

Suggested change: jsonFrameSignature.Signer changed from *common.Address to *hexutil.Bytes and DecodeJSONTx now stores any-length signer bytes; EncodeJSONTx still only emits a signer for exactly 20 bytes.

@redpandabot redpandabot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed 9 changed file(s) @ dabf895f — no blocking issues found.

@pk910
pk910 merged commit 077b33c into master Oct 20, 2026
7 checks passed
@pk910
pk910 deleted the fix/bogota-frame-rpc-recovery branch October 20, 2026 11:50
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.

2 participants