[BLADE-97] Refactor tx signer (#113)

* Refactoring the tx singer logic

* Build fix

* Fix tests

crypto/txsigner.go

@@ -3,76 +3,84 @@ package crypto
 import (
 	"crypto/ecdsa"
 	"errors"
-	"fmt"
 	"math/big"
 
 	"github.com/0xPolygon/polygon-edge/chain"
 	"github.com/0xPolygon/polygon-edge/helper/keccak"
 	"github.com/0xPolygon/polygon-edge/types"
+	"github.com/umbracle/fastrlp"
 )
 
-// Magic numbers from Ethereum, used in v calculation
+// Magic numbers, taken from the Ethereum, used in the calculation of the V value
+// Only matters in pre-EIP-2930 (pre-Berlin) transactions
 var (
-	big27 = big.NewInt(27)
-	big35 = big.NewInt(35)
+	big27 = big.NewInt(27) // pre-EIP-155
+	big35 = big.NewInt(35) // EIP-155
 )
 
-// TxSigner is a utility interface used to recover data from a transaction
+// RLP encoding helper
+var arenaPool fastrlp.ArenaPool
+
+// TxSigner is a utility interface used to work with transaction signatures
 type TxSigner interface {
 	// Hash returns the hash of the transaction
-	Hash(tx *types.Transaction) types.Hash
+	Hash(*types.Transaction) types.Hash
 
 	// Sender returns the sender of the transaction
-	Sender(tx *types.Transaction) (types.Address, error)
+	Sender(*types.Transaction) (types.Address, error)
 
-	// SignTx signs a transaction
-	SignTx(tx *types.Transaction, priv *ecdsa.PrivateKey) (*types.Transaction, error)
+	// SingTx takes the original transaction as input and returns its signed version
+	SignTx(*types.Transaction, *ecdsa.PrivateKey) (*types.Transaction, error)
 }
 
-// NewSigner creates a new signer object (EIP155 or FrontierSigner)
+// NewSigner creates a new signer based on currently supported forks
 func NewSigner(forks chain.ForksInTime, chainID uint64) TxSigner {
 	var signer TxSigner
 
-	if forks.EIP155 {
-		signer = NewEIP155Signer(chainID, forks.Homestead)
+	if forks.London {
+		signer = NewLondonSigner(chainID)
+	} else if forks.Berlin {
+		signer = NewBerlinSigner(chainID)
+	} else if forks.EIP155 {
+		signer = NewEIP155Signer(chainID)
+	} else if forks.Homestead {
+		signer = NewHomesteadSigner()
 	} else {
-		signer = NewFrontierSigner(forks.Homestead)
-	}
-
-	// London signer requires a fallback signer that is defined above.
-	// This is the reason why the london signer check is separated.
-	if forks.London || forks.Berlin {
-		return NewLondonOrBerlinSigner(chainID, forks.Homestead, signer)
+		signer = NewFrontierSigner()
 	}
 
 	return signer
 }
 
-// encodeSignature generates a signature value based on the R, S and V value
-func encodeSignature(R, S, V *big.Int, isHomestead bool) ([]byte, error) {
-	if !ValidateSignatureValues(V, R, S, isHomestead) {
-		return nil, fmt.Errorf("invalid txn signature")
+// encodeSignature generates a signature based on the R, S and parity values
+//
+// The signature encoding format is as follows:
+// (32-bytes R, 32-bytes S, 1-byte parity)
+//
+// Note: although the signature value V, based on different standards, is calculated and encoded in different ways,
+// the encodeSignature function expects parity of Y coordinate as third input and that is what will be encoded
+func encodeSignature(r, s, parity *big.Int, isHomestead bool) ([]byte, error) {
+	if !ValidateSignatureValues(parity, r, s, isHomestead) {
+		return nil, errors.New("Signature encoding failed: Invalid transaction signature")
 	}
 
-	sig := make([]byte, 65)
-	copy(sig[32-len(R.Bytes()):32], R.Bytes())
-	copy(sig[64-len(S.Bytes()):64], S.Bytes())
-	sig[64] = byte(V.Int64()) // here is safe to convert it since ValidateSignatureValues will validate the v value
+	signature := make([]byte, 65)
 
-	return sig, nil
+	copy(signature[32-len(r.Bytes()):32], r.Bytes())
+	copy(signature[64-len(s.Bytes()):64], s.Bytes())
+	signature[64] = byte(parity.Int64())
+
+	return signature, nil
 }
 
-// recoverAddress recovers the sender address from a transaction hash and signature parameters.
-// It takes the transaction hash, r, s, v values of the signature,
-// and a flag indicating if the transaction is in the Homestead format.
-// It returns the recovered address and an error if any.
-func recoverAddress(txHash types.Hash, r, s, v *big.Int, isHomestead bool) (types.Address, error) {
-	sig, err := encodeSignature(r, s, v, isHomestead)
+// recoverAddress recovers the sender address from the transaction hash and signature R, S and parity values
+func recoverAddress(txHash types.Hash, r, s, parity *big.Int, isHomestead bool) (types.Address, error) {
+	signature, err := encodeSignature(r, s, parity, isHomestead)
 	if err != nil {
 		return types.ZeroAddress, err
 	}
 
-	pub, err := Ecrecover(txHash.Bytes(), sig)
+	publicKey, err := Ecrecover(txHash.Bytes(), signature)
 	if err != nil {
 		return types.ZeroAddress, err
 	}
@@ -81,9 +89,12 @@ func recoverAddress(txHash types.Hash, r, s, v *big.Int, isHomestead bool) (type
 		return types.ZeroAddress, errors.New("invalid public key")
 	}
 
-	buf := Keccak256(pub[1:])[12:]
+	// First byte of the publicKey indicates that it is serialized in uncompressed form (it has the value 0x04), so we ommit that
+	hash := Keccak256(publicKey[1:])
+
+	address := hash[12:]
 
-	return types.BytesToAddress(buf), nil
+	return types.BytesToAddress(address), nil
 }
 
 // calcTxHash calculates the transaction hash (keccak256 hash of the RLP value)
@@ -98,7 +109,7 @@ func calcTxHash(tx *types.Transaction, chainID uint64) types.Hash {
 
 	switch tx.Type() {
 	case types.AccessListTx:
-		a := signerPool.Get()
+		a := arenaPool.Get()
 		v := a.NewArray()
 
 		v.Set(a.NewUint(chainID))
@@ -137,12 +148,12 @@ func calcTxHash(tx *types.Transaction, chainID uint64) types.Hash {
 
 		hash = keccak.PrefixedKeccak256Rlp([]byte{byte(tx.Type())}, nil, v)
 
-		signerPool.Put(a)
+		arenaPool.Put(a)
 
 		return types.BytesToHash(hash)
 
 	case types.DynamicFeeTx, types.LegacyTx, types.StateTx:
-		a := signerPool.Get()
+		a := arenaPool.Get()
 		isDynamicFeeTx := tx.Type() == types.DynamicFeeTx
 
 		v := a.NewArray()
@@ -207,7 +218,7 @@ func calcTxHash(tx *types.Transaction, chainID uint64) types.Hash {
 			hash = keccak.Keccak256Rlp(nil, v)
 		}
 
-		signerPool.Put(a)
+		arenaPool.Put(a)
 	}
 
 	return types.BytesToHash(hash)

crypto/txsignerBerlin.go

@@ -0,0 +1,157 @@
+package crypto
+
+import (
+	"crypto/ecdsa"
+	"errors"
+	"math/big"
+
+	"github.com/0xPolygon/polygon-edge/helper/keccak"
+	"github.com/0xPolygon/polygon-edge/types"
+)
+
+// BerlinSigner may be used for signing legacy (pre-EIP-155 and EIP-155) and EIP-2930 transactions
+type BerlinSigner struct {
+	EIP155Signer
+}
+
+// NewBerlinSigner returns new BerlinSinger object (constructor)
+//
+// BerlinSigner accepts the following types of transactions:
+//   - EIP-2930 access list transactions,
+//   - EIP-155 replay protected transactions, and
+//   - pre-EIP-155 legacy transactions
+func NewBerlinSigner(chainID uint64) *BerlinSigner {
+	return &BerlinSigner{
+		EIP155Signer: EIP155Signer{
+			chainID:         chainID,
+			HomesteadSigner: HomesteadSigner{},
+		},
+	}
+}
+
+// Hash returns the keccak256 hash of the transaction
+//
+// The EIP-2930 transaction hash preimage is as follows:
+// (0x01 || RLP(chainId, nonce, gasPrice, gas, to, value, input, accessList)
+//
+// Specification: https://eips.ethereum.org/EIPS/eip-2930#specification
+func (signer *BerlinSigner) Hash(tx *types.Transaction) types.Hash {
+	if tx.Type() != types.AccessListTx {
+		return signer.EIP155Signer.Hash(tx)
+	}
+
+	var hash []byte
+
+	RLP := arenaPool.Get()
+
+	// RLP(-, -, -, -, -, -, -, -)
+	hashPreimage := RLP.NewArray()
+
+	// RLP(chainId, -, -, -, -, -, -, -)
+	hashPreimage.Set(RLP.NewUint(signer.chainID))
+
+	// RLP(chainId, nonce, -, -, -, -, -, -)
+	hashPreimage.Set(RLP.NewUint(tx.Nonce()))
+
+	// RLP(chainId, nonce, gasPrice, -, -, -, -, -)
+	hashPreimage.Set(RLP.NewBigInt(tx.GasPrice()))
+
+	// RLP(chainId, nonce, gasPrice, gas, -, -, -, -)
+	hashPreimage.Set(RLP.NewUint(tx.Gas()))
+
+	// Checking whether the transaction is a smart contract deployment
+	if tx.To() == nil {
+
+		// RLP(chainId, nonce, gasPrice, gas, to, -, -, -)
+		hashPreimage.Set(RLP.NewNull())
+	} else {
+
+		// RLP(chainId, nonce, gasPrice, gas, to, -, -, -)
+		hashPreimage.Set(RLP.NewCopyBytes((*(tx.To())).Bytes()))
+	}
+
+	// RLP(chainId, nonce, gasPrice, gas, to, value, -, -)
+	hashPreimage.Set(RLP.NewBigInt(tx.Value()))
+
+	// RLP(chainId, nonce, gasPrice, gas, to, value, input, -)
+	hashPreimage.Set(RLP.NewCopyBytes(tx.Input()))
+
+	// Serialization format of the access list: [[{20-bytes address}, [{32-bytes key}, ...]], ...] where `...` denotes zero or more items
+	accessList := RLP.NewArray()
+
+	if tx.AccessList() != nil {
+
+		// accessTuple contains (address, storageKeys[])
+		for _, accessTuple := range tx.AccessList() {
+
+			accessTupleSerFormat := RLP.NewArray()
+			accessTupleSerFormat.Set(RLP.NewCopyBytes(accessTuple.Address.Bytes()))
+
+			storageKeysSerFormat := RLP.NewArray()
+
+			for _, storageKey := range accessTuple.StorageKeys {
+				storageKeysSerFormat.Set(RLP.NewCopyBytes(storageKey.Bytes()))
+			}
+
+			accessTupleSerFormat.Set(storageKeysSerFormat)
+			accessList.Set(accessTupleSerFormat)
+		}
+	}
+
+	// RLP(chainId, nonce, gasPrice, gas, to, value, input, accessList)
+	hashPreimage.Set(accessList)
+
+	// keccak256(0x01 || RLP(chainId, nonce, gasPrice, gas, to, value, input, accessList)
+	hash = keccak.PrefixedKeccak256Rlp([]byte{byte(tx.Type())}, nil, hashPreimage)
+
+	arenaPool.Put(RLP)
+
+	return types.BytesToHash(hash)
+}
+
+// Sender returns the sender of the transaction
+func (signer *BerlinSigner) Sender(tx *types.Transaction) (types.Address, error) {
+	if tx.Type() != types.AccessListTx {
+		return signer.EIP155Signer.Sender(tx)
+	}
+
+	v, r, s := tx.RawSignatureValues()
+
+	return recoverAddress(signer.Hash(tx), r, s, v, true)
+}
+
+// SingTx takes the original transaction as input and returns its signed version
+func (signer *BerlinSigner) SignTx(tx *types.Transaction, privateKey *ecdsa.PrivateKey) (*types.Transaction, error) {
+	if tx.Type() != types.AccessListTx {
+		return signer.EIP155Signer.SignTx(tx, privateKey)
+	}
+
+	tx = tx.Copy()
+
+	h := signer.Hash(tx)
+
+	sig, err := Sign(privateKey, h[:])
+	if err != nil {
+		return nil, err
+	}
+
+	r := new(big.Int).SetBytes(sig[:32])
+	s := new(big.Int).SetBytes(sig[32:64])
+
+	if s.Cmp(secp256k1NHalf) > 0 {
+		return nil, errors.New("SignTx method: S must be inclusively lower than secp256k1n/2")
+	}
+
+	v := new(big.Int).SetBytes(signer.calculateV(sig[64]))
+
+	tx.SetSignatureValues(v, r, s)
+
+	return tx, nil
+}
+
+// Private method calculateV returns the V value for the EIP-2930 transactions
+//
+// V represents the parity of the Y coordinate
+func (e *BerlinSigner) calculateV(parity byte) []byte {
+	return big.NewInt(int64(parity)).Bytes()
+}