op-chain-ops: validation + test coverage

op-chain-ops/safe/batch_file.go

@@ -4,6 +4,7 @@
 package safe
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
 	"golang.org/x/exp/maps"
@@ -13,6 +14,7 @@ import (
 	"github.com/ethereum/go-ethereum/accounts/abi"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
+	"github.com/ethereum/go-ethereum/crypto"
 )
 
 // Batch represents a Safe tx-builder transaction.
@@ -93,6 +95,16 @@ func (b *Batch) AddCall(to common.Address, value *big.Int, sig string, args []an
 	return nil
 }
 
+// Check will check the batch for errors
+func (b *Batch) Check() error {
+	for _, tx := range b.Transactions {
+		if err := tx.Check(); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // bathcFileMarshaling is a helper type used for JSON marshaling.
 type batchMarshaling struct {
 	Version      string             `json:"version"`
@@ -154,6 +166,48 @@ type BatchTransaction struct {
 	InputValues map[string]string `json:"contractInputsValues"`
 }
 
+// Check will check the batch transaction for errors.
+// An error is defined by:
+// - incorrectly encoded calldata
+// - mismatch in number of arguments
+func (bt *BatchTransaction) Check() error {
+	if len(bt.Method.Inputs) != len(bt.InputValues) {
+		return fmt.Errorf("expected %d inputs but got %d", len(bt.Method.Inputs), len(bt.InputValues))
+	}
+
+	if len(bt.Data) > 0 && bt.Method.Name != "fallback" {
+		if len(bt.Data) < 4 {
+			return fmt.Errorf("must have at least 4 bytes of calldata, got %d", len(bt.Data))
+		}
+		sig := bt.Signature()
+		selector := crypto.Keccak256([]byte(sig))[0:4]
+		if !bytes.Equal(bt.Data[0:4], selector) {
+			return fmt.Errorf("data does not match signature")
+		}
+	}
+	return nil
+}
+
+// Signature returns the function signature of the batch transaction.
+func (bt *BatchTransaction) Signature() string {
+	types := make([]string, len(bt.Method.Inputs))
+	for i, input := range bt.Method.Inputs {
+		types[i] = buildFunctionSignature(input)
+	}
+	return fmt.Sprintf("%s(%s)", bt.Method.Name, strings.Join(types, ","))
+}
+
+func buildFunctionSignature(input ContractInput) string {
+	if input.Type == "tuple" {
+		types := make([]string, len(input.Components))
+		for i, component := range input.Components {
+			types[i] = buildFunctionSignature(component)
+		}
+		return fmt.Sprintf("(%s)", strings.Join(types, ","))
+	}
+	return input.InternalType
+}
+
 // UnmarshalJSON will unmarshal a BatchTransaction from JSON.
 func (b *BatchTransaction) UnmarshalJSON(data []byte) error {
 	var bt batchTransactionMarshaling
@@ -162,6 +216,9 @@ func (b *BatchTransaction) UnmarshalJSON(data []byte) error {
 	}
 	b.To = common.HexToAddress(bt.To)
 	b.Value = new(big.Int).SetUint64(bt.Value)
+	if bt.Data != nil {
+		b.Data = common.CopyBytes(*bt.Data)
+	}
 	b.Method = bt.Method
 	b.InputValues = bt.InputValues
 	return nil

op-chain-ops/safe/batch_file_test.go

@@ -3,7 +3,7 @@ package safe
 import (
 	"bytes"
 	"encoding/json"
-	//"fmt"
+	"errors"
 	"math/big"
 	"os"
 	"testing"
@@ -59,6 +59,9 @@ func TestBatchAddCallFinalizeWithdrawalTransaction(t *testing.T) {
 	value := big.NewInt(222)
 
 	require.NoError(t, batch.AddCall(to, value, sig, argument, portalABI))
+	require.NoError(t, batch.Check())
+	require.Equal(t, batch.Transactions[0].Signature(), "finalizeWithdrawalTransaction((uint256,address,address,uint256,uint256,bytes))")
+
 	expected, err := os.ReadFile("testdata/finalize-withdrawal-tx.json")
 	require.NoError(t, err)
 
@@ -87,6 +90,9 @@ func TestBatchAddCallDespostTransaction(t *testing.T) {
 	}
 
 	require.NoError(t, batch.AddCall(to, value, sig, argument, portalABI))
+	require.NoError(t, batch.Check())
+	require.Equal(t, batch.Transactions[0].Signature(), "depositTransaction(address,uint256,uint64,bool,bytes)")
+
 	expected, err := os.ReadFile("testdata/deposit-tx.json")
 	require.NoError(t, err)
 
@@ -94,3 +100,57 @@ func TestBatchAddCallDespostTransaction(t *testing.T) {
 	require.NoError(t, err)
 	require.JSONEq(t, string(expected), string(serialized))
 }
+
+// TestBatchCheck checks for the various failure cases of Batch.Check
+// as well as a simple check for a valid batch.
+func TestBatchCheck(t *testing.T) {
+	cases := []struct {
+		name string
+		bt   BatchTransaction
+		err  error
+	}{
+		{
+			name: "bad-input-count",
+			bt: BatchTransaction{
+				Method: ContractMethod{},
+				InputValues: map[string]string{
+					"foo": "bar",
+				},
+			},
+			err: errors.New("expected 0 inputs but got 1"),
+		},
+		{
+			name: "bad-calldata-too-small",
+			bt: BatchTransaction{
+				Data: []byte{0x01},
+			},
+			err: errors.New("must have at least 4 bytes of calldata, got 1"),
+		},
+		{
+			name: "bad-calldata-mismatch",
+			bt: BatchTransaction{
+				Data: []byte{0x01, 0x02, 0x03, 0x04},
+				Method: ContractMethod{
+					Name: "foo",
+				},
+			},
+			err: errors.New("data does not match signature"),
+		},
+		{
+			name: "good-calldata",
+			bt: BatchTransaction{
+				Data: []byte{0xc2, 0x98, 0x55, 0x78},
+				Method: ContractMethod{
+					Name: "foo",
+				},
+			},
+			err: nil,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			require.Equal(t, tc.err, tc.bt.Check())
+		})
+	}
+}

op-chain-ops/safe/batch_helpers.go

@@ -162,3 +162,16 @@ func stringifyType(t abi.Type) (string, error) {
 		return "", fmt.Errorf("unknown type: %d", t.T)
 	}
 }
+
+// buildFunctionSignature builds a function signature from a ContractInput.
+// It is recursive to handle tuples.
+func buildFunctionSignature(input ContractInput) string {
+	if input.Type == "tuple" {
+		types := make([]string, len(input.Components))
+		for i, component := range input.Components {
+			types[i] = buildFunctionSignature(component)
+		}
+		return fmt.Sprintf("(%s)", strings.Join(types, ","))
+	}
+	return input.InternalType
+}