[BLADE-97] Refactor tx signer

crypto/crypto.go

@@ -30,6 +30,7 @@ var (
 	ErrInvalidBLSSignature = errors.New("invalid BLS Signature")
 	errHashOfInvalidLength = errors.New("message hash of invalid length")
 	errInvalidSignature    = errors.New("invalid signature")
+	errInvalidChainID      = errors.New("invalid chain id for signer")
 )
 
 type KeyType string

crypto/txsigner.go

@@ -7,208 +7,106 @@ import (
 	"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.London {
+		return NewLondonSigner(chainID)
+	}
+
+	if forks.Berlin {
+		return NewBerlinSigner(chainID)
+	}
 
 	if forks.EIP155 {
-		signer = NewEIP155Signer(chainID, forks.Homestead)
-	} else {
-		signer = NewFrontierSigner(forks.Homestead)
+		return NewEIP155Signer(chainID)
 	}
 
-	// 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)
+	if forks.Homestead {
+		return NewHomesteadSigner()
 	}
 
-	return signer
+	return NewFrontierSigner()
 }
 
-// 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, because transaction signature is invalid")
 	}
 
-	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
 	}
 
-	if len(pub) == 0 || pub[0] != 4 {
+	if len(publicKey) == 0 || publicKey[0] != 4 {
 		return types.ZeroAddress, errors.New("invalid public key")
 	}
 
-	buf := Keccak256(pub[1:])[12:]
-
-	return types.BytesToAddress(buf), nil
-}
+	// 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:])
 
-// calcTxHash calculates the transaction hash (keccak256 hash of the RLP value)
-// LegacyTx:
-// keccak256(RLP(nonce, gasPrice, gas, to, value, input, chainId, 0, 0))
-// AccessListsTx:
-// keccak256(RLP(type, chainId, nonce, gasPrice, gas, to, value, input, accessList))
-// DynamicFeeTx:
-// keccak256(RLP(type, chainId, nonce, gasTipCap, gasFeeCap, gas, to, value, input, accessList))
-func calcTxHash(tx *types.Transaction, chainID uint64) types.Hash {
-	var hash []byte
+	address := hash[12:]
 
-	switch tx.Type() {
-	case types.AccessListTxType:
-		a := signerPool.Get()
-		v := a.NewArray()
-
-		v.Set(a.NewUint(chainID))
-		v.Set(a.NewUint(tx.Nonce()))
-		v.Set(a.NewBigInt(tx.GasPrice()))
-		v.Set(a.NewUint(tx.Gas()))
-
-		if tx.To() == nil {
-			v.Set(a.NewNull())
-		} else {
-			v.Set(a.NewCopyBytes((*(tx.To())).Bytes()))
-		}
-
-		v.Set(a.NewBigInt(tx.Value()))
-		v.Set(a.NewCopyBytes(tx.Input()))
-
-		// add accessList
-		accessListVV := a.NewArray()
-
-		if tx.AccessList() != nil {
-			for _, accessTuple := range tx.AccessList() {
-				accessTupleVV := a.NewArray()
-				accessTupleVV.Set(a.NewCopyBytes(accessTuple.Address.Bytes()))
-
-				storageKeysVV := a.NewArray()
-				for _, storageKey := range accessTuple.StorageKeys {
-					storageKeysVV.Set(a.NewCopyBytes(storageKey.Bytes()))
-				}
-
-				accessTupleVV.Set(storageKeysVV)
-				accessListVV.Set(accessTupleVV)
-			}
-		}
-
-		v.Set(accessListVV)
-
-		hash = keccak.PrefixedKeccak256Rlp([]byte{byte(tx.Type())}, nil, v)
-
-		signerPool.Put(a)
-
-		return types.BytesToHash(hash)
-
-	case types.DynamicFeeTxType, types.LegacyTxType, types.StateTxType:
-		a := signerPool.Get()
-		isDynamicFeeTx := tx.Type() == types.DynamicFeeTxType
-
-		v := a.NewArray()
-
-		if isDynamicFeeTx {
-			v.Set(a.NewUint(chainID))
-		}
-
-		v.Set(a.NewUint(tx.Nonce()))
-
-		if isDynamicFeeTx {
-			v.Set(a.NewBigInt(tx.GasTipCap()))
-			v.Set(a.NewBigInt(tx.GasFeeCap()))
-		} else {
-			v.Set(a.NewBigInt(tx.GasPrice()))
-		}
-
-		v.Set(a.NewUint(tx.Gas()))
-
-		if tx.To() == nil {
-			v.Set(a.NewNull())
-		} else {
-			v.Set(a.NewCopyBytes((*(tx.To())).Bytes()))
-		}
-
-		v.Set(a.NewBigInt(tx.Value()))
-
-		v.Set(a.NewCopyBytes(tx.Input()))
-
-		if isDynamicFeeTx {
-			// Convert TxAccessList to RLP format and add it to the vv array.
-			accessListVV := a.NewArray()
-
-			if tx.AccessList() != nil {
-				for _, accessTuple := range tx.AccessList() {
-					accessTupleVV := a.NewArray()
-					accessTupleVV.Set(a.NewCopyBytes(accessTuple.Address.Bytes()))
-
-					storageKeysVV := a.NewArray()
-					for _, storageKey := range accessTuple.StorageKeys {
-						storageKeysVV.Set(a.NewCopyBytes(storageKey.Bytes()))
-					}
-
-					accessTupleVV.Set(storageKeysVV)
-					accessListVV.Set(accessTupleVV)
-				}
-			}
-
-			v.Set(accessListVV)
-		} else {
-			// EIP155
-			if chainID != 0 {
-				v.Set(a.NewUint(chainID))
-				v.Set(a.NewUint(0))
-				v.Set(a.NewUint(0))
-			}
-		}
+	return types.BytesToAddress(address), nil
+}
 
-		if isDynamicFeeTx {
-			hash = keccak.PrefixedKeccak256Rlp([]byte{byte(tx.Type())}, nil, v)
-		} else {
-			hash = keccak.Keccak256Rlp(nil, v)
-		}
+// validateTxChainID checks if the transaction chain ID matches the expected chain ID
+func validateTxChainID(tx *types.Transaction, chainID uint64) error {
+	txChainID := tx.ChainID()
 
-		signerPool.Put(a)
+	if txChainID == nil || txChainID.Uint64() != chainID {
+		return fmt.Errorf("%w: have %d want %d", errInvalidChainID, tx.ChainID(), chainID)
 	}
 
-	return types.BytesToHash(hash)
+	return nil
 }