diff --git a/cmd/devp2p/internal/ethtest/suite.go b/cmd/devp2p/internal/ethtest/suite.go index 295486b59d..fedb400ddd 100644 --- a/cmd/devp2p/internal/ethtest/suite.go +++ b/cmd/devp2p/internal/ethtest/suite.go @@ -81,7 +81,7 @@ func (s *Suite) EthTests() []utesting.Test { {Name: "ZeroRequestID", Fn: s.TestZeroRequestID}, // get history {Name: "GetBlockBodies", Fn: s.TestGetBlockBodies}, - {Name: "GetReceipts70", Fn: s.TestGetReceipts}, + {Name: "GetReceipts", Fn: s.TestGetReceipts}, // test transactions {Name: "LargeTxRequest", Fn: s.TestLargeTxRequest, Slow: true}, {Name: "Transaction", Fn: s.TestTransaction}, diff --git a/eth/downloader/queue.go b/eth/downloader/queue.go index 2c97ebd520..4d8810b28b 100644 --- a/eth/downloader/queue.go +++ b/eth/downloader/queue.go @@ -679,7 +679,7 @@ func (q *queue) deliver(id string, taskPool map[common.Hash]*types.Header, // If no data items were retrieved, mark them as unavailable for the origin peer if results == 0 { for _, header := range request.Headers { - request.Peer.MarkLacking(header.Hash()) //todo? + request.Peer.MarkLacking(header.Hash()) } } // Assemble each of the results with their headers and retrieved data parts @@ -720,7 +720,6 @@ func (q *queue) deliver(id string, taskPool map[common.Hash]*types.Header, resDropMeter.Mark(int64(results - accepted)) // Return all failed or missing fetches to the queue - //todo if incomplete { for _, header := range request.Headers[from+accepted : from+results] { taskQueue.Push(header, -int64(header.Number.Uint64())) diff --git a/eth/downloader/queue_test.go b/eth/downloader/queue_test.go index a38a87764f..0e973bc84c 100644 --- a/eth/downloader/queue_test.go +++ b/eth/downloader/queue_test.go @@ -274,9 +274,9 @@ func TestEmptyBlocks(t *testing.T) { } } -// TestPartialReceiptDelivery checks two points below -// 1. Receipts failed validation should be re requested to the other peers -// 2. Partial delivery should not be expired +// TestPartialReceiptDelivery checks two points: +// 1. Receipts that fail validation should be re-requested from other peers. +// 2. Partial delivery should not expire. func TestPartialReceiptDelivery(t *testing.T) { blocks, receipts := makeChain(64, 0, testGenesis, blockConfig{txPeriod: 1, txCount: 5}) chain := chainData{blocks: blocks, receipts: receipts, offset: 0} @@ -305,7 +305,7 @@ func TestPartialReceiptDelivery(t *testing.T) { t.Logf("request: length %d", len(req.Headers)) - // 1. Deliver partial receipt: should not clear the remaining receipts from pending list + // 1. Deliver a partial receipt: this must not clear the remaining receipts from the pending list firstCutoff := len(req.Headers) / 3 receiptRLP, rcHashes := getPartialReceiptsDelivery(0, firstCutoff, receipts) accepted, err := q.DeliverReceipts(peer.id, receiptRLP, rcHashes, true, 0) @@ -333,7 +333,7 @@ func TestPartialReceiptDelivery(t *testing.T) { t.Fatalf("there should be in flight receipts") } - // 2. Deliver partial receipt with invalid receipt: should clear invalid receipt from pending list + // 2. Deliver a partial receipt containing an invalid entry: the invalid receipt should be removed from the pending list secondCutoff := firstCutoff + len(req.Headers)/3 receiptRLP, rcHashes = getPartialReceiptsDelivery(firstCutoff, secondCutoff, receipts) // one invalid receipt @@ -346,7 +346,7 @@ func TestPartialReceiptDelivery(t *testing.T) { t.Fatalf("delivery should fail") } - // invalid receipt should returned back to the pending pool + // The invalid receipt should be returned to the pending pool if pending := q.PendingReceipts(); pending != numBlock-len(req.Headers)+1 { t.Fatalf("wrong pending receipt length, got %d, exp %d", pending, numBlock-len(req.Headers)) } @@ -364,7 +364,7 @@ func TestPartialReceiptDelivery(t *testing.T) { } } - // 3. Deliver partial receipt to complete request + // 3. Deliver the remaining receipts to complete the request thirdCutoff := len(req.Headers) receiptRLP, rcHashes = getPartialReceiptsDelivery(secondCutoff, thirdCutoff, receipts) accepted, err = q.DeliverReceipts(peer.id, receiptRLP, rcHashes, false, secondCutoff) diff --git a/eth/protocols/eth/handler_test.go b/eth/protocols/eth/handler_test.go index 8938ed43c9..f934d394ff 100644 --- a/eth/protocols/eth/handler_test.go +++ b/eth/protocols/eth/handler_test.go @@ -553,7 +553,7 @@ func testGetBlockReceipts(t *testing.T, protocol uint) { func TestGetBlockPartialReceipts(t *testing.T) { testGetBlockPartialReceipts(t, ETH70) } func testGetBlockPartialReceipts(t *testing.T, protocol int) { - // First generate chain and overwrite receipts + // First, generate the chain and overwrite the receipts. generator := func(_ int, block *core.BlockGen) { for j := 0; j < 5; j++ { tx, err := types.SignTx( @@ -573,7 +573,7 @@ func testGetBlockPartialReceipts(t *testing.T, protocol int) { blockCutoff := 2 receiptCutoff := 4 - // Replace receipts in DB with larger receipts + // Replace the receipts in the database with larger receipts. targetBlock := backend.chain.GetBlockByNumber(uint64(blockCutoff)) receipts := backend.chain.GetReceiptsByHash(targetBlock.Hash()) receiptSize := params.MaxTxGas / params.LogDataGas // ~2MiB per receipt @@ -626,7 +626,7 @@ func testGetBlockPartialReceipts(t *testing.T, protocol int) { t.Errorf("receipts mismatch: %v", err) } - // Simulate re-request + // Simulate the continued request partialReceipt = []*ReceiptList69{NewReceiptList69(receipts[receiptCutoff:])} p2p.Send(peer.app, GetReceiptsMsg, &GetReceiptsPacket70{ diff --git a/eth/protocols/eth/handlers.go b/eth/protocols/eth/handlers.go index 551fce7b1c..6bba81fa79 100644 --- a/eth/protocols/eth/handlers.go +++ b/eth/protocols/eth/handlers.go @@ -353,8 +353,8 @@ func serviceGetReceiptsQuery69(chain *core.BlockChain, query GetReceiptsRequest) } // serviceGetReceiptsQuery70 assembles the response to a receipt query. -// If the size of receipts is larger than 10MB, it would cut it and flag lastBlockIncomplete -// It omits up to firstBlockReceiptIndex receipt from the first receipt list +// 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. func serviceGetReceiptsQuery70(chain *core.BlockChain, query GetReceiptsRequest, firstBlockReceiptIndex uint64) ([]rlp.RawValue, bool) { var ( bytes int diff --git a/eth/protocols/eth/peer.go b/eth/protocols/eth/peer.go index e2fa0bd25c..0f6d54c444 100644 --- a/eth/protocols/eth/peer.go +++ b/eth/protocols/eth/peer.go @@ -363,7 +363,7 @@ func (p *Peer) RequestReceipts(hashes []common.Hash, sink chan *Response) (*Requ return req, nil } -// handlePartialReceipts re-request partial receipts +// HandlePartialReceipts re-request partial receipts func (p *Peer) HandlePartialReceipts(previousId uint64) error { split := p.receiptBuffer[previousId].idx id := rand.Uint64() @@ -385,8 +385,8 @@ func (p *Peer) HandlePartialReceipts(previousId uint64) error { return p.dispatchRequest(req) } -// validateReceipt validate and check completion of partial request -// This function also modifies packet (trim partial response or append previously collected receipts) +// ValidateReceipt validates a receipt packet and checks whether a partial request is complete. +// It also mutates the packet in place, trimming the partial response or appending previously collected receipts. func (p *Peer) ValidateReceipt(packet *ReceiptsPacket70) (int, error) { from := 0 requestId := packet.RequestId @@ -394,8 +394,8 @@ func (p *Peer) ValidateReceipt(packet *ReceiptsPacket70) (int, error) { return 0, fmt.Errorf("receipt list size 0") } - // process first block - // : partially collected before and completed by this response + // Process the first block + // If the request was partially collected earlier, append the buffered data so this response completes it. firstReceipt := packet.List[0] if firstReceipt == nil { return 0, fmt.Errorf("nil first receipt") @@ -407,7 +407,7 @@ func (p *Peer) ValidateReceipt(packet *ReceiptsPacket70) (int, error) { delete(p.receiptBuffer, requestId) } - // process last block + // Trim and buffer the last block when the response is incomplete. if packet.LastBlockIncomplete { lastReceipts := packet.List[len(packet.List)-1] if lastReceipts == nil { @@ -421,25 +421,25 @@ func (p *Peer) ValidateReceipt(packet *ReceiptsPacket70) (int, error) { previousLog = buffer.size } - // 1. Verify the total number of tx delivered + // 1. Verify that the total number of transactions delivered is under the limit. if uint64(previousTxs+len(lastReceipts.items)) > lastReceipts.items[0].GasUsed/21_000 { // should be dropped, don't clear the buffer return 0, fmt.Errorf("total number of tx exceeded limit") } - // 2. Verify the size of each receipt against the gas limit of the corresponding transaction + // 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 { return 0, fmt.Errorf("total size of receipt exceeded limit") } previousLog += uint64(len(rc.Logs)) } - // 3. Verify the total download receipt size is no longer than allowed by the block gas limit + // 3. Verify that the overall downloaded receipt size does not exceed the block gas limit. if previousLog > params.MaxGasLimit/params.LogDataGas { return 0, fmt.Errorf("total download receipt size exceeded the limit") } - // Update buffer & trim packet + // Update the buffered data and trim the packet to exclude the incomplete block. if buffer, ok := p.receiptBuffer[requestId]; ok { buffer.idx = buffer.idx + len(packet.List) - 1 buffer.list.items = append(buffer.list.items, lastReceipts.items...)