diff --git a/core/vm/eips.go b/core/vm/eips.go index 4a2988a805..005e4c4313 100644 --- a/core/vm/eips.go +++ b/core/vm/eips.go @@ -90,6 +90,15 @@ func opChainID(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memo return nil, nil } +// enable2200Sloppy applies EIP-2200 (Rebalance net-metered SSTORE) +// WITHOUT IMPLEMENTING THE GAS REPRICING FOR SLOAD OPCODE. +func enable2200Sloppy(jt *JumpTable) { + // This value is wrong on purpose; it makes the "sloppiness" explicit. + jt[SLOAD].constantGas = vars.SloadGasEIP150 // 200 + + jt[SSTORE].dynamicGas = gasSStoreEIP2200 +} + // enable2200 applies EIP-2200 (Rebalance net-metered SSTORE) func enable2200(jt *JumpTable) { jt[SLOAD].constantGas = vars.SloadGasEIP2200 diff --git a/core/vm/jump_table.go b/core/vm/jump_table.go index 24d285f3f5..42edd98452 100644 --- a/core/vm/jump_table.go +++ b/core/vm/jump_table.go @@ -189,7 +189,17 @@ func instructionSetForConfig(config ctypes.ChainConfigurator, bn *big.Int) JumpT if config.IsEnabled(config.GetECIP1080Transition, bn) { enableSelfBalance(&instructionSet) } - if config.IsEnabled(config.GetEIP2200Transition, bn) && !config.IsEnabled(config.GetEIP2200DisableTransition, bn) { + + // EIP2200 was originally implemented incorrectly (not meeting specifications) by ethereum/go-ethereum, multi-geth, and Parity + // clients. + // ECIP1086 is a specification to allow this "bad" implementation, which is useful for ETC testnets Kotti and Mordor. + // + is2200enabled := config.IsEnabled(config.GetEIP2200Transition, bn) && !config.IsEnabled(config.GetEIP2200DisableTransition, bn) + if is2200enabled && + config.IsEnabled(config.GetECIP1086Transition, bn) && + !config.IsEnabled(config.GetEIP1884Transition, bn) { + enable2200Sloppy(&instructionSet) + } else if is2200enabled { enable2200(&instructionSet) // Net metered SSTORE - https://eips.ethereum.org/EIPS/eip-2200 } return instructionSet diff --git a/core/vm/jump_table_test.go b/core/vm/jump_table_test.go new file mode 100644 index 0000000000..d46c9dfb91 --- /dev/null +++ b/core/vm/jump_table_test.go @@ -0,0 +1,87 @@ +package vm + +import ( + "math/big" + "testing" + + "github.com/ethereum/go-ethereum/params" + "github.com/ethereum/go-ethereum/params/confp" + "github.com/ethereum/go-ethereum/params/vars" +) + +// TestECIP1086 shows that SLOAD constant gas is never bumped to 800 on Kotti the testnet config. +// Despite EIP2200 being installed and it's specification calling for 800 gas, this was incorrectly +// implemented in several clients, and thus, via ECIP1086, made into a "canonical mistake," which is tested +// here. +// This tests shows that although EIP2200 is implemented for a window (sloppyWindow), the gas cost for SLOAD does not change. +func TestECIP1086_Kotti(t *testing.T) { + kc := params.KottiChainConfig + + sloppyWindowStart := uint64(2_058_191) + sloppyWindowEnd := uint64(2_208_203) + + // Since "sloppy" EIP2200 did not actually modify SLOAD gas (and ECIP1086 makes this canonical), + // we want to show that SLOAD=200 gas is always used on Kotti whether EIP2200 is enabled or, afterwards, disabled. + wantSloadGasAlways := vars.SloadGasEIP150 + + if wantSloadGasAlways != vars.NetSstoreDirtyGas { + // https://corepaper.org/ethereum/fork/istanbul/#aztlan-fix + /* + Design Failure + + Aztlan decided to include EIP-2200 for SSTORE net gas metering. + However, it did not thoroughly consider the facts that EIP-2200 is designed with the full context + of Istanbul hard fork, whose pre-conditions that Aztlan does not fulfill. + + Net gas metering has a family of EIP specifications, including EIP-1283, EIP-1706 and EIP-2200. + The specifications contain a parameter of "dirty gas", which is charged when a storage value has + already been modified in the same transaction context. This value, as in the design intention, + is expected to always equal to the value of SLOAD gas cost. + In EIP-2200, this "dirty gas" is set to be 800, so as to accommodate EIP-1884’s gas cost change of SLOAD from 200 to 800. + However, Aztlan does not include EIP-1884 but only applied EIP-2200, + resulting in inconsistency in the EIP design intention, and may lead to unknown gas cost issues. + + This has also led to confusions in implementations. For example, a contributor previously merged an + incorrect version of Aztlan hard fork into Parity Ethereum. + This, if left unspotted, will lead to consensus split on the whole Ethereum Classic network. + + Recommendation: Remove EIP-2200 from the applied specification list, and add EIP-1283 with EIP-1706 instead. + */ + t.Fatal("'dirty gas' parameter should be same as SLOAD cost") + } + + for _, f := range confp.Forks(kc) { + for _, ft := range []uint64{f - 1, f, f + 1} { + + jt := instructionSetForConfig(kc, new(big.Int).SetUint64(ft)) + + // Yes, this logic could be simplified, but I want to show the window/post-window no-op verbosely. + if ft >= sloppyWindowStart && ft < sloppyWindowEnd { + if jt[SLOAD].constantGas != wantSloadGasAlways { + t.Error(ft, "bad gas", jt[SLOAD].constantGas) + } + + } else if ft >= sloppyWindowEnd { + // EIP1283 and EIP1706 are activated, but neither modify the SLOAD constant gas price. + if jt[SLOAD].constantGas != wantSloadGasAlways { + t.Error(ft, "bad gas 2", jt[SLOAD].constantGas) + } + } + } + } +} + +// TestECIP1086_ETCMainnet is a variant of TestECIP1086_Kotti, +// showing that SLOAD gas is never 800. +func TestECIP1086_ETCMainnet(t *testing.T) { + etc := params.ClassicChainConfig + + for _, f := range confp.Forks(etc) { + for _, ft := range []uint64{f - 1, f, f + 1} { + jt := instructionSetForConfig(etc, new(big.Int).SetUint64(ft)) + if jt[SLOAD].constantGas == vars.SloadGasEIP2200 { + t.Error("bad gas") + } + } + } +} diff --git a/params/config_kotti.go b/params/config_kotti.go index 35a9abc121..d28b63e9c1 100644 --- a/params/config_kotti.go +++ b/params/config_kotti.go @@ -71,6 +71,8 @@ var ( EIP2028FBlock: big.NewInt(2058191), EIP2200FBlock: big.NewInt(2058191), // RePetersburg (== re-1283) + ECIP1086FBlock: big.NewInt(2058191), + // ECIP-1078, aka Phoenix Fix EIP2200DisableFBlock: big.NewInt(2_208_203), EIP1283FBlock: big.NewInt(2_208_203), diff --git a/params/config_mordor.go b/params/config_mordor.go index a1104d6ae5..d19cb4d31b 100644 --- a/params/config_mordor.go +++ b/params/config_mordor.go @@ -65,6 +65,8 @@ var ( EIP2028FBlock: big.NewInt(778507), EIP2200FBlock: big.NewInt(778507), // RePetersburg (== re-1283) + ECIP1086FBlock: big.NewInt(778507), + // ECIP-1078, aka Phoenix Fix EIP2200DisableFBlock: big.NewInt(976_231), EIP1283FBlock: big.NewInt(976_231), diff --git a/params/types/ctypes/configurator_iface.go b/params/types/ctypes/configurator_iface.go index 809fee63c6..85d33ec152 100644 --- a/params/types/ctypes/configurator_iface.go +++ b/params/types/ctypes/configurator_iface.go @@ -120,6 +120,9 @@ type ProtocolSpecifier interface { SetECIP1080Transition(n *uint64) error GetEIP1706Transition() *uint64 SetEIP1706Transition(n *uint64) error + + GetECIP1086Transition() *uint64 + SetECIP1086Transition(n *uint64) error } type Forker interface { diff --git a/params/types/genesisT/genesis.go b/params/types/genesisT/genesis.go index be8982f8c5..f19a3acf4e 100644 --- a/params/types/genesisT/genesis.go +++ b/params/types/genesisT/genesis.go @@ -536,6 +536,14 @@ func (g Genesis) SetEIP1706Transition(n *uint64) error { return g.Config.SetEIP1706Transition(n) } +func (g Genesis) GetECIP1086Transition() *uint64 { + return g.Config.GetECIP1086Transition() +} + +func (g Genesis) SetECIP1086Transition(n *uint64) error { + return g.Config.SetECIP1086Transition(n) +} + func (g *Genesis) IsEnabled(fn func() *uint64, n *big.Int) bool { return g.Config.IsEnabled(fn, n) } diff --git a/params/types/goethereum/goethereum_configurator.go b/params/types/goethereum/goethereum_configurator.go index 50ca0b7845..55bb128707 100644 --- a/params/types/goethereum/goethereum_configurator.go +++ b/params/types/goethereum/goethereum_configurator.go @@ -382,6 +382,17 @@ func (c *ChainConfig) SetEIP1706Transition(n *uint64) error { return nil } +func (c *ChainConfig) GetECIP1086Transition() *uint64 { + return nil +} + +func (c *ChainConfig) SetECIP1086Transition(n *uint64) error { + if n == nil { + return nil + } + return ctypes.ErrUnsupportedConfigFatal +} + func (c *ChainConfig) IsEnabled(fn func() *uint64, n *big.Int) bool { f := fn() if f == nil || n == nil { diff --git a/params/types/multigeth/chain_config.go b/params/types/multigeth/chain_config.go index f18821af4b..daf8a40f52 100644 --- a/params/types/multigeth/chain_config.go +++ b/params/types/multigeth/chain_config.go @@ -169,9 +169,12 @@ type MultiGethChainConfig struct { ECIP1017FBlock *big.Int `json:"ecip1017FBlock,omitempty"` ECIP1017EraRounds *big.Int `json:"ecip1017EraRounds,omitempty"` // ECIP1017 era rounds ECIP1080FBlock *big.Int `json:"ecip1080FBlock,omitempty"` - DisposalBlock *big.Int `json:"disposalBlock,omitempty"` // Bomb disposal HF block - SocialBlock *big.Int `json:"socialBlock,omitempty"` // Ethereum Social Reward block - EthersocialBlock *big.Int `json:"ethersocialBlock,omitempty"` // Ethersocial Reward block + + ECIP1086FBlock *big.Int `json:"ecip1086FBlock,omitempty"` + + DisposalBlock *big.Int `json:"disposalBlock,omitempty"` // Bomb disposal HF block + SocialBlock *big.Int `json:"socialBlock,omitempty"` // Ethereum Social Reward block + EthersocialBlock *big.Int `json:"ethersocialBlock,omitempty"` // Ethersocial Reward block // Various consensus engines Ethash *ctypes.EthashConfig `json:"ethash,omitempty"` diff --git a/params/types/multigeth/chain_config_configurator.go b/params/types/multigeth/chain_config_configurator.go index 4c0d824b50..0a6efa5174 100644 --- a/params/types/multigeth/chain_config_configurator.go +++ b/params/types/multigeth/chain_config_configurator.go @@ -372,6 +372,15 @@ func (c *MultiGethChainConfig) SetEIP1706Transition(n *uint64) error { return nil } +func (c *MultiGethChainConfig) GetECIP1086Transition() *uint64 { + return bigNewU64(c.ECIP1086FBlock) +} + +func (c *MultiGethChainConfig) SetECIP1086Transition(n *uint64) error { + c.ECIP1086FBlock = setBig(c.ECIP1086FBlock, n) + return nil +} + func (c *MultiGethChainConfig) IsEnabled(fn func() *uint64, n *big.Int) bool { f := fn() if f == nil || n == nil { diff --git a/params/types/multigethv0/multigethv0_chain_config_configurator.go b/params/types/multigethv0/multigethv0_chain_config_configurator.go index 30b27aefa3..948bd05756 100644 --- a/params/types/multigethv0/multigethv0_chain_config_configurator.go +++ b/params/types/multigethv0/multigethv0_chain_config_configurator.go @@ -385,6 +385,17 @@ func (c *ChainConfig) SetEIP1706Transition(n *uint64) error { return ctypes.ErrUnsupportedConfigFatal } +func (c *ChainConfig) GetECIP1086Transition() *uint64 { + return nil +} + +func (c *ChainConfig) SetECIP1086Transition(n *uint64) error { + if n == nil { + return nil + } + return ctypes.ErrUnsupportedConfigFatal +} + func (c *ChainConfig) IsEnabled(fn func() *uint64, n *big.Int) bool { f := fn() if f == nil || n == nil { diff --git a/params/types/parity/parity_configurator.go b/params/types/parity/parity_configurator.go index 0ef50256a8..6de3439f6b 100644 --- a/params/types/parity/parity_configurator.go +++ b/params/types/parity/parity_configurator.go @@ -477,6 +477,17 @@ func (c *ParityChainSpec) SetEIP1706Transition(n *uint64) error { return nil } +func (c *ParityChainSpec) GetECIP1086Transition() *uint64 { + return nil +} + +func (c *ParityChainSpec) SetECIP1086Transition(n *uint64) error { + if n == nil { + return nil + } + return ctypes.ErrUnsupportedConfigFatal +} + func (spec *ParityChainSpec) IsEnabled(fn func() *uint64, n *big.Int) bool { f := fn() if f == nil || n == nil {