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.

accounts/abi/bind/auth.go

@@ -52,7 +52,7 @@ func NewKeyedTransactor(key *ecdsa.PrivateKey) *TransactOpts {
 			if address != keyAddr {
 				return nil, errors.New("not authorized to sign this account")
 			}
-			signature, err := crypto.SignEthereum(signer.Hash(tx).Bytes(), key)
+			signature, err := crypto.Sign(signer.Hash(tx).Bytes(), key)
 			if err != nil {
 				return nil, err
 			}

accounts/account_manager.go

@@ -136,42 +136,29 @@ func (am *Manager) DeleteAccount(a Account, passphrase string) error {
 	return err
 }
 
-// Sign calculates a ECDSA signature for the given hash.
-// Note, Ethereum signatures have a particular format as described in the
-// yellow paper. Use the SignEthereum function to calculate a signature
-// in Ethereum format.
+// Sign calculates a ECDSA signature for the given hash. The produced signature
+// is in the [R || S || V] format where V is 0 or 1.
 func (am *Manager) Sign(addr common.Address, hash []byte) ([]byte, error) {
 	am.mu.RLock()
 	defer am.mu.RUnlock()
-	unlockedKey, found := am.unlocked[addr]
-	if !found {
-		return nil, ErrLocked
-	}
-	return crypto.Sign(hash, unlockedKey.PrivateKey)
-}
 
-// SignEthereum calculates a ECDSA signature for the given hash.
-// The signature has the format as described in the Ethereum yellow paper.
-func (am *Manager) SignEthereum(addr common.Address, hash []byte) ([]byte, error) {
-	am.mu.RLock()
-	defer am.mu.RUnlock()
 	unlockedKey, found := am.unlocked[addr]
 	if !found {
 		return nil, ErrLocked
 	}
-	return crypto.SignEthereum(hash, unlockedKey.PrivateKey)
+	return crypto.Sign(hash, unlockedKey.PrivateKey)
 }
 
-// SignWithPassphrase signs hash if the private key matching the given
-// address can be decrypted with the given passphrase.
+// SignWithPassphrase signs hash if the private key matching the given address
+// can be decrypted with the given passphrase. The produced signature is in the
+// [R || S || V] format where V is 0 or 1.
 func (am *Manager) SignWithPassphrase(addr common.Address, passphrase string, hash []byte) (signature []byte, err error) {
 	_, key, err := am.getDecryptedKey(Account{Address: addr}, passphrase)
 	if err != nil {
 		return nil, err
 	}
-
 	defer zeroKey(key.PrivateKey)
-	return crypto.SignEthereum(hash, key.PrivateKey)
+	return crypto.Sign(hash, key.PrivateKey)
 }
 
 // Unlock unlocks the given account indefinitely.

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())

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.

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 {

core/types/transaction_test.go

@@ -47,7 +47,7 @@ var (
 		common.FromHex("5544"),
 	).WithSignature(
 		HomesteadSigner{},
-		common.Hex2Bytes("98ff921201554726367d2be8c804a7ff89ccf285ebc57dff8ae4c44b9c19ac4a8887321be575c8095f789dd4c743dfe42c1820f9231f98a962b210e3ac2452a31c"),
+		common.Hex2Bytes("98ff921201554726367d2be8c804a7ff89ccf285ebc57dff8ae4c44b9c19ac4a8887321be575c8095f789dd4c743dfe42c1820f9231f98a962b210e3ac2452a301"),
 	)
 )
 

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)

crypto/crypto.go

@@ -167,25 +167,19 @@ func GenerateKey() (*ecdsa.PrivateKey, error) {
 	return ecdsa.GenerateKey(secp256k1.S256(), rand.Reader)
 }
 
