From 22ab6697acb5256357b74044c15d8ffa337f76f1 Mon Sep 17 00:00:00 2001 From: healthykim Date: Wed, 26 Nov 2025 16:21:53 +0900 Subject: [PATCH] fix: fix validation logic and add tests --- eth/protocols/eth/handlers.go | 2 +- eth/protocols/eth/peer.go | 20 +++-- eth/protocols/eth/peer_test.go | 130 +++++++++++++++++++++++++++++++-- 3 files changed, 138 insertions(+), 14 deletions(-) diff --git a/eth/protocols/eth/handlers.go b/eth/protocols/eth/handlers.go index 89f30640f1..abf29d9f38 100644 --- a/eth/protocols/eth/handlers.go +++ b/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 { diff --git a/eth/protocols/eth/peer.go b/eth/protocols/eth/peer.go index 156549d9b5..723ac8488d 100644 --- a/eth/protocols/eth/peer.go +++ b/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") } diff --git a/eth/protocols/eth/peer_test.go b/eth/protocols/eth/peer_test.go index 366d9b0e63..90e0a0df78 100644 --- a/eth/protocols/eth/peer_test.go +++ b/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"), - common.HexToHash("0xbb"), - common.HexToHash("0xcc"), - common.HexToHash("0xdd"), + backend.chain.GetBlockByNumber(1).Hash(), + backend.chain.GetBlockByNumber(2).Hash(), + backend.chain.GetBlockByNumber(3).Hash(), + 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") + } }