rpc: drop size from eth_getHeaderBy* responses - #26912
MysticRyuujin wants to merge 1 commit into
Conversation
| let Some(block) = self.recovered_block(block_id).await? else { return Ok(None) }; | ||
| let header = | ||
| let mut header = | ||
| self.converter().convert_header(block.clone_sealed_header(), block.rlp_length())?; |
There was a problem hiding this comment.
I think the better solution here would be to change convert_header so that this accepts Optional size or remove that arg entirely, leaning towards removing entirely
There was a problem hiding this comment.
Both need a reth-core change. FromConsensusHeader::from_consensus_header takes block_size: usize, so the () impl of HeaderConverter cannot build a size-free header today.
convert_header is also the only place block size gets set. Block flattens its header, and into_rpc_block* passes rlp_length to the header builder. Drop the arg and the block path has to set size after conversion, which needs a setter bound on the RPC header type in into_rpc_block*. Same trait as this PR, moved to the block path.
I prefer that version. Size is a block property, so headers never carry it and blocks add it. I can open the reth-core PR that drops the arg from FromConsensusHeader and adds the setter for into_rpc_block*, then rebase this one on the release. If you would rather take the smaller Option<usize> step first, tell me.
|
Opened paradigmxyz/reth-core#44 for the removal. I have the matching reth side ready locally and checked it against that branch through a temporary Two notes on that version:
I will push it here once reth-core cuts a release with that change. This PR stays on the current version until then. |
|
We will finalize this after paradigmxyz/reth-core#44 is merged and included in the next minor reth-core release. |
|
Status update. ethereum/execution-apis#877 is out of draft, and so are the Nethermind, Besu, Erigon and ethrex PRs. go-ethereum #35627 is merged. This PR stays a draft, as agreed. It waits on reth-core#44 and the next minor reth-core release. The replacement version is ready locally and verified against that branch through a temporary |
eth_getHeaderByNumberandeth_getHeaderByHashno longer emitsize. The value was the whole-block RLP length, the same numbereth_getBlockBy*reports, so it leaked body data into a header-only response. geth, Nethermind, and Erigon'serigon_getHeaderBy*all omit it.Implements the semantics proposed in ethereum/execution-apis#877 (ethereum/execution-apis#874). Block responses keep
size.Mechanics:
RpcTypes::Headergains aSizedHeaderbound with aclear_sizeimplementation foralloy_rpc_types_eth::Header, andrpc_block_headerclears the field after conversion. An alternative is changingFromConsensusHeaderin reth-core to take an optional size; happy to redo it that way if preferred.Follow-up candidate: a header-only lookup path, since
rpc_block_headerloads and recovers the full block only to discard the body.