From 147e26cf62d050e8a82f1a2770b183aafd8c8f93 Mon Sep 17 00:00:00 2001 From: Andrew Ashikhmin <34320705+yperbasis@users.noreply.github.com> Date: Fri, 4 Oct 2024 09:58:38 +0200 Subject: [PATCH] Update EIP-7702: simpler transaction validity (#12212) Cherry pick #12146 --- core/state_transition.go | 11 ---- crypto/crypto.go | 12 ++--- erigon-lib/crypto/secp256k1.go | 4 +- erigon-lib/txpool/pool.go | 6 --- erigon-lib/txpool/pool_test.go | 48 +++++++++++++++++ erigon-lib/txpool/txpool_grpc_server.go | 2 +- erigon-lib/txpool/txpoolcfg/txpoolcfg.go | 69 ++++++++++++------------ erigon-lib/types/txn.go | 7 +-- erigon-lib/types/txn_test.go | 57 +++++++++++++++++++- 9 files changed, 149 insertions(+), 67 deletions(-) diff --git a/core/state_transition.go b/core/state_transition.go index 8a1d56e41e0..9c71ea733ce 100644 --- a/core/state_transition.go +++ b/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) diff --git a/crypto/crypto.go b/crypto/crypto.go index 7fc20070dce..ff575dc6c15 100644 --- a/crypto/crypto.go +++ b/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") diff --git a/erigon-lib/crypto/secp256k1.go b/erigon-lib/crypto/secp256k1.go index e004445787c..3e8f6dc2ff0 100644 --- a/erigon-lib/crypto/secp256k1.go +++ b/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 } diff --git a/erigon-lib/txpool/pool.go b/erigon-lib/txpool/pool.go index 89dca8e1ec0..be681044858 100644 --- a/erigon-lib/txpool/pool.go +++ b/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 diff --git a/erigon-lib/txpool/pool_test.go b/erigon-lib/txpool/pool_test.go index 4fdc5935b5d..dfe1eaa5b1a 100644 --- a/erigon-lib/txpool/pool_test.go +++ b/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) diff --git a/erigon-lib/txpool/txpool_grpc_server.go b/erigon-lib/txpool/txpool_grpc_server.go index c638d3b37e0..431a045c51d 100644 --- a/erigon-lib/txpool/txpool_grpc_server.go +++ b/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: diff --git a/erigon-lib/txpool/txpoolcfg/txpoolcfg.go b/erigon-lib/txpool/txpoolcfg/txpoolcfg.go index e5eabf3db65..feb7a40fa53 100644 --- a/erigon-lib/txpool/txpoolcfg/txpoolcfg.go +++ b/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)) } diff --git a/erigon-lib/types/txn.go b/erigon-lib/types/txn.go index 32939c93d2d..a211892eb2c 100644 --- a/erigon-lib/types/txn.go +++ b/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 diff --git a/erigon-lib/types/txn_test.go b/erigon-lib/types/txn_test.go index 99ce6b0cfaa..c7765f2af34 100644 --- a/erigon-lib/types/txn_test.go +++ b/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)) +}