Merge branch 'feat/ecip-1086' into etclabscore-master

Rel etclabscore/multi-geth-fork#151

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

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

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")
+			}
+		}
+	}
+}

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),

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),

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 {

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)
 }

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 {

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"`

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 {

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 {

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 {