+// ValidateSignatureValues verifies whether the signature values are valid with
+// the given chain rules. The v value is assumed to be either 0 or 1.
 func ValidateSignatureValues(v byte, r, s *big.Int, homestead bool) bool {
 	if r.Cmp(common.Big1) < 0 || s.Cmp(common.Big1) < 0 {
 		return false
 	}
-	vint := uint32(v)
 	// reject upper range of s values (ECDSA malleability)
 	// see discussion in secp256k1/libsecp256k1/include/secp256k1.h
 	if homestead && s.Cmp(secp256k1.HalfN) > 0 {
 		return false
 	}
 	// Frontier: allow s to be in full N range
-	if s.Cmp(secp256k1.N) >= 0 {
-		return false
-	}
-	if r.Cmp(secp256k1.N) < 0 && (vint == 27 || vint == 28) {
-		return true
-	} else {
-		return false
-	}
+	return r.Cmp(secp256k1.N) < 0 && s.Cmp(secp256k1.N) < 0 && (v == 0 || v == 1)
 }
 
 func SigToPub(hash, sig []byte) (*ecdsa.PublicKey, error) {
@@ -199,14 +193,13 @@ func SigToPub(hash, sig []byte) (*ecdsa.PublicKey, error) {
 }
 
 // Sign calculates an ECDSA signature.
+//
 // This function is susceptible to choosen plaintext attacks that can leak
 // information about the private key that is used for signing. Callers must
 // be aware that the given hash cannot be choosen by an adversery. Common
 // solution is to hash any input before calculating the signature.
 //
-// Note: the calculated signature is not Ethereum compliant. The yellow paper
-// dictates Ethereum singature to have a V value with and offset of 27 v in [27,28].
-// Use SignEthereum to get an Ethereum compliant signature.
+// The produced signature is in the [R || S || V] format where V is 0 or 1.
 func Sign(data []byte, prv *ecdsa.PrivateKey) (sig []byte, err error) {
 	if len(data) != 32 {
 		return nil, fmt.Errorf("hash is required to be exactly 32 bytes (%d)", len(data))
@@ -218,20 +211,6 @@ func Sign(data []byte, prv *ecdsa.PrivateKey) (sig []byte, err error) {
 	return
 }
 
-// SignEthereum calculates an Ethereum ECDSA signature.
-// This function is susceptible to choosen plaintext attacks that can leak
-// information about the private key that is used for signing. Callers must
-// be aware that the given hash cannot be freely choosen by an adversery.
-// Common solution is to hash the message before calculating the signature.
-func SignEthereum(data []byte, prv *ecdsa.PrivateKey) ([]byte, error) {
-	sig, err := Sign(data, prv)
-	if err != nil {
-		return nil, err
-	}
-	sig[64] += 27 // as described in the yellow paper
-	return sig, err
-}
-
 func Encrypt(pub *ecdsa.PublicKey, message []byte) ([]byte, error) {
 	return ecies.Encrypt(rand.Reader, ecies.ImportECDSAPublic(pub), message, nil, nil)
 }

crypto/crypto_test.go

@@ -80,22 +80,15 @@ func Test0Key(t *testing.T) {
 	}
 }
 
-func testSign(signfn func([]byte, *ecdsa.PrivateKey) ([]byte, error), t *testing.T) {
+func TestSign(t *testing.T) {
 	key, _ := HexToECDSA(testPrivHex)
 	addr := common.HexToAddress(testAddrHex)
 
 	msg := Keccak256([]byte("foo"))
-	sig, err := signfn(msg, key)
+	sig, err := Sign(msg, key)
 	if err != nil {
 		t.Errorf("Sign error: %s", err)
 	}
-
-	// signfn can return a recover id of either [0,1] or [27,28].
-	// In the latter case its an Ethereum signature, adjust recover id.
-	if sig[64] == 27 || sig[64] == 28 {
-		sig[64] -= 27
-	}
-
 	recoveredPub, err := Ecrecover(msg, sig)
 	if err != nil {
 		t.Errorf("ECRecover error: %s", err)
@@ -117,34 +110,15 @@ func testSign(signfn func([]byte, *ecdsa.PrivateKey) ([]byte, error), t *testing
 	}
 }
 
-func TestSign(t *testing.T) {
-	testSign(Sign, t)
-}
-
-func TestSignEthereum(t *testing.T) {
-	testSign(SignEthereum, t)
-}
-
-func testInvalidSign(signfn func([]byte, *ecdsa.PrivateKey) ([]byte, error), t *testing.T) {
-	_, err := signfn(make([]byte, 1), nil)
-	if err == nil {
+func TestInvalidSign(t *testing.T) {
+	if _, err := Sign(make([]byte, 1), nil); err == nil {
 		t.Errorf("expected sign with hash 1 byte to error")
 	}
-
-	_, err = signfn(make([]byte, 33), nil)
-	if err == nil {
+	if _, err := Sign(make([]byte, 33), nil); err == nil {
 		t.Errorf("expected sign with hash 33 byte to error")
 	}
 }
 
-func TestInvalidSign(t *testing.T) {
-	testInvalidSign(Sign, t)
-}
-
-func TestInvalidSignEthereum(t *testing.T) {
-	testInvalidSign(SignEthereum, t)
-}
-
 func TestNewContractAddress(t *testing.T) {
 	key, _ := HexToECDSA(testPrivHex)
 	addr := common.HexToAddress(testAddrHex)
@@ -207,38 +181,38 @@ func TestValidateSignatureValues(t *testing.T) {
 	secp256k1nMinus1 := new(big.Int).Sub(secp256k1.N, common.Big1)
 
 	// correct v,r,s
-	check(true, 27, one, one)
-	check(true, 28, one, one)
+	check(true, 0, one, one)
+	check(true, 1, one, one)
 	// incorrect v, correct r,s,
-	check(false, 30, one, one)
-	check(false, 26, one, one)
+	check(false, 2, one, one)
+	check(false, 3, one, one)
 
 	// incorrect v, combinations of incorrect/correct r,s at lower limit
+	check(false, 2, zero, zero)
+	check(false, 2, zero, one)
+	check(false, 2, one, zero)
+	check(false, 2, one, one)
+
+	// correct v for any combination of incorrect r,s
 	check(false, 0, zero, zero)
 	check(false, 0, zero, one)
 	check(false, 0, one, zero)
-	check(false, 0, one, one)
-
-	// correct v for any combination of incorrect r,s
-	check(false, 27, zero, zero)
-	check(false, 27, zero, one)
-	check(false, 27, one, zero)
 
-	check(false, 28, zero, zero)
-	check(false, 28, zero, one)
-	check(false, 28, one, zero)
+	check(false, 1, zero, zero)
+	check(false, 1, zero, one)
+	check(false, 1, one, zero)
 
 	// correct sig with max r,s
-	check(true, 27, secp256k1nMinus1, secp256k1nMinus1)
+	check(true, 0, secp256k1nMinus1, secp256k1nMinus1)
 	// correct v, combinations of incorrect r,s at upper limit
-	check(false, 27, secp256k1.N, secp256k1nMinus1)
-	check(false, 27, secp256k1nMinus1, secp256k1.N)
-	check(false, 27, secp256k1.N, secp256k1.N)
+	check(false, 0, secp256k1.N, secp256k1nMinus1)
+	check(false, 0, secp256k1nMinus1, secp256k1.N)
+	check(false, 0, secp256k1.N, secp256k1.N)
 
 	// current callers ensures r,s cannot be negative, but let's test for that too
 	// as crypto package could be used stand-alone
-	check(false, 27, minusOne, one)
-	check(false, 27, one, minusOne)
+	check(false, 0, minusOne, one)
+	check(false, 0, one, minusOne)
 }
 
 func checkhash(t *testing.T, name string, f func([]byte) []byte, msg, exp []byte) {

internal/ethapi/api.go

@@ -287,11 +287,19 @@ func signHash(data []byte) []byte {
 // Sign calculates an Ethereum ECDSA signature for:
 // keccack256("\x19Ethereum Signed Message:\n" + len(message) + message))
 //
+// Note, the produced signature conforms to the secp256k1 curve R, S and V values,
+// where the V value will be 27 or 28 for legacy reasons.
+//
 // The key used to calculate the signature is decrypted with the given password.
 //
 // https://github.com/ethereum/go-ethereum/wiki/Management-APIs#personal_sign
 func (s *PrivateAccountAPI) Sign(ctx context.Context, data hexutil.Bytes, addr common.Address, passwd string) (hexutil.Bytes, error) {
-	return s.b.AccountManager().SignWithPassphrase(addr, passwd, signHash(data))
+	signature, err := s.b.AccountManager().SignWithPassphrase(addr, passwd, signHash(data))
+	if err != nil {
+		return nil, err
+	}
+	signature[64] += 27 // SignWithPassphrase uses canonical secp256k1 signatures (v = 0 or 1), transform to yellow paper
+	return signature, nil
 }
 
 // EcRecover returns the address for the account that was used to create the signature.
@@ -300,15 +308,19 @@ func (s *PrivateAccountAPI) Sign(ctx context.Context, data hexutil.Bytes, addr c
 // hash = keccak256("\x19Ethereum Signed Message:\n"${message length}${message})
 // addr = ecrecover(hash, signature)
 //
+// Note, the signature must conform to the secp256k1 curve R, S and V values, where
+// the V value must be be 27 or 28 for legacy reasons.
+//
 // https://github.com/ethereum/go-ethereum/wiki/Management-APIs#personal_ecRecover
 func (s *PrivateAccountAPI) EcRecover(ctx context.Context, data, sig hexutil.Bytes) (common.Address, error) {
 	if len(sig) != 65 {
 		return common.Address{}, fmt.Errorf("signature must be 65 bytes long")
 	}
-	// see crypto.Ecrecover description
-	if sig[64] == 27 || sig[64] == 28 {
-		sig[64] -= 27
+	if sig[64] != 27 && sig[64] != 28 {
+		return common.Address{}, fmt.Errorf("invalid Ethereum signature (V is not 27 or 28)")
 	}
+	sig[64] -= 27 // Transform yellow paper signatures to canonical secp256k1 form
+
 	rpk, err := crypto.Ecrecover(signHash(data), sig)
 	if err != nil {
 		return common.Address{}, err
@@ -964,7 +976,7 @@ func (s *PublicTransactionPoolAPI) GetTransactionReceipt(txHash common.Hash) (ma
 func (s *PublicTransactionPoolAPI) sign(addr common.Address, tx *types.Transaction) (*types.Transaction, error) {
 	signer := types.MakeSigner(s.b.ChainConfig(), s.b.CurrentBlock().Number())
 
-	signature, err := s.b.AccountManager().SignEthereum(addr, signer.Hash(tx).Bytes())
+	signature, err := s.b.AccountManager().Sign(addr, signer.Hash(tx).Bytes())
 	if err != nil {
 		return nil, err
 	}
@@ -1046,11 +1058,10 @@ func (s *PublicTransactionPoolAPI) SendTransaction(ctx context.Context, args Sen
 	}
 	tx := args.toTransaction()
 	signer := types.MakeSigner(s.b.ChainConfig(), s.b.CurrentBlock().Number())
-	signature, err := s.b.AccountManager().SignEthereum(args.From, signer.Hash(tx).Bytes())
+	signature, err := s.b.AccountManager().Sign(args.From, signer.Hash(tx).Bytes())
 	if err != nil {
 		return common.Hash{}, err
 	}
-
 	return submitTransaction(ctx, s.b, tx, signature)
 }
 
@@ -1084,11 +1095,19 @@ func (s *PublicTransactionPoolAPI) SendRawTransaction(ctx context.Context, encod
 // Sign calculates an ECDSA signature for:
 // keccack256("\x19Ethereum Signed Message:\n" + len(message) + message).
 //
+// Note, the produced signature conforms to the secp256k1 curve R, S and V values,
+// where the V value will be 27 or 28 for legacy reasons.
+//
 // The account associated with addr must be unlocked.
 //
 // https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_sign
 func (s *PublicTransactionPoolAPI) Sign(addr common.Address, data hexutil.Bytes) (hexutil.Bytes, error) {
-	return s.b.AccountManager().SignEthereum(addr, signHash(data))
+	signature, err := s.b.AccountManager().Sign(addr, signHash(data))
+	if err == nil {
+		// Sign uses canonical secp256k1 signatures (v = 0 or 1), transform to yellow paper
+		signature[64] += 27
+	}
+	return signature, err
 }
 
 // SignTransactionResult represents a RLP encoded signed transaction.