accounts, core, crypto, internal: use normalised V during signature handling (#3455)

To address increasing complexity in code that handles signatures, this PR
discards all notion of "different" signature types at the library level. Both
the crypto and accounts package is reduced to only be able to produce plain
canonical secp256k1 signatures. This makes the crpyto APIs much cleaner,
simpler and harder to abuse.

5 files changed, 27 insertions, 96 deletions

diff --git a/core/types/block_test.go b/core/types/block_test.go
index b95bddcfc..93435ca00 100644
--- a/core/types/block_test.go
+++ b/core/types/block_test.go
@@ -53,7 +53,7 @@ func TestBlockEncoding(t *testing.T) {
 
 	tx1 := NewTransaction(0, common.HexToAddress("095e7baea6a6c7c4c2dfeb977efac326af552d87"), big.NewInt(10), big.NewInt(50000), big.NewInt(10), nil)
 
-	tx1, _ = tx1.WithSignature(HomesteadSigner{}, common.Hex2Bytes("9bea4c4daac7c7c52e093e6a4c35dbbcf8856f1af7b059ba20253e70848d094f8a8fae537ce25ed8cb5af9adac3f141af69bd515bd2ba031522df09b97dd72b11b"))
+	tx1, _ = tx1.WithSignature(HomesteadSigner{}, common.Hex2Bytes("9bea4c4daac7c7c52e093e6a4c35dbbcf8856f1af7b059ba20253e70848d094f8a8fae537ce25ed8cb5af9adac3f141af69bd515bd2ba031522df09b97dd72b100"))
 	fmt.Println(block.Transactions()[0].Hash())
 	fmt.Println(tx1.data)
 	fmt.Println(tx1.Hash())
diff --git a/core/types/transaction.go b/core/types/transaction.go
index f566dc365..87b54ab30 100644
--- a/core/types/transaction.go
+++ b/core/types/transaction.go
@@ -199,9 +199,9 @@ func (tx *Transaction) UnmarshalJSON(input []byte) error {
 
 	var V byte
 	if isProtectedV((*big.Int)(dec.V)) {
-		V = normaliseV(NewEIP155Signer(deriveChainId((*big.Int)(dec.V))), (*big.Int)(dec.V))
+		V = byte((new(big.Int).Sub((*big.Int)(dec.V), deriveChainId((*big.Int)(dec.V))).Uint64()) - 35)
 	} else {
-		V = byte(((*big.Int)(dec.V)).Uint64())
+		V = byte(((*big.Int)(dec.V)).Uint64() - 27)
 	}
 	if !crypto.ValidateSignatureValues(V, (*big.Int)(dec.R), (*big.Int)(dec.S), false) {
 		return ErrInvalidSig
@@ -272,51 +272,6 @@ func (tx *Transaction) Size() common.StorageSize {
 	return common.StorageSize(c)
 }
 
-/*
-// From returns the address derived from the signature (V, R, S) using secp256k1
-// elliptic curve and an error if it failed deriving or upon an incorrect
-// signature.
-//
-// From Uses the homestead consensus rules to determine whether the signature is
-// valid.
-//
-// From caches the address, allowing it to be used regardless of
-// Frontier / Homestead. however, the first time called it runs
-// signature validations, so we need two versions. This makes it
-// easier to ensure backwards compatibility of things like package rpc
-// where eth_getblockbynumber uses tx.From() and needs to work for
-// both txs before and after the first homestead block. Signatures
-// valid in homestead are a subset of valid ones in Frontier)
-func (tx *Transaction) From() (common.Address, error) {
-	if tx.signer == nil {
-		return common.Address{}, errNoSigner
-	}
-
-	if from := tx.from.Load(); from != nil {
-		return from.(common.Address), nil
-	}
-
-	pubkey, err := tx.signer.PublicKey(tx)
-	if err != nil {
-		return common.Address{}, err
-	}
-	var addr common.Address
-	copy(addr[:], crypto.Keccak256(pubkey[1:])[12:])
-	tx.from.Store(addr)
-	return addr, nil
-}
-
-// SignatureValues returns the ECDSA signature values contained in the transaction.
-func (tx *Transaction) SignatureValues() (v byte, r *big.Int, s *big.Int, err error) {
-	if tx.signer == nil {
-		return 0, nil, nil,errNoSigner
-	}
-
-	return normaliseV(tx.signer, tx.data.V), new(big.Int).Set(tx.data.R),new(big.Int).Set(tx.data.S), nil
-}
-
-*/
-
 // AsMessage returns the transaction as a core.Message.
 //
 // AsMessage requires a signer to derive the sender.
diff --git a/core/types/transaction_signing.go b/core/types/transaction_signing.go
index 7d571643f..8952bd574 100644
--- a/core/types/transaction_signing.go
+++ b/core/types/transaction_signing.go
@@ -53,7 +53,7 @@ func MakeSigner(config *params.ChainConfig, blockNumber *big.Int) Signer {
 // SignECDSA signs the transaction using the given signer and private key
 func SignECDSA(s Signer, tx *Transaction, prv *ecdsa.PrivateKey) (*Transaction, error) {
 	h := s.Hash(tx)
-	sig, err := crypto.SignEthereum(h[:], prv)
+	sig, err := crypto.Sign(h[:], prv)
 	if err != nil {
 		return nil, err
 	}
@@ -91,11 +91,6 @@ func Sender(signer Signer, tx *Transaction) (common.Address, error) {
 	return addr, nil
 }
 
-// SignatureValues returns the ECDSA signature values contained in the transaction.
-func SignatureValues(signer Signer, tx *Transaction) (v byte, r *big.Int, s *big.Int) {
-	return normaliseV(signer, tx.data.V), new(big.Int).Set(tx.data.R), new(big.Int).Set(tx.data.S)
-}
-
 type Signer interface {
 	// Hash returns the rlp encoded hash for signatures
 	Hash(tx *Transaction) common.Hash
@@ -143,17 +138,16 @@ func (s EIP155Signer) PublicKey(tx *Transaction) ([]byte, error) {
 		return nil, ErrInvalidChainId
 	}
 
-	V := normaliseV(s, tx.data.V)
+	V := byte(new(big.Int).Sub(tx.data.V, s.chainIdMul).Uint64() - 35)
 	if !crypto.ValidateSignatureValues(V, tx.data.R, tx.data.S, true) {
 		return nil, ErrInvalidSig
 	}
-
 	// encode the signature in uncompressed format
 	R, S := tx.data.R.Bytes(), tx.data.S.Bytes()
 	sig := make([]byte, 65)
 	copy(sig[32-len(R):32], R)
 	copy(sig[64-len(S):64], S)
-	sig[64] = V - 27
+	sig[64] = V
 
 	// recover the public key from the signature
 	hash := s.Hash(tx)
@@ -167,8 +161,8 @@ func (s EIP155Signer) PublicKey(tx *Transaction) ([]byte, error) {
 	return pub, nil
 }
 
-// WithSignature returns a new transaction with the given signature.
-// This signature needs to be formatted as described in the yellow paper (v+27).
+// WithSignature returns a new transaction with the given signature. This signature
+// needs to be in the [R || S || V] format where V is 0 or 1.
 func (s EIP155Signer) WithSignature(tx *Transaction, sig []byte) (*Transaction, error) {
 	if len(sig) != 65 {
 		panic(fmt.Sprintf("wrong size for snature: got %d, want 65", len(sig)))
@@ -179,7 +173,7 @@ func (s EIP155Signer) WithSignature(tx *Transaction, sig []byte) (*Transaction,
 	cpy.data.S = new(big.Int).SetBytes(sig[32:64])
 	cpy.data.V = new(big.Int).SetBytes([]byte{sig[64]})
 	if s.chainId.BitLen() > 0 {
-		cpy.data.V = big.NewInt(int64(sig[64] - 27 + 35))
+		cpy.data.V = big.NewInt(int64(sig[64] + 35))
 		cpy.data.V.Add(cpy.data.V, s.chainIdMul)
 	}
 	return cpy, nil
@@ -201,7 +195,7 @@ func (s EIP155Signer) Hash(tx *Transaction) common.Hash {
 
 func (s EIP155Signer) SigECDSA(tx *Transaction, prv *ecdsa.PrivateKey) (*Transaction, error) {
 	h := s.Hash(tx)
-	sig, err := crypto.SignEthereum(h[:], prv)
+	sig, err := crypto.Sign(h[:], prv)
 	if err != nil {
 		return nil, err
 	}
@@ -217,8 +211,8 @@ func (s HomesteadSigner) Equal(s2 Signer) bool {
 	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).
+// WithSignature returns a new transaction with the given signature. This signature
+// needs to be in the [R || S || V] format where V is 0 or 1.
 func (hs HomesteadSigner) WithSignature(tx *Transaction, sig []byte) (*Transaction, error) {
 	if len(sig) != 65 {
 		panic(fmt.Sprintf("wrong size for snature: got %d, want 65", len(sig)))
@@ -226,13 +220,13 @@ func (hs HomesteadSigner) WithSignature(tx *Transaction, sig []byte) (*Transacti
 	cpy := &Transaction{data: tx.data}
 	cpy.data.R = new(big.Int).SetBytes(sig[:32])
 	cpy.data.S = new(big.Int).SetBytes(sig[32:64])
-	cpy.data.V = new(big.Int).SetBytes([]byte{sig[64]})
+	cpy.data.V = new(big.Int).SetBytes([]byte{sig[64] + 27})
 	return cpy, nil
 }
 
 func (hs HomesteadSigner) SignECDSA(tx *Transaction, prv *ecdsa.PrivateKey) (*Transaction, error) {
 	h := hs.Hash(tx)
-	sig, err := crypto.SignEthereum(h[:], prv)
+	sig, err := crypto.Sign(h[:], prv)
 	if err != nil {
 		return nil, err
 	}
@@ -243,7 +237,7 @@ func (hs HomesteadSigner) PublicKey(tx *Transaction) ([]byte, error) {
 	if tx.data.V.BitLen() > 8 {
 		return nil, ErrInvalidSig
 	}
-	V := byte(tx.data.V.Uint64())
+	V := byte(tx.data.V.Uint64() - 27)
 	if !crypto.ValidateSignatureValues(V, tx.data.R, tx.data.S, true) {
 		return nil, ErrInvalidSig
 	}
@@ -252,7 +246,7 @@ func (hs HomesteadSigner) PublicKey(tx *Transaction) ([]byte, error) {
 	sig := make([]byte, 65)
 	copy(sig[32-len(r):32], r)
 	copy(sig[64-len(s):64], s)
-	sig[64] = V - 27
+	sig[64] = V
 
 	// recover the public key from the snature
 	hash := hs.Hash(tx)
@@ -273,8 +267,8 @@ func (s FrontierSigner) Equal(s2 Signer) bool {
 	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).
+// WithSignature returns a new transaction with the given signature. This signature
+// needs to be in the [R || S || V] format where V is 0 or 1.
 func (fs FrontierSigner) WithSignature(tx *Transaction, sig []byte) (*Transaction, error) {
 	if len(sig) != 65 {
 		panic(fmt.Sprintf("wrong size for snature: got %d, want 65", len(sig)))
@@ -282,13 +276,13 @@ func (fs FrontierSigner) WithSignature(tx *Transaction, sig []byte) (*Transactio
 	cpy := &Transaction{data: tx.data}
 	cpy.data.R = new(big.Int).SetBytes(sig[:32])
 	cpy.data.S = new(big.Int).SetBytes(sig[32:64])
-	cpy.data.V = new(big.Int).SetBytes([]byte{sig[64]})
+	cpy.data.V = new(big.Int).SetBytes([]byte{sig[64] + 27})
 	return cpy, nil
 }
 
 func (fs FrontierSigner) SignECDSA(tx *Transaction, prv *ecdsa.PrivateKey) (*Transaction, error) {
 	h := fs.Hash(tx)
-	sig, err := crypto.SignEthereum(h[:], prv)
+	sig, err := crypto.Sign(h[:], prv)
 	if err != nil {
 		return nil, err
 	}
@@ -313,7 +307,7 @@ func (fs FrontierSigner) PublicKey(tx *Transaction) ([]byte, error) {
 		return nil, ErrInvalidSig
 	}
 
-	V := byte(tx.data.V.Uint64())
+	V := byte(tx.data.V.Uint64() - 27)
 	if !crypto.ValidateSignatureValues(V, tx.data.R, tx.data.S, false) {
 		return nil, ErrInvalidSig
 	}
@@ -322,7 +316,7 @@ func (fs FrontierSigner) PublicKey(tx *Transaction) ([]byte, error) {
 	sig := make([]byte, 65)
 	copy(sig[32-len(r):32], r)
 	copy(sig[64-len(s):64], s)
-	sig[64] = V - 27
+	sig[64] = V
 
 	// recover the public key from the snature
 	hash := fs.Hash(tx)
@@ -336,18 +330,6 @@ func (fs FrontierSigner) PublicKey(tx *Transaction) ([]byte, error) {
 	return pub, nil
 }
 
-// normaliseV returns the Ethereum version of the V parameter
-func normaliseV(s Signer, v *big.Int) byte {
-	if s, ok := s.(EIP155Signer); ok {
-		stdV := v.BitLen() <= 8 && (v.Uint64() == 27 || v.Uint64() == 28)
-		if s.chainId.BitLen() > 0 && !stdV {
-			nv := byte((new(big.Int).Sub(v, s.chainIdMul).Uint64()) - 35 + 27)
-			return nv
-		}
-	}
-	return byte(v.Uint64())
-}
-
 // deriveChainId derives the chain id from the given v parameter
 func deriveChainId(v *big.Int) *big.Int {
 	if v.BitLen() <= 64 {
diff --git a/core/types/transaction_test.go b/core/types/transaction_test.go
index ca105566a..4a38462e3 100644
--- a/core/types/transaction_test.go
+++ b/core/types/transaction_test.go
@@ -47,7 +47,7 @@ var (
 		common.FromHex("5544"),
 	).WithSignature(
 		HomesteadSigner{},
-		common.Hex2Bytes("98ff921201554726367d2be8c804a7ff89ccf285ebc57dff8ae4c44b9c19ac4a8887321be575c8095f789dd4c743dfe42c1820f9231f98a962b210e3ac2452a31c"),
+		common.Hex2Bytes("98ff921201554726367d2be8c804a7ff89ccf285ebc57dff8ae4c44b9c19ac4a8887321be575c8095f789dd4c743dfe42c1820f9231f98a962b210e3ac2452a301"),
 	)
 )
 
diff --git a/core/vm/contracts.go b/core/vm/contracts.go
index b45f14724..5cd0d0bb1 100644
--- a/core/vm/contracts.go
+++ b/core/vm/contracts.go
@@ -89,21 +89,15 @@ func ecrecoverFunc(in []byte) []byte {
 
 	r := common.BytesToBig(in[64:96])
 	s := common.BytesToBig(in[96:128])
-	// Treat V as a 256bit integer
-	vbig := common.Bytes2Big(in[32:64])
-	v := byte(vbig.Uint64())
+	v := in[63] - 27
 
 	// tighter sig s values in homestead only apply to tx sigs
-	if !crypto.ValidateSignatureValues(v, r, s, false) {
+	if common.Bytes2Big(in[32:63]).BitLen() > 0 || !crypto.ValidateSignatureValues(v, r, s, false) {
 		glog.V(logger.Detail).Infof("ECRECOVER error: v, r or s value invalid")
 		return nil
 	}
-
-	// v needs to be at the end and normalized for libsecp256k1
-	vbignormal := new(big.Int).Sub(vbig, big.NewInt(27))
-	vnormal := byte(vbignormal.Uint64())
-	rsv := append(in[64:128], vnormal)
-	pubKey, err := crypto.Ecrecover(in[:32], rsv)
+	// v needs to be at the end for libsecp256k1
+	pubKey, err := crypto.Ecrecover(in[:32], append(in[64:128], v))
 	// make sure the public key is a valid one
 	if err != nil {
 		glog.V(logger.Detail).Infoln("ECRECOVER error: ", err)