From fc12dbe40bdb3fd77d0ea6049570b8c870421859 Mon Sep 17 00:00:00 2001 From: Sina M <1591639+s1na@users.noreply.github.com> Date: Fri, 31 Jan 2025 18:34:22 +0100 Subject: [PATCH] eth/catalyst: fix validation of type 0 request (#31103) I caught this error on Hive. It was introduced by https://github.com/ethereum/go-ethereum/pull/31071 because after adding the equality check the request type 0 will be rejected. --- eth/catalyst/api.go | 6 ++--- eth/catalyst/api_test.go | 58 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go index 7a9707dc1572..e6f29c970b5b 100644 --- a/eth/catalyst/api.go +++ b/eth/catalyst/api.go @@ -1272,18 +1272,16 @@ func convertRequests(hex []hexutil.Bytes) [][]byte { // validateRequests checks that requests are ordered by their type and are not empty. func validateRequests(requests [][]byte) error { - var last byte - for _, req := range requests { + for i, req := range requests { // No empty requests. if len(req) < 2 { return fmt.Errorf("empty request: %v", req) } // Check that requests are ordered by their type. // Each type must appear only once. - if req[0] <= last { + if i > 0 && req[0] <= requests[i-1][0] { return fmt.Errorf("invalid request order: %v", req) } - last = req[0] } return nil } diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index 46988277314b..407f0db3eb72 100644 --- a/eth/catalyst/api_test.go +++ b/eth/catalyst/api_test.go @@ -1737,3 +1737,61 @@ func TestGetClientVersion(t *testing.T) { t.Fatalf("client info does match expected, got %s", info.String()) } } + +func TestValidateRequests(t *testing.T) { + tests := []struct { + name string + requests [][]byte + wantErr bool + }{ + { + name: "valid ascending", + requests: [][]byte{ + {0x00, 0xAA, 0xBB}, // type 0x00 + {0x01, 0xCC}, // type 0x01 + {0x02, 0xDD}, // type 0x02 + }, + wantErr: false, + }, + { + name: "empty request (too short)", + requests: [][]byte{ + {0x00}, // only 1 byte: type with no data + }, + wantErr: true, + }, + { + name: "duplicate type", + requests: [][]byte{ + {0x00, 0x11}, + {0x01, 0x22}, + {0x01, 0x33}, // duplicate type 0x01 + }, + wantErr: true, + }, + { + name: "out of order", + requests: [][]byte{ + {0x01, 0xAA}, // type 0x01 + {0x00, 0xBB}, // type 0x00 out of order (should be ascending) + }, + wantErr: true, + }, + { + name: "single request valid", + requests: [][]byte{ + {0x01, 0xAB}, + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateRequests(tt.requests) + if (err != nil) != tt.wantErr { + t.Errorf("validateRequests(%v) error = %v, wantErr = %v", + tt.requests, err, tt.wantErr) + } + }) + } +}