go-ethereum/cmd/devp2p/internal/ethtest/chain_test.go
ozpool 80b58f649a
cmd/devp2p/internal/ethtest: fix header stride in reverse GetHeaders (#35362)
`Chain.GetHeaders` steps by `1 + Skip` in the forward branch but by `1 -
Skip` in the reverse branch. `Skip` is a `uint64`, so the reverse stride
is wrong for every `Skip > 0`:

| Skip | reverse step | result |
| --- | --- | --- |
| 0 | `-= 1` | correct |
| 1 | `-= 0` | block number never moves, the same header is returned
`Amount` times |
| >= 2 | `1 - Skip` underflows | walks *forward* instead of back (`Skip
= 2` on block 100 lands on 101) |

Per the `GetBlockHeadersRequest` definition, `Skip` is "Blocks to skip
between consecutive headers", so the stride is `Skip + 1` in whichever
direction the query runs. Subtracting `1 + Skip` makes the reverse
branch mirror the forward one.

The existing table test covers forward + `Skip: 1` and reverse + `Skip:
0` (which works by accident, since `1 - 0 == 1`). Added the missing
reverse + `Skip: 1` case; it fails on master:

```
--- FAIL: TestChainGetHeaders/3
    Test: TestChainGetHeaders/3
FAIL	github.com/ethereum/go-ethereum/cmd/devp2p/internal/ethtest
```

and passes with the fix (4/4).

No caller sends a reverse request with `Skip > 0` today, so nothing is
broken in the current suite. The helper computes the expected headers
that responses are checked against, though, so the moment a reverse+skip
case is added it would silently assert the wrong headers rather than
fail loudly.
2026-07-17 18:51:45 +02:00

215 lines
4.9 KiB
Go

// Copyright 2020 The go-ethereum Authors
// This file is part of go-ethereum.
//
// go-ethereum is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// go-ethereum is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with go-ethereum. If not, see <http://www.gnu.org/licenses/>.
package ethtest
import (
"path/filepath"
"strconv"
"testing"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/eth/protocols/eth"
"github.com/ethereum/go-ethereum/p2p"
"github.com/stretchr/testify/assert"
)
// TestEthProtocolNegotiation tests whether the test suite
// can negotiate the highest eth protocol in a status message exchange
func TestEthProtocolNegotiation(t *testing.T) {
t.Parallel()
var tests = []struct {
conn *Conn
caps []p2p.Cap
expected uint32
}{
{
conn: &Conn{
ourHighestProtoVersion: 65,
},
caps: []p2p.Cap{
{Name: "eth", Version: 63},
{Name: "eth", Version: 64},
{Name: "eth", Version: 65},
},
expected: uint32(65),
},
{
conn: &Conn{
ourHighestProtoVersion: 65,
},
caps: []p2p.Cap{
{Name: "eth", Version: 63},
{Name: "eth", Version: 64},
{Name: "eth", Version: 65},
},
expected: uint32(65),
},
{
conn: &Conn{
ourHighestProtoVersion: 65,
},
caps: []p2p.Cap{
{Name: "eth", Version: 63},
{Name: "eth", Version: 64},
{Name: "eth", Version: 65},
},
expected: uint32(65),
},
{
conn: &Conn{
ourHighestProtoVersion: 64,
},
caps: []p2p.Cap{
{Name: "eth", Version: 63},
{Name: "eth", Version: 64},
{Name: "eth", Version: 65},
},
expected: 64,
},
{
conn: &Conn{
ourHighestProtoVersion: 65,
},
caps: []p2p.Cap{
{Name: "eth", Version: 0},
{Name: "eth", Version: 89},
{Name: "eth", Version: 65},
},
expected: uint32(65),
},
{
conn: &Conn{
ourHighestProtoVersion: 64,
},
caps: []p2p.Cap{
{Name: "eth", Version: 63},
{Name: "eth", Version: 64},
{Name: "wrongProto", Version: 65},
},
expected: uint32(64),
},
{
conn: &Conn{
ourHighestProtoVersion: 65,
},
caps: []p2p.Cap{
{Name: "eth", Version: 63},
{Name: "eth", Version: 64},
{Name: "wrongProto", Version: 65},
},
expected: uint32(64),
},
}
for i, tt := range tests {
t.Run(strconv.Itoa(i), func(t *testing.T) {
tt.conn.negotiateEthProtocol(tt.caps)
assert.Equal(t, tt.expected, uint32(tt.conn.negotiatedProtoVersion))
})
}
}
// TestChainGetHeaders tests whether the test suite can correctly
// respond to a GetBlockHeaders request from a node.
func TestChainGetHeaders(t *testing.T) {
t.Parallel()
dir, err := filepath.Abs("./testdata")
if err != nil {
t.Fatal(err)
}
chain, err := NewChain(dir)
if err != nil {
t.Fatal(err)
}
var tests = []struct {
req eth.GetBlockHeadersPacket
expected []*types.Header
}{
{
req: eth.GetBlockHeadersPacket{
GetBlockHeadersRequest: &eth.GetBlockHeadersRequest{
Origin: eth.HashOrNumber{Number: uint64(2)},
Amount: uint64(5),
Skip: 1,
Reverse: false,
},
},
expected: []*types.Header{
chain.blocks[2].Header(),
chain.blocks[4].Header(),
chain.blocks[6].Header(),
chain.blocks[8].Header(),
chain.blocks[10].Header(),
},
},
{
req: eth.GetBlockHeadersPacket{
GetBlockHeadersRequest: &eth.GetBlockHeadersRequest{
Origin: eth.HashOrNumber{Number: uint64(chain.Len() - 1)},
Amount: uint64(3),
Skip: 0,
Reverse: true,
},
},
expected: []*types.Header{
chain.blocks[chain.Len()-1].Header(),
chain.blocks[chain.Len()-2].Header(),
chain.blocks[chain.Len()-3].Header(),
},
},
{
req: eth.GetBlockHeadersPacket{
GetBlockHeadersRequest: &eth.GetBlockHeadersRequest{
Origin: eth.HashOrNumber{Hash: chain.Head().Hash()},
Amount: uint64(1),
Skip: 0,
Reverse: false,
},
},
expected: []*types.Header{
chain.Head().Header(),
},
},
{
req: eth.GetBlockHeadersPacket{
GetBlockHeadersRequest: &eth.GetBlockHeadersRequest{
Origin: eth.HashOrNumber{Number: uint64(10)},
Amount: uint64(3),
Skip: 1,
Reverse: true,
},
},
expected: []*types.Header{
chain.blocks[10].Header(),
chain.blocks[8].Header(),
chain.blocks[6].Header(),
},
},
}
for i, tt := range tests {
t.Run(strconv.Itoa(i), func(t *testing.T) {
headers, err := chain.GetHeaders(&tt.req)
if err != nil {
t.Fatal(err)
}
assert.Equal(t, headers, tt.expected)
})
}
}