fix(evm): a second bug-hunt pass, five subsystems plus nwo and statedelta

.github/workflows/nightly-fuzz.yml

@@ -120,6 +120,10 @@ jobs:
             dir: ./x/token/services/network/evm
             pkg: .
             func: FuzzComputeAnchor
+          - name: evm-anchor-from-txid
+            dir: ./x/token/services/network/evm
+            pkg: ./keys
+            func: FuzzAnchorFromTxID
           - name: evm-abi-decode-bytes
             dir: ./x/token/services/network/evm
             pkg: ./abi
@@ -148,6 +152,14 @@ jobs:
             dir: ./x/token/services/network/evm
             pkg: ./client
             func: FuzzBytesToAddress
+          - name: evm-json-log-to-log
+            dir: ./x/token/services/network/evm
+            pkg: ./client
+            func: FuzzJSONLogToLog
+          - name: evm-json-receipt-to-receipt
+            dir: ./x/token/services/network/evm
+            pkg: ./client
+            func: FuzzJSONReceiptToReceipt
 
     steps:
       - name: Checkout code

.gitignore

@@ -15,6 +15,7 @@ cmd/token_validation_service/out/
 coverage.out
 /.codex/
 plan.md
+x/BUG_HUNT_PROMPT.md
 
 # The EVM integration suites generate a node binary tree and a network under out/ and testdata/.
 # Both hold real Go files, so a stray "git add -A" commits them and a linter then reports on them.

x/token/services/network/evm/abi/fuzz_test.go

@@ -102,7 +102,21 @@ func FuzzDecodeUint64(f *testing.F) {
 	f.Add(make([]byte, wordLength*2))
 
 	f.Fuzz(func(t *testing.T, ret []byte) {
-		_, _ = DecodeUint64(ret)
+		got, err := DecodeUint64(ret)
+		if err != nil {
+			return
+		}
+		if len(ret) < wordLength {
+			t.Fatalf("decoded a uint64 from a %d byte response, shorter than one word", len(ret))
+		}
+		// The value must actually fit: every byte outside the low 8 has to be zero, checked here by an
+		// independent comparison rather than the same byte-loop DecodeUint64 itself uses.
+		if !bytes.Equal(ret[:wordLength-8], make([]byte, wordLength-8)) {
+			t.Fatalf("accepted a value whose high bytes are not zero: it does not fit in a uint64")
+		}
+		if want := binary.BigEndian.Uint64(ret[wordLength-8 : wordLength]); got != want {
+			t.Fatalf("decoded %d, but the low 8 bytes actually encode %d", got, want)
+		}
 	})
 }
 

x/token/services/network/evm/client/evmclient.go

@@ -43,20 +43,29 @@ type Log struct {
 	Data        []byte
 	TxHash      Hash
 	BlockNumber uint64
+	// Removed is true when the node is reporting that a reorg has undone the block this log was
+	// mined in. A caller treating a log as evidence of a committed, permanent effect must not trust
+	// one with Removed set.
+	Removed bool
 }
 
 // LogFilter selects logs by contract address, block range and indexed topics.
 // Topics follows the eth_getLogs convention: position i lists the acceptable values for topic i;
 // an empty inner slice matches any value at that position.
 //
-// ToBlock == 0 means "latest": a range ending at genesis is never a useful query, and searching up to
-// the head is what a caller looking for an event actually wants, so the zero value is spent on the
-// common case rather than requiring a separate round trip to read the current block number.
+// The upper end of the range is ToBlockTag when set, otherwise ToBlock, with ToBlock == 0 meaning
+// "latest": a range ending at genesis is never a useful query, and searching up to the head is what a
+// caller looking for an event actually wants, so the zero value is spent on the common case rather
+// than requiring a separate round trip to read the current block number. ToBlockTag lets a caller that
+// cares about reorg safety search only up to its configured tag (e.g. "finalized") instead.
 type LogFilter struct {
 	Address   Address
 	FromBlock uint64
 	ToBlock   uint64
-	Topics    [][]Hash
+	// ToBlockTag, when non-empty, is sent as the upper bound instead of ToBlock (e.g. BlockTagFinalized
+	// or BlockTagSafe).
+	ToBlockTag string
+	Topics     [][]Hash
 }
 
 // GasFees carries the EIP-1559 fee parameters suggested by the node.

