Skip to content

Conversation

@healthykim
Copy link
Contributor

@healthykim healthykim commented Nov 11, 2025

This is a draft PR to add support for EIP-7975.
Overall changes

  • Each response is buffered in the peer’s receipt buffer when the lastBlockIncomplete field is true.
  • Continued request uses the same request id of its original request(RequestPartialReceipts).
  • Partial responses are verified in validateLastBlockReceipt.
  • Even if all receipts for partial blocks of the request are collected, those partial results are not sinked to the downloader, to avoid complexity. This assumes that partial response and buffering occur only in exceptional cases. (The complexity added to the queue can be checked in 1f32d8959)

@healthykim healthykim changed the title WIP: add eip-7975 EIP-7975: eth/70 - partial block receipt lists Nov 15, 2025
@healthykim healthykim changed the title EIP-7975: eth/70 - partial block receipt lists eth: implement EIP-7975 (eth/70 - partial block receipt lists) Nov 15, 2025
@healthykim healthykim marked this pull request as ready for review November 18, 2025 13:37
buffer.list = append(buffer.list, packet.List...)
buffer.lastLogSize = logSize
} else {
p.receiptBuffer[requestId] = &partialReceipt{
Copy link
Contributor

Choose a reason for hiding this comment

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

The peer should not be able to create buffered items for unrequested blocks. So we need to create request ID entries explicitly when the request is made, instead of just making a buffer on demand here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the request tracking here and the validation here.

The buffer is cleared when the request is canceled or completed.

It is also cleared when an empty response is received.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could do this with just one map. Maybe keep an entry in p.receiptBuffer for all requests, even non-partial ones? It seems simpler this way

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I changed this, now receiptBuffer stores requested hashes and buffered receipts

buffer.list = append(buffer.list, packet.List...)
buffer.lastLogSize = logSize
} else {
p.receiptBuffer[requestId] = &partialReceipt{
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could do this with just one map. Maybe keep an entry in p.receiptBuffer for all requests, even non-partial ones? It seems simpler this way


// 2. Verify the size of each receipt against the maximum transaction size.
for _, rc := range lastReceipts.items {
if uint64(len(rc.Logs)) > params.MaxTxGas/params.LogDataGas {
Copy link
Contributor

Choose a reason for hiding this comment

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

This check is weird. We only added MaxTxGas in the next fork, which has not even activated on mainnet yet, and I'm not sure this limit is always defined in other chains. The check against header gaslimit should be enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am confused here, the check against the header’s gasLimit will always pass if the third condition passes. Or should we remove the second condition?

Copy link
Contributor

Choose a reason for hiding this comment

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

We agreed to remove condition 2.

Copy link
Member

Choose a reason for hiding this comment

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

Downloading block receipts across multiple messages creates new attack surface. Partial receipt lists cannot be verified against the block header, so in responses with lastBlockIncomplete = 1, the last receipts list must be validated in a different way:

It's technically verifiable. The receipts can be proved with a range proof (although I agree there is some complexity around the 0th element whose index is 0x80), any particular reason to not attaching the proof for the paginated receipts?

reqCancel chan *cancel // Dispatch channel to cancel pending requests and untrack them
resDispatch chan *response // Dispatch channel to fulfil pending requests and untrack them

receiptBuffer map[uint64]*partialReceipt // requestId to receiptlist map
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment could be improved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the comment to // Previously requested receipts to buffer partial receipts


// serviceGetReceiptsQuery70 assembles the response to a receipt query.
// If the receipts exceed 10 MiB, it trims them and sets the lastBlockIncomplete flag.
// Indices smaller than firstBlockReceiptIndex are omitted from the first block receipt list.
Copy link
Member

Choose a reason for hiding this comment

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

nitpick, usually we limit the length of comment to 80 chars for better readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated this. Thank you!


for lookups, hash := range query {
if bytes >= softResponseLimit || len(receipts) >= maxReceiptsServe ||
lookups >= 2*maxReceiptsServe {
Copy link
Member

Choose a reason for hiding this comment

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

this lookup restriction is unnecessary? (similar in eth68, eth69)

the number of requested receipts is already limited by len(receipts) >= maxReceiptsServe?

Peer string // Demultiplexer if cross-peer requests are batched together
Sent time.Time // Timestamp when the request was sent

continued bool
Copy link
Member

Choose a reason for hiding this comment

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

It's not used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this. Thank you!

lastReceipt := len(p.receiptBuffer[id].list[lastBlock].items)

req := &Request{
id: id,
Copy link
Member

Choose a reason for hiding this comment

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

I think the hacky thing in the implementation is we try to reuse the same request ID for the subsequent requests.

How about this: if we detect that the response is incomplete and subsequent requests are required, we internally maintain the mapping between the original request ID and the following IDs, so that the following responses can be correctly detect and be assembled properly?

buffer := p.receiptBuffer[requestId]

// Do not assign buffer to the response not requested
if buffer == nil {
Copy link
Member

Choose a reason for hiding this comment

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

I think it's a bit weird to always allocate an item in the receiptBuffer even the first response is complete.

The incomplete response for mega-receipt is not common. I think we should only maintain the additional records for this special case.

Copy link
Contributor Author

@healthykim healthykim Nov 28, 2025

Choose a reason for hiding this comment

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

I found that the same data (requested hashes) is already stored in dispatcher.go (pending). Can we utilize that?


// 2. Verify the size of each receipt against the maximum transaction size.
for _, rc := range lastReceipts.items {
if uint64(len(rc.Logs)) > params.MaxTxGas/params.LogDataGas {
Copy link
Member

Choose a reason for hiding this comment

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

Downloading block receipts across multiple messages creates new attack surface. Partial receipt lists cannot be verified against the block header, so in responses with lastBlockIncomplete = 1, the last receipts list must be validated in a different way:

It's technically verifiable. The receipts can be proved with a range proof (although I agree there is some complexity around the 0th element whose index is 0x80), any particular reason to not attaching the proof for the paginated receipts?

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