Conversation
… from the chain config The receipts subscription built its signer with types.MakeSigner(nil, blockNum, 0), which returns a zero Signer: no chain ID and every fork flag false, so it rejects every transaction type. Sender recovery only worked while the transaction still cached its sender; otherwise SubscribeReceiptsReply.From stayed nil. MarshalSubscribeReceipt then passed that nil to ConvertH160toAddress, which dereferences it. The subscription goroutine recovers the panic with dbg.LogPanic, but that sends SIGINT, so a missing sender shuts the node down. Build the signer once from the chain config, as geth does in FilterAPI.TransactionReceipts, and guard From on the RPC side the way To and ContractAddress already are.
The fallback rebuilt the zero signer this branch removes. Pass the chain config from the three test call sites instead.
There was a problem hiding this comment.
🟢 Approval recommended
The focused fix addresses both failure paths and includes appropriate regression coverage.
Pull request overview
This PR prevents receipt subscriptions from crashing when sender data is not cached by recovering senders with the configured chain signer and safely handling missing senders.
Changes:
- Build and reuse a receipt signer from the chain configuration.
- Guard missing
Fromvalues during RPC receipt marshaling. - Add regression coverage and update test backend setup.
File summaries
| File | Description |
|---|---|
node/privateapi/receiptsfilter.go |
Recovers receipt senders with the configured signer. |
node/privateapi/receiptsfilter_test.go |
Tests uncached sender recovery. |
node/privateapi/ethbackend.go |
Passes chain configuration to the receipt filter. |
execution/types/ethutils/receipt.go |
Avoids dereferencing a missing sender. |
execution/types/ethutils/receipt_test.go |
Tests marshaling without sender data. |
rpc/jsonrpc/eth_subscribe_test.go |
Supplies chain configuration in subscription tests. |
rpc/jsonrpc/debug_api_test.go |
Supplies chain configuration in debug tests. |
cmd/rpcdaemon/rpcdaemontest/test_util.go |
Supplies chain configuration to test servers. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
The signer fix is right. One gap in the guard.
It is still reachable after the signer fix. Also worth a line in the doc comment: |
RPCReceipt.From was a value common.Address, so a receipt whose sender the backend could not recover serialised as "from":"0x0000...0000", which a subscriber cannot tell from a real zero address. Make it a *common.Address and run the subscribe path through nonZeroAddress, as To and ContractAddress already do. MarshalReceipt keeps setting the pointer whenever the receipt is signed, so eth_getTransactionReceipt and eth_getBlockReceipts still emit an address: execution-apis lists "from" as required with no null variant, and geth also falls back to the zero address there. Document that NewEthBackendServer now needs a non-nil chainConfig.
Part of #23986.
receiptNotificationToProtobuilt its signer withtypes.MakeSigner(nil, ...), which returns a zeroSignerthat rejects every transaction type. Recovery worked only while the transaction still cached its sender; otherwiseSubscribeReceiptsReply.Fromstayed nil, andMarshalSubscribeReceiptpassed that nil toConvertH160toAddress, which dereferences it.dbg.LogPanicrecovers the panic but sends SIGINT, so a missing sender shuts the node down.Fix: build the signer once from the chain config with
types.LatestSigner, as geth does inFilterAPI.TransactionReceipts, and guardFromon the RPC side the wayToandContractAddressalready are.Both tests fail on
main: one segfaults inConvertH160toAddress, the other signs a dynamic-fee transaction so its sender is not cached.The rest of #23986 (log fields,
effectiveGasPrice, one notification per block) changes the payload format and needs a proto field; it follows in a stacked PR.