x/token/services/network/evm/client/fuzz_test.go

@@ -8,6 +8,7 @@ package client
 
 import (
 	"bytes"
+	"encoding/json"
 	"strings"
 	"testing"
 )
@@ -93,3 +94,114 @@ func FuzzBytesToAddress(f *testing.F) {
 		}
 	})
 }
+
+// FuzzJSONLogToLog fuzzes the eth_getLogs record decoder. It is the point where a response body from
+// whatever node the driver was pointed at (a compromised, buggy, or simply mismatched one, per the
+// design's own framing of alternative EVM backends) turns into a driver type, so the property is that
+// a malformed record is rejected rather than panicking the caller.
+func FuzzJSONLogToLog(f *testing.F) {
+	f.Add(
+		`{"address":"0x5FbDB2315678afecb367f032d93F642f64180aa3",` +
+			`"topics":["0x853f272fffc6efc284fc16a254decca742d2347e05703e501c59968f78f81ffa"],"data":"0xabcd",` +
+			`"transactionHash":"0x853f272fffc6efc284fc16a254decca742d2347e05703e501c59968f78f81ffa","blockNumber":"0x1"}`,
+	)
+	f.Add(`{}`)
+	f.Add(`{"topics":[""]}`)
+	f.Add(`{"topics":["not hex"]}`)
+	f.Add(`{"data":"not hex"}`)
+	f.Add(`{"blockNumber":"not hex"}`)
+	f.Add(`not json at all`)
+
+	f.Fuzz(func(t *testing.T, raw string) {
+		var j jsonLog
+		if err := json.Unmarshal([]byte(raw), &j); err != nil {
+			return
+		}
+		log, err := j.toLog()
+		if err != nil {
+			return
+		}
+		assertLogMatchesRaw(t, log, j)
+	})
+}
+
+// assertLogMatchesRaw re-derives every field of log directly from j's raw strings and requires an
+// exact match. toLog is glue over the already-fuzzed HexToAddress/HexToHash/decodeHexBytes/
+// parseHexUint, so this does not re-implement hex parsing; it proves toLog actually wires each field
+// through its parser and propagates that parser's error, rather than, say, silently substituting a
+// zero value for a field it failed to parse.
+func assertLogMatchesRaw(t *testing.T, log Log, j jsonLog) {
+	t.Helper()
+
+	addr, err := HexToAddress(j.Address)
+	if err != nil || log.Address != addr {
+		t.Fatalf("toLog succeeded but Address does not match parsing j.Address directly (err=%v)", err)
+	}
+	txHash, err := HexToHash(j.TxHash)
+	if err != nil || log.TxHash != txHash {
+		t.Fatalf("toLog succeeded but TxHash does not match parsing j.TxHash directly (err=%v)", err)
+	}
+	data, err := decodeHexBytes(j.Data)
+	if err != nil || !bytes.Equal(log.Data, data) {
+		t.Fatalf("toLog succeeded but Data does not match parsing j.Data directly (err=%v)", err)
+	}
+	blockNumber, err := parseHexUint(j.BlockNumber)
+	if err != nil || log.BlockNumber != blockNumber {
+		t.Fatalf("toLog succeeded but BlockNumber does not match parsing j.BlockNumber directly (err=%v)", err)
+	}
+	if len(log.Topics) != len(j.Topics) {
+		t.Fatalf("toLog succeeded but topic count changed: got %d, raw had %d", len(log.Topics), len(j.Topics))
+	}
+	for i, raw := range j.Topics {
+		h, err := HexToHash(raw)
+		if err != nil || log.Topics[i] != h {
+			t.Fatalf("toLog succeeded but topic %d does not match parsing it directly (err=%v)", i, err)
+		}
+	}
+}
+
+// FuzzJSONReceiptToReceipt fuzzes the eth_getTransactionReceipt record decoder, the same node-supplied
+// boundary as FuzzJSONLogToLog, including the nested logs it decodes through jsonLog.toLog.
+func FuzzJSONReceiptToReceipt(f *testing.F) {
+	f.Add(`{"transactionHash":"0x853f272fffc6efc284fc16a254decca742d2347e05703e501c59968f78f81ffa","blockNumber":"0x1","status":"0x1","logs":[]}`)
+	f.Add(`{}`)
+	f.Add(`{"status":"not hex"}`)
+	f.Add(`{"blockNumber":null,"status":"0x0"}`)
+	f.Add(`{"logs":[{"data":"not hex"}]}`)
+	f.Add(`not json at all`)
+
+	f.Fuzz(func(t *testing.T, raw string) {
+		var j jsonReceipt
+		if err := json.Unmarshal([]byte(raw), &j); err != nil {
+			return
+		}
+		receipt, err := j.toReceipt()
+		if err != nil {
+			return
+		}
+
+		txHash, err := HexToHash(j.TxHash)
+		if err != nil || receipt.TxHash != txHash {
+			t.Fatalf("toReceipt succeeded but TxHash does not match parsing j.TxHash directly (err=%v)", err)
+		}
+		status, err := parseHexUint(j.Status)
+		if err != nil || receipt.Status != status {
+			t.Fatalf("toReceipt succeeded but Status does not match parsing j.Status directly (err=%v)", err)
+		}
+		if (j.BlockNumber == nil) != (receipt.BlockNumber == nil) {
+			t.Fatalf("toReceipt succeeded but BlockNumber presence does not match the raw field")
+		}
+		if j.BlockNumber != nil {
+			bn, err := parseHexUint(*j.BlockNumber)
+			if err != nil || *receipt.BlockNumber != bn {
+				t.Fatalf("toReceipt succeeded but BlockNumber does not match parsing it directly (err=%v)", err)
+			}
+		}
+		if len(receipt.Logs) != len(j.Logs) {
+			t.Fatalf("toReceipt succeeded but log count changed: got %d, raw had %d", len(receipt.Logs), len(j.Logs))
+		}
+		for i := range j.Logs {
+			assertLogMatchesRaw(t, receipt.Logs[i], j.Logs[i])
+		}
+	})
+}

