From 80b58f649a1cb3cf59fcc0defe88720e57b96ca4 Mon Sep 17 00:00:00 2001
From: ozpool <151670776+ozpool@users.noreply.github.com>
Date: Fri, 17 Jul 2026 22:21:45 +0530
Subject: [PATCH] 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.
---
 cmd/devp2p/internal/ethtest/chain.go      |  2 +-
 cmd/devp2p/internal/ethtest/chain_test.go | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/cmd/devp2p/internal/ethtest/chain.go b/cmd/devp2p/internal/ethtest/chain.go
index 36c395cdef..16b5a36558 100644
--- a/cmd/devp2p/internal/ethtest/chain.go
+++ b/cmd/devp2p/internal/ethtest/chain.go
@@ -231,7 +231,7 @@ func (c *Chain) GetHeaders(req *eth.GetBlockHeadersPacket) ([]*types.Header, err
 	}
 	if req.Reverse {
 		for i := 1; i < int(req.Amount); i++ {
-			blockNumber -= (1 - req.Skip)
+			blockNumber -= (1 + req.Skip)
 			headers[i] = c.blocks[blockNumber].Header()
 		}
 		return headers, nil
diff --git a/cmd/devp2p/internal/ethtest/chain_test.go b/cmd/devp2p/internal/ethtest/chain_test.go
index 62bd6d26ea..31c90c6c04 100644
--- a/cmd/devp2p/internal/ethtest/chain_test.go
+++ b/cmd/devp2p/internal/ethtest/chain_test.go
@@ -186,6 +186,21 @@ func TestChainGetHeaders(t *testing.T) {
 				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 {