fix: fix validation logic and add tests

eth/protocols/eth/handlers.go

@@ -527,7 +527,7 @@ func handleReceipts70(backend Backend, msg Decoder, peer *Peer) error {
 		return err
 	}
 
-	if err := peer.BufferReceiptsPacket(res); err != nil {
+	if err := peer.BufferReceiptsPacket(res, backend); err != nil {
 		return err
 	}
 	if res.LastBlockIncomplete {

eth/protocols/eth/peer.go

@@ -404,7 +404,7 @@ func (p *Peer) RequestPartialReceipts(id uint64) error {
 
 // BufferReceiptsPacket validates a receipt packet and buffer the incomplete packet.
 // If the request is completed, it appends previously collected receipts.
-func (p *Peer) BufferReceiptsPacket(packet *ReceiptsPacket70) error {
+func (p *Peer) BufferReceiptsPacket(packet *ReceiptsPacket70, backend Backend) error {
 	requestId := packet.RequestId
 
 	// Do not assign buffer to the response not requested
@@ -418,14 +418,22 @@ func (p *Peer) BufferReceiptsPacket(packet *ReceiptsPacket70) error {
 
 	// Buffer the last block when the response is incomplete.
 	if packet.LastBlockIncomplete {
-		logSize, err := p.validateLastBlockReceipt(packet.List, requestId)
+		lastBlock := len(packet.List) - 1
+		if _, ok := p.receiptBuffer[requestId]; ok {
+			lastBlock += len(p.receiptBuffer[requestId].list) - 1
+		}
+		header := backend.Chain().GetHeaderByHash(p.requestedReceipts[requestId][lastBlock])
+		logSize, err := p.validateLastBlockReceipt(packet.List, requestId, header)
 		if err != nil {
 			return err
 		}
 
 		// Update the buffered data and trim the packet to exclude the incomplete block.
 		if buffer, ok := p.receiptBuffer[requestId]; ok {
-			buffer.list = append(buffer.list, packet.List...)
+			// If the buffer is already allocated, it means that the previous response was incomplete
+			// Append the first block receipts
+			buffer.list[len(buffer.list)-1].Append(packet.List[0])
+			buffer.list = append(buffer.list, packet.List[1:]...)
 			buffer.lastLogSize = logSize
 		} else {
 			p.receiptBuffer[requestId] = &partialReceipt{
@@ -451,7 +459,7 @@ func (p *Peer) BufferReceiptsPacket(packet *ReceiptsPacket70) error {
 // This function is called only when the `lastBlockincomplete == true`.
 // Note that the last receipt response (which completes receiptLists of a pending block) is not verified here.
 // Those response doesn't need hueristics below since they can be verified by its trie root.
-func (p *Peer) validateLastBlockReceipt(receiptLists []*ReceiptList69, id uint64) (uint64, error) {
+func (p *Peer) validateLastBlockReceipt(receiptLists []*ReceiptList69, id uint64, header *types.Header) (uint64, error) {
 	lastReceipts := receiptLists[len(receiptLists)-1]
 
 	// If the receipt is in the middle of retreival, use the buffered data.
@@ -468,7 +476,7 @@ func (p *Peer) validateLastBlockReceipt(receiptLists []*ReceiptList69, id uint64
 	}
 
 	// 1. Verify that the total number of transactions delivered is under the limit.
-	if uint64(previousTxs+len(lastReceipts.items)) > params.MaxGasLimit/21_000 {
+	if uint64(previousTxs+len(lastReceipts.items)) > header.GasUsed/21_000 {
 		// should be dropped, don't clear the buffer
 		return 0, fmt.Errorf("total number of tx exceeded limit")
 	}
@@ -481,7 +489,7 @@ func (p *Peer) validateLastBlockReceipt(receiptLists []*ReceiptList69, id uint64
 		log += uint64(len(rc.Logs))
 	}
 	// 3. Verify that the overall downloaded receipt size does not exceed the block gas limit.
-	if previousLog+log > params.MaxGasLimit/params.LogDataGas {
+	if previousLog+log > header.GasUsed/params.LogDataGas {
 		return 0, fmt.Errorf("total download receipt size exceeded the limit")
 	}
 

eth/protocols/eth/peer_test.go

@@ -21,12 +21,16 @@ package eth
 
 import (
 	"crypto/rand"
+	"math/big"
 	"testing"
 	"time"
 
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/core"
+	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/p2p"
 	"github.com/ethereum/go-ethereum/p2p/enode"
+	"github.com/ethereum/go-ethereum/params"
 	"github.com/ethereum/go-ethereum/rlp"
 )
 
@@ -92,6 +96,18 @@ func TestPeerSet(t *testing.T) {
 }
 
 func TestPartialReceipt(t *testing.T) {
+	gen := func(_ int, g *core.BlockGen) {
+		signer := types.HomesteadSigner{}
+
+		for range 4 {
+			tx, _ := types.SignTx(types.NewTransaction(g.TxNonce(testAddr), testAddr, big.NewInt(10), params.TxGas, g.BaseFee(), nil), signer, testKey)
+			g.AddTx(tx)
+		}
+	}
+
+	backend := newTestBackendWithGenerator(4, true, true, gen)
+	defer backend.close()
+
 	app, net := p2p.MsgPipe()
 	var id enode.ID
 	if _, err := rand.Read(id[:]); err != nil {
@@ -121,10 +137,10 @@ func TestPartialReceipt(t *testing.T) {
 	}()
 
 	hashes := []common.Hash{
-		common.HexToHash("0xaa"),
+		backend.chain.GetBlockByNumber(1).Hash(),
-		common.HexToHash("0xbb"),
+		backend.chain.GetBlockByNumber(2).Hash(),
-		common.HexToHash("0xcc"),
+		backend.chain.GetBlockByNumber(3).Hash(),
-		common.HexToHash("0xdd"),
+		backend.chain.GetBlockByNumber(4).Hash(),
 	}
 
 	sink := make(chan *Response, 1)
@@ -154,7 +170,7 @@ func TestPartialReceipt(t *testing.T) {
 			},
 		},
 	}
-	if err := peer.BufferReceiptsPacket(delivery); err != nil {
+	if err := peer.BufferReceiptsPacket(delivery, backend); err != nil {
 		t.Fatalf("first ReconstructReceiptsPacket failed: %v", err)
 	}
 
@@ -201,7 +217,7 @@ func TestPartialReceipt(t *testing.T) {
 			},
 		},
 	}
-	if err := peer.BufferReceiptsPacket(delivery); err != nil {
+	if err := peer.BufferReceiptsPacket(delivery, backend); err != nil {
 		t.Fatalf("second ReconstructReceiptsPacket failed: %v", err)
 	}
 	if _, ok := peer.receiptBuffer[rereq.RequestId]; ok {
@@ -213,6 +229,18 @@ func TestPartialReceipt(t *testing.T) {
 }
 
 func TestPartialReceiptFailure(t *testing.T) {
+	gen := func(_ int, g *core.BlockGen) {
+		signer := types.HomesteadSigner{}
+
+		for range 4 {
+			tx, _ := types.SignTx(types.NewTransaction(g.TxNonce(testAddr), testAddr, big.NewInt(10), params.TxGas, g.BaseFee(), nil), signer, testKey)
+			g.AddTx(tx)
+		}
+	}
+
+	backend := newTestBackendWithGenerator(4, true, true, gen)
+	defer backend.close()
+
 	app, net := p2p.MsgPipe()
 	var id enode.ID
 	if _, err := rand.Read(id[:]); err != nil {
@@ -258,8 +286,96 @@ func TestPartialReceiptFailure(t *testing.T) {
 			},
 		},
 	}
-	err := peer.BufferReceiptsPacket(delivery)
+	err := peer.BufferReceiptsPacket(delivery, backend)
 	if err == nil {
 		t.Fatal("Unknown response should be dropped")
 	}
+
+	// If a peer deliverse excessive amount of receipts, it should also fail the validation
+	hashes := []common.Hash{
+		backend.chain.GetBlockByNumber(1).Hash(),
+		backend.chain.GetBlockByNumber(2).Hash(),
+		backend.chain.GetBlockByNumber(3).Hash(),
+		backend.chain.GetBlockByNumber(4).Hash(),
+	}
+
+	// Case 1 ) The number of receipts exceeds maximum tx count
+	sink := make(chan *Response, 1)
+	req, err := peer.RequestReceipts(hashes, sink)
+	if err != nil {
+		t.Fatalf("RequestReceipts failed: %v", err)
+	}
+	select {
+	case _ = <-packetCh:
+	case <-time.After(2 * time.Second):
+		t.Fatalf("timeout waiting for request packet")
+	}
+
+	maxTxCount := backend.chain.GetBlockByNumber(1).GasUsed() / 21_000
+	delivery = &ReceiptsPacket70{
+		RequestId:           req.id,
+		LastBlockIncomplete: true,
+		List: []*ReceiptList69{{
+			items: []Receipt{
+				{Logs: rlp.RawValue(make([]byte, 1))},
+			},
+		}},
+	}
+	for range maxTxCount {
+		delivery.List[0].items = append(delivery.List[0].items, Receipt{Logs: rlp.RawValue(make([]byte, 1))})
+	}
+	err = peer.BufferReceiptsPacket(delivery, backend)
+	if err == nil {
+		t.Fatal("Response with the excessive number of receipts should fail the validation")
+	}
+
+	// Case 2 ) The size of each receipt exceeds maximum log size against the max tx size
+	req, err = peer.RequestReceipts(hashes, sink)
+	if err != nil {
+		t.Fatalf("RequestReceipts failed: %v", err)
+	}
+	select {
+	case _ = <-packetCh:
+	case <-time.After(2 * time.Second):
+		t.Fatalf("timeout waiting for request packet")
+	}
+	maxLogSize := params.MaxTxGas / params.LogDataGas
+	delivery = &ReceiptsPacket70{
+		RequestId:           req.id,
+		LastBlockIncomplete: true,
+		List: []*ReceiptList69{{
+			items: []Receipt{
+				{Logs: rlp.RawValue(make([]byte, maxLogSize+1))},
+			},
+		}},
+	}
+	err = peer.BufferReceiptsPacket(delivery, backend)
+	if err == nil {
+		t.Fatal("Response with the excessive number of receipts should fail the validation")
+	}
+
+	// Case 3 ) Total receipt size exceeds the block gas limit
+	req, err = peer.RequestReceipts(hashes, sink)
+	if err != nil {
+		t.Fatalf("RequestReceipts failed: %v", err)
+	}
+	select {
+	case _ = <-packetCh:
+	case <-time.After(2 * time.Second):
+		t.Fatalf("timeout waiting for request packet")
+	}
+	maxReceiptSize := backend.chain.GetBlockByNumber(1).GasUsed() / params.LogDataGas
+	delivery = &ReceiptsPacket70{
+		RequestId:           req.id,
+		LastBlockIncomplete: true,
+		List: []*ReceiptList69{{
+			items: []Receipt{
+				{Logs: rlp.RawValue(make([]byte, maxReceiptSize+1))},
+			},
+		}},
+	}
+	err = peer.BufferReceiptsPacket(delivery, backend)
+	if err == nil {
+		t.Fatal("Response with the excessive number of receipts should fail the validation")
+	}
 }