From 86f57ee036dfe1b48cdeaa04c116c86261b0b933 Mon Sep 17 00:00:00 2001 From: Ian Norden Date: Thu, 30 Apr 2020 08:52:15 -0500 Subject: [PATCH] enforce eip1559 gasPrice >= baseFee; update MaxGasEIP1559 to 20 mil --- accounts/abi/bind/backends/simulated.go | 7 ++-- core/block_validator_test.go | 15 +++++---- core/chain_makers_test.go | 23 +++++++------ core/state_transition.go | 20 +++++------ core/tx_pool.go | 3 ++ core/tx_pool_test.go | 4 +-- internal/ethapi/api.go | 45 ++++++++++++++++++++++--- light/txpool.go | 11 ++++++ params/protocol_params.go | 2 +- 9 files changed, 92 insertions(+), 38 deletions(-) diff --git a/accounts/abi/bind/backends/simulated.go b/accounts/abi/bind/backends/simulated.go index b932bf1a9..9392e74bc 100644 --- a/accounts/abi/bind/backends/simulated.go +++ b/accounts/abi/bind/backends/simulated.go @@ -607,8 +607,11 @@ func (b *SimulatedBackend) SendTransaction(ctx context.Context, tx *types.Transa defer b.mu.Unlock() // EIP1559 guards - eip1559 := b.config.IsEIP1559(b.blockchain.CurrentBlock().Number()) - eip1559Finalized := b.config.IsEIP1559Finalized(b.blockchain.CurrentBlock().Number()) + eip1559 := b.config.IsEIP1559(b.pendingBlock.Number()) + eip1559Finalized := b.config.IsEIP1559Finalized(b.pendingBlock.Number()) + if eip1559 && b.pendingBlock.BaseFee() == nil { + return core.ErrNoBaseFee + } if eip1559Finalized && (tx.GasPremium() == nil || tx.FeeCap() == nil || tx.GasPrice() != nil) { return core.ErrTxNotEIP1559 } diff --git a/core/block_validator_test.go b/core/block_validator_test.go index 9860991e5..a96960eca 100644 --- a/core/block_validator_test.go +++ b/core/block_validator_test.go @@ -90,7 +90,7 @@ func TestHeaderVerificationEIP1559(t *testing.T) { testdb = rawdb.NewMemoryDatabase() gspec = &Genesis{ Config: params.EIP1559ChainConfig, - Alloc: GenesisAlloc{addr1: {Balance: big.NewInt(1000000)}}, + Alloc: GenesisAlloc{addr1: {Balance: big.NewInt(1000000)}, addr2: {Balance: new(big.Int).SetUint64((params.EIP1559InitialBaseFee * params.TxGas) + 1000)}}, BaseFee: new(big.Int).SetUint64(params.EIP1559InitialBaseFee)} genesis = gspec.MustCommit(testdb) signer = types.HomesteadSigner{} @@ -104,7 +104,7 @@ func TestHeaderVerificationEIP1559(t *testing.T) { // In block 2, addr1 sends some more ether to addr2. // addr2 attempts to pass it on to addr3 using a EIP1559 transaction tx1, _ := types.SignTx(types.NewTransaction(gen.TxNonce(addr1), addr2, big.NewInt(1000), params.TxGas, new(big.Int), nil, nil, nil), signer, key1) - tx2, _ := types.SignTx(types.NewTransaction(gen.TxNonce(addr2), addr3, big.NewInt(1000), params.TxGas, nil, nil, new(big.Int), new(big.Int)), signer, key2) + tx2, _ := types.SignTx(types.NewTransaction(gen.TxNonce(addr2), addr3, big.NewInt(1000), params.TxGas, nil, nil, new(big.Int), new(big.Int).SetUint64(params.EIP1559InitialBaseFee)), signer, key2) gen.AddTx(tx1) gen.AddTx(tx2) case 2: @@ -172,8 +172,9 @@ func TestHeaderVerificationEIP1559Finalized(t *testing.T) { addr3 = crypto.PubkeyToAddress(key3.PublicKey) testdb = rawdb.NewMemoryDatabase() gspec = &Genesis{ - Config: params.EIP1559FinalizedChainConfig, - Alloc: GenesisAlloc{addr1: {Balance: new(big.Int).SetUint64((params.EIP1559InitialBaseFee * params.TxGas) + 1000000)}}, + Config: params.EIP1559FinalizedChainConfig, + Alloc: GenesisAlloc{addr1: {Balance: new(big.Int).SetUint64((params.EIP1559InitialBaseFee * params.TxGas * 2) + 11000)}, + addr2: {Balance: new(big.Int).SetUint64((params.EIP1559InitialBaseFee * params.TxGas) + 1000)}}, BaseFee: new(big.Int).SetUint64(params.EIP1559InitialBaseFee)} genesis = gspec.MustCommit(testdb) signer = types.HomesteadSigner{} @@ -186,8 +187,8 @@ func TestHeaderVerificationEIP1559Finalized(t *testing.T) { case 1: // In block 2, addr1 sends some more ether to addr2. // addr2 attempts to pass it on to addr3 using a EIP1559 transaction - tx1, _ := types.SignTx(types.NewTransaction(gen.TxNonce(addr1), addr2, big.NewInt(1000), params.TxGas, nil, nil, new(big.Int), new(big.Int)), signer, key1) - tx2, _ := types.SignTx(types.NewTransaction(gen.TxNonce(addr2), addr3, big.NewInt(1000), params.TxGas, nil, nil, new(big.Int), new(big.Int)), signer, key2) + tx1, _ := types.SignTx(types.NewTransaction(gen.TxNonce(addr1), addr2, big.NewInt(1000), params.TxGas, nil, nil, new(big.Int), new(big.Int).SetUint64(params.EIP1559InitialBaseFee)), signer, key1) + tx2, _ := types.SignTx(types.NewTransaction(gen.TxNonce(addr2), addr3, big.NewInt(1000), params.TxGas, nil, nil, new(big.Int), new(big.Int).SetUint64(params.EIP1559InitialBaseFee)), signer, key2) gen.AddTx(tx1) gen.AddTx(tx2) case 2: @@ -645,7 +646,7 @@ func TestCalcGasLimitAndBaseFee(t *testing.T) { big.NewInt(1059276716), new(big.Int).SetUint64(params.EIP1559ForkFinalizedBlockNumber + 10000), params.MaxGasEIP1559, - new(big.Int).SetUint64(1138722469), + new(big.Int).SetUint64(1191686305), }, { params.EIP1559FinalizedChainConfig, diff --git a/core/chain_makers_test.go b/core/chain_makers_test.go index ec0baf290..b5fd2b7f8 100644 --- a/core/chain_makers_test.go +++ b/core/chain_makers_test.go @@ -193,7 +193,7 @@ func generateChainDuringTransition(t *testing.T) { // Ensure that key1 has some funds in the genesis block. gspec := &Genesis{ Config: params.EIP1559ChainConfig, - Alloc: GenesisAlloc{addr1: {Balance: big.NewInt(1000000)}}, + Alloc: GenesisAlloc{addr1: {Balance: big.NewInt(1000000)}, addr2: {Balance: new(big.Int).SetUint64((params.EIP1559InitialBaseFee * params.TxGas) + 1000)}}, BaseFee: new(big.Int).SetUint64(params.EIP1559InitialBaseFee), } genesis := gspec.MustCommit(db) @@ -212,7 +212,7 @@ func generateChainDuringTransition(t *testing.T) { // In block 2, addr1 sends some more ether to addr2. // addr2 attempts to pass it on to addr3 using a EIP1559 transaction tx1, _ := types.SignTx(types.NewTransaction(gen.TxNonce(addr1), addr2, big.NewInt(1000), params.TxGas, new(big.Int), nil, nil, nil), signer, key1) - tx2, _ := types.SignTx(types.NewTransaction(gen.TxNonce(addr2), addr3, big.NewInt(1000), params.TxGas, nil, nil, new(big.Int), new(big.Int)), signer, key2) + tx2, _ := types.SignTx(types.NewTransaction(gen.TxNonce(addr2), addr3, big.NewInt(1000), params.TxGas, nil, nil, new(big.Int), new(big.Int).SetUint64(params.EIP1559InitialBaseFee)), signer, key2) gen.AddTx(tx1) gen.AddTx(tx2) case 2: @@ -245,7 +245,7 @@ func generateChainDuringTransition(t *testing.T) { if state.GetBalance(addr1).Uint64() != 989000 { t.Fatalf("expected balance of addr1 to equal %d got %d", 989000, state.GetBalance(addr1).Uint64()) } - if state.GetBalance(addr2).Uint64() != 10000 { + if state.GetBalance(addr2).Uint64() != 4917051584000 { t.Fatalf("expected balance of addr2 to equal %d got %d", 10000, state.GetBalance(addr2).Uint64()) } // This value is different because the test config we use has Constantinople active (uses ConstantinopleBlockReward) @@ -338,8 +338,9 @@ func generateChainAfterFinalization2(t *testing.T) { // Ensure that key1 has some funds in the genesis block. gspec := &Genesis{ - Config: params.EIP1559FinalizedChainConfig, - Alloc: GenesisAlloc{addr1: {Balance: new(big.Int).SetUint64((params.EIP1559InitialBaseFee * params.TxGas) + 1000000)}}, + Config: params.EIP1559FinalizedChainConfig, + Alloc: GenesisAlloc{addr1: {Balance: new(big.Int).SetUint64((params.EIP1559InitialBaseFee * params.TxGas * 2) + 11000)}, + addr2: {Balance: new(big.Int).SetUint64((params.EIP1559InitialBaseFee * params.TxGas) + 1000)}}, BaseFee: new(big.Int).SetUint64(params.EIP1559InitialBaseFee), } genesis := gspec.MustCommit(db) @@ -357,8 +358,8 @@ func generateChainAfterFinalization2(t *testing.T) { case 1: // In block 2, addr1 sends some more ether to addr2. // addr2 attempts to pass it on to addr3 using a EIP1559 transaction - tx1, _ := types.SignTx(types.NewTransaction(gen.TxNonce(addr1), addr2, big.NewInt(1000), params.TxGas, nil, nil, new(big.Int), new(big.Int)), signer, key1) - tx2, _ := types.SignTx(types.NewTransaction(gen.TxNonce(addr2), addr3, big.NewInt(1000), params.TxGas, nil, nil, new(big.Int), new(big.Int)), signer, key2) + tx1, _ := types.SignTx(types.NewTransaction(gen.TxNonce(addr1), addr2, big.NewInt(1000), params.TxGas, nil, nil, new(big.Int), new(big.Int).SetUint64(params.EIP1559InitialBaseFee)), signer, key1) + tx2, _ := types.SignTx(types.NewTransaction(gen.TxNonce(addr2), addr3, big.NewInt(1000), params.TxGas, nil, nil, new(big.Int), new(big.Int).SetUint64(params.EIP1559InitialBaseFee)), signer, key2) gen.AddTx(tx1) gen.AddTx(tx2) case 2: @@ -388,11 +389,11 @@ func generateChainAfterFinalization2(t *testing.T) { if blockchain.CurrentBlock().Number().Uint64() != 5 { t.Fatalf("expected last block to equal %d got %d", 5, blockchain.CurrentBlock().Number().Uint64()) } - if state.GetBalance(addr1).Uint64() != 2625000989000 { - t.Fatalf("expected balance of addr1 to equal %d got %d", 989000, state.GetBalance(addr1).Uint64()) + if state.GetBalance(addr1).Uint64() != 7542051573000 { + t.Fatalf("expected balance of addr1 to equal %d got %d", 7542051573000, state.GetBalance(addr1).Uint64()) } - if state.GetBalance(addr2).Uint64() != 10000 { - t.Fatalf("expected balance of addr2 to equal %d got %d", 10000, state.GetBalance(addr2).Uint64()) + if state.GetBalance(addr2).Uint64() != 4917051584000 { + t.Fatalf("expected balance of addr2 to equal %d got %d", 4917051584000, state.GetBalance(addr2).Uint64()) } // This value is different than in TestGenerateChain because the test config we use has Constantinople active (uses ConstantinopleBlockReward) bal, _ := new(big.Int).SetString("7875000000000001000", 10) diff --git a/core/state_transition.go b/core/state_transition.go index faf5fa477..1094c8054 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -26,8 +26,8 @@ import ( ) var ( - errInsufficientBalanceForGas = errors.New("insufficient balance to pay for gas") - errInsufficientCoinbaseBalance = errors.New("insufficient coinbase balance to apply a negative coinbase credit") + errInsufficientBalanceForGas = errors.New("insufficient balance to pay for gas") + ErrEIP1559GasPriceLessThanBaseFee = errors.New("EIP11559 GasPrice is less than the current BaseFee") ) /* @@ -262,6 +262,12 @@ func (st *StateTransition) preCheck() error { if st.msg.GasPrice() == nil && (st.msg.GasPremium() == nil || st.msg.FeeCap() == nil) { return ErrMissingGasFields } + // If it is an EIp1559 transaction, make sure the derived gasPrice is >= baseFee + if st.isEIP1559 { + if st.eip1559GasPrice.Cmp(st.evm.BaseFee) < 0 { + return ErrEIP1559GasPriceLessThanBaseFee + } + } return st.buyGas() } @@ -328,15 +334,7 @@ func (st *StateTransition) TransitionDb() (*ExecutionResult, error) { if st.isEIP1559 { // block.coinbase gains (gasprice - BASEFEE) * gasused coinBaseCredit := new(big.Int).Mul(new(big.Int).Sub(st.eip1559GasPrice, st.evm.BaseFee), new(big.Int).SetUint64(st.gasUsed())) - // If gasprice < BASEFEE (due to the fee_cap), this means that the block.coinbase loses funds from this operation; - // in this case, check that the post-balance is non-negative and throw an exception if it is negative. - if coinBaseCredit.Sign() < 0 { - coinbaseBal := st.state.GetBalance(st.evm.Coinbase) - postBalance := new(big.Int).Add(coinbaseBal, coinBaseCredit) - if postBalance.Sign() < 0 { - return nil, 0, vmerr != nil, errInsufficientCoinbaseBalance - } - } + // coinbaseCredit cannot be negative since we precheck that eip1559GasPrice >= st.evm.BaseFee st.state.AddBalance(st.evm.Coinbase, coinBaseCredit) return ret, st.gasUsed(), vmerr != nil, err diff --git a/core/tx_pool.go b/core/tx_pool.go index f19df6ed5..a794ec9b5 100644 --- a/core/tx_pool.go +++ b/core/tx_pool.go @@ -587,6 +587,9 @@ func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error { if gasPrice.Cmp(tx.FeeCap()) > 0 { gasPrice.Set(tx.FeeCap()) } + if gasPrice.Cmp(pool.chain.CurrentBlock().BaseFee()) < 0 { + return ErrEIP1559GasPriceLessThanBaseFee + } } // Heuristic limit, reject transactions over 32KB to prevent DOS attacks diff --git a/core/tx_pool_test.go b/core/tx_pool_test.go index 8392dc2e0..d117e3326 100644 --- a/core/tx_pool_test.go +++ b/core/tx_pool_test.go @@ -3755,7 +3755,7 @@ func TestTransactionPoolUnderpricingEIP1559(t *testing.T) { if err := pool.AddLocal(ltx); err != nil { t.Fatalf("failed to append underpriced local transaction: %v", err) } - ltx = eip1559Transaction(0, 100000, keys[3], big.NewInt(0), big.NewInt(0)) + ltx = eip1559Transaction(0, 100000, keys[3], big.NewInt(0), big.NewInt(1)) if err := pool.AddLocal(ltx); err != nil { t.Fatalf("failed to add new underpriced local transaction: %v", err) } @@ -3854,7 +3854,7 @@ func TestTransactionPoolUnderpricingEIP1559Finalized(t *testing.T) { t.Fatalf("pool internal state corrupted: %v", err) } // Ensure that adding local transactions can push out even higher priced ones - ltx = eip1559Transaction(1, 100000, keys[2], big.NewInt(0), big.NewInt(0)) + ltx = eip1559Transaction(1, 100000, keys[2], big.NewInt(0), big.NewInt(1)) if err := pool.AddLocal(ltx); err != nil { t.Fatalf("failed to append underpriced local transaction: %v", err) } diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 89243b52f..da1ba2000 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -810,6 +810,9 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo // EIP1559 guards eip1559 := b.ChainConfig().IsEIP1559(b.CurrentBlock().Number()) eip1559Finalized := b.ChainConfig().IsEIP1559Finalized(b.CurrentBlock().Number()) + if eip1559 && b.CurrentBlock().BaseFee() == nil { + return nil, 0, false, core.ErrNoBaseFee + } if eip1559Finalized && (args.GasPremium == nil || args.FeeCap == nil || args.GasPrice != nil) { return nil, 0, false, core.ErrTxNotEIP1559 } @@ -822,8 +825,17 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo if args.FeeCap != nil && args.GasPremium == nil { return nil, 0, false, errors.New("if FeeCap is set, GasPremium must be set") } - if args.GasPremium != nil && args.FeeCap == nil { - return nil, 0, false, errors.New("if GasPremium is set, FeeCap must be set") + if args.GasPremium != nil { + if args.FeeCap == nil { + return nil, 0, false, errors.New("if GasPremium is set, FeeCap must be set") + } + gasPrice := new(big.Int).Add(b.CurrentBlock().BaseFee(), args.GasPremium.ToInt()) + if gasPrice.Cmp(args.FeeCap.ToInt()) > 0 { + gasPrice.Set(args.FeeCap.ToInt()) + } + if gasPrice.Cmp(b.CurrentBlock().BaseFee()) < 0 { + return nil, 0, false, core.ErrEIP1559GasPriceLessThanBaseFee + } } state, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) @@ -1495,6 +1507,9 @@ func (args *SendTxArgs) setDefaults(ctx context.Context, b Backend) error { // EIP1559 guards eip1559 := b.ChainConfig().IsEIP1559(b.CurrentBlock().Number()) eip1559Finalized := b.ChainConfig().IsEIP1559Finalized(b.CurrentBlock().Number()) + if eip1559 && b.CurrentBlock().BaseFee() == nil { + return core.ErrNoBaseFee + } if eip1559Finalized && (args.GasPremium == nil || args.FeeCap == nil || args.GasPrice != nil) { return core.ErrTxNotEIP1559 } @@ -1507,9 +1522,19 @@ func (args *SendTxArgs) setDefaults(ctx context.Context, b Backend) error { if args.FeeCap != nil && args.GasPremium == nil { return errors.New("if FeeCap is set, GasPremium must be set") } - if args.GasPremium != nil && args.FeeCap == nil { - return errors.New("if GasPremium is set, FeeCap must be set") + if args.GasPremium != nil { + if args.FeeCap == nil { + return errors.New("if GasPremium is set, FeeCap must be set") + } + gasPrice := new(big.Int).Add(b.CurrentBlock().BaseFee(), args.GasPremium.ToInt()) + if gasPrice.Cmp(args.FeeCap.ToInt()) > 0 { + gasPrice.Set(args.FeeCap.ToInt()) + } + if gasPrice.Cmp(b.CurrentBlock().BaseFee()) < 0 { + return core.ErrEIP1559GasPriceLessThanBaseFee + } } + // If EIP1559 is activated but not finalized and neither a GasPrice, GasPremium, or FeeCap are provided default to suggesting a GasPrice if args.GasPrice == nil && args.GasPremium == nil { price, err := b.SuggestPrice(ctx) @@ -1661,6 +1686,9 @@ func (s *PublicTransactionPoolAPI) SendRawTransaction(ctx context.Context, encod // EIP1559 guards eip1559 := s.b.ChainConfig().IsEIP1559(s.b.CurrentBlock().Number()) eip1559Finalized := s.b.ChainConfig().IsEIP1559Finalized(s.b.CurrentBlock().Number()) + if eip1559 && s.b.CurrentBlock().BaseFee() == nil { + return common.Hash{}, core.ErrNoBaseFee + } if eip1559Finalized && (tx.GasPremium() == nil || tx.FeeCap() == nil || tx.GasPrice() != nil) { return common.Hash{}, core.ErrTxNotEIP1559 } @@ -1673,6 +1701,15 @@ func (s *PublicTransactionPoolAPI) SendRawTransaction(ctx context.Context, encod if tx.GasPrice() == nil && (tx.GasPremium() == nil || tx.FeeCap() == nil) { return common.Hash{}, core.ErrMissingGasFields } + if tx.GasPremium() != nil { + gasPrice := new(big.Int).Add(s.b.CurrentBlock().BaseFee(), tx.GasPremium()) + if gasPrice.Cmp(tx.FeeCap()) > 0 { + gasPrice.Set(tx.FeeCap()) + } + if gasPrice.Cmp(s.b.CurrentBlock().BaseFee()) < 0 { + return common.Hash{}, core.ErrEIP1559GasPriceLessThanBaseFee + } + } return SubmitTransaction(ctx, s.b, tx) } diff --git a/light/txpool.go b/light/txpool.go index 5a204a4c1..6098237f8 100644 --- a/light/txpool.go +++ b/light/txpool.go @@ -397,6 +397,17 @@ func (pool *TxPool) validateTx(ctx context.Context, tx *types.Transaction) error return core.ErrEIP1559GasLimit } + // Derive the gasPrice from the tx.GasPremium() and tx.FeeCap() (EIP1559 transaction) to ensure it is greater than BaseFee + if tx.GasPremium() != nil { + gasPrice := new(big.Int).Add(pool.chain.CurrentHeader().BaseFee, tx.GasPremium()) + if gasPrice.Cmp(tx.FeeCap()) > 0 { + gasPrice.Set(tx.FeeCap()) + } + if gasPrice.Cmp(pool.chain.CurrentHeader().BaseFee) < 0 { + return core.ErrEIP1559GasPriceLessThanBaseFee + } + } + // Transactions can't be negative. This may never happen // using RLP decoded transactions but may occur if you create // a transaction using the RPC for example. diff --git a/params/protocol_params.go b/params/protocol_params.go index 260acd48b..e39c2aa4d 100644 --- a/params/protocol_params.go +++ b/params/protocol_params.go @@ -145,7 +145,7 @@ const ( EIP1559ForkFinalizedBlockNumber = EIP1559ForkBlockNumber + (MaxGasEIP1559 / 20) BaseFeeMaxChangeDenominator uint64 = 8 TargetGasUsed uint64 = 10000000 - MaxGasEIP1559 uint64 = 16000000 + MaxGasEIP1559 uint64 = 20000000 EIP1559DecayRange = EIP1559ForkFinalizedBlockNumber - EIP1559ForkBlockNumber EIP1559GasIncrementAmount = (MaxGasEIP1559 / 2) / EIP1559DecayRange // We need to shift (MaxGasEIP1559 / 2) gas from the legacy pool into the EIP1559 pool over the EIP1559DecayRange PerTransactionGasLimit uint64 = 8000000