Update EIP-7702: simpler transaction validity (#12212)

Cherry pick #12146

core/state_transition.go

@@ -375,17 +375,6 @@ func (st *StateTransition) TransitionDb(refunds bool, gasBailout bool) (*evmtype
 			}
 
 			// 2. authority recover
-
-			// TODO: these signature checks should ideally be in RecoverSigner, a new PR on 7702 should simplify this
-			// adding this to pass tests for now
-			if auth.S.Cmp(crypto.Secp256k1halfN) > 0 {
-				return nil, fmt.Errorf("invalid signature S, skipping, auth index %d", i)
-			}
-
-			if !auth.V.Eq(u256.Num0) && !auth.V.Eq(u256.Num1) {
-				return nil, fmt.Errorf("invalid v value: %d", auth.V.Uint64())
-			}
-
 			authorityPtr, err := auth.RecoverSigner(data, b[:])
 			if err != nil {
 				log.Debug("authority recover failed, skipping", "err", err, "auth index", i)

crypto/crypto.go

@@ -24,20 +24,19 @@ import (
 	"encoding/hex"
 	"errors"
 	"fmt"
-	"github.com/ledgerwatch/erigon-lib/common/hexutil"
 	"hash"
 	"io"
 	"math/big"
 	"os"
 
 	"github.com/holiman/uint256"
-	libcommon "github.com/ledgerwatch/erigon-lib/common"
 	"golang.org/x/crypto/sha3"
 
-	"github.com/ledgerwatch/erigon/crypto/cryptopool"
+	libcommon "github.com/ledgerwatch/erigon-lib/common"
+	"github.com/ledgerwatch/erigon-lib/common/hexutil"
 
 	"github.com/ledgerwatch/erigon/common/math"
-	"github.com/ledgerwatch/erigon/common/u256"
+	"github.com/ledgerwatch/erigon/crypto/cryptopool"
 	"github.com/ledgerwatch/erigon/rlp"
 )
 
@@ -51,9 +50,8 @@ const RecoveryIDOffset = 64
 const DigestLength = 32
 
 var (
-	secp256k1N     = new(uint256.Int).SetBytes(hexutil.MustDecode("0xfffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141"))
-	secp256k1NBig  = secp256k1N.ToBig()
-	Secp256k1halfN = new(uint256.Int).Div(secp256k1N, u256.Num2)
+	secp256k1N    = new(uint256.Int).SetBytes(hexutil.MustDecode("0xfffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141"))
+	secp256k1NBig = secp256k1N.ToBig()
 )
 
 var errInvalidPubkey = errors.New("invalid secp256k1 public key")

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/txpool/pool.go

@@ -40,7 +40,6 @@ 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"
@@ -910,11 +909,6 @@ func (p *TxPool) validateTx(txn *types.TxSlot, isLocal bool, stateCache kvcache.
 		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

erigon-lib/txpool/pool_test.go

@@ -744,6 +744,54 @@ func TestShanghaiValidateTx(t *testing.T) {
 	}
 }
 
+func TestSetCodeTxValidationWithLargeAuthorizationValues(t *testing.T) {
+	maxUint256 := new(uint256.Int).SetAllOne()
+
+	ch := make(chan types.Announcements, 1)
+	_, coreDB := memdb.NewTestPoolDB(t), memdb.NewTestDB(t)
+	cfg := txpoolcfg.DefaultConfig
+	chainID := *maxUint256
+	cache := &kvcache.DummyCache{}
+	logger := log.New()
+	pool, err := New(ch, coreDB, cfg, cache, chainID, common.Big0 /* shanghaiTime */, nil, /* agraBlock */
+		common.Big0 /* cancunTime */, common.Big0 /* pragueTime */, fixedgas.DefaultMaxBlobsPerBlock, nil, logger)
+	assert.NoError(t, err)
+	ctx := context.Background()
+	tx, err := coreDB.BeginRw(ctx)
+	defer tx.Rollback()
+	assert.NoError(t, err)
+
+	sndr := sender{nonce: 0, balance: *uint256.NewInt(math.MaxUint64)}
+	sndrBytes := make([]byte, types.EncodeSenderLengthForStorage(sndr.nonce, sndr.balance))
+	types.EncodeSender(sndr.nonce, sndr.balance, sndrBytes)
+	err = tx.Put(kv.PlainState, []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, sndrBytes)
+	assert.NoError(t, err)
+
+	txn := &types.TxSlot{
+		FeeCap:         *uint256.NewInt(21000),
+		Gas:            500000,
+		SenderID:       0,
+		Type:           types.SetCodeTxType,
+		Authorizations: make([]types.Signature, 1),
+	}
+	txn.Authorizations[0].ChainID = chainID
+	txn.Authorizations[0].V.Set(maxUint256)
+	txn.Authorizations[0].R.Set(maxUint256)
+	txn.Authorizations[0].S.Set(maxUint256)
+
+	txns := types.TxSlots{
+		Txs:     append([]*types.TxSlot{}, txn),
+		Senders: types.Addresses{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
+	}
+	err = pool.senders.registerNewSenders(&txns, logger)
+	assert.NoError(t, err)
+	view, err := cache.View(ctx, tx)
+	assert.NoError(t, err)
+
+	result := pool.validateTx(txn, false /* isLocal */, view)
+	assert.Equal(t, txpoolcfg.Success, result)
+}
+
 // Blob gas price bump + other requirements to replace existing txns in the pool
 func TestBlobTxReplacement(t *testing.T) {
 	assert, require := assert.New(t), require.New(t)

erigon-lib/txpool/txpool_grpc_server.go

@@ -242,7 +242,7 @@ func mapDiscardReasonToProto(reason txpoolcfg.DiscardReason) txpool_proto.Import
 	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:
+		txpoolcfg.UnmatchedBlobTxExt, txpoolcfg.NoAuthorizations:
 		// 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:

erigon-lib/txpool/txpoolcfg/txpoolcfg.go

@@ -84,40 +84,39 @@ var DefaultConfig = Config{
 type DiscardReason uint8
 
 const (
-	NotSet               DiscardReason = 0 // analog of "nil-value", means it will be set in future
-	Success              DiscardReason = 1
-	AlreadyKnown         DiscardReason = 2
-	Mined                DiscardReason = 3
-	ReplacedByHigherTip  DiscardReason = 4
-	UnderPriced          DiscardReason = 5
-	ReplaceUnderpriced   DiscardReason = 6 // if a transaction is attempted to be replaced with a different one without the required price bump.
-	FeeTooLow            DiscardReason = 7
-	OversizedData        DiscardReason = 8
-	InvalidSender        DiscardReason = 9
-	NegativeValue        DiscardReason = 10 // ensure no one is able to specify a transaction with a negative value.
-	Spammer              DiscardReason = 11
-	PendingPoolOverflow  DiscardReason = 12
-	BaseFeePoolOverflow  DiscardReason = 13
-	QueuedPoolOverflow   DiscardReason = 14
-	GasUintOverflow      DiscardReason = 15
-	IntrinsicGas         DiscardReason = 16
-	RLPTooLong           DiscardReason = 17
-	NonceTooLow          DiscardReason = 18
-	InsufficientFunds    DiscardReason = 19
-	NotReplaced          DiscardReason = 20 // There was an existing transaction with the same sender and nonce, not enough price bump to replace
-	DuplicateHash        DiscardReason = 21 // There was an existing transaction with the same hash
-	InitCodeTooLarge     DiscardReason = 22 // EIP-3860 - transaction init code is too large
-	TypeNotActivated     DiscardReason = 23 // For example, an EIP-4844 transaction is submitted before Cancun activation
-	InvalidCreateTxn     DiscardReason = 24 // EIP-4844 & 7702 transactions cannot have the form of a create transaction
-	NoBlobs              DiscardReason = 25 // Blob transactions must have at least one blob
-	TooManyBlobs         DiscardReason = 26 // There's a limit on how many blobs a block (and thus any transaction) may have
-	UnequalBlobTxExt     DiscardReason = 27 // blob_versioned_hashes, blobs, commitments and proofs must have equal number
-	BlobHashCheckFail    DiscardReason = 28 // KZGcommitment's versioned hash has to be equal to blob_versioned_hash at the same index
-	UnmatchedBlobTxExt   DiscardReason = 29 // KZGcommitments must match the corresponding blobs and proofs
-	BlobTxReplace        DiscardReason = 30 // Cannot replace type-3 blob txn with another type of txn
-	BlobPoolOverflow     DiscardReason = 31 // The total number of blobs (through blob txs) in the pool has reached its limit
-	NoAuthorizations     DiscardReason = 32 // EIP-7702 transactions with an empty authorization list are invalid
-	InvalidAuthorization DiscardReason = 33 // Authorization signature is invalid (EIP-7702)
+	NotSet              DiscardReason = 0 // analog of "nil-value", means it will be set in future
+	Success             DiscardReason = 1
+	AlreadyKnown        DiscardReason = 2
+	Mined               DiscardReason = 3
+	ReplacedByHigherTip DiscardReason = 4
+	UnderPriced         DiscardReason = 5
+	ReplaceUnderpriced  DiscardReason = 6 // if a transaction is attempted to be replaced with a different one without the required price bump.
+	FeeTooLow           DiscardReason = 7
+	OversizedData       DiscardReason = 8
+	InvalidSender       DiscardReason = 9
+	NegativeValue       DiscardReason = 10 // ensure no one is able to specify a transaction with a negative value.
+	Spammer             DiscardReason = 11
+	PendingPoolOverflow DiscardReason = 12
+	BaseFeePoolOverflow DiscardReason = 13
+	QueuedPoolOverflow  DiscardReason = 14
+	GasUintOverflow     DiscardReason = 15
+	IntrinsicGas        DiscardReason = 16
+	RLPTooLong          DiscardReason = 17
+	NonceTooLow         DiscardReason = 18
+	InsufficientFunds   DiscardReason = 19
+	NotReplaced         DiscardReason = 20 // There was an existing transaction with the same sender and nonce, not enough price bump to replace
+	DuplicateHash       DiscardReason = 21 // There was an existing transaction with the same hash
+	InitCodeTooLarge    DiscardReason = 22 // EIP-3860 - transaction init code is too large
+	TypeNotActivated    DiscardReason = 23 // For example, an EIP-4844 transaction is submitted before Cancun activation
+	InvalidCreateTxn    DiscardReason = 24 // EIP-4844 & 7702 transactions cannot have the form of a create transaction
+	NoBlobs             DiscardReason = 25 // Blob transactions must have at least one blob
+	TooManyBlobs        DiscardReason = 26 // There's a limit on how many blobs a block (and thus any transaction) may have
+	UnequalBlobTxExt    DiscardReason = 27 // blob_versioned_hashes, blobs, commitments and proofs must have equal number
+	BlobHashCheckFail   DiscardReason = 28 // KZGcommitment's versioned hash has to be equal to blob_versioned_hash at the same index
+	UnmatchedBlobTxExt  DiscardReason = 29 // KZGcommitments must match the corresponding blobs and proofs
+	BlobTxReplace       DiscardReason = 30 // Cannot replace type-3 blob txn with another type of txn
+	BlobPoolOverflow    DiscardReason = 31 // The total number of blobs (through blob txs) in the pool has reached its limit
+	NoAuthorizations    DiscardReason = 32 // EIP-7702 transactions with an empty authorization list are invalid
 )
 
 func (r DiscardReason) String() string {
@@ -182,8 +181,6 @@ func (r DiscardReason) String() string {
 		return "blobs limit in txpool is full"
 	case NoAuthorizations:
 		return "EIP-7702 transactions with an empty authorization list are invalid"
-	case InvalidAuthorization:
-		return "Authorization signature is invalid (EIP-7702)"
 	default:
 		panic(fmt.Sprintf("discard reason: %d", r))
 	}

erigon-lib/types/txn.go

@@ -323,9 +323,6 @@ func parseSignature(payload []byte, pos int, legacy bool, cfgChainId *uint256.In
 			}
 		}
 	} else {
-		if sig.V.GtUint64(1) {
-			return 0, 0, fmt.Errorf("v is loo large: %s", &sig.V)
-		}
 		yParity = byte(sig.V.Uint64())
 	}
 
@@ -586,6 +583,10 @@ func (ctx *TxParseContext) parseTransactionBody(payload []byte, pos, p0 int, slo
 			sigHashLen += uint(chainIDLen) // For chainId
 			sigHashLen += 2                // For two extra zeros
 		}
+	} else {
+		if ctx.Signature.V.GtUint64(1) {
+			return 0, fmt.Errorf("%w: v is loo large: %s", ErrParseTxn, &ctx.Signature.V)
+		}
 	}
 
 	// For legacy transactions, hash the full payload

erigon-lib/types/txn_test.go

@@ -327,7 +327,7 @@ func TestSetCodeTxParsing(t *testing.T) {
 	assert.Equal(t, 0, len(tx2.Authorizations))
 	assert.Equal(t, SetCodeTxType, tx2.Type)
 
-	// generated using this in encdec_test.go
+	// generated using this in core/types/encdec_test.go
 	/*
 		func TestGenerateSetCodeTxRlp(t *testing.T) {
 			tr := NewTRand()
@@ -360,3 +360,58 @@ func TestSetCodeTxParsing(t *testing.T) {
 		}
 	*/
 }
+
+// See https://github.com/ethereum/EIPs/pull/8845
+func TestSetCodeTxParsingWithLargeAuthorizationValues(t *testing.T) {
+	// generated using this in core/types/encdec_test.go
+	/*
+		func TestGenerateSetCodeTxRlpWithLargeAuthorizationValues(t *testing.T) {
+			tr := NewTRand()
+			var tx Transaction
+			requiredAuthLen := 1
+			for tx = tr.RandTransaction(); tx.Type() != types2.SetCodeTxType || len(tx.(*SetCodeTransaction).GetAuthorizations()) != requiredAuthLen; tx = tr.RandTransaction() {
+			}
+			v, _, _ := tx.RawSignatureValues()
+			v.SetUint64(uint64(randIntInRange(0, 2)))
+
+			tx.GetChainID().SetUint64(1)
+
+			auths := tx.(*SetCodeTransaction).GetAuthorizations()
+			for i := range auths {
+				auths[i].ChainID.SetAllOne()
+				auths[i].Nonce = math.MaxUint64
+				auths[i].V.SetAllOne()
+				auths[i].R.SetAllOne()
+				auths[i].S.SetAllOne()
+			}
+			w := bytes.NewBuffer(nil)
+			if err := tx.MarshalBinary(w); err != nil {
+				t.Error(err)
+			}
+
+			hex := hexutility.Bytes(w.Bytes()).String()
+			fmt.Println("tx", hex)
+		}
+	*/
+	bodyRlxHex := "0x04f90546018820d75f30c9812472883efdaaf328683c8f88791ecd238942f142882f6a9ebaec6c1af49484d85e90da36f0392202193df1101863eb895d498883bf224e11f95355b902540e61088a043ef305b21d1bab6e4c14edeae66b3c0d6c183e423324c562b33b98ca2900d6a0adc2c13fdce4835debb623bf9f02860959bf589e88392a98a57db341a182e7d063945aaa4153010dd1d58d508d27ddd07e82b2f95af341b76b3acfbe00fde8d890867b6794ac95a4096dccbfaf9f92f446d6e4f3a718185f875ce49308ad24cf8b7316f1a73da7fb626e92e7d45d3b6ffeb2e3d153e92bc2499ccd8a43a6295db0be96dffcb857ed409b3e4f29a126e38982bf927efa9e637f670f631b36795dc158cb32069e846f28e4721b9d3004c6951075bf14f27e9f81dfa409e344e713143da8b8b35bd5ebfbc2b5c1e6909fadc7a5681ca63ecd387b745bbd3fff1169308a02e313671f2e5e5e914e8085236112fe12ae52403f54a95230c5807e5697b34bea0bf932a8cd45b6bf7ac1b318c594de85c523b1b82358ee8fce4f94d04bc30369c5a5482668a354bb36bb0f6e8943662be124cc3650f8e563a9c62808fb65444f425ff274974cd6cce771e67c2d45a95431a731ae0db60ef83793d9c257d48a24dae64e43870ecaf56a7f3e95415cdaacee3da94ad569e430ba4cdf5f0ac7cd8bd2e5507a1d0f16849e1d1823fcce1670c861b591051170c60883a32c23bc0a552415203baad7d1a6d44e3229956387ae014e944dbd7b918fa1c52fc7a4ee1ea813f1ba4a1bdedd7a23b4a93faca01eb1127d51953603eae70499d485dc3c8633b778dbae199158e03977b003a9da717ae8f31780949c53f82c5d09dd7428747426f77e39349dcd4fb8e3793d6a46cd4e9be6d6614b285d79e772bae41fb8a1625d23b38cef14911092e78c86f901f0f85994e805c10b96ec670ff8d6b684ed413eb71babf735f842a0e1b6f8621d3f4513ac7d491849fdcf3a8264e6cf4a1503357414a38682662555a027d3e5a0f5e7478629ef06e831162a8e99c4d9cd973919e21c162d29ac1410e6f87a94a5bfb70f5ffb19b2b2173e2d80a1bbfb6ff10dc4f863a0e129afdd8d7ac6e5c52708e658ded4859b06f1b483497e0474f6599b72dd2f75a0a7169872faa1fdfc3c2d80c207bc64ae817301b4087979563ef690b02b0d7964a0fe009fc656d1f0fcfc1c7c93b7ab67bdc9dd06f06a37fdaecbd43ed5d1423a11f87a94feae80b40ef29bdd926061a9000c1b0875cb0be3f863a0a5a381867886a9780e6f955d62cf73849e338918751dba1442d632fccb8d52a8a0ee8501f7bbc25d0dde1f60771167abc421be099cf0ec07636f4e5092f8f6b66ba0660a025a421dd73f4747e3a262a726ab8ff76b893c23e4d595ac50c86b9ae8c0f89b94d94f45974bbdb5d2e27419b5202fd2eec115fe26f884a04ee78854a71d2c0965a62080f2cff53b788a3dc0f8c6a55bf0535e0197a8ba77a08eb0010ea0fbe6e9251421dcbc26d83abe55bc24c6466b1833365dd87b253623a0040fa96b15c24f6a6e6dfd1e7be34b7fb58e8cfc26e4ed16b45d8eb567b60089a0cbfa955cc7c5435e0b6694e1a3d64d773a8baa3c4a85d6eddad407dbff5e645df8a4f8a2a0ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff942d9986714cb0a7f215d435013a08af34c42406be88ffffffffffffffffa0ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffa0ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffa0ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff01886417de6405dde7ee88813b5026c7af887a"
+	bodyRlx := hexutility.MustDecodeHex(bodyRlxHex)
+
+	ctx := NewTxParseContext(*uint256.NewInt(1))
+	ctx.withSender = false
+
+	var tx TxSlot
+	txType, err := PeekTransactionType(bodyRlx)
+	require.NoError(t, err)
+	assert.Equal(t, SetCodeTxType, txType)
+
+	_, err = ctx.ParseTransaction(bodyRlx, 0, &tx, nil, false /* hasEnvelope */, false, nil)
+	require.NoError(t, err)
+	assert.Equal(t, SetCodeTxType, tx.Type)
+	require.Equal(t, 1, len(tx.Authorizations))
+
+	maxUint256 := new(uint256.Int).SetAllOne()
+	assert.True(t, tx.Authorizations[0].ChainID.Eq(maxUint256))
+	assert.True(t, tx.Authorizations[0].V.Eq(maxUint256))
+	assert.True(t, tx.Authorizations[0].R.Eq(maxUint256))
+	assert.True(t, tx.Authorizations[0].S.Eq(maxUint256))
+}