Skip to content

Commit

Permalink
PIP-35 for amoy: enforce gas related configs (#1285)
Browse files Browse the repository at this point in the history
* enforce gas related configs for amoy

* internal/cli/server: revert changes to default config.toml for test

* eth: check nil genesis for tests
  • Loading branch information
manav2401 authored Jul 11, 2024
1 parent 32c9405 commit 1a05223
Show file tree
Hide file tree
Showing 18 changed files with 100 additions and 54 deletions.
6 changes: 3 additions & 3 deletions builder/files/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ syncmode = "full"

[txpool]
nolocals = true
pricelimit = 30000000000
pricelimit = 30000000000 # 25000000000 for amoy
accountslots = 16
globalslots = 32768
accountqueue = 16
Expand All @@ -69,7 +69,7 @@ syncmode = "full"

[miner]
gaslimit = 30000000
gasprice = "30000000000"
gasprice = "30000000000" # 25000000000 for amoy
# mine = true
# etherbase = "VALIDATOR ADDRESS"
# extradata = ""
Expand Down Expand Up @@ -127,7 +127,7 @@ syncmode = "full"
# maxheaderhistory = 1024
# maxblockhistory = 1024
# maxprice = "5000000000000"
ignoreprice = "30000000000"
ignoreprice = "30000000000" # 25000000000 for amoy

[telemetry]
metrics = true
Expand Down
23 changes: 16 additions & 7 deletions core/txpool/legacypool/legacypool.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,9 @@ var DefaultConfig = Config{
Journal: "transactions.rlp",
Rejournal: time.Hour,

PriceLimit: params.BorDefaultTxPoolPriceLimit,
// (PIP-35): Update price limit to `params.BorDefaultTxPoolPriceLimit` once
// the change is applied over all networks.
PriceLimit: 1,
PriceBump: 10,

AccountSlots: 16,
Expand All @@ -161,16 +163,23 @@ var DefaultConfig = Config{

// sanitize checks the provided user configurations and changes anything that's
// unreasonable or unworkable.
func (config *Config) sanitize() Config {
func (config *Config) sanitize(chainId *big.Int) Config {
conf := *config
if conf.Rejournal < time.Second {
log.Warn("Sanitizing invalid txpool journal time", "provided", conf.Rejournal, "updated", time.Second)
conf.Rejournal = time.Second
}
// enforce txpool price limit to 30gwei in bor
if conf.PriceLimit != params.BorDefaultTxPoolPriceLimit {
log.Warn("Sanitizing invalid txpool price limit", "provided", conf.PriceLimit, "updated", DefaultConfig.PriceLimit)
conf.PriceLimit = DefaultConfig.PriceLimit
// (PIP-35): Only enforce the 25gwei txpool price limit for amoy
if chainId.Cmp(params.AmoyChainConfig.ChainID) == 0 {
if conf.PriceLimit != params.BorDefaultTxPoolPriceLimit {
log.Warn("Sanitizing invalid txpool price limit", "provided", conf.PriceLimit, "updated", params.BorDefaultTxPoolPriceLimit)
conf.PriceLimit = params.BorDefaultTxPoolPriceLimit
}
} else {
if conf.PriceLimit < 1 {
log.Warn("Sanitizing invalid txpool price limit", "provided", conf.PriceLimit, "updated", DefaultConfig.PriceLimit)
conf.PriceLimit = DefaultConfig.PriceLimit
}
}
if conf.PriceBump < 1 {
log.Warn("Sanitizing invalid txpool price bump", "provided", conf.PriceBump, "updated", DefaultConfig.PriceBump)
Expand Down Expand Up @@ -250,7 +259,7 @@ type txpoolResetRequest struct {
// transactions from the network.
func New(config Config, chain BlockChain, options ...func(pool *LegacyPool)) *LegacyPool {
// Sanitize the input to ensure no vulnerable gas prices are set
config = (&config).sanitize()
config = (&config).sanitize(chain.Config().ChainID)

// Create the transaction pool with its initial settings
pool := &LegacyPool{
Expand Down
5 changes: 4 additions & 1 deletion core/txpool/legacypool/legacypool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func init() {
testTxPoolConfig = DefaultConfig
testTxPoolConfig.Journal = ""
/*
Given the introduction of `BorDefaultTxPoolPriceLimit=30gwei`,
Given the introduction of `BorDefaultTxPoolPriceLimit=25gwei`,
we set `testTxPoolConfig.PriceLimit = 1` to avoid rewriting all `legacypool_test.go` tests,
causing code divergence from geth, as this has been widely tested on different networks.
Also, `worker_test.go` has been adapted to reflect such changes.
Expand Down Expand Up @@ -296,6 +296,9 @@ func (c *testChain) State() (*state.StateDB, error) {

// TestTxPoolDefaultPriceLimit ensures the bor default tx pool price limit is set correctly.
func TestTxPoolDefaultPriceLimit(t *testing.T) {
// (PIP-35): Only applicable to amoy
t.Skip("Skipped because the price enforcement is only applied to amoy")

t.Parallel()

pool, _ := setupPool()
Expand Down
6 changes: 3 additions & 3 deletions docs/cli/example_config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ devfakeauthor = false # Run miner without validator set authorization
nolocals = false # Disables price exemptions for locally submitted transactions
journal = "transactions.rlp" # Disk journal for local transaction to survive node restarts
rejournal = "1h0m0s" # Time interval to regenerate the local transaction journal
pricelimit = 30000000000 # Minimum gas price limit to enforce for acceptance into the pool. Regardless the value set, it will be enforced to 30000000000 in bor.
pricelimit = 30000000000 # Minimum gas price limit to enforce for acceptance into the pool. Recommended for mainnet = 30000000000 (not enforced). For Amoy network, regardless the value set, it will be enforced to 25000000000 in bor.
pricebump = 10 # Price bump percentage to replace an already existing transaction
accountslots = 16 # Minimum number of executable transaction slots guaranteed per account
globalslots = 32768 # Maximum number of executable transaction slots for all accounts
Expand All @@ -74,7 +74,7 @@ devfakeauthor = false # Run miner without validator set authorization
etherbase = "" # Public address for block mining rewards
extradata = "" # Block extra data set by the miner (default = client version)
gaslimit = 30000000 # Target gas ceiling for mined blocks
gasprice = "30000000000" # Minimum gas price for mining a transaction. Regardless the value set, it will be enforced to 30000000000 in bor, default suitable for amoy/mumbai/devnet.
gasprice = "30000000000" # Minimum gas price for mining a transaction. Recommended for mainnet = 30000000000 (not enforced). For Amoy network, regardless the value set, it will be enforced to 25000000000 in bor.
recommit = "2m5s" # The time interval for miner to re-create mining work
commitinterrupt = true # Interrupt the current mining work when time is exceeded and create partial blocks

Expand Down Expand Up @@ -128,7 +128,7 @@ devfakeauthor = false # Run miner without validator set authorization
maxheaderhistory = 1024 # Maximum header history of gasprice oracle
maxblockhistory = 1024 # Maximum block history of gasprice oracle
maxprice = "5000000000000" # Maximum gas price will be recommended by gpo
ignoreprice = "2" # Gas price below which gpo will ignore transactions (recommended for mainnet = 30000000000, default suitable for amoy/mumbai/devnet)
ignoreprice = "2" # Gas price below which gpo will ignore transactions. Recommended for mainnet = 30000000000 (not enforced). For Amoy network, regardless the value set, it will be enforced to 25000000000 in bor.

[telemetry]
metrics = false # Enable metrics collection and reporting
Expand Down
6 changes: 3 additions & 3 deletions docs/cli/server.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ The ```bor server``` command runs the Bor client.

- ```gpo.blocks```: Number of recent blocks to check for gas prices (default: 20)

- ```gpo.ignoreprice```: Gas price below which gpo will ignore transactions (default: 30000000000). It's set to 30gwei in bor
- ```gpo.ignoreprice```: Gas price below which gpo will ignore transactions (default: 2)

- ```gpo.maxblockhistory```: Maximum block history of gasprice oracle (default: 1024)

Expand Down Expand Up @@ -250,7 +250,7 @@ The ```bor server``` command runs the Bor client.

- ```miner.gaslimit```: Target gas ceiling (gas limit) for mined blocks (default: 30000000)

- ```miner.gasprice```: Minimum gas price for mining a transaction (default: 30000000000). It's set to 30gwei in bor
- ```miner.gasprice```: Minimum gas price for mining a transaction (default: 1000000000)

- ```miner.interruptcommit```: Interrupt block commit when block creation time is passed (default: true)

Expand Down Expand Up @@ -306,6 +306,6 @@ The ```bor server``` command runs the Bor client.

- ```txpool.pricebump```: Price bump percentage to replace an already existing transaction (default: 10)

- ```txpool.pricelimit```: Minimum gas price limit to enforce the acceptance of txs into the pool (default: 30000000000). It's set to 30gwei in bor
- ```txpool.pricelimit```: Minimum gas price limit to enforce for acceptance into the pool (default: 1)

- ```txpool.rejournal```: Time interval to regenerate the local transaction journal (default: 1h0m0s)
27 changes: 23 additions & 4 deletions eth/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,17 @@ func New(stack *node.Node, config *ethconfig.Config) (*Ethereum, error) {
if !config.SyncMode.IsValid() {
return nil, fmt.Errorf("invalid sync mode %d", config.SyncMode)
}
// enforce minimum gas price of 30 gwei in bor
if config.Miner.GasPrice == nil || config.Miner.GasPrice.Cmp(big.NewInt(params.BorDefaultMinerGasPrice)) != 0 {
log.Warn("Sanitizing invalid miner gas price", "provided", config.Miner.GasPrice, "updated", ethconfig.Defaults.Miner.GasPrice)
config.Miner.GasPrice = new(big.Int).Set(ethconfig.Defaults.Miner.GasPrice)
// (PIP-35): Only enforce the minimum gas price of 25 gwei for amoy
if config.Genesis != nil && config.Genesis.Config.ChainID.Cmp(params.AmoyChainConfig.ChainID) == 0 {
if config.Miner.GasPrice == nil || config.Miner.GasPrice.Cmp(big.NewInt(params.BorDefaultMinerGasPrice)) != 0 {
log.Warn("Sanitizing invalid miner gas price", "provided", config.Miner.GasPrice, "updated", params.BorDefaultMinerGasPrice)
config.Miner.GasPrice = new(big.Int).SetUint64(params.BorDefaultMinerGasPrice)
}
} else {
if config.Miner.GasPrice == nil || config.Miner.GasPrice.Cmp(common.Big0) <= 0 {
log.Warn("Sanitizing invalid miner gas price", "provided", config.Miner.GasPrice, "updated", ethconfig.Defaults.Miner.GasPrice)
config.Miner.GasPrice = new(big.Int).Set(ethconfig.Defaults.Miner.GasPrice)
}
}
if config.NoPruning && config.TrieDirtyCache > 0 {
if config.SnapshotCache > 0 {
Expand Down Expand Up @@ -251,6 +258,10 @@ func New(stack *node.Node, config *ethconfig.Config) (*Ethereum, error) {
eth.blockchain, err = core.NewBlockChain(chainDb, cacheConfig, config.Genesis, &overrides, eth.engine, vmConfig, eth.shouldPreserve, &config.TxLookupLimit, checker)
}

// (PIP-35): Update the default ignore price for amoy
if chainConfig.ChainID.Cmp(params.AmoyChainConfig.ChainID) == 0 {
gasprice.DefaultIgnorePriceAmoy = new(big.Int).SetUint64(params.BorDefaultGpoIgnorePrice)
}
eth.APIBackend.gpo = gasprice.NewOracle(eth.APIBackend, gpoParams)
if err != nil {
return nil, err
Expand All @@ -277,6 +288,14 @@ func New(stack *node.Node, config *ethconfig.Config) (*Ethereum, error) {
if err != nil {
return nil, err
}

// The `config.TxPool.PriceLimit` used above doesn't reflect the sanitized/enforced changes
// made in the txpool. Update the `gasTip` explicitly to reflect the enforced value.
// (PIP-35): This change is only applied for Amoy network. Remove the checks once it's applied for all networks.
if chainConfig.ChainID.Cmp(params.AmoyChainConfig.ChainID) == 0 {
eth.txPool.SetGasTip(new(big.Int).SetUint64(params.BorDefaultTxPoolPriceLimit))
}

// Permit the downloader to use the trie cache allowance during fast sync
cacheLimit := cacheConfig.TrieCleanLimit + cacheConfig.TrieDirtyLimit + cacheConfig.SnapshotLimit
if eth.handler, err = newHandler(&handlerConfig{
Expand Down
4 changes: 2 additions & 2 deletions eth/catalyst/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func generateMergeChain(n int, merged bool) (*core.Genesis, []*types.Block) {
generate := func(i int, g *core.BlockGen) {
g.OffsetTime(5)
g.SetExtra([]byte("test"))
tx, _ := types.SignTx(types.NewTransaction(testNonce, common.HexToAddress("0x9a9070028361F7AAbeB3f2F2Dc07F82C4a98A02a"), big.NewInt(1), params.TxGas, big.NewInt(params.InitialBaseFee*32), nil), types.LatestSigner(&config), testKey)
tx, _ := types.SignTx(types.NewTransaction(testNonce, common.HexToAddress("0x9a9070028361F7AAbeB3f2F2Dc07F82C4a98A02a"), big.NewInt(1), params.TxGas, big.NewInt(params.InitialBaseFee*2), nil), types.LatestSigner(&config), testKey)
g.AddTx(tx)
testNonce++
}
Expand Down Expand Up @@ -604,7 +604,7 @@ func TestNewPayloadOnInvalidChain(t *testing.T) {
Nonce: statedb.GetNonce(testAddr),
Value: new(big.Int),
Gas: 1000000,
GasPrice: big.NewInt(32 * params.InitialBaseFee),
GasPrice: big.NewInt(2 * params.InitialBaseFee),
Data: logCode,
})
ethservice.TxPool().Add([]*types.Transaction{tx}, false, true)
Expand Down
23 changes: 16 additions & 7 deletions eth/gasprice/gasprice.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@ import (
const sampleNumber = 3 // Number of transactions sampled in a block

var (
DefaultMaxPrice = big.NewInt(500 * params.GWei)
DefaultIgnorePrice = big.NewInt(params.BorDefaultGpoIgnorePrice)
DefaultMaxPrice = big.NewInt(500 * params.GWei)
DefaultIgnorePrice = big.NewInt(2 * params.Wei)
DefaultIgnorePriceAmoy *big.Int // (PIP-35): Default ignore price for amoy (to be removed later)
)

type Config struct {
Expand Down Expand Up @@ -100,13 +101,21 @@ func NewOracle(backend OracleBackend, params Config) *Oracle {
log.Warn("Sanitizing invalid gasprice oracle price cap", "provided", params.MaxPrice, "updated", maxPrice)
}

// (PIP-35): Enforce the ignore price for amoy. The default value for amoy (which is
// an indicator of being on amoy) should be set by the caller.
ignorePrice := params.IgnorePrice
if ignorePrice == nil || ignorePrice.Int64() != DefaultIgnorePrice.Int64() {
ignorePrice = DefaultIgnorePrice
log.Warn("Sanitizing invalid gasprice oracle ignore price", "provided", params.IgnorePrice, "updated", ignorePrice)
} else if ignorePrice.Int64() > 0 {
log.Info("Gasprice oracle is ignoring threshold set", "threshold", ignorePrice)
if DefaultIgnorePriceAmoy != nil {
if ignorePrice == nil || ignorePrice.Int64() != DefaultIgnorePriceAmoy.Int64() {
ignorePrice = DefaultIgnorePriceAmoy
log.Warn("Sanitizing invalid gasprice oracle ignore price", "provided", params.IgnorePrice, "updated", ignorePrice)
}
} else {
if ignorePrice == nil || ignorePrice.Int64() <= 0 {
ignorePrice = DefaultIgnorePrice
log.Warn("Sanitizing invalid gasprice oracle ignore price", "provided", params.IgnorePrice, "updated", ignorePrice)
}
}
log.Info("Gasprice oracle is ignoring threshold set", "threshold", ignorePrice)

maxHeaderHistory := params.MaxHeaderHistory
if maxHeaderHistory < 1 {
Expand Down
6 changes: 3 additions & 3 deletions internal/cli/server/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,7 @@ func DefaultConfig() *Config {
NoLocals: false,
Journal: "transactions.rlp",
Rejournal: 1 * time.Hour,
PriceLimit: params.BorDefaultTxPoolPriceLimit, // bor's default
PriceLimit: 1, // Default for every network except Amoy
PriceBump: 10,
AccountSlots: 16,
GlobalSlots: 32768,
Expand All @@ -666,8 +666,8 @@ func DefaultConfig() *Config {
Sealer: &SealerConfig{
Enabled: false,
Etherbase: "",
GasCeil: 30_000_000, // geth's default
GasPrice: big.NewInt(params.BorDefaultMinerGasPrice), // bor's default
GasCeil: 30_000_000, // geth's default
GasPrice: big.NewInt(1 * params.GWei), // Default for every network except Amoy
ExtraData: "",
Recommit: 125 * time.Second,
CommitInterruptFlag: true,
Expand Down
3 changes: 3 additions & 0 deletions internal/cli/server/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ func TestConfigDefault(t *testing.T) {

// assertBorDefaultGasPrice asserts the bor default gas price is set correctly.
func assertBorDefaultGasPrice(t *testing.T, ethConfig *ethconfig.Config) {
// (PIP-35): Only applicable to amoy
t.Skip("Skipped because the price enforcement is only applied to amoy")

assert.NotNil(t, ethConfig)
assert.Equal(t, ethConfig.Miner.GasPrice, big.NewInt(params.BorDefaultMinerGasPrice))
}
Expand Down
6 changes: 3 additions & 3 deletions internal/cli/server/testdata/default.toml
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ devfakeauthor = false
nolocals = false
journal = "transactions.rlp"
rejournal = "1h0m0s"
pricelimit = 30000000000
pricelimit = 1
pricebump = 10
accountslots = 16
globalslots = 32768
Expand All @@ -71,7 +71,7 @@ devfakeauthor = false
etherbase = ""
extradata = ""
gaslimit = 30000000
gasprice = "30000000000"
gasprice = "1000000000"
recommit = "2m5s"
commitinterrupt = true

Expand Down Expand Up @@ -127,7 +127,7 @@ devfakeauthor = false
maxheaderhistory = 1024
maxblockhistory = 1024
maxprice = "500000000000"
ignoreprice = "30000000000"
ignoreprice = "2"

[telemetry]
metrics = false
Expand Down
7 changes: 5 additions & 2 deletions miner/miner.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,11 @@ type Config struct {

// DefaultConfig contains default settings for miner.
var DefaultConfig = Config{
GasCeil: 30000000,
GasPrice: big.NewInt(params.BorDefaultMinerGasPrice), // enforces minimum gas price of 30 gwei in bor
GasCeil: 30000000,

// (PIP-35): Update the gas price to `params.BorDefaultMinerGasPrice` once
// the change is applied over all networks.
GasPrice: big.NewInt(params.GWei),

// The default recommit time is chosen as two seconds since
// consensus-layer usually will wait a half slot of time(6s)
Expand Down
2 changes: 1 addition & 1 deletion miner/worker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ func (b *testWorkerBackend) newRandomTxWithNonce(creation bool, nonce uint64) *t
func (b *testWorkerBackend) newStorageCreateContractTx() (*types.Transaction, common.Address) {
var tx *types.Transaction

gasPrice := big.NewInt(30 * params.InitialBaseFee)
gasPrice := big.NewInt(10 * params.InitialBaseFee)

tx, _ = types.SignTx(types.NewContractCreation(b.txPool.Nonce(TestBankAddress), big.NewInt(0), testGas, gasPrice, common.FromHex(storageContractByteCode)), types.HomesteadSigner{}, testBankKey)
contractAddr := crypto.CreateAddress(TestBankAddress, b.txPool.Nonce(TestBankAddress))
Expand Down
6 changes: 3 additions & 3 deletions packaging/templates/testnet-amoy/archive/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ gcmode = "archive"
# locals = []
# journal = ""
# rejournal = "1h0m0s"
# pricelimit = 30000000000
# pricelimit = 25000000000
# pricebump = 10

[miner]
gaslimit = 30000000
# gasprice = "30000000000"
# gasprice = "25000000000"
# mine = false
# etherbase = ""
# extradata = ""
Expand Down Expand Up @@ -119,7 +119,7 @@ gcmode = "archive"
# maxheaderhistory = 1024
# maxblockhistory = 1024
# maxprice = "5000000000000"
# ignoreprice = "30000000000"
# ignoreprice = "25000000000"

[telemetry]
metrics = true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ syncmode = "full"
# locals = []
# journal = ""
# rejournal = "1h0m0s"
# pricelimit = 30000000000
# pricelimit = 25000000000
# pricebump = 10

[miner]
gaslimit = 30000000
# gasprice = "30000000000"
# gasprice = "25000000000"
# mine = false
# etherbase = ""
# extradata = ""
Expand Down Expand Up @@ -119,7 +119,7 @@ syncmode = "full"
# maxheaderhistory = 1024
# maxblockhistory = 1024
# maxprice = "5000000000000"
# ignoreprice = "30000000000"
# ignoreprice = "25000000000"

[telemetry]
metrics = true
Expand Down
Loading

0 comments on commit 1a05223

Please sign in to comment.