From f2a07f093b2f22eddce0e71b00ec1dbe64c5eaa5 Mon Sep 17 00:00:00 2001 From: minh-bq <97180373+minh-bq@users.noreply.github.com> Date: Wed, 14 Aug 2024 10:46:06 +0700 Subject: [PATCH 1/3] core/vm: correct the Istanbul bn256 precompiled contracts (#521) In commit 268d950147 ("core/vm: change Consortium precompiled mapping into global variable"), we mistakenly copy the Byzantium precompiled contracts to Istanbul and add blake2F at address 0x9 only. This is incorrect as the bn256 precompiled contracts at 0x6, 0x7, 0x8 are changed too. This commit fixes Istanbul precompiled contracts to have correct bn256 version. --- core/vm/contracts.go | 3 +++ core/vm/contracts_test.go | 40 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/core/vm/contracts.go b/core/vm/contracts.go index 1efaf504b0..f9bc2d8724 100644 --- a/core/vm/contracts.go +++ b/core/vm/contracts.go @@ -114,6 +114,9 @@ func init() { PrecompiledContractsByzantium[common.BytesToAddress([]byte{8})] = &bn256PairingByzantium{} PrecompiledContractsIstanbul = copyPrecompiledContract(PrecompiledContractsByzantium) + PrecompiledContractsIstanbul[common.BytesToAddress([]byte{6})] = &bn256AddIstanbul{} + PrecompiledContractsIstanbul[common.BytesToAddress([]byte{7})] = &bn256ScalarMulIstanbul{} + PrecompiledContractsIstanbul[common.BytesToAddress([]byte{8})] = &bn256PairingIstanbul{} PrecompiledContractsIstanbul[common.BytesToAddress([]byte{9})] = &blake2F{} PrecompiledContractsConsortium = copyPrecompiledContract(PrecompiledContractsIstanbul) diff --git a/core/vm/contracts_test.go b/core/vm/contracts_test.go index dad72ac640..b951ab74fe 100644 --- a/core/vm/contracts_test.go +++ b/core/vm/contracts_test.go @@ -28,6 +28,7 @@ import ( "github.com/ethereum/go-ethereum/accounts/abi" "github.com/ethereum/go-ethereum/log" + "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/common" ) @@ -427,3 +428,42 @@ func BenchmarkPrecompiledBLS12381G2MultiExpWorstCase(b *testing.B) { } benchmarkPrecompiled("0f", testcase, b) } + +func TestIstanbulPrecompile(t *testing.T) { + evm := EVM{ + chainConfig: ¶ms.ChainConfig{}, + chainRules: params.Rules{ + IsIstanbul: true, + }, + } + caller := AccountRef(common.Address{0x1}) + precompiledContract, ok := evm.precompile(caller, common.BytesToAddress([]byte{6})) + if !ok { + t.Fatal("Failed to get Istanbul precompiled contract") + } + + _, ok = precompiledContract.(*bn256AddIstanbul) + if !ok { + t.Fatal("Incorrect precompiled contract") + } + + precompiledContract, ok = evm.precompile(caller, common.BytesToAddress([]byte{7})) + if !ok { + t.Fatal("Failed to get Istanbul precompiled contract") + } + + _, ok = precompiledContract.(*bn256ScalarMulIstanbul) + if !ok { + t.Fatal("Incorrect precompiled contract") + } + + precompiledContract, ok = evm.precompile(caller, common.BytesToAddress([]byte{8})) + if !ok { + t.Fatal("Failed to get Istanbul precompiled contract") + } + + _, ok = precompiledContract.(*bn256PairingIstanbul) + if !ok { + t.Fatal("Incorrect precompiled contract") + } +} From 71f207de651f8ad288d218ecc7833c82dbf873a1 Mon Sep 17 00:00:00 2001 From: minh-bq <97180373+minh-bq@users.noreply.github.com> Date: Wed, 14 Aug 2024 17:21:14 +0700 Subject: [PATCH 2/3] core/vm: fix Berlin precompiled address list (#520) * core/vm: add Consortium precompiled contract call gas test * core/vm: fix Berlin precompiled address list Commit 268d950147 ("core/vm: change Consortium precompiled mapping into global variable") tries to optimize how precompiled contracts mapping is calculated. However, it introduces a breaking change. In the current running network after Berlin, PrecompiledAddressesBerlin does not contain Consortium precompiled addresses even though the Consortium precompiled contracts are already activated. It is a bug. However, PrecompiledAddressesBerlin is used in ActivePrecompiles, which is called to get the active precompiled contracts to add to access list at the start of transaction. So we cannot fix this by just changing PrecompiledAddressesBerlin as it makes the breaking change in gas calculation. Therefore, we need to make PrecompiledAddressesBerlin the same as running network, which is equal to PrecompiledAddressesIstanbul. Consortium precompiled addresses are added in later hardfork. --- core/vm/contracts.go | 19 +++++- core/vm/contracts_test.go | 125 ++++++++++++++++++++++++++++++++++++++ core/vm/evm_test.go | 69 --------------------- 3 files changed, 141 insertions(+), 72 deletions(-) diff --git a/core/vm/contracts.go b/core/vm/contracts.go index f9bc2d8724..a56c7a8a9b 100644 --- a/core/vm/contracts.go +++ b/core/vm/contracts.go @@ -147,9 +147,22 @@ func init() { for k := range PrecompiledContractsConsortiumMiko { PrecompiledAddressesMiko = append(PrecompiledAddressesMiko, k) } - for k := range PrecompiledContractsBerlin { - PrecompiledAddressesBerlin = append(PrecompiledAddressesBerlin, k) - } + + // Commit 268d950147 ("core/vm: change Consortium precompiled mapping into global variable") + // tries to optimize how precompiled contracts mapping is calculated. However, it introduces + // a breaking change. + // + // In the current running network after Berlin, PrecompiledAddressesBerlin does not contain + // Consortium precompiled addresses even though the Consortium precompiled contracts are + // already activated. It is a bug. However, PrecompiledAddressesBerlin is used in + // ActivePrecompiles, which is called to get the active precompiled contracts to add to access + // list at the start of transaction. So we cannot fix this by just changing + // PrecompiledAddressesBerlin as it makes the breaking change in gas calculation. + // + // Therefore, we need to make PrecompiledAddressesBerlin the same as running network, which + // is equal to PrecompiledAddressesIstanbul. Consortium precompiled addresses are added in + // later hardfork. + PrecompiledAddressesBerlin = PrecompiledAddressesIstanbul } // ActivePrecompiles returns the precompiles enabled with the current configuration. diff --git a/core/vm/contracts_test.go b/core/vm/contracts_test.go index b951ab74fe..8b620722b5 100644 --- a/core/vm/contracts_test.go +++ b/core/vm/contracts_test.go @@ -21,12 +21,15 @@ import ( "encoding/json" "fmt" "io/ioutil" + "math/big" "os" "strings" "testing" "time" "github.com/ethereum/go-ethereum/accounts/abi" + "github.com/ethereum/go-ethereum/core/rawdb" + "github.com/ethereum/go-ethereum/core/state" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/params" @@ -467,3 +470,125 @@ func TestIstanbulPrecompile(t *testing.T) { t.Fatal("Incorrect precompiled contract") } } + +func TestConsortiumPrecompileQuery(t *testing.T) { + evm := EVM{ + chainConfig: ¶ms.ChainConfig{}, + chainRules: params.Rules{ + IsMiko: true, + }, + } + caller := AccountRef(common.Address{0x1}) + precompiledContract, ok := evm.precompile(caller, common.BytesToAddress([]byte{106})) + if !ok { + t.Fatal("Failed to get Miko precompiled contract") + } + + p, ok := precompiledContract.(*consortiumValidateProofOfPossession) + if !ok { + t.Fatal("Incorrect precompiled contract") + } + + if p.evm != &evm { + t.Fatal("Incorrect evm in precompiled contract") + } + + if p.caller != caller { + t.Fatal("Incorrect caller in precompiled contract") + } + + precompiledContract, ok = evm.precompile(caller, common.BytesToAddress([]byte{1})) + if !ok { + t.Fatal("Failed to get ecrecover precompiled contract") + } + if _, ok := precompiledContract.(*ecrecover); !ok { + t.Fatal("Wrong ecrecover contract") + } + + // Check precompiled contract after Berlin + evm.chainRules.IsBerlin = true + precompiledContract, ok = evm.precompile(caller, common.BytesToAddress([]byte{106})) + if !ok { + t.Fatal("Failed to get Miko precompiled contract") + } + p, ok = precompiledContract.(*consortiumValidateProofOfPossession) + if !ok { + t.Fatal("Incorrect precompiled contract") + } + + if p.evm != &evm { + t.Fatal("Incorrect evm in precompiled contract") + } + + if p.caller != caller { + t.Fatal("Incorrect caller in precompiled contract") + } +} + +func BenchmarkConsortiumPrecompileQuery(b *testing.B) { + evm := EVM{ + chainConfig: ¶ms.ChainConfig{}, + chainRules: params.Rules{ + IsMiko: true, + }, + } + + b.ResetTimer() + b.ReportAllocs() + for i := 0; i < b.N; i++ { + evm.precompile(nil, common.BytesToAddress([]byte{106})) + } +} + +func TestConsortiumPrecompileCallGas(t *testing.T) { + statedb, _ := state.New(common.Hash{}, state.NewDatabase(rawdb.NewMemoryDatabase()), nil) + chainConfig := params.ChainConfig{ + EIP155Block: big.NewInt(0), + EIP158Block: big.NewInt(0), + ByzantiumBlock: big.NewInt(0), + ConstantinopleBlock: big.NewInt(0), + PetersburgBlock: big.NewInt(0), + IstanbulBlock: big.NewInt(0), + MuirGlacierBlock: big.NewInt(0), + ConsortiumV2Block: big.NewInt(0), + MikoBlock: big.NewInt(0), + BerlinBlock: big.NewInt(0), + LondonBlock: big.NewInt(0), + } + + env := NewEVM(BlockContext{ + BlockNumber: common.Big0, + CanTransfer: func(_ StateDB, _ common.Address, _ *big.Int) bool { return true }, + Transfer: func(_ StateDB, _, _ common.Address, _ *big.Int) {}, + }, TxContext{}, statedb, &chainConfig, Config{}) + contractAddr := common.Address{0x2} + code := []byte{ + byte(PUSH1), byte(0x0), // retLength + byte(PUSH1), byte(0x0), // retOffset + byte(PUSH1), byte(0x0), // argsLength + byte(PUSH1), byte(0x0), // argsOffset + byte(PUSH1), byte(0x0), // value + byte(PUSH20), + } + code = append(code, common.BytesToAddress([]byte{104}).Bytes()...) // consortium precompiled address + code = append(code, byte(PUSH3), byte(0x10), byte(0x00), byte(0x00)) // gas + code = append(code, byte(CALL)) // call + code = append(code, byte(STOP)) + + initialGas := uint64(100_000_000) + contract := &Contract{ + Code: code, + self: contractRef{addr: contractAddr}, + Gas: initialGas, + } + + statedb.Prepare(env.chainRules, common.Address{}, common.Address{}, &contractAddr, ActivePrecompiles(env.chainRules), nil) + _, err := env.interpreter.Run(contract, nil, false) + if err != nil { + t.Fatal(err) + } + gasUsed := initialGas - contract.Gas + if gasUsed != 1051197 { + t.Fatalf("Expect gas used to be %d, got %d", 1051197, gasUsed) + } +} diff --git a/core/vm/evm_test.go b/core/vm/evm_test.go index a9bdfc0f28..afc649f741 100644 --- a/core/vm/evm_test.go +++ b/core/vm/evm_test.go @@ -740,72 +740,3 @@ func TestInternalTransactionOrderDelegateCalls(t *testing.T) { } } } - -func TestConsortiumPrecompileQuery(t *testing.T) { - evm := EVM{ - chainConfig: ¶ms.ChainConfig{}, - chainRules: params.Rules{ - IsMiko: true, - }, - } - caller := AccountRef(common.Address{0x1}) - precompiledContract, ok := evm.precompile(caller, common.BytesToAddress([]byte{106})) - if !ok { - t.Fatal("Failed to get Miko precompiled contract") - } - - p, ok := precompiledContract.(*consortiumValidateProofOfPossession) - if !ok { - t.Fatal("Incorrect precompiled contract") - } - - if p.evm != &evm { - t.Fatal("Incorrect evm in precompiled contract") - } - - if p.caller != caller { - t.Fatal("Incorrect caller in precompiled contract") - } - - precompiledContract, ok = evm.precompile(caller, common.BytesToAddress([]byte{1})) - if !ok { - t.Fatal("Failed to get ecrecover precompiled contract") - } - if _, ok := precompiledContract.(*ecrecover); !ok { - t.Fatal("Wrong ecrecover contract") - } - - // Check precompiled contract after Berlin - evm.chainRules.IsBerlin = true - precompiledContract, ok = evm.precompile(caller, common.BytesToAddress([]byte{106})) - if !ok { - t.Fatal("Failed to get Miko precompiled contract") - } - p, ok = precompiledContract.(*consortiumValidateProofOfPossession) - if !ok { - t.Fatal("Incorrect precompiled contract") - } - - if p.evm != &evm { - t.Fatal("Incorrect evm in precompiled contract") - } - - if p.caller != caller { - t.Fatal("Incorrect caller in precompiled contract") - } -} - -func BenchmarkConsortiumPrecompileQuery(b *testing.B) { - evm := EVM{ - chainConfig: ¶ms.ChainConfig{}, - chainRules: params.Rules{ - IsMiko: true, - }, - } - - b.ResetTimer() - b.ReportAllocs() - for i := 0; i < b.N; i++ { - evm.precompile(nil, common.BytesToAddress([]byte{106})) - } -} From c004b7ddad91f44c64fbb25bba7152bdac5564c5 Mon Sep 17 00:00:00 2001 From: minh-bq <97180373+minh-bq@users.noreply.github.com> Date: Fri, 16 Aug 2024 10:44:02 +0700 Subject: [PATCH 3/3] consortium-v2: prepare access list and transient storage in system transaction (#509) * consortium-v2: prepare access list and transient storage in system transaction In 576852519 ("core/state: remove newAccessList in SetTxContext"), we remove the access list reset in SetTxContext because later in statedb.Prepare we already reset the list. However, system transaction does not call to statedb.Prepare before processing transaction. This results in a consensus breaking change. This commit create a new access list reset function and adds to system transaction path to fix the issue. Moreover, we add the statedb.Prepare call to system transaction path after Shanghai to make EIP-3651: warm coinbase and EIP-1153 - Transient storage opcodes work correctly in system transaction. * consortium-v2: add access list and transient storage test for system transaction --- consensus/consortium/common/contract.go | 8 ++ consensus/consortium/common/contract_test.go | 95 ++++++++++++++++++++ core/state/statedb.go | 5 ++ 3 files changed, 108 insertions(+) diff --git a/consensus/consortium/common/contract.go b/consensus/consortium/common/contract.go index f6dae68ef4..89b391f4c1 100644 --- a/consensus/consortium/common/contract.go +++ b/consensus/consortium/common/contract.go @@ -549,6 +549,14 @@ func applyMessage( // Create a new context to be used in the EVM environment opts.EVMContext.CurrentTransaction = tx from, _ := types.Sender(types.MakeSigner(opts.ChainConfig, opts.Header.Number), tx) + + chainRules := opts.ChainConfig.Rules(opts.Header.Number) + if chainRules.IsShanghai { + opts.State.Prepare(chainRules, from, from, tx.To(), vm.ActivePrecompiles(chainRules), nil) + } else if chainRules.IsBerlin { + opts.State.ResetAccessList() + } + // Create a new environment which holds all relevant information // about the transaction and calling mechanisms. vmenv := vm.NewEVM(*opts.EVMContext, vm.TxContext{Origin: from, GasPrice: big.NewInt(0)}, opts.State, opts.ChainConfig, vm.Config{}) diff --git a/consensus/consortium/common/contract_test.go b/consensus/consortium/common/contract_test.go index d07860c435..000342b6f6 100644 --- a/consensus/consortium/common/contract_test.go +++ b/consensus/consortium/common/contract_test.go @@ -8,6 +8,7 @@ import ( "reflect" "testing" + "github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/common" legacyProfile "github.com/ethereum/go-ethereum/consensus/consortium/generated_contracts/legacy_profile" "github.com/ethereum/go-ethereum/consensus/consortium/generated_contracts/profile" @@ -17,6 +18,7 @@ import ( "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/core/state" "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/params" ) @@ -292,3 +294,96 @@ func TestContractCall(t *testing.T) { } } } + +func TestApplyTransaction(t *testing.T) { + minerKey, _ := crypto.GenerateKey() + minerAddr := crypto.PubkeyToAddress(minerKey.PublicKey) + + chainId := big.NewInt(2020) + signer := types.NewMikoSigner(chainId) + chainConfig := params.ChainConfig{ + ChainID: chainId, + ByzantiumBlock: common.Big0, + BerlinBlock: common.Big0, + } + tx, err := types.SignNewTx(minerKey, signer, &types.LegacyTx{ + To: &common.Address{0x22}, + }) + if err != nil { + t.Fatal(err) + } + msg := types.NewMessage(minerAddr, tx.To(), tx.Nonce(), tx.Value(), tx.Gas(), + tx.GasPrice(), tx.GasFeeCap(), tx.GasTipCap(), tx.Data(), nil, false) + + state, err := state.New(common.Hash{}, state.NewDatabase(rawdb.NewMemoryDatabase()), nil) + if err != nil { + t.Fatal(err) + } + dummyAccount := common.Address{0x11} + state.AddAddressToAccessList(dummyAccount) + + var ( + txs []*types.Transaction + receipts []*types.Receipt + ) + opts := ApplyTransactOpts{ + ApplyMessageOpts: &ApplyMessageOpts{ + Header: &types.Header{ + Coinbase: minerAddr, + Number: common.Big0, + }, + EVMContext: &vm.BlockContext{ + Transfer: func(_ vm.StateDB, _, _ common.Address, _ *big.Int) {}, + }, + State: state, + ChainConfig: &chainConfig, + }, + Signer: signer, + SignTxFn: func(_ accounts.Account, tx *types.Transaction, chainId *big.Int) (*types.Transaction, error) { + return types.SignTx(tx, signer, minerKey) + }, + Mining: true, + UsedGas: new(uint64), + Txs: &txs, + Receipts: &receipts, + } + err = ApplyTransaction(msg, &opts) + if err != nil { + t.Fatalf("Failed to apply transaction, err %v", err) + } + + if state.AddressInAccessList(dummyAccount) { + t.Fatalf("Expect the access list to be reset after Berlin") + } + + state.AddAddressToAccessList(dummyAccount) + dummyKey := common.Hash{0x33} + state.SetTransientState(dummyAccount, dummyKey, common.Hash{0x44}) + chainConfig.ShanghaiBlock = common.Big0 + err = ApplyTransaction(msg, &opts) + if err != nil { + t.Fatalf("Failed to apply transaction, err %v", err) + } + + if state.AddressInAccessList(dummyAccount) { + t.Fatalf("Expect the access list to be reset after Berlin") + } + + if state.GetTransientState(dummyAccount, dummyKey) != (common.Hash{}) { + t.Fatalf("Expect the transient state to be reset") + } + + if !state.AddressInAccessList(minerAddr) { + t.Fatalf("Expect sender to be in the access list after Shanghai") + } + + if !state.AddressInAccessList(*tx.To()) { + t.Fatalf("Expect recipient to be in the access list after Shanghai") + } + + for _, addr := range vm.ActivePrecompiles(chainConfig.Rules(common.Big0)) { + if !state.AddressInAccessList(addr) { + t.Fatalf("Expect precompile %v to be in the access list after Shanghai", addr) + } + } +} diff --git a/core/state/statedb.go b/core/state/statedb.go index dfdef93fcc..fc252bbcd1 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -1033,6 +1033,11 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (common.Hash, error) { return root, err } +// ResetAccessList sets access list to empty +func (s *StateDB) ResetAccessList() { + s.accessList = newAccessList() +} + // Prepare handles the preparatory steps for executing a state transition with. // This method must be invoked before state transition. //