From d9bb3517f0b0e08106d3bf9dfa245183a926a81f Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Thu, 16 Feb 2023 10:38:15 -0800 Subject: [PATCH 1/5] Remove unnecessary gasprice updater logic and tests --- core/tx_pool.go | 4 +- plugin/evm/block_builder_test.go | 18 ++++ plugin/evm/gasprice_update.go | 81 ------------------ plugin/evm/gasprice_update_test.go | 129 ----------------------------- plugin/evm/vm.go | 15 +--- 5 files changed, 22 insertions(+), 225 deletions(-) delete mode 100644 plugin/evm/gasprice_update.go delete mode 100644 plugin/evm/gasprice_update_test.go diff --git a/core/tx_pool.go b/core/tx_pool.go index 7b4e50c376..25a5b0d52c 100644 --- a/core/tx_pool.go +++ b/core/tx_pool.go @@ -190,7 +190,7 @@ var DefaultTxPoolConfig = TxPoolConfig{ Journal: "transactions.rlp", Rejournal: time.Hour, - PriceLimit: 1, + PriceLimit: 0, PriceBump: 10, AccountSlots: 16, @@ -328,7 +328,9 @@ func NewTxPool(config TxPoolConfig, chainconfig *params.ChainConfig, chain block initDoneCh: make(chan struct{}), generalShutdownChan: make(chan struct{}), gasPrice: new(big.Int).SetUint64(config.PriceLimit), + minimumFee: new(big.Int).Add(common.Big0, chainconfig.FeeConfig.MinBaseFee), } + pool.locals = newAccountSet(pool.signer) for _, addr := range config.Locals { log.Info("Setting new local account", "address", addr) diff --git a/plugin/evm/block_builder_test.go b/plugin/evm/block_builder_test.go index c178edea8d..401ba262f3 100644 --- a/plugin/evm/block_builder_test.go +++ b/plugin/evm/block_builder_test.go @@ -14,6 +14,24 @@ import ( "github.com/ava-labs/avalanchego/snow" ) +func attemptAwait(t *testing.T, wg *sync.WaitGroup, delay time.Duration) { + ticker := make(chan struct{}) + + // Wait for [wg] and then close [ticket] to indicate that + // the wait group has finished. + go func() { + wg.Wait() + close(ticker) + }() + + select { + case <-time.After(delay): + t.Fatal("Timed out waiting for wait group to complete") + case <-ticker: + // The wait group completed without issue + } +} + func TestBlockBuilderShutsDown(t *testing.T) { shutdownChan := make(chan struct{}) wg := &sync.WaitGroup{} diff --git a/plugin/evm/gasprice_update.go b/plugin/evm/gasprice_update.go deleted file mode 100644 index ff5e994959..0000000000 --- a/plugin/evm/gasprice_update.go +++ /dev/null @@ -1,81 +0,0 @@ -// (c) 2019-2020, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package evm - -import ( - "math/big" - "sync" - "time" - - "github.com/ava-labs/subnet-evm/params" -) - -type gasPriceUpdater struct { - setter gasPriceSetter - chainConfig *params.ChainConfig - shutdownChan <-chan struct{} - - wg *sync.WaitGroup -} - -type gasPriceSetter interface { - SetGasPrice(price *big.Int) - SetMinFee(price *big.Int) -} - -// handleGasPriceUpdates creates and runs an instance of -func (vm *VM) handleGasPriceUpdates() { - gpu := &gasPriceUpdater{ - setter: vm.txPool, - chainConfig: vm.chainConfig, - shutdownChan: vm.shutdownChan, - wg: &vm.shutdownWg, - } - - gpu.start() -} - -// start handles the appropriate gas price and minimum fee updates required by [gpu.chainConfig] -func (gpu *gasPriceUpdater) start() { - // Updates to the minimum gas price as of Subnet EVM if it's already in effect or starts a goroutine to enable it at the correct time - if disabled := gpu.handleUpdate(gpu.setter.SetGasPrice, gpu.chainConfig.SubnetEVMTimestamp, big.NewInt(0)); disabled { - return - } - minBaseFee := gpu.chainConfig.FeeConfig.MinBaseFee - // Updates to the minimum gas price as of Subnet EVM if it's already in effect or starts a goroutine to enable it at the correct time - gpu.handleUpdate(gpu.setter.SetMinFee, gpu.chainConfig.SubnetEVMTimestamp, minBaseFee) -} - -// handleUpdate handles calling update(price) at the appropriate time based on -// the value of [timestamp]. -// 1) If [timestamp] is nil, update is never called -// 2) If [timestamp] has already passed, update is called immediately -// 3) [timestamp] is some time in the future, starts a goroutine that will call update(price) at the time -// given by [timestamp]. -func (gpu *gasPriceUpdater) handleUpdate(update func(price *big.Int), timestamp *big.Int, price *big.Int) bool { - if timestamp == nil { - return true - } - - currentTime := time.Now() - upgradeTime := time.Unix(timestamp.Int64(), 0) - if currentTime.After(upgradeTime) { - update(price) - } else { - gpu.wg.Add(1) - go gpu.updatePrice(update, time.Until(upgradeTime), price) - } - return false -} - -// updatePrice calls update(updatedPrice) after waiting for [duration] or shuts down early -// if the [shutdownChan] is closed. -func (gpu *gasPriceUpdater) updatePrice(update func(price *big.Int), duration time.Duration, updatedPrice *big.Int) { - defer gpu.wg.Done() - select { - case <-time.After(duration): - update(updatedPrice) - case <-gpu.shutdownChan: - } -} diff --git a/plugin/evm/gasprice_update_test.go b/plugin/evm/gasprice_update_test.go deleted file mode 100644 index d590de8809..0000000000 --- a/plugin/evm/gasprice_update_test.go +++ /dev/null @@ -1,129 +0,0 @@ -// (c) 2019-2020, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package evm - -import ( - "math/big" - "sync" - "testing" - "time" - - "github.com/ava-labs/subnet-evm/params" -) - -type mockGasPriceSetter struct { - lock sync.Mutex - price, minFee *big.Int -} - -func (m *mockGasPriceSetter) SetGasPrice(price *big.Int) { - m.lock.Lock() - defer m.lock.Unlock() - - m.price = price -} - -func (m *mockGasPriceSetter) SetMinFee(minFee *big.Int) { - m.lock.Lock() - defer m.lock.Unlock() - - m.minFee = minFee -} - -func (m *mockGasPriceSetter) GetStatus() (*big.Int, *big.Int) { - m.lock.Lock() - defer m.lock.Unlock() - - return m.price, m.minFee -} - -func attemptAwait(t *testing.T, wg *sync.WaitGroup, delay time.Duration) { - ticker := make(chan struct{}) - - // Wait for [wg] and then close [ticket] to indicate that - // the wait group has finished. - go func() { - wg.Wait() - close(ticker) - }() - - select { - case <-time.After(delay): - t.Fatal("Timed out waiting for wait group to complete") - case <-ticker: - // The wait group completed without issue - } -} - -func TestUpdateGasPriceShutsDown(t *testing.T) { - shutdownChan := make(chan struct{}) - wg := &sync.WaitGroup{} - config := *params.TestChainConfig - // Set SubnetEVMBlockTime one hour in the future so that it will - // create a goroutine waiting for an hour before updating the gas price - config.SubnetEVMTimestamp = big.NewInt(time.Now().Add(time.Hour).Unix()) - - gpu := &gasPriceUpdater{ - setter: &mockGasPriceSetter{price: big.NewInt(1)}, - chainConfig: &config, - shutdownChan: shutdownChan, - wg: wg, - } - - gpu.start() - // Close [shutdownChan] and ensure that the wait group finishes in a reasonable - // amount of time. - close(shutdownChan) - attemptAwait(t, wg, 5*time.Second) -} - -func TestUpdateGasPriceInitializesPrice(t *testing.T) { - shutdownChan := make(chan struct{}) - wg := &sync.WaitGroup{} - gpu := &gasPriceUpdater{ - setter: &mockGasPriceSetter{price: big.NewInt(1)}, - chainConfig: params.TestChainConfig, - shutdownChan: shutdownChan, - wg: wg, - } - - gpu.start() - // The wait group should finish immediately since no goroutine - // should be created when all prices should be set from the start - attemptAwait(t, wg, time.Millisecond) - - if gpu.setter.(*mockGasPriceSetter).price.Cmp(big.NewInt(0)) != 0 { - t.Fatalf("Expected price to match minimum base fee for subnet-evm") - } - - if minFee := gpu.setter.(*mockGasPriceSetter).minFee; minFee == nil || minFee.Cmp(params.DefaultFeeConfig.MinBaseFee) != 0 { - t.Fatalf("Expected min fee to match minimum fee for subnet-evm, but found: %d", minFee) - } -} - -func TestUpdateGasPriceUpdatesPrice(t *testing.T) { - shutdownChan := make(chan struct{}) - wg := &sync.WaitGroup{} - config := *params.TestChainConfig - config.SubnetEVMTimestamp = big.NewInt(time.Now().Add(1 * time.Second).Unix()) - - gpu := &gasPriceUpdater{ - setter: &mockGasPriceSetter{price: big.NewInt(1)}, - chainConfig: &config, - shutdownChan: shutdownChan, - wg: wg, - } - - gpu.start() - - // Confirm Subnet EVM settings are applied at the very end. - attemptAwait(t, wg, 5*time.Second) - price, minFee := gpu.setter.(*mockGasPriceSetter).GetStatus() - if price.Cmp(big.NewInt(0)) != 0 { - t.Fatalf("Expected price to match minimum base fee for subnet-evm") - } - if minFee == nil || minFee.Cmp(params.DefaultFeeConfig.MinBaseFee) != 0 { - t.Fatalf("Expected min fee to match minimum fee for subnet-evm, but found: %d", minFee) - } -} diff --git a/plugin/evm/vm.go b/plugin/evm/vm.go index 92a9f05dfd..e21b051741 100644 --- a/plugin/evm/vm.go +++ b/plugin/evm/vm.go @@ -218,17 +218,6 @@ type VM struct { warpBackend warp.WarpBackend } -/* - ****************************************************************************** - ********************************* Snowman API ******************************** - ****************************************************************************** - */ - -// implements SnowmanPlusPlusVM interface -func (vm *VM) GetActivationTime() time.Time { - return time.Unix(vm.chainConfig.SubnetEVMTimestamp.Int64(), 0) -} - // Initialize implements the snowman.ChainVM interface func (vm *VM) Initialize( _ context.Context, @@ -390,6 +379,7 @@ func (vm *VM) Initialize( vm.chainConfig = g.Config vm.networkID = vm.ethConfig.NetworkId + // TODO: remove SkipSubnetEVMUpgradeCheck after next network upgrade if !vm.config.SkipSubnetEVMUpgradeCheck { // check that subnetEVM upgrade is enabled from genesis before upgradeBytes if !vm.chainConfig.IsSubnetEVM(common.Big0) { @@ -482,9 +472,6 @@ func (vm *VM) initializeChain(lastAcceptedHash common.Hash, ethConfig ethconfig. vm.blockChain = vm.eth.BlockChain() vm.miner = vm.eth.Miner() - // start goroutines to update the tx pool gas minimum gas price when upgrades go into effect - vm.handleGasPriceUpdates() - vm.eth.Start() return vm.initChainState(vm.blockChain.LastAcceptedBlock()) } From 19d0cd5c2cb8d4beb22466346c81c4fbca0fe6d2 Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Thu, 16 Feb 2023 10:43:10 -0800 Subject: [PATCH 2/5] Remove comment referencing gas price updater --- core/tx_pool.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/tx_pool.go b/core/tx_pool.go index 25a5b0d52c..57b818edae 100644 --- a/core/tx_pool.go +++ b/core/tx_pool.go @@ -1443,10 +1443,6 @@ func (pool *TxPool) reset(oldHead, newHead *types.Header) { // when we reset txPool we should explicitly check if fee struct for min base fee has changed // so that we can correctly drop txs with < minBaseFee from tx pool. - // TODO: this should be checking IsSubnetEVM since we also support minimumFee for SubnetEVM - // without requiring FeeConfigManager is enabled. - // This is already being set by SetMinFee when gas price updater starts. - // However tests are currently failing if we change this check to IsSubnetEVM. if pool.chainconfig.IsFeeConfigManager(new(big.Int).SetUint64(newHead.Time)) { feeConfig, _, err := pool.chain.GetFeeConfigAt(newHead) if err != nil { From 55f47dfee122cde41156d16ea3d0efd0454303cb Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Thu, 16 Feb 2023 10:57:08 -0800 Subject: [PATCH 3/5] Revert default price limit change --- core/tx_pool.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/tx_pool.go b/core/tx_pool.go index 57b818edae..8377526a93 100644 --- a/core/tx_pool.go +++ b/core/tx_pool.go @@ -190,7 +190,7 @@ var DefaultTxPoolConfig = TxPoolConfig{ Journal: "transactions.rlp", Rejournal: time.Hour, - PriceLimit: 0, + PriceLimit: 1, PriceBump: 10, AccountSlots: 16, From 4226c75ab15185cb9642ce34393b8b8ff13444e9 Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Thu, 16 Feb 2023 10:58:12 -0800 Subject: [PATCH 4/5] Move set min fee back to vm.go --- core/tx_pool.go | 1 - plugin/evm/vm.go | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/core/tx_pool.go b/core/tx_pool.go index 8377526a93..8ea11e9e55 100644 --- a/core/tx_pool.go +++ b/core/tx_pool.go @@ -328,7 +328,6 @@ func NewTxPool(config TxPoolConfig, chainconfig *params.ChainConfig, chain block initDoneCh: make(chan struct{}), generalShutdownChan: make(chan struct{}), gasPrice: new(big.Int).SetUint64(config.PriceLimit), - minimumFee: new(big.Int).Add(common.Big0, chainconfig.FeeConfig.MinBaseFee), } pool.locals = newAccountSet(pool.signer) diff --git a/plugin/evm/vm.go b/plugin/evm/vm.go index e21b051741..e38d87d342 100644 --- a/plugin/evm/vm.go +++ b/plugin/evm/vm.go @@ -469,6 +469,7 @@ func (vm *VM) initializeChain(lastAcceptedHash common.Hash, ethConfig ethconfig. } vm.eth.SetEtherbase(ethConfig.Miner.Etherbase) vm.txPool = vm.eth.TxPool() + vm.txPool.SetGasPrice(vm.chainConfig.FeeConfig.MinBaseFee) vm.blockChain = vm.eth.BlockChain() vm.miner = vm.eth.Miner() From 2891958c0598f2872d0587a7fd60e959633d5042 Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Thu, 16 Feb 2023 11:03:41 -0800 Subject: [PATCH 5/5] Update to retain previous tx pool gas price setting behavior --- core/tx_pool.go | 1 - plugin/evm/vm.go | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/core/tx_pool.go b/core/tx_pool.go index 8ea11e9e55..ed5502d94b 100644 --- a/core/tx_pool.go +++ b/core/tx_pool.go @@ -329,7 +329,6 @@ func NewTxPool(config TxPoolConfig, chainconfig *params.ChainConfig, chain block generalShutdownChan: make(chan struct{}), gasPrice: new(big.Int).SetUint64(config.PriceLimit), } - pool.locals = newAccountSet(pool.signer) for _, addr := range config.Locals { log.Info("Setting new local account", "address", addr) diff --git a/plugin/evm/vm.go b/plugin/evm/vm.go index e38d87d342..ce6f80b3d8 100644 --- a/plugin/evm/vm.go +++ b/plugin/evm/vm.go @@ -8,6 +8,7 @@ import ( "encoding/json" "errors" "fmt" + "math/big" "os" "path/filepath" "strings" @@ -469,7 +470,8 @@ func (vm *VM) initializeChain(lastAcceptedHash common.Hash, ethConfig ethconfig. } vm.eth.SetEtherbase(ethConfig.Miner.Etherbase) vm.txPool = vm.eth.TxPool() - vm.txPool.SetGasPrice(vm.chainConfig.FeeConfig.MinBaseFee) + vm.txPool.SetMinFee(vm.chainConfig.FeeConfig.MinBaseFee) + vm.txPool.SetGasPrice(big.NewInt(0)) vm.blockChain = vm.eth.BlockChain() vm.miner = vm.eth.Miner()