Skip to content

Commit

Permalink
Stricter validation of EIP-7702 transactions (#11885)
Browse files Browse the repository at this point in the history
Check (in the txpool and in the main code) that an EIP-7702 transaction
is
[valid](https://eips.ethereum.org/EIPS/eip-7702#set-code-transaction),
namely that:

- `to != nil`
- `len(authorization_list) != 0`
- `authorization.chain_id` is `uint256`
- `authorization.nonce` is `uint64`
- `authorization.address` is `bytes20`
- `authorization.y_parity == 0 || authorization.y_parity == 1`
- `authorization.r` is `uint256`
- `authorization.s` is `uint256` and `authorization.s <= secp256k1n/2`

This PR doesn't implement ethereum/EIPs#8865 or
ethereum/EIPs#8845
  • Loading branch information
yperbasis authored Sep 12, 2024
1 parent dafc99b commit 60675ae
Show file tree
Hide file tree
Showing 14 changed files with 260 additions and 183 deletions.
4 changes: 3 additions & 1 deletion core/types/authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import (

libcommon "github.com/erigontech/erigon-lib/common"
"github.com/erigontech/erigon-lib/common/length"
libcrypto "github.com/erigontech/erigon-lib/crypto"
rlp2 "github.com/erigontech/erigon-lib/rlp"

"github.com/erigontech/erigon/common/u256"
"github.com/erigontech/erigon/crypto"
"github.com/erigontech/erigon/params"
Expand Down Expand Up @@ -75,7 +77,7 @@ 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) {
if !libcrypto.TransactionSignatureIsValid(sig[64], &ath.R, &ath.S, false /* allowPreEip2s */) {
return nil, errors.New("invalid signature")
}

Expand Down
12 changes: 7 additions & 5 deletions core/types/set_code_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,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
Expand Down Expand Up @@ -225,13 +229,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
}
Expand Down
7 changes: 3 additions & 4 deletions core/types/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,14 @@ import (
"github.com/holiman/uint256"
"github.com/protolambda/ztyp/codec"

"github.com/erigontech/erigon-lib/log/v3"

"github.com/erigontech/erigon-lib/chain"
libcommon "github.com/erigontech/erigon-lib/common"
"github.com/erigontech/erigon-lib/common/fixedgas"
libcrypto "github.com/erigontech/erigon-lib/crypto"
"github.com/erigontech/erigon-lib/log/v3"
types2 "github.com/erigontech/erigon-lib/types"

"github.com/erigontech/erigon/common/math"
"github.com/erigontech/erigon/crypto"
"github.com/erigontech/erigon/rlp"
)

Expand Down Expand Up @@ -284,7 +283,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
}

Expand Down
3 changes: 2 additions & 1 deletion core/types/transaction_signing.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (

"github.com/erigontech/erigon-lib/chain"
libcommon "github.com/erigontech/erigon-lib/common"
libcrypto "github.com/erigontech/erigon-lib/crypto"

"github.com/erigontech/erigon/common/u256"
"github.com/erigontech/erigon/crypto"
Expand Down Expand Up @@ -364,7 +365,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
Expand Down
3 changes: 2 additions & 1 deletion core/vm/contracts.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (

"github.com/erigontech/erigon-lib/chain"
libcommon "github.com/erigontech/erigon-lib/common"
libcrypto "github.com/erigontech/erigon-lib/crypto"
"github.com/erigontech/erigon-lib/crypto/blake2b"
libkzg "github.com/erigontech/erigon-lib/crypto/kzg"

Expand Down Expand Up @@ -242,7 +243,7 @@ func (c *ecrecover) Run(input []byte) ([]byte, error) {
v := input[63] - 27

// tighter sig s values input homestead only apply to txn sigs
if !allZero(input[32:63]) || !crypto.ValidateSignatureValues(v, r, s, false) {
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
Expand Down
15 changes: 0 additions & 15 deletions crypto/crypto.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,21 +301,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)
Expand Down
48 changes: 0 additions & 48 deletions crypto/crypto_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,12 @@ import (
"reflect"
"testing"

"github.com/holiman/uint256"
"golang.org/x/crypto/sha3"

libcommon "github.com/erigontech/erigon-lib/common"
"github.com/erigontech/erigon-lib/common/hexutil"

"github.com/erigontech/erigon/common"
"github.com/erigontech/erigon/common/u256"
)

var testAddrHex = "970e8128ab834e8eac17ab8e3812f010678cf791"
Expand Down Expand Up @@ -279,52 +277,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) {
Expand Down
4 changes: 2 additions & 2 deletions erigon-lib/crypto/secp256k1.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}

Expand Down
74 changes: 74 additions & 0 deletions erigon-lib/crypto/secp256k1_test.go
Original file line number Diff line number Diff line change
@@ -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/erigontech/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)
}
20 changes: 17 additions & 3 deletions erigon-lib/txpool/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import (
"github.com/erigontech/erigon-lib/common/fixedgas"
"github.com/erigontech/erigon-lib/common/hexutility"
"github.com/erigontech/erigon-lib/common/u256"
"github.com/erigontech/erigon-lib/crypto"
libkzg "github.com/erigontech/erigon-lib/crypto/kzg"
"github.com/erigontech/erigon-lib/gointerfaces"
"github.com/erigontech/erigon-lib/gointerfaces/grpcutil"
Expand Down Expand Up @@ -772,7 +773,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 txn with a low enough intrinsic gas to include so carry on
continue
Expand Down Expand Up @@ -852,7 +854,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 {
Expand Down Expand Up @@ -896,10 +898,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
Expand All @@ -909,7 +923,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))
}
Expand Down
7 changes: 5 additions & 2 deletions erigon-lib/txpool/txpool_grpc_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,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
Expand Down
Loading

0 comments on commit 60675ae

Please sign in to comment.