core/types: fix chain ID handling in eip2930Signer

core/types/access_list_tx.go

@@ -110,6 +110,6 @@ func (tx *AccessListTx) rawSignatureValues() (v, r, s *big.Int) {
 	return tx.V, tx.R, tx.S
 }
 
-func (tx *AccessListTx) setSignatureValues(v, r, s *big.Int) {
-	tx.V, tx.R, tx.S = v, r, s
+func (tx *AccessListTx) setSignatureValues(chainID, v, r, s *big.Int) {
+	tx.ChainID, tx.V, tx.R, tx.S = chainID, v, r, s
 }

core/types/legacy_tx.go

@@ -106,6 +106,6 @@ func (tx *LegacyTx) rawSignatureValues() (v, r, s *big.Int) {
 	return tx.V, tx.R, tx.S
 }
 
-func (tx *LegacyTx) setSignatureValues(v, r, s *big.Int) {
+func (tx *LegacyTx) setSignatureValues(chainID, v, r, s *big.Int) {
 	tx.V, tx.R, tx.S = v, r, s
 }

core/types/transaction.go

@@ -79,7 +79,7 @@ type TxData interface {
 	to() *common.Address
 
 	rawSignatureValues() (v, r, s *big.Int)
-	setSignatureValues(v, r, s *big.Int)
+	setSignatureValues(chainID, v, r, s *big.Int)
 }
 
 // EncodeRLP implements rlp.Encoder
@@ -337,7 +337,7 @@ func (tx *Transaction) WithSignature(signer Signer, sig []byte) (*Transaction, e
 		return nil, err
 	}
 	cpy := tx.inner.copy()
-	cpy.setSignatureValues(v, r, s)
+	cpy.setSignatureValues(signer.ChainID(), v, r, s)
 	return &Transaction{inner: cpy, time: tx.time}, nil
 }
 

core/types/transaction_signing.go

@@ -27,9 +27,7 @@ import (
 	"github.com/ethereum/go-ethereum/params"
 )
 
-var (
-	ErrInvalidChainId = errors.New("invalid chain id for signer")
-)
+var ErrInvalidChainId = errors.New("invalid chain id for signer")
 
 // sigCache is used to cache the derived sender and contains
 // the signer used to derive it.
@@ -56,8 +54,8 @@ func MakeSigner(config *params.ChainConfig, blockNumber *big.Int) Signer {
 
 // LatestSigner returns the 'most permissive' Signer available for the given chain
 // configuration. Specifically, this enables support of EIP-155 replay protection and
-// EIP-2930 access list transactions depending on whether their respective forks are
-// scheduled to occur at any block number in the config.
+// EIP-2930 access list transactions when their respective forks are scheduled to occur at
+// any block number in the chain config.
 //
 // Use this in transaction-handling code where the current block number is unknown. If you
 // have the current block number available, use MakeSigner instead.
@@ -144,16 +142,25 @@ func Sender(signer Signer, tx *Transaction) (common.Address, error) {
 	return addr, nil
 }
 
-// Signer encapsulates transaction signature handling. Note that this interface is not a
-// stable API and may change at any time to accommodate new protocol rules.
+// Signer encapsulates transaction signature handling. The name of this type is slightly
+// misleading because Signers don't actually sign, they're just for validating and
+// processing of signatures.
+//
+// Note that this interface is not a stable API and may change at any time to accommodate
+// new protocol rules.
 type Signer interface {
 	// Sender returns the sender address of the transaction.
 	Sender(tx *Transaction) (common.Address, error)
+
 	// SignatureValues returns the raw R, S, V values corresponding to the
 	// given signature.
 	SignatureValues(tx *Transaction, sig []byte) (r, s, v *big.Int, err error)
-	// Hash returns the hash to be signed.
+	ChainID() *big.Int
+
+	// Hash returns 'signature hash', i.e. the transaction hash that is signed by the
+	// private key. This hash does not uniquely identify the transaction.
 	Hash(tx *Transaction) common.Hash
+
 	// Equal returns true if the given signer is the same as the receiver.
 	Equal(Signer) bool
 }
@@ -166,6 +173,10 @@ func NewEIP2930Signer(chainId *big.Int) Signer {
 	return eip2930Signer{NewEIP155Signer(chainId)}
 }
 
+func (s eip2930Signer) ChainID() *big.Int {
+	return s.chainId
+}
+
 func (s eip2930Signer) Equal(s2 Signer) bool {
 	x, ok := s2.(eip2930Signer)
 	return ok && x.chainId.Cmp(s.chainId) == 0
@@ -194,14 +205,19 @@ func (s eip2930Signer) Sender(tx *Transaction) (common.Address, error) {
 }
 
 func (s eip2930Signer) SignatureValues(tx *Transaction, sig []byte) (R, S, V *big.Int, err error) {
-	switch tx.Type() {
-	case LegacyTxType:
+	switch txdata := tx.inner.(type) {
+	case *LegacyTx:
 		R, S, V = decodeSignature(sig)
 		if s.chainId.Sign() != 0 {
 			V = big.NewInt(int64(sig[64] + 35))
 			V.Add(V, s.chainIdMul)
 		}
-	case AccessListTxType:
+	case *AccessListTx:
+		// Check that chain ID of tx matches the signer. We also accept ID zero here,
+		// because it indicates that the chain ID was not specified in the tx.
+		if txdata.ChainID.Sign() != 0 && txdata.ChainID.Cmp(s.chainId) != 0 {
+			return nil, nil, nil, ErrInvalidChainId
+		}
 		R, S, _ = decodeSignature(sig)
 		V = big.NewInt(int64(sig[64]))
 	default:
@@ -228,7 +244,7 @@ func (s eip2930Signer) Hash(tx *Transaction) common.Hash {
 		return prefixedRlpHash(
 			tx.Type(),
 			[]interface{}{
-				tx.ChainId(),
+				s.chainId,
 				tx.Nonce(),
 				tx.GasPrice(),
 				tx.Gas(),
@@ -262,6 +278,10 @@ func NewEIP155Signer(chainId *big.Int) EIP155Signer {
 	}
 }
 
+func (s EIP155Signer) ChainID() *big.Int {
+	return s.chainId
+}
+
 func (s EIP155Signer) Equal(s2 Signer) bool {
 	eip155, ok := s2.(EIP155Signer)
 	return ok && eip155.chainId.Cmp(s.chainId) == 0
@@ -317,6 +337,10 @@ func (s EIP155Signer) Hash(tx *Transaction) common.Hash {
 // homestead rules.
 type HomesteadSigner struct{ FrontierSigner }
 
+func (s HomesteadSigner) ChainID() *big.Int {
+	return nil
+}
+
 func (s HomesteadSigner) Equal(s2 Signer) bool {
 	_, ok := s2.(HomesteadSigner)
 	return ok
@@ -338,6 +362,10 @@ func (hs HomesteadSigner) Sender(tx *Transaction) (common.Address, error) {
 
 type FrontierSigner struct{}
 
+func (s FrontierSigner) ChainID() *big.Int {
+	return nil
+}
+
 func (s FrontierSigner) Equal(s2 Signer) bool {
 	_, ok := s2.(FrontierSigner)
 	return ok

core/types/transaction_test.go

@@ -111,26 +111,79 @@ func TestEIP2718TransactionSigHash(t *testing.T) {
 	}
 }
 
-func TestEIP2718SigHashes(t *testing.T) {
-	// the signer chainid doesn't matter for the sighash
-	signer := NewEIP2930Signer(big.NewInt(0))
-	for i, tc := range []struct {
-		rlpData  string
-		sigHash  common.Hash
-		fullHash common.Hash
+// This test checks signature operations on access list transactions.
+func TestEIP2930Signer(t *testing.T) {
+
+	var (
+		key, _  = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291")
+		keyAddr = crypto.PubkeyToAddress(key.PublicKey)
+		signer1 = NewEIP2930Signer(big.NewInt(1))
+		signer2 = NewEIP2930Signer(big.NewInt(2))
+		tx0     = NewTx(&AccessListTx{Nonce: 1})
+		tx1     = NewTx(&AccessListTx{ChainID: big.NewInt(1), Nonce: 1})
+		tx2, _  = SignNewTx(key, signer2, &AccessListTx{ChainID: big.NewInt(2), Nonce: 1})
+	)
+
+	tests := []struct {
+		tx             *Transaction
+		signer         Signer
+		wantSignerHash common.Hash
+		wantSenderErr  error
+		wantSignErr    error
+		wantHash       common.Hash // after signing
 	}{
 		{
-			rlpData:  "0xb8a701f8a486796f6c6f763380843b9aca008262d4948a8eafb1cf62bfbeb1741769dae1a9dd479961928080f838f7940000000000000000000000000000000000001337e1a0000000000000000000000000000000000000000000000000000000000000000080a0775101f92dcca278a56bfe4d613428624a1ebfc3cd9e0bcc1de80c41455b9021a06c9deac205afe7b124907d4ba54a9f46161498bd3990b90d175aac12c9a40ee9",
-			sigHash:  common.HexToHash("0xf8eb2089f9add782b02e4c0ce41540817688bf579c14736576cb1d6d562c2f6b"),
-			fullHash: common.HexToHash("0x212a85be428a85d00fb5335b013bc8d3cf7511ffdd8938de768f4ca8bf1caf50"),
+			tx:             tx0,
+			signer:         signer1,
+			wantSignerHash: common.HexToHash("846ad7672f2a3a40c1f959cd4a8ad21786d620077084d84c8d7c077714caa139"),
+			wantSenderErr:  ErrInvalidChainId,
+			wantHash:       common.HexToHash("1ccd12d8bbdb96ea391af49a35ab641e219b2dd638dea375f2bc94dd290f2549"),
+		},
+		{
+			tx:             tx1,
+			signer:         signer1,
+			wantSenderErr:  ErrInvalidSig,
+			wantSignerHash: common.HexToHash("846ad7672f2a3a40c1f959cd4a8ad21786d620077084d84c8d7c077714caa139"),
+			wantHash:       common.HexToHash("1ccd12d8bbdb96ea391af49a35ab641e219b2dd638dea375f2bc94dd290f2549"),
+		},
+		{
+			// This checks what happens when trying to sign an unsigned tx for the wrong chain.
+			tx:             tx1,
+			signer:         signer2,
+			wantSenderErr:  ErrInvalidChainId,
+			wantSignerHash: common.HexToHash("367967247499343401261d718ed5aa4c9486583e4d89251afce47f4a33c33362"),
+			wantSignErr:    ErrInvalidChainId,
+		},
+		{
+			// This checks what happens when trying to re-sign a signed tx for the wrong chain.
+			tx:             tx2,
+			signer:         signer1,
+			wantSenderErr:  ErrInvalidChainId,
+			wantSignerHash: common.HexToHash("846ad7672f2a3a40c1f959cd4a8ad21786d620077084d84c8d7c077714caa139"),
+			wantSignErr:    ErrInvalidChainId,
 		},
-	} {
-		var tx Transaction
-		rlp.DecodeBytes(common.FromHex(tc.rlpData), &tx)
-		hash := tx.Hash()
-		sigHash := signer.Hash(&tx)
-		if sigHash != tc.sigHash || hash != tc.fullHash {
-			t.Fatalf("test %d: got\nsighash %x want %x\nhash: %x want %x\n", i, sigHash, tc.sigHash, hash, tc.fullHash)
+	}
+
+	for i, test := range tests {
+		sigHash := test.signer.Hash(test.tx)
+		if sigHash != test.wantSignerHash {
+			t.Errorf("test %d: wrong sig hash: got %x, want %x", i, sigHash, test.wantSignerHash)
+		}
+		sender, err := Sender(test.signer, test.tx)
+		if err != test.wantSenderErr {
+			t.Errorf("test %d: wrong Sender error %q", i, err)
+		}
+		if err == nil && sender != keyAddr {
+			t.Errorf("test %d: wrong sender address %x", i, sender)
+		}
+		signedTx, err := SignTx(test.tx, test.signer, key)
+		if err != test.wantSignErr {
+			t.Fatalf("test %d: wrong SignTx error %q", i, err)
+		}
+		if signedTx != nil {
+			if signedTx.Hash() != test.wantHash {
+				t.Errorf("test %d: wrong tx hash after signing: got %x, want %x", i, signedTx.Hash(), test.wantHash)
+			}
 		}
 	}
 }
@@ -163,7 +216,6 @@ func TestEIP2718TransactionEncode(t *testing.T) {
 func decodeTx(data []byte) (*Transaction, error) {
 	var tx Transaction
 	t, err := &tx, rlp.Decode(bytes.NewReader(data), &tx)
-
 	return t, err
 }