Polish ERC-6492 errors and edge-case coverage

eip1271.go

@@ -36,7 +36,7 @@ func VerifyEIP1271(
 
 	input, err := encodeEIP1271Call(hash, signature)
 	if err != nil {
-		return Result{}, fmt.Errorf("%w: encode eip1271 calldata: %v", ErrInvalidABIOutput, err)
+		return Result{}, fmt.Errorf("%w: encode eip1271 calldata: %v", ErrInvalidABIInput, err)
 	}
 
 	call := ethereum.CallMsg{

eip1271_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/ethereum/go-ethereum"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/vm"
+	"github.com/ethereum/go-ethereum/crypto"
 )
 
 func TestVerifyEIP1271ValidMagicValue(t *testing.T) {
@@ -300,6 +301,22 @@ func TestVerifyEIP1271WithFrom(t *testing.T) {
 	}
 }
 
+func TestEIP1271SelectorAndMagicValue(t *testing.T) {
+	selector := crypto.Keccak256([]byte("isValidSignature(bytes32,bytes)"))[:4]
+
+	if !bytes.Equal(selector, eip1271IsValidSignatureSelector[:]) {
+		t.Fatalf(
+			"eip1271 selector = 0x%x, want 0x%x",
+			eip1271IsValidSignatureSelector,
+			selector,
+		)
+	}
+
+	if !bytes.Equal(selector, eip1271MagicValue[:]) {
+		t.Fatalf("eip1271 magic value = 0x%x, want selector 0x%x", eip1271MagicValue, selector)
+	}
+}
+
 type recordingContractCaller struct {
 	output []byte
 	err    error

erc6492.go

@@ -54,6 +54,10 @@ func VerifyERC6492(
 			return Result{}, ErrMissingERC6492Factory
 		}
 
+		if cfg.erc6492Factory == (common.Address{}) {
+			return Result{}, ErrZeroERC6492FactoryAddress
+		}
+
 		wrapped, err := WrapERC6492(cfg.erc6492Factory, cfg.erc6492FactoryData, signature)
 		if err != nil {
 			return Result{}, err
@@ -66,9 +70,13 @@ func VerifyERC6492(
 		return Result{}, ErrDeploylessVerifierMissing
 	}
 
+	if cfg.erc6492VerifierAddress == (common.Address{}) {
+		return Result{}, ErrZeroERC6492VerifierAddress
+	}
+
 	input, err := encodeERC6492VerifierCall(signer, hash, wrappedSignature)
 	if err != nil {
-		return Result{}, fmt.Errorf("%w: encode erc6492 verifier calldata: %v", ErrInvalidABIOutput, err)
+		return Result{}, fmt.Errorf("%w: encode erc6492 verifier calldata: %v", ErrInvalidABIInput, err)
 	}
 
 	call := ethereum.CallMsg{

erc6492_test.go

@@ -205,6 +205,10 @@ func TestUnwrapERC6492MalformedSignatures(t *testing.T) {
 			name:      "missing suffix",
 			signature: []byte("missing suffix"),
 		},
+		{
+			name:      "suffix only",
+			signature: append([]byte(nil), erc6492MagicSuffix[:]...),
+		},
 		{
 			name:      "malformed abi with suffix",
 			signature: append([]byte("not valid abi"), erc6492MagicSuffix[:]...),
@@ -323,6 +327,34 @@ func TestVerifyERC6492UnwrappedWithoutFactory(t *testing.T) {
 	}
 }
 
+func TestVerifyERC6492UnwrappedWithZeroFactoryAddress(t *testing.T) {
+	caller := &recordingERC6492Caller{
+		output: mustPackERC6492VerifierBool(t, true),
+	}
+
+	result, err := VerifyERC6492(
+		context.Background(),
+		caller,
+		common.HexToAddress("0x3333333333333333333333333333333333333333"),
+		common.HexToHash("0xabababababababababababababababababababababababababababababababab"),
+		[]byte{0x01, 0x02, 0x03},
+		WithERC6492Factory(common.Address{}, []byte{0x04, 0x05}),
+		WithERC6492VerifierAddress(common.HexToAddress("0x4444444444444444444444444444444444444444")),
+	)
+
+	if !errors.Is(err, ErrZeroERC6492FactoryAddress) {
+		t.Fatalf("VerifyERC6492 error = %v, want ErrZeroERC6492FactoryAddress", err)
+	}
+
+	if result != (Result{}) {
+		t.Fatalf("expected zero result on error, got %+v", result)
+	}
+
+	if caller.calls != 0 {
+		t.Fatalf("expected verifier not to be called, got %d calls", caller.calls)
+	}
+}
+
 func TestVerifyERC6492MalformedWrappedSignature(t *testing.T) {
 	caller := &recordingERC6492Caller{
 		output: mustPackERC6492VerifierBool(t, true),
@@ -352,6 +384,42 @@ func TestVerifyERC6492MalformedWrappedSignature(t *testing.T) {
 	}
 }
 
+func TestVerifyERC6492ZeroVerifierAddress(t *testing.T) {
+	wrapped, err := WrapERC6492(
+		common.HexToAddress("0x6666666666666666666666666666666666666666"),
+		[]byte{0x01},
+		[]byte{0x02},
+	)
+	if err != nil {
+		t.Fatalf("WrapERC6492 returned error: %v", err)
+	}
+
+	caller := &recordingERC6492Caller{
+		output: mustPackERC6492VerifierBool(t, true),
+	}
+
+	result, err := VerifyERC6492(
+		context.Background(),
+		caller,
+		common.HexToAddress("0x7777777777777777777777777777777777777777"),
+		common.HexToHash("0xcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcd"),
+		wrapped,
+		WithERC6492VerifierAddress(common.Address{}),
+	)
+
+	if !errors.Is(err, ErrZeroERC6492VerifierAddress) {
+		t.Fatalf("VerifyERC6492 error = %v, want ErrZeroERC6492VerifierAddress", err)
+	}
+
+	if result != (Result{}) {
+		t.Fatalf("expected zero result on error, got %+v", result)
+	}
+
+	if caller.calls != 0 {
+		t.Fatalf("expected verifier not to be called, got %d calls", caller.calls)
+	}
+}
+
 func TestVerifyERC6492MissingVerifierAddressReturnsDeploylessGuard(t *testing.T) {
 	factory := common.HexToAddress("0x6666666666666666666666666666666666666666")
 	wrapped, err := WrapERC6492(factory, []byte{0x01}, []byte{0x02})

errors.go

@@ -3,24 +3,35 @@ package erc6492
 import "errors"
 
 var (
-	// ErrNilCaller is returned when a verification path requires a caller but nil was provided.
+	// ErrNilCaller is returned when verification requires a caller but nil was provided.
 	ErrNilCaller = errors.New("erc6492: nil caller")
 
-	// ErrMalformedERC6492Signature is returned when an ERC-6492 wrapper is missing or cannot be ABI-decoded.
+	// ErrMalformedERC6492Signature is returned for missing or malformed ERC-6492 wrappers.
 	ErrMalformedERC6492Signature = errors.New("erc6492: malformed ERC-6492 signature")
 
-	// ErrMissingERC6492Factory is returned when an unwrapped ERC-6492 signature is verified without factory data.
+	// ErrMissingERC6492Factory is returned when an unwrapped signature cannot be
+	// prepared for ERC-6492 verification because factory data was not provided.
 	ErrMissingERC6492Factory = errors.New("erc6492: missing ERC-6492 factory")
 
-	// ErrMissingERC6492Verifier is returned when a deployed ERC-6492 verifier address is required but missing.
-	ErrMissingERC6492Verifier = errors.New("erc6492: missing ERC-6492 verifier address")
+	// ErrZeroERC6492FactoryAddress is returned when ERC-6492 wrapping is
+	// requested with the zero factory address.
+	ErrZeroERC6492FactoryAddress = errors.New("erc6492: zero ERC-6492 factory address")
 
-	// ErrDeploylessVerifierMissing is returned when deployless ERC-6492 verification is requested before verifier bytecode provenance has been added.
+	// ErrZeroERC6492VerifierAddress is returned when ERC-6492 verification is
+	// configured with the zero verifier address.
+	ErrZeroERC6492VerifierAddress = errors.New("erc6492: zero ERC-6492 verifier address")
+
+	// ErrDeploylessVerifierMissing is returned when ERC-6492 verification needs
+	// a verifier, but no deployed verifier was configured.
 	ErrDeploylessVerifierMissing = errors.New("erc6492: deployless verifier unavailable; provide WithERC6492VerifierAddress")
 
-	// ErrInvalidABIOutput is returned when ABI encoding or return-data decoding fails.
+	// ErrInvalidABIInput is returned when calldata cannot be ABI-encoded.
+	ErrInvalidABIInput = errors.New("erc6492: invalid ABI input")
+
+	// ErrInvalidABIOutput is returned when contract return data cannot be decoded.
 	ErrInvalidABIOutput = errors.New("erc6492: invalid ABI output")
 
-	// ErrUnexpectedVerifierData is returned when an ERC-6492 verifier returns data in an unexpected format.
+	// ErrUnexpectedVerifierData is returned when an ERC-6492 verifier returns
+	// data that does not match the expected ABI.
 	ErrUnexpectedVerifierData = errors.New("erc6492: unexpected verifier return data")
 )

verify_test.go

@@ -1,6 +1,7 @@
 package erc6492
 
 import (
+	"bytes"
 	"context"
 	"errors"
 	"math/big"
@@ -137,6 +138,72 @@ func TestVerifyWithERC6492FactoryRoutesBeforeCodeAt(t *testing.T) {
 	}
 }
 
+func TestVerifyWithERC6492FactoryAndAlreadyWrappedSignatureDoesNotDoubleWrap(t *testing.T) {
+	ctx := context.Background()
+	signer := common.HexToAddress("0x1414141414141414141414141414141414141414")
+	hash := common.HexToHash("0x1515151515151515151515151515151515151515151515151515151515151515")
+	verifier := common.HexToAddress("0x1616161616161616161616161616161616161616")
+
+	originalFactory := common.HexToAddress("0x1717171717171717171717171717171717171717")
+	originalFactoryData := []byte{0x01, 0x02, 0x03}
+	innerSignature := []byte{0x04, 0x05, 0x06}
+
+	wrapped, err := WrapERC6492(originalFactory, originalFactoryData, innerSignature)
+	if err != nil {
+		t.Fatalf("WrapERC6492 returned error: %v", err)
+	}
+
+	unusedFactory := common.HexToAddress("0x1818181818181818181818181818181818181818")
+	unusedFactoryData := []byte{0xaa, 0xbb, 0xcc}
+
+	caller := &recordingUniversalCaller{
+		codeErr:    errors.New("CodeAt should not be called"),
+		callOutput: mustPackERC6492VerifierBool(t, true),
+	}
+
+	result, err := Verify(
+		ctx,
+		caller,
+		signer,
+		hash,
+		wrapped,
+		WithERC6492Factory(unusedFactory, unusedFactoryData),
+		WithERC6492VerifierAddress(verifier),
+	)
+	assertNoError(t, err)
+	assertResult(t, result, true, MethodERC6492)
+
+	if caller.codeCalls != 0 {
+		t.Fatalf("expected CodeAt not to be called, got %d calls", caller.codeCalls)
+	}
+
+	if caller.callCalls != 1 {
+		t.Fatalf("expected 1 verifier call, got %d", caller.callCalls)
+	}
+
+	_, _, verifierSignature := mustUnpackERC6492VerifierCallArgs(t, caller.call.Data[4:])
+	if !bytes.Equal(verifierSignature, wrapped) {
+		t.Fatalf("verifier received signature = %x, want original wrapped signature %x", verifierSignature, wrapped)
+	}
+
+	decoded, err := UnwrapERC6492(verifierSignature)
+	if err != nil {
+		t.Fatalf("UnwrapERC6492 returned error: %v", err)
+	}
+
+	if decoded.Factory != originalFactory {
+		t.Fatalf("wrapped factory = %s, want original factory %s", decoded.Factory.Hex(), originalFactory.Hex())
+	}
+
+	if !bytes.Equal(decoded.FactoryData, originalFactoryData) {
+		t.Fatalf("wrapped factory data = %x, want original factory data %x", decoded.FactoryData, originalFactoryData)
+	}
+
+	if !bytes.Equal(decoded.Signature, innerSignature) {
+		t.Fatalf("wrapped inner signature = %x, want original inner signature %x", decoded.Signature, innerSignature)
+	}
+}
+
 func TestVerifyERC6492VerifierFalseIsFinal(t *testing.T) {
 	ctx := context.Background()
 	signer := common.HexToAddress("0x7777777777777777777777777777777777777777")
@@ -193,6 +260,29 @@ func TestVerifyMalformedWrappedERC6492ErrorsBeforeCodeAt(t *testing.T) {
 	}
 }
 
+func TestVerifySuffixOnlyERC6492SignatureErrorsBeforeCodeAt(t *testing.T) {
+	ctx := context.Background()
+	signer := common.HexToAddress("0x1919191919191919191919191919191919191919")
+	hash := common.HexToHash("0x2020202020202020202020202020202020202020202020202020202020202020")
+	signature := append([]byte(nil), erc6492MagicSuffix[:]...)
+
+	caller := &recordingUniversalCaller{
+		codeErr: errors.New("CodeAt should not be called"),
+	}
+
+	result, err := Verify(ctx, caller, signer, hash, signature)
+	assertErrorIs(t, err, ErrMalformedERC6492Signature)
+	assertZeroResult(t, result)
+
+	if caller.codeCalls != 0 {
+		t.Fatalf("expected CodeAt not to be called, got %d calls", caller.codeCalls)
+	}
+
+	if caller.callCalls != 0 {
+		t.Fatalf("expected verifier not to be called, got %d calls", caller.callCalls)
+	}
+}
+
 func TestVerifyWithERC6492FactoryMissingVerifierErrorsBeforeCodeAt(t *testing.T) {
 	ctx := context.Background()
 	signer := common.HexToAddress("0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb")