-
Notifications
You must be signed in to change notification settings - Fork 221
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[wip] decouple precompiles from core/vm #1320
base: master
Are you sure you want to change the base?
Changes from 4 commits
b1bfabb
22dd760
32a3895
7684553
34baeec
7221490
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
// (c) 2024, Ava Labs, Inc. All rights reserved. | ||
// See the file LICENSE for licensing terms. | ||
|
||
package core | ||
|
||
import ( | ||
"github.com/ava-labs/avalanchego/snow" | ||
"github.com/ava-labs/subnet-evm/constants" | ||
"github.com/ava-labs/subnet-evm/core/vm" | ||
"github.com/ava-labs/subnet-evm/params" | ||
"github.com/ava-labs/subnet-evm/precompile/contract" | ||
"github.com/ava-labs/subnet-evm/precompile/contracts/deployerallowlist" | ||
"github.com/ava-labs/subnet-evm/precompile/modules" | ||
"github.com/ava-labs/subnet-evm/precompile/precompileconfig" | ||
"github.com/ethereum/go-ethereum/common" | ||
"github.com/ethereum/go-ethereum/log" | ||
) | ||
|
||
var ( | ||
newEVM = vm.NewEVM | ||
) | ||
|
||
func init() { | ||
vm.NewEVM = NewEVM | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This and the above capture of the original of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can go back to a style that names the new function where it's used. We already configure the VM by importing packages and altering global state, such as the precompile module registry. I agree that substituting a global is up on the list of textbook anti-patterns, but I think it is preferable to callsite modifications. If we have a few such cases, we can document them explicitly. What is your suggested alternative? |
||
} | ||
|
||
// IsProhibited returns true if [addr] is in the prohibited list of addresses which should | ||
// not be allowed as an EOA or newly created contract address. | ||
func IsProhibited(addr common.Address) bool { | ||
if addr == constants.BlackholeAddr { | ||
return true | ||
} | ||
|
||
return modules.ReservedAddress(addr) | ||
} | ||
|
||
type EVM struct { | ||
*vm.EVM | ||
|
||
chainConfig *params.ChainConfig | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is needed to return it as a The purpose of this type is to keep our modifications of the EVM out of geth code, so we wrap all avalanche specific inputs and configurations and the geth code will try to call a generic callback to call precompiles instead (changing from |
||
} | ||
|
||
func NewEVM( | ||
blockCtx vm.BlockContext, txCtx vm.TxContext, statedb vm.StateDB, | ||
chainConfig vm.ChainConfig, config vm.Config, | ||
) *vm.EVM { | ||
customChainConfig, ok := chainConfig.(*params.ChainConfig) | ||
if !ok { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In what circumstances would someone pass another type? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this PR it is not. But in the future it may be possible that geth / libevm code could use that, until the code is fully separated and there is no "bad imports" |
||
// If the chainConfig is not a params.ChainConfig, then we can't use the custom | ||
// EVM implementation, so we fall back to the default implementation. | ||
log.Warn("ChainConfig is not a *params.ChainConfig, falling back to default EVM") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does passing a non-geth type result in using the native geth implementation and vice versa? That seems back to front. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry if it was unclear, at the moment I do not consider When we are able to properly instantiate geth without these avalanche modifications with its own configuration, many of the merge conflicts (especially in tests) and nuanced test modifications will begin to disappear (we can replace any critical behavior checks with tests that instantiate an avalanche VM) |
||
return newEVM(blockCtx, txCtx, statedb, chainConfig, config) | ||
} | ||
evm := &EVM{ | ||
chainConfig: customChainConfig, | ||
} | ||
config.IsProhibited = IsProhibited | ||
config.DeployerAllowed = evm.DeployerAllowed | ||
config.CustomPrecompiles = evm.CustomPrecompiles | ||
|
||
evm.EVM = newEVM(blockCtx, txCtx, statedb, chainConfig, config) | ||
return evm.EVM | ||
} | ||
|
||
func (evm *EVM) GetBlockContext() contract.BlockContext { | ||
return &evm.EVM.Context | ||
} | ||
|
||
func (evm *EVM) GetStateDB() contract.StateDB { | ||
return evm.StateDB | ||
} | ||
|
||
func (evm *EVM) GetChainConfig() precompileconfig.ChainConfig { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this being returned as a My immediate thought, before inspecting the interface definition, was that it seems to be a non sequitur. We're in the Upon inspection I see that it only has There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is already being returned as
Probably you looked at coreth, subnet-evm also includes Some precompiles already access parameters from |
||
return evm.chainConfig | ||
} | ||
|
||
func (evm *EVM) GetSnowContext() *snow.Context { | ||
return evm.chainConfig.SnowCtx | ||
} | ||
|
||
func (evm *EVM) DeployerAllowed(addr common.Address) bool { | ||
rules := evm.chainConfig.Rules(evm.Context.BlockNumber, evm.Context.Time) | ||
if rules.IsPrecompileEnabled(deployerallowlist.ContractAddress) { | ||
allowListRole := deployerallowlist.GetContractDeployerAllowListStatus(evm.StateDB, evm.TxContext.Origin) | ||
if !allowListRole.IsEnabled() { | ||
return false | ||
} | ||
} | ||
return true | ||
} | ||
|
||
func (evm *EVM) CustomPrecompiles(addr common.Address) (vm.RunFunc, bool) { | ||
rules := evm.chainConfig.Rules(evm.Context.BlockNumber, evm.Context.Time) | ||
if _, ok := rules.ActivePrecompiles[addr]; !ok { | ||
return nil, false | ||
} | ||
module, ok := modules.GetPrecompileModuleByAddress(addr) | ||
if !ok { | ||
return nil, false | ||
} | ||
|
||
return func( | ||
caller common.Address, input []byte, suppliedGas uint64, readOnly bool, | ||
) (ret []byte, remainingGas uint64, err error) { | ||
return module.Contract.Run(evm, caller, addr, input, suppliedGas, readOnly) | ||
}, true | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,8 +34,6 @@ import ( | |
"math/big" | ||
|
||
"github.com/ava-labs/subnet-evm/params" | ||
"github.com/ava-labs/subnet-evm/precompile/contract" | ||
"github.com/ava-labs/subnet-evm/precompile/modules" | ||
"github.com/ava-labs/subnet-evm/vmerrs" | ||
"github.com/ethereum/go-ethereum/common" | ||
"github.com/ethereum/go-ethereum/common/math" | ||
|
@@ -57,81 +55,81 @@ type PrecompiledContract interface { | |
|
||
// PrecompiledContractsHomestead contains the default set of pre-compiled Ethereum | ||
// contracts used in the Frontier and Homestead releases. | ||
var PrecompiledContractsHomestead = map[common.Address]contract.StatefulPrecompiledContract{ | ||
common.BytesToAddress([]byte{1}): newWrappedPrecompiledContract(&ecrecover{}), | ||
common.BytesToAddress([]byte{2}): newWrappedPrecompiledContract(&sha256hash{}), | ||
common.BytesToAddress([]byte{3}): newWrappedPrecompiledContract(&ripemd160hash{}), | ||
common.BytesToAddress([]byte{4}): newWrappedPrecompiledContract(&dataCopy{}), | ||
var PrecompiledContractsHomestead = map[common.Address]PrecompiledContract{ | ||
common.BytesToAddress([]byte{1}): &ecrecover{}, | ||
common.BytesToAddress([]byte{2}): &sha256hash{}, | ||
common.BytesToAddress([]byte{3}): &ripemd160hash{}, | ||
common.BytesToAddress([]byte{4}): &dataCopy{}, | ||
} | ||
|
||
// PrecompiledContractsByzantium contains the default set of pre-compiled Ethereum | ||
// contracts used in the Byzantium release. | ||
var PrecompiledContractsByzantium = map[common.Address]contract.StatefulPrecompiledContract{ | ||
common.BytesToAddress([]byte{1}): newWrappedPrecompiledContract(&ecrecover{}), | ||
common.BytesToAddress([]byte{2}): newWrappedPrecompiledContract(&sha256hash{}), | ||
common.BytesToAddress([]byte{3}): newWrappedPrecompiledContract(&ripemd160hash{}), | ||
common.BytesToAddress([]byte{4}): newWrappedPrecompiledContract(&dataCopy{}), | ||
common.BytesToAddress([]byte{5}): newWrappedPrecompiledContract(&bigModExp{eip2565: false}), | ||
common.BytesToAddress([]byte{6}): newWrappedPrecompiledContract(&bn256AddByzantium{}), | ||
common.BytesToAddress([]byte{7}): newWrappedPrecompiledContract(&bn256ScalarMulByzantium{}), | ||
common.BytesToAddress([]byte{8}): newWrappedPrecompiledContract(&bn256PairingByzantium{}), | ||
var PrecompiledContractsByzantium = map[common.Address]PrecompiledContract{ | ||
common.BytesToAddress([]byte{1}): &ecrecover{}, | ||
common.BytesToAddress([]byte{2}): &sha256hash{}, | ||
common.BytesToAddress([]byte{3}): &ripemd160hash{}, | ||
common.BytesToAddress([]byte{4}): &dataCopy{}, | ||
common.BytesToAddress([]byte{5}): &bigModExp{eip2565: false}, | ||
common.BytesToAddress([]byte{6}): &bn256AddByzantium{}, | ||
common.BytesToAddress([]byte{7}): &bn256ScalarMulByzantium{}, | ||
common.BytesToAddress([]byte{8}): &bn256PairingByzantium{}, | ||
} | ||
|
||
// PrecompiledContractsIstanbul contains the default set of pre-compiled Ethereum | ||
// contracts used in the Istanbul release. | ||
var PrecompiledContractsIstanbul = map[common.Address]contract.StatefulPrecompiledContract{ | ||
common.BytesToAddress([]byte{1}): newWrappedPrecompiledContract(&ecrecover{}), | ||
common.BytesToAddress([]byte{2}): newWrappedPrecompiledContract(&sha256hash{}), | ||
common.BytesToAddress([]byte{3}): newWrappedPrecompiledContract(&ripemd160hash{}), | ||
common.BytesToAddress([]byte{4}): newWrappedPrecompiledContract(&dataCopy{}), | ||
common.BytesToAddress([]byte{5}): newWrappedPrecompiledContract(&bigModExp{eip2565: false}), | ||
common.BytesToAddress([]byte{6}): newWrappedPrecompiledContract(&bn256AddIstanbul{}), | ||
common.BytesToAddress([]byte{7}): newWrappedPrecompiledContract(&bn256ScalarMulIstanbul{}), | ||
common.BytesToAddress([]byte{8}): newWrappedPrecompiledContract(&bn256PairingIstanbul{}), | ||
common.BytesToAddress([]byte{9}): newWrappedPrecompiledContract(&blake2F{}), | ||
var PrecompiledContractsIstanbul = map[common.Address]PrecompiledContract{ | ||
common.BytesToAddress([]byte{1}): &ecrecover{}, | ||
common.BytesToAddress([]byte{2}): &sha256hash{}, | ||
common.BytesToAddress([]byte{3}): &ripemd160hash{}, | ||
common.BytesToAddress([]byte{4}): &dataCopy{}, | ||
common.BytesToAddress([]byte{5}): &bigModExp{eip2565: false}, | ||
common.BytesToAddress([]byte{6}): &bn256AddIstanbul{}, | ||
common.BytesToAddress([]byte{7}): &bn256ScalarMulIstanbul{}, | ||
common.BytesToAddress([]byte{8}): &bn256PairingIstanbul{}, | ||
common.BytesToAddress([]byte{9}): &blake2F{}, | ||
} | ||
|
||
// PrecompiledContractsBerlin contains the default set of pre-compiled Ethereum | ||
// contracts used in the Berlin release. | ||
var PrecompiledContractsBerlin = map[common.Address]contract.StatefulPrecompiledContract{ | ||
common.BytesToAddress([]byte{1}): newWrappedPrecompiledContract(&ecrecover{}), | ||
common.BytesToAddress([]byte{2}): newWrappedPrecompiledContract(&sha256hash{}), | ||
common.BytesToAddress([]byte{3}): newWrappedPrecompiledContract(&ripemd160hash{}), | ||
common.BytesToAddress([]byte{4}): newWrappedPrecompiledContract(&dataCopy{}), | ||
common.BytesToAddress([]byte{5}): newWrappedPrecompiledContract(&bigModExp{eip2565: true}), | ||
common.BytesToAddress([]byte{6}): newWrappedPrecompiledContract(&bn256AddIstanbul{}), | ||
common.BytesToAddress([]byte{7}): newWrappedPrecompiledContract(&bn256ScalarMulIstanbul{}), | ||
common.BytesToAddress([]byte{8}): newWrappedPrecompiledContract(&bn256PairingIstanbul{}), | ||
common.BytesToAddress([]byte{9}): newWrappedPrecompiledContract(&blake2F{}), | ||
var PrecompiledContractsBerlin = map[common.Address]PrecompiledContract{ | ||
common.BytesToAddress([]byte{1}): &ecrecover{}, | ||
common.BytesToAddress([]byte{2}): &sha256hash{}, | ||
common.BytesToAddress([]byte{3}): &ripemd160hash{}, | ||
common.BytesToAddress([]byte{4}): &dataCopy{}, | ||
common.BytesToAddress([]byte{5}): &bigModExp{eip2565: true}, | ||
common.BytesToAddress([]byte{6}): &bn256AddIstanbul{}, | ||
common.BytesToAddress([]byte{7}): &bn256ScalarMulIstanbul{}, | ||
common.BytesToAddress([]byte{8}): &bn256PairingIstanbul{}, | ||
common.BytesToAddress([]byte{9}): &blake2F{}, | ||
} | ||
|
||
// PrecompiledContractsCancun contains the default set of pre-compiled Ethereum | ||
// contracts used in the Cancun release. | ||
var PrecompiledContractsCancun = map[common.Address]contract.StatefulPrecompiledContract{ | ||
common.BytesToAddress([]byte{1}): newWrappedPrecompiledContract(&ecrecover{}), | ||
common.BytesToAddress([]byte{2}): newWrappedPrecompiledContract(&sha256hash{}), | ||
common.BytesToAddress([]byte{3}): newWrappedPrecompiledContract(&ripemd160hash{}), | ||
common.BytesToAddress([]byte{4}): newWrappedPrecompiledContract(&dataCopy{}), | ||
common.BytesToAddress([]byte{5}): newWrappedPrecompiledContract(&bigModExp{eip2565: true}), | ||
common.BytesToAddress([]byte{6}): newWrappedPrecompiledContract(&bn256AddIstanbul{}), | ||
common.BytesToAddress([]byte{7}): newWrappedPrecompiledContract(&bn256ScalarMulIstanbul{}), | ||
common.BytesToAddress([]byte{8}): newWrappedPrecompiledContract(&bn256PairingIstanbul{}), | ||
common.BytesToAddress([]byte{9}): newWrappedPrecompiledContract(&blake2F{}), | ||
common.BytesToAddress([]byte{0x0a}): newWrappedPrecompiledContract(&kzgPointEvaluation{}), | ||
var PrecompiledContractsCancun = map[common.Address]PrecompiledContract{ | ||
common.BytesToAddress([]byte{1}): &ecrecover{}, | ||
common.BytesToAddress([]byte{2}): &sha256hash{}, | ||
common.BytesToAddress([]byte{3}): &ripemd160hash{}, | ||
common.BytesToAddress([]byte{4}): &dataCopy{}, | ||
common.BytesToAddress([]byte{5}): &bigModExp{eip2565: true}, | ||
common.BytesToAddress([]byte{6}): &bn256AddIstanbul{}, | ||
common.BytesToAddress([]byte{7}): &bn256ScalarMulIstanbul{}, | ||
common.BytesToAddress([]byte{8}): &bn256PairingIstanbul{}, | ||
common.BytesToAddress([]byte{9}): &blake2F{}, | ||
common.BytesToAddress([]byte{0x0a}): &kzgPointEvaluation{}, | ||
} | ||
|
||
// PrecompiledContractsBLS contains the set of pre-compiled Ethereum | ||
// contracts specified in EIP-2537. These are exported for testing purposes. | ||
var PrecompiledContractsBLS = map[common.Address]contract.StatefulPrecompiledContract{ | ||
common.BytesToAddress([]byte{10}): newWrappedPrecompiledContract(&bls12381G1Add{}), | ||
common.BytesToAddress([]byte{11}): newWrappedPrecompiledContract(&bls12381G1Mul{}), | ||
common.BytesToAddress([]byte{12}): newWrappedPrecompiledContract(&bls12381G1MultiExp{}), | ||
common.BytesToAddress([]byte{13}): newWrappedPrecompiledContract(&bls12381G2Add{}), | ||
common.BytesToAddress([]byte{14}): newWrappedPrecompiledContract(&bls12381G2Mul{}), | ||
common.BytesToAddress([]byte{15}): newWrappedPrecompiledContract(&bls12381G2MultiExp{}), | ||
common.BytesToAddress([]byte{16}): newWrappedPrecompiledContract(&bls12381Pairing{}), | ||
common.BytesToAddress([]byte{17}): newWrappedPrecompiledContract(&bls12381MapG1{}), | ||
common.BytesToAddress([]byte{18}): newWrappedPrecompiledContract(&bls12381MapG2{}), | ||
var PrecompiledContractsBLS = map[common.Address]PrecompiledContract{ | ||
common.BytesToAddress([]byte{10}): &bls12381G1Add{}, | ||
common.BytesToAddress([]byte{11}): &bls12381G1Mul{}, | ||
common.BytesToAddress([]byte{12}): &bls12381G1MultiExp{}, | ||
common.BytesToAddress([]byte{13}): &bls12381G2Add{}, | ||
common.BytesToAddress([]byte{14}): &bls12381G2Mul{}, | ||
common.BytesToAddress([]byte{15}): &bls12381G2MultiExp{}, | ||
common.BytesToAddress([]byte{16}): &bls12381Pairing{}, | ||
common.BytesToAddress([]byte{17}): &bls12381MapG1{}, | ||
common.BytesToAddress([]byte{18}): &bls12381MapG2{}, | ||
} | ||
|
||
var ( | ||
|
@@ -175,15 +173,6 @@ func init() { | |
for _, k := range addrsList { | ||
PrecompileAllNativeAddresses[k] = struct{}{} | ||
} | ||
|
||
// Ensure that this package will panic during init if there is a conflict present with the declared | ||
// precompile addresses. | ||
for _, module := range modules.RegisteredModules() { | ||
address := module.Address | ||
if _, ok := PrecompileAllNativeAddresses[address]; ok { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We also probably don't need this |
||
panic(fmt.Errorf("precompile address collides with existing native address: %s", address)) | ||
} | ||
} | ||
} | ||
Comment on lines
-179
to
176
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if this can be added as a UT instead |
||
|
||
// ActivePrecompiles returns the precompiles enabled with the current configuration. | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -114,7 +114,7 @@ func enable1344(jt *JumpTable) { | |
|
||
// opChainID implements CHAINID opcode | ||
func opChainID(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) { | ||
chainId, _ := uint256.FromBig(interpreter.evm.chainConfig.ChainID) | ||
chainId, _ := uint256.FromBig(interpreter.evm.chainRules.ChainID) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why the change? Upstream uses There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this PR makes chainConfig an interface and we cannot reference a field on that. Open to other suggestions |
||
scope.Stack.push(chainId) | ||
return nil, nil | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this live in
core
instead ofvm
? Being here blurs any existing separation of concerns.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It cannot be in
core/vm
since the purpose is moving to a direction where avalanche packages are not imported in geth code.