x/token/services/network/evm/client/jsonrpc.go

@@ -72,14 +72,25 @@ func (c *JSONRPCClient) Ping(ctx context.Context) error {
 	return err
 }
 
-// Call performs a read-only contract call at the given block tag.
+// Call performs a read-only contract call at the given block tag. A revert is classified the same way
+// EstimateGas classifies one (ErrExecutionReverted), so a caller that later adds a Call against a
+// method capable of reverting does not have to rediscover this distinction: eth_call and
+// eth_estimateGas fail the same way against the same node.
 func (c *JSONRPCClient) Call(ctx context.Context, to Address, data []byte, blockTag string) ([]byte, error) {
 	if blockTag == "" {
 		blockTag = BlockTagFinalized
 	}
 	arg := map[string]any{"to": to.Hex(), "data": encodeHexBytes(data)}
 	var out string
-	if err := c.call(ctx, "eth_call", &out, arg, blockTag); err != nil {
+	rpcErr, err := c.invoke(ctx, "eth_call", &out, arg, blockTag)
+	if rpcErr != nil {
+		if isReverted(rpcErr) {
+			return nil, errors.Wrapf(ErrExecutionReverted, "eth_call failed: %s", rpcErr.Message)
+		}
+
+		return nil, errors.Wrap(rpcErr, "eth_call failed")
+	}
+	if err != nil {
 		return nil, err
 	}
 
@@ -102,7 +113,10 @@ func (c *JSONRPCClient) GetLogs(ctx context.Context, q LogFilter) ([]Log, error)
 		topics = append(topics, values)
 	}
 	toBlock := any(encodeHexUint(q.ToBlock))
-	if q.ToBlock == 0 {
+	switch {
+	case q.ToBlockTag != "":
+		toBlock = q.ToBlockTag
+	case q.ToBlock == 0:
 		toBlock = BlockTagLatest
 	}
 	arg := map[string]any{
@@ -218,14 +232,20 @@ func (c *JSONRPCClient) SuggestGasFees(ctx context.Context) (GasFees, error) {
 }
 
 // baseFee reads the base fee of the latest block. A chain with no base fee at all (a pre-London or
-// zero-fee configuration) reports none, which is a base fee of zero rather than an error.
+// zero-fee configuration) reports the field absent, which is a base fee of zero rather than an error.
+// A null result, by contrast, means the node did not return a block at all and is an error: head is a
+// pointer so that case is distinguishable, since unmarshaling JSON null into a non-pointer target is a
+// silent no-op that would otherwise be indistinguishable from the legitimate absent-field case.
 func (c *JSONRPCClient) baseFee(ctx context.Context) (*big.Int, error) {
-	var head struct {
+	var head *struct {
 		BaseFeePerGas string `json:"baseFeePerGas"`
 	}
 	if err := c.call(ctx, "eth_getBlockByNumber", &head, "latest", false); err != nil {
 		return nil, err
 	}
+	if head == nil {
+		return nil, errors.New("evm client: eth_getBlockByNumber(\"latest\") returned no block")
+	}
 	if head.BaseFeePerGas == "" {
 		return new(big.Int), nil
 	}
@@ -413,6 +433,7 @@ type jsonLog struct {
 	Data        string   `json:"data"`
 	TxHash      string   `json:"transactionHash"`
 	BlockNumber string   `json:"blockNumber"`
+	Removed     bool     `json:"removed"`
 }
 
 func (j *jsonLog) toLog() (Log, error) {
@@ -441,7 +462,9 @@ func (j *jsonLog) toLog() (Log, error) {
 		return Log{}, errors.Wrap(err, "invalid log block number")
 	}
 
-	return Log{Address: addr, Topics: topics, Data: data, TxHash: txHash, BlockNumber: blockNumber}, nil
+	return Log{
+		Address: addr, Topics: topics, Data: data, TxHash: txHash, BlockNumber: blockNumber, Removed: j.Removed,
+	}, nil
 }
 
 type jsonReceipt struct {
@@ -500,9 +523,9 @@ func encodeHexBytes(b []byte) string {
 	return "0x" + hex.EncodeToString(b)
 }
 
-// decodeHexBytes decodes 0x-prefixed hex data, tolerating an empty or bare "0x" value.
+// decodeHexBytes decodes 0x/0X-prefixed hex data, tolerating an empty or bare prefix value.
 func decodeHexBytes(s string) ([]byte, error) {
-	s = strings.TrimPrefix(strings.TrimSpace(s), "0x")
+	s = trimHexPrefix(s)
 	if s == "" {
 		return nil, nil
 	}
@@ -514,9 +537,9 @@ func decodeHexBytes(s string) ([]byte, error) {
 	return b, nil
 }
 
-// parseHexUint parses a 0x-prefixed hex quantity into a uint64.
+// parseHexUint parses a 0x/0X-prefixed hex quantity into a uint64.
 func parseHexUint(s string) (uint64, error) {
-	trimmed := strings.TrimPrefix(strings.TrimSpace(s), "0x")
+	trimmed := trimHexPrefix(s)
 	if trimmed == "" {
 		return 0, errors.Errorf("empty hex quantity")
 	}
@@ -528,9 +551,9 @@ func parseHexUint(s string) (uint64, error) {
 	return v, nil
 }
 
-// parseHexBig parses a 0x-prefixed hex quantity into a big.Int.
+// parseHexBig parses a 0x/0X-prefixed hex quantity into a big.Int.
 func parseHexBig(s string) (*big.Int, error) {
-	trimmed := strings.TrimPrefix(strings.TrimSpace(s), "0x")
+	trimmed := trimHexPrefix(s)
 	if trimmed == "" {
 		return nil, errors.Errorf("empty hex quantity")
 	}