Stricter validation of EIP-7702 transactions (#12201)

Cherry pick #11885

core/types/authorization.go

@@ -7,7 +7,10 @@ import (
 	"io"
 
 	"github.com/holiman/uint256"
+
 	libcommon "github.com/ledgerwatch/erigon-lib/common"
+	libcrypto "github.com/ledgerwatch/erigon-lib/crypto"
+
 	"github.com/ledgerwatch/erigon-lib/common/length"
 	rlp2 "github.com/ledgerwatch/erigon-lib/rlp"
 	"github.com/ledgerwatch/erigon/common/u256"
@@ -74,8 +77,8 @@ func (ath *Authorization) RecoverSigner(data *bytes.Buffer, b []byte) (*libcommo
 		return nil, fmt.Errorf("invalid v value: %d", ath.V.Uint64())
 	}
 
-	if !crypto.ValidateSignatureValues(sig[64], &ath.R, &ath.S, false) {
-		return nil, fmt.Errorf("invalid signature")
+	if !libcrypto.TransactionSignatureIsValid(sig[64], &ath.R, &ath.S, false /* allowPreEip2s */) {
+		return nil, errors.New("invalid signature")
 	}
 
 	pubkey, err := crypto.Ecrecover(hash.Bytes(), sig[:])

core/types/set_code_tx.go

@@ -132,7 +132,11 @@ func (tx *SetCodeTransaction) AsMessage(s Signer, baseFee *big.Int, rules *chain
 		msg.gasPrice.Set(tx.FeeCap)
 	}
 
+	if len(tx.Authorizations) == 0 {
+		return msg, errors.New("SetCodeTransaction without authorizations is invalid")
+	}
 	msg.authorizations = tx.Authorizations
+
 	var err error
 	msg.from, err = tx.Sender(s)
 	return msg, err
@@ -234,13 +238,11 @@ func (tx *SetCodeTransaction) DecodeRLP(s *rlp.Stream) error {
 	if b, err = s.Bytes(); err != nil {
 		return err
 	}
-	if len(b) > 0 && len(b) != 20 {
+	if len(b) != 20 {
 		return fmt.Errorf("wrong size for To: %d", len(b))
 	}
-	if len(b) > 0 {
-		tx.To = &libcommon.Address{}
-		copy((*tx.To)[:], b)
-	}
+	tx.To = &libcommon.Address{}
+	copy((*tx.To)[:], b)
 	if b, err = s.Uint256Bytes(); err != nil {
 		return err
 	}

core/types/transaction.go

@@ -31,10 +31,10 @@ import (
 	"github.com/ledgerwatch/erigon-lib/chain"
 	libcommon "github.com/ledgerwatch/erigon-lib/common"
 	"github.com/ledgerwatch/erigon-lib/common/fixedgas"
+	libcrypto "github.com/ledgerwatch/erigon-lib/crypto"
 	types2 "github.com/ledgerwatch/erigon-lib/types"
 
 	"github.com/ledgerwatch/erigon/common/math"
-	"github.com/ledgerwatch/erigon/crypto"
 	"github.com/ledgerwatch/erigon/rlp"
 )
 
@@ -280,7 +280,7 @@ func sanityCheckSignature(v *uint256.Int, r *uint256.Int, s *uint256.Int, maybeP
 		// must already be equal to the recovery id.
 		plainV = byte(v.Uint64())
 	}
-	if !crypto.ValidateSignatureValues(plainV, r, s, false) {
+	if !libcrypto.TransactionSignatureIsValid(plainV, r, s, true /* allowPreEip2s */) {
 		return ErrInvalidSig
 	}
 

core/types/transaction_signing.go

@@ -23,9 +23,11 @@ import (
 	"math/big"
 
 	"github.com/holiman/uint256"
+	"github.com/ledgerwatch/secp256k1"
+
 	"github.com/ledgerwatch/erigon-lib/chain"
 	libcommon "github.com/ledgerwatch/erigon-lib/common"
-	"github.com/ledgerwatch/secp256k1"
+	libcrypto "github.com/ledgerwatch/erigon-lib/crypto"
 
 	"github.com/ledgerwatch/erigon/common/u256"
 	"github.com/ledgerwatch/erigon/crypto"
@@ -359,7 +361,7 @@ func recoverPlain(context *secp256k1.Context, sighash libcommon.Hash, R, S, Vb *
 		return libcommon.Address{}, ErrInvalidSig
 	}
 	V := byte(Vb.Uint64() - 27)
-	if !crypto.ValidateSignatureValues(V, R, S, homestead) {
+	if !libcrypto.TransactionSignatureIsValid(V, R, S, !homestead) {
 		return libcommon.Address{}, ErrInvalidSig
 	}
 	// encode the signature in uncompressed format

core/vm/contracts.go

@@ -30,6 +30,7 @@ import (
 
 	"github.com/ledgerwatch/erigon-lib/chain"
 	libcommon "github.com/ledgerwatch/erigon-lib/common"
+	libcrypto "github.com/ledgerwatch/erigon-lib/crypto"
 	"github.com/ledgerwatch/erigon-lib/crypto/blake2b"
 	libkzg "github.com/ledgerwatch/erigon-lib/crypto/kzg"
 
@@ -238,8 +239,8 @@ func (c *ecrecover) Run(input []byte) ([]byte, error) {
 	s := new(uint256.Int).SetBytes(input[96:128])
 	v := input[63] - 27
 
-	// tighter sig s values input homestead only apply to tx sigs
-	if !allZero(input[32:63]) || !crypto.ValidateSignatureValues(v, r, s, false) {
+	// tighter sig s values input homestead only apply to txn sigs
+	if !allZero(input[32:63]) || !libcrypto.TransactionSignatureIsValid(v, r, s, true /* allowPreEip2s */) {
 		return nil, nil
 	}
 	// We must make sure not to modify the 'input', so placing the 'v' along with

crypto/crypto.go

@@ -298,21 +298,6 @@ func GenerateKey() (*ecdsa.PrivateKey, error) {
 	return ecdsa.GenerateKey(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 *uint256.Int, homestead bool) bool {
-	if r.IsZero() || s.IsZero() {
-		return false
-	}
-	// reject upper range of s values (ECDSA malleability)
-	// see discussion in secp256k1/libsecp256k1/include/secp256k1.h
-	if homestead && s.Gt(secp256k1halfN) {
-		return false
-	}
-	// Frontier: allow s to be in full N range
-	return r.Lt(secp256k1N) && s.Lt(secp256k1N) && (v == 0 || v == 1)
-}
-
 // DESCRIBED: docs/programmers_guide/guide.md#address---identifier-of-an-account
 func PubkeyToAddress(p ecdsa.PublicKey) libcommon.Address {
 	pubBytes := MarshalPubkey(&p)

crypto/crypto_test.go

@@ -20,18 +20,16 @@ import (
 	"bytes"
 	"crypto/ecdsa"
 	"encoding/hex"
-	"github.com/ledgerwatch/erigon-lib/common/hexutil"
 	"os"
 	"reflect"
 	"testing"
 
 	"golang.org/x/crypto/sha3"
 
-	"github.com/holiman/uint256"
 	libcommon "github.com/ledgerwatch/erigon-lib/common"
+	"github.com/ledgerwatch/erigon-lib/common/hexutil"
 
 	"github.com/ledgerwatch/erigon/common"
-	"github.com/ledgerwatch/erigon/common/u256"
 )
 
 var testAddrHex = "970e8128ab834e8eac17ab8e3812f010678cf791"
@@ -276,52 +274,6 @@ func TestSaveECDSA(t *testing.T) {
 	}
 }
 
-func TestValidateSignatureValues(t *testing.T) {
-	check := func(expected bool, v byte, r, s *uint256.Int) {
-		if ValidateSignatureValues(v, r, s, false) != expected {
-			t.Errorf("mismatch for v: %d r: %d s: %d want: %v", v, r, s, expected)
-		}
-	}
-	minusOne := uint256.NewInt(0).SetAllOne()
-	one := u256.Num1
-	zero := u256.Num0
-	secp256k1nMinus1 := new(uint256.Int).Sub(secp256k1N, u256.Num1)
-
-	// correct v,r,s
-	check(true, 0, one, one)
-	check(true, 1, one, one)
-	// incorrect v, correct r,s,
-	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, 1, zero, zero)
-	check(false, 1, zero, one)
-	check(false, 1, one, zero)
-
-	// correct sig with max r,s
-	check(true, 0, secp256k1nMinus1, secp256k1nMinus1)
-	// correct v, combinations of incorrect r,s at upper limit
-	check(false, 0, secp256k1N, secp256k1nMinus1)
-	check(false, 0, secp256k1nMinus1, secp256k1N)
-	check(false, 0, secp256k1N, secp256k1N)
-
-	// 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, 0, minusOne, one)
-	check(false, 0, one, minusOne)
-}
-
 func checkhash(t *testing.T, name string, f func([]byte) []byte, msg, exp []byte) {
 	sum := f(msg)
 	if !bytes.Equal(exp, sum) {

erigon-lib/crypto/secp256k1.go

@@ -24,7 +24,7 @@ import (
 
 var (
 	secp256k1N     = new(uint256.Int).SetBytes(hexutility.MustDecodeHex("fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141"))
-	secp256k1halfN = new(uint256.Int).Rsh(secp256k1N, 1)
+	Secp256k1halfN = new(uint256.Int).Rsh(secp256k1N, 1)
 )
 
 // See Appendix F "Signing Transactions" of the Yellow Paper
@@ -34,7 +34,7 @@ func TransactionSignatureIsValid(v byte, r, s *uint256.Int, allowPreEip2s bool)
 	}
 
 	// See EIP-2: Homestead Hard-fork Changes
-	if !allowPreEip2s && s.Gt(secp256k1halfN) {
+	if !allowPreEip2s && s.Gt(Secp256k1halfN) {
 		return false
 	}
 

erigon-lib/crypto/secp256k1_test.go

@@ -0,0 +1,74 @@
+// Copyright 2014 The go-ethereum Authors
+// (original work)
+// Copyright 2024 The Erigon Authors
+// (modifications)
+// This file is part of Erigon.
+//
+// Erigon is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Lesser General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Erigon is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with Erigon. If not, see <http://www.gnu.org/licenses/>.
+
+package crypto
+
+import (
+	"testing"
+
+	"github.com/holiman/uint256"
+
+	"github.com/ledgerwatch/erigon-lib/common/u256"
+)
+
+func TestTransactionSignatureIsValid(t *testing.T) {
+	check := func(expected bool, v byte, r, s *uint256.Int) {
+		if TransactionSignatureIsValid(v, r, s, true) != expected {
+			t.Errorf("mismatch for v: %d r: %d s: %d want: %v", v, r, s, expected)
+		}
+	}
+	minusOne := uint256.NewInt(0).SetAllOne()
+	one := u256.N1
+	zero := u256.N0
+	secp256k1nMinus1 := new(uint256.Int).Sub(secp256k1N, u256.N1)
+
+	// correct v,r,s
+	check(true, 0, one, one)
+	check(true, 1, one, one)
+	// incorrect v, correct r,s,
+	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, 1, zero, zero)
+	check(false, 1, zero, one)
+	check(false, 1, one, zero)
+
+	// correct sig with max r,s
+	check(true, 0, secp256k1nMinus1, secp256k1nMinus1)
+	// correct v, combinations of incorrect r,s at upper limit
+	check(false, 0, secp256k1N, secp256k1nMinus1)
+	check(false, 0, secp256k1nMinus1, secp256k1N)
+	check(false, 0, secp256k1N, secp256k1N)
+
+	// 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, 0, minusOne, one)
+	check(false, 0, one, minusOne)
+}

erigon-lib/txpool/pool.go

@@ -40,6 +40,7 @@ import (
 	"github.com/hashicorp/golang-lru/v2/simplelru"
 	"github.com/holiman/uint256"
 	"github.com/ledgerwatch/erigon-lib/common/hexutility"
+	"github.com/ledgerwatch/erigon-lib/crypto"
 	"github.com/ledgerwatch/log/v3"
 
 	"github.com/ledgerwatch/erigon-lib/chain"
@@ -773,7 +774,8 @@ func (p *TxPool) best(n uint16, txs *types.TxsRlp, tx kv.Tx, onTopOf, availableG
 		// make sure we have enough gas in the caller to add this transaction.
 		// not an exact science using intrinsic gas but as close as we could hope for at
 		// this stage
-		intrinsicGas, _ := txpoolcfg.CalcIntrinsicGas(uint64(mt.Tx.DataLen), uint64(mt.Tx.DataNonZeroLen), uint64(mt.Tx.AuthorizationLen), nil, mt.Tx.Creation, true, true, isShanghai)
+		authorizationLen := uint64(len(mt.Tx.Authorizations))
+		intrinsicGas, _ := txpoolcfg.CalcIntrinsicGas(uint64(mt.Tx.DataLen), uint64(mt.Tx.DataNonZeroLen), authorizationLen, nil, mt.Tx.Creation, true, true, isShanghai)
 		if intrinsicGas > availableGas {
 			// we might find another TX with a low enough intrinsic gas to include so carry on
 			continue
@@ -853,7 +855,7 @@ func (p *TxPool) validateTx(txn *types.TxSlot, isLocal bool, stateCache kvcache.
 			return txpoolcfg.TypeNotActivated
 		}
 		if txn.Creation {
-			return txpoolcfg.CreateBlobTxn
+			return txpoolcfg.InvalidCreateTxn
 		}
 		blobCount := uint64(len(txn.BlobHashes))
 		if blobCount == 0 {
@@ -897,10 +899,22 @@ func (p *TxPool) validateTx(txn *types.TxSlot, isLocal bool, stateCache kvcache.
 		}
 	}
 
+	authorizationLen := len(txn.Authorizations)
 	if txn.Type == types.SetCodeTxType {
 		if !p.isPrague() {
 			return txpoolcfg.TypeNotActivated
 		}
+		if txn.Creation {
+			return txpoolcfg.InvalidCreateTxn
+		}
+		if authorizationLen == 0 {
+			return txpoolcfg.NoAuthorizations
+		}
+		for i := 0; i < authorizationLen; i++ {
+			if txn.Authorizations[i].S.Gt(crypto.Secp256k1halfN) {
+				return txpoolcfg.InvalidAuthorization
+			}
+		}
 	}
 
 	// Drop non-local transactions under our own minimal accepted gas price or tip
@@ -910,7 +924,7 @@ func (p *TxPool) validateTx(txn *types.TxSlot, isLocal bool, stateCache kvcache.
 		}
 		return txpoolcfg.UnderPriced
 	}
-	gas, reason := txpoolcfg.CalcIntrinsicGas(uint64(txn.DataLen), uint64(txn.DataNonZeroLen), uint64(txn.AuthorizationLen), nil, txn.Creation, true, true, isShanghai)
+	gas, reason := txpoolcfg.CalcIntrinsicGas(uint64(txn.DataLen), uint64(txn.DataNonZeroLen), uint64(authorizationLen), nil, txn.Creation, true, true, isShanghai)
 	if txn.Traced {
 		p.logger.Info(fmt.Sprintf("TX TRACING: validateTx intrinsic gas idHash=%x gas=%d", txn.IDHash, gas))
 	}

erigon-lib/txpool/txpool_grpc_server.go

@@ -239,8 +239,11 @@ func mapDiscardReasonToProto(reason txpoolcfg.DiscardReason) txpool_proto.Import
 		return txpool_proto.ImportResult_ALREADY_EXISTS
 	case txpoolcfg.UnderPriced, txpoolcfg.ReplaceUnderpriced, txpoolcfg.FeeTooLow:
 		return txpool_proto.ImportResult_FEE_TOO_LOW
-	case txpoolcfg.InvalidSender, txpoolcfg.NegativeValue, txpoolcfg.OversizedData, txpoolcfg.InitCodeTooLarge, txpoolcfg.RLPTooLong, txpoolcfg.CreateBlobTxn, txpoolcfg.NoBlobs, txpoolcfg.TooManyBlobs, txpoolcfg.TypeNotActivated, txpoolcfg.UnequalBlobTxExt, txpoolcfg.BlobHashCheckFail, txpoolcfg.UnmatchedBlobTxExt:
-		// TODO(eip-4844) TypeNotActivated may be transient (e.g. a blob transaction is submitted 1 sec prior to Cancun activation)
+	case txpoolcfg.InvalidSender, txpoolcfg.NegativeValue, txpoolcfg.OversizedData, txpoolcfg.InitCodeTooLarge,
+		txpoolcfg.RLPTooLong, txpoolcfg.InvalidCreateTxn, txpoolcfg.NoBlobs, txpoolcfg.TooManyBlobs,
+		txpoolcfg.TypeNotActivated, txpoolcfg.UnequalBlobTxExt, txpoolcfg.BlobHashCheckFail,
+		txpoolcfg.UnmatchedBlobTxExt, txpoolcfg.NoAuthorizations, txpoolcfg.InvalidAuthorization:
+		// TODO(EIP-7702) TypeNotActivated may be transient (e.g. a set code transaction is submitted 1 sec prior to the Pectra activation)
 		return txpool_proto.ImportResult_INVALID
 	default:
 		return txpool_proto.ImportResult_INTERNAL_ERROR