core, core/types: refactored tx chain id checking

Refactored explicit chain id checking in to the Sender deriviation method

2 files changed, 46 insertions, 2 deletions

diff --git a/core/types/transaction_signing.go b/core/types/transaction_signing.go
index 48209e2d8..7d571643f 100644
--- a/core/types/transaction_signing.go
+++ b/core/types/transaction_signing.go
@@ -21,13 +21,14 @@ import (
 	"errors"
 	"fmt"
 	"math/big"
-	"reflect"
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/params"
 )
 
+var ErrInvalidChainId = errors.New("invalid chaid id for signer")
+
 // sigCache is used to cache the derived sender and contains
 // the signer used to derive it.
 type sigCache struct {
@@ -75,7 +76,7 @@ func Sender(signer Signer, tx *Transaction) (common.Address, error) {
 		// If the signer used to derive from in a previous
 		// call is not the same as used current, invalidate
 		// the cache.
-		if reflect.TypeOf(sigCache.signer) == reflect.TypeOf(signer) {
+		if sigCache.signer.Equal(signer) {
 			return sigCache.from, nil
 		}
 	}
@@ -104,6 +105,8 @@ type Signer interface {
 	SignECDSA(tx *Transaction, prv *ecdsa.PrivateKey) (*Transaction, error)
 	// WithSignature returns a copy of the transaction with the given signature
 	WithSignature(tx *Transaction, sig []byte) (*Transaction, error)
+	// Checks for equality on the signers
+	Equal(Signer) bool
 }
 
 // EIP155Transaction implements TransactionInterface using the
@@ -121,6 +124,11 @@ func NewEIP155Signer(chainId *big.Int) EIP155Signer {
 	}
 }
 
+func (s EIP155Signer) Equal(s2 Signer) bool {
+	eip155, ok := s2.(EIP155Signer)
+	return ok && eip155.chainId.Cmp(s.chainId) == 0
+}
+
 func (s EIP155Signer) SignECDSA(tx *Transaction, prv *ecdsa.PrivateKey) (*Transaction, error) {
 	return SignECDSA(s, tx, prv)
 }
@@ -131,6 +139,10 @@ func (s EIP155Signer) PublicKey(tx *Transaction) ([]byte, error) {
 		return (HomesteadSigner{}).PublicKey(tx)
 	}
 
+	if tx.ChainId().Cmp(s.chainId) != 0 {
+		return nil, ErrInvalidChainId
+	}
+
 	V := normaliseV(s, tx.data.V)
 	if !crypto.ValidateSignatureValues(V, tx.data.R, tx.data.S, true) {
 		return nil, ErrInvalidSig
@@ -200,6 +212,11 @@ func (s EIP155Signer) SigECDSA(tx *Transaction, prv *ecdsa.PrivateKey) (*Transac
 // homestead rules.
 type HomesteadSigner struct{ FrontierSigner }
 
+func (s HomesteadSigner) Equal(s2 Signer) bool {
+	_, ok := s2.(HomesteadSigner)
+	return ok
+}
+
 // WithSignature returns a new transaction with the given snature.
 // This snature needs to be formatted as described in the yellow paper (v+27).
 func (hs HomesteadSigner) WithSignature(tx *Transaction, sig []byte) (*Transaction, error) {
@@ -251,6 +268,11 @@ func (hs HomesteadSigner) PublicKey(tx *Transaction) ([]byte, error) {
 
 type FrontierSigner struct{}
 
+func (s FrontierSigner) Equal(s2 Signer) bool {
+	_, ok := s2.(FrontierSigner)
+	return ok
+}
+
 // WithSignature returns a new transaction with the given snature.
 // This snature needs to be formatted as described in the yellow paper (v+27).
 func (fs FrontierSigner) WithSignature(tx *Transaction, sig []byte) (*Transaction, error) {
diff --git a/core/types/transaction_signing_test.go b/core/types/transaction_signing_test.go
index 89c590262..dc618e570 100644
--- a/core/types/transaction_signing_test.go
+++ b/core/types/transaction_signing_test.go
@@ -114,3 +114,25 @@ func TestEIP155SigningVitalik(t *testing.T) {
 
 	}
 }
+
+func TestChainId(t *testing.T) {
+	key, _ := defaultTestKey()
+
+	tx := NewTransaction(0, common.Address{}, new(big.Int), new(big.Int), new(big.Int), nil)
+
+	var err error
+	tx, err = tx.SignECDSA(NewEIP155Signer(big.NewInt(1)), key)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = Sender(NewEIP155Signer(big.NewInt(2)), tx)
+	if err != ErrInvalidChainId {
+		t.Error("expected error:", ErrInvalidChainId)
+	}
+
+	_, err = Sender(NewEIP155Signer(big.NewInt(1)), tx)
+	if err != nil {
+		t.Error("expected no error")
+	}
+}