Skip to content

core/vm: clean up EVM environmental structure #31061

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.

Sign up for GitHub

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

Merged
rjl493456442 merged 2 commits into from
Feb 18, 2025
Merged

core/vm: clean up EVM environmental structure #31061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
98b4bb9
core/vm: clean up vm
rjl493456442 Jan 20, 2025
e71bbae
core/vm: remove setCodeOptionalHash
rjl493456442 Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions core/state_processor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func ProcessBeaconBlockRoot(beaconRoot common.Hash, evm *vm.EVM) {
}
evm.SetTxContext(NewEVMTxContext(msg))
evm.StateDB.AddAddressToAccessList(params.BeaconRootsAddress)
_, _, _ = evm.Call(vm.AccountRef(msg.From), *msg.To, msg.Data, 30_000_000, common.U2560)
_, _, _ = evm.Call(msg.From, *msg.To, msg.Data, 30_000_000, common.U2560)
evm.StateDB.Finalise(true)
}

Expand All @@ -253,7 +253,7 @@ func ProcessParentBlockHash(prevHash common.Hash, evm *vm.EVM) {
}
evm.SetTxContext(NewEVMTxContext(msg))
evm.StateDB.AddAddressToAccessList(params.HistoryStorageAddress)
_, _, err := evm.Call(vm.AccountRef(msg.From), *msg.To, msg.Data, 30_000_000, common.U2560)
_, _, err := evm.Call(msg.From, *msg.To, msg.Data, 30_000_000, common.U2560)
if err != nil {
panic(err)
}
Expand Down Expand Up @@ -292,7 +292,7 @@ func processRequestsSystemCall(requests *[][]byte, evm *vm.EVM, requestType byte
}
evm.SetTxContext(NewEVMTxContext(msg))
evm.StateDB.AddAddressToAccessList(addr)
ret, _, _ := evm.Call(vm.AccountRef(msg.From), *msg.To, msg.Data, 30_000_000, common.U2560)
ret, _, _ := evm.Call(msg.From, *msg.To, msg.Data, 30_000_000, common.U2560)
evm.StateDB.Finalise(true)
if len(ret) == 0 {
return // skip empty output
Expand Down
5 changes: 2 additions & 3 deletions core/state_transition.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,6 @@ func (st *stateTransition) execute() (*ExecutionResult, error) {

var (
msg = st.msg
sender = vm.AccountRef(msg.From)
rules = st.evm.ChainConfig().Rules(st.evm.Context.BlockNumber, st.evm.Context.Random != nil, st.evm.Context.Time)
contractCreation = msg.To == nil
floorDataGas uint64
Expand Down Expand Up @@ -484,7 +483,7 @@ func (st *stateTransition) execute() (*ExecutionResult, error) {
vmerr error // vm errors do not effect consensus and are therefore not assigned to err
)
if contractCreation {
ret, _, st.gasRemaining, vmerr = st.evm.Create(sender, msg.Data, st.gasRemaining, value)
ret, _, st.gasRemaining, vmerr = st.evm.Create(msg.From, msg.Data, st.gasRemaining, value)
} else {
// Increment the nonce for the next transaction.
st.state.SetNonce(msg.From, st.state.GetNonce(msg.From)+1, tracing.NonceChangeEoACall)
Expand All @@ -507,7 +506,7 @@ func (st *stateTransition) execute() (*ExecutionResult, error) {
}

// Execute the transaction's call.
ret, st.gasRemaining, vmerr = st.evm.Call(sender, st.to(), msg.Data, st.gasRemaining, value)
ret, st.gasRemaining, vmerr = st.evm.Call(msg.From, st.to(), msg.Data, st.gasRemaining, value)
}

// Compute refund counter, capped to a refund quotient.
Expand Down
86 changes: 20 additions & 66 deletions core/vm/contract.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,39 +22,20 @@ import (
"github.com/holiman/uint256"
)

// ContractRef is a reference to the contract's backing object
type ContractRef interface {
Address() common.Address
}

// AccountRef implements ContractRef.
//
// Account references are used during EVM initialisation and
// its primary use is to fetch addresses. Removing this object
// proves difficult because of the cached jump destinations which
// are fetched from the parent contract (i.e. the caller), which
// is a ContractRef.
type AccountRef common.Address

// Address casts AccountRef to an Address
func (ar AccountRef) Address() common.Address { return (common.Address)(ar) }

// Contract represents an ethereum contract in the state database. It contains
// the contract code, calling arguments. Contract implements ContractRef
type Contract struct {
// CallerAddress is the result of the caller which initialised this
// contract. However when the "call method" is delegated this value
// needs to be initialised to that of the caller's caller.
CallerAddress common.Address
caller ContractRef
self ContractRef
// caller is the result of the caller which initialised this
// contract. However, when the "call method" is delegated this
// value needs to be initialised to that of the caller's caller.
caller common.Address
address common.Address

jumpdests map[common.Hash]bitvec // Aggregated result of JUMPDEST analysis.
analysis bitvec // Locally cached result of JUMPDEST analysis

Code []byte
CodeHash common.Hash
CodeAddr *common.Address
Input []byte

// is the execution frame represented by this object a contract deployment
Expand All @@ -66,23 +47,18 @@ type Contract struct {
}

// NewContract returns a new contract environment for the execution of EVM.
func NewContract(caller ContractRef, object ContractRef, value *uint256.Int, gas uint64) *Contract {
c := &Contract{CallerAddress: caller.Address(), caller: caller, self: object}

if parent, ok := caller.(*Contract); ok {
// Reuse JUMPDEST analysis from parent context if available.
c.jumpdests = parent.jumpdests
} else {
c.jumpdests = make(map[common.Hash]bitvec)
func NewContract(caller common.Address, address common.Address, value *uint256.Int, gas uint64, jumpDests map[common.Hash]bitvec) *Contract {
Copy link
Copy Markdown
Contributor

@s1na s1na Feb 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about adding code and codeHash here? It seems in all cases we are doing:

c := NewContract()
c.SetCallCode

// Initialize the jump analysis map if it's nil, mostly for tests
if jumpDests == nil {
jumpDests = make(map[common.Hash]bitvec)
}
return &Contract{
caller: caller,
address: address,
jumpdests: jumpDests,
Gas: gas,
value: value,
}

// Gas should be a pointer so it can safely be reduced through the run
// This pointer will be off the state transition
c.Gas = gas
// ensures a value is set
c.value = value

return c
}

func (c *Contract) validJumpdest(dest *uint256.Int) bool {
Expand Down Expand Up @@ -132,18 +108,6 @@ func (c *Contract) isCode(udest uint64) bool {
return c.analysis.codeSegment(udest)
}

// AsDelegate sets the contract to be a delegate call and returns the current
// contract (for chaining calls)
func (c *Contract) AsDelegate() *Contract {
// NOTE: caller must, at all times be a contract. It should never happen
// that caller is something other than a Contract.
parent := c.caller.(*Contract)
c.CallerAddress = parent.CallerAddress
c.value = parent.value

return c
}

// GetOp returns the n'th element in the contract's byte array
func (c *Contract) GetOp(n uint64) OpCode {
if n < uint64(len(c.Code)) {
Expand All @@ -158,7 +122,7 @@ func (c *Contract) GetOp(n uint64) OpCode {
// Caller will recursively call caller when the contract is a delegate
// call, including that of caller's caller.
func (c *Contract) Caller() common.Address {
return c.CallerAddress
return c.caller
}

// UseGas attempts the use gas and subtracts it and returns true on success
Expand Down Expand Up @@ -186,26 +150,16 @@ func (c *Contract) RefundGas(gas uint64, logger *tracing.Hooks, reason tracing.G

// Address returns the contracts address
func (c *Contract) Address() common.Address {
return c.self.Address()
return c.address
}

// Value returns the contract's value (sent to it from it's caller)
func (c *Contract) Value() *uint256.Int {
return c.value
}

// SetCallCode sets the code of the contract and address of the backing data
// object
func (c *Contract) SetCallCode(addr *common.Address, hash common.Hash, code []byte) {
// SetCallCode sets the code of the contract,
func (c *Contract) SetCallCode(hash common.Hash, code []byte) {
c.Code = code
c.CodeHash = hash
c.CodeAddr = addr
Copy link
Copy Markdown
Member

@MariusVanDerWijden MariusVanDerWijden Jan 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was never used, right? weird...

Copy link
Copy Markdown
Member Author

@rjl493456442 rjl493456442 Jan 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know... At least it's not used now

}

// SetCodeOptionalHash can be used to provide code, but it's optional to provide hash.
// In case hash is not provided, the jumpdest analysis will not be saved to the parent context
func (c *Contract) SetCodeOptionalHash(addr *common.Address, codeAndHash *codeAndHash) {
c.Code = codeAndHash.code
c.CodeHash = codeAndHash.hash
c.CodeAddr = addr
}
8 changes: 2 additions & 6 deletions core/vm/eips.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,13 +338,9 @@ func opExtCodeCopyEIP4762(pc *uint64, interpreter *EVMInterpreter, scope *ScopeC
}
addr := common.Address(a.Bytes20())
code := interpreter.evm.StateDB.GetCode(addr)
contract := &Contract{
Code: code,
self: AccountRef(addr),
}
paddedCodeCopy, copyOffset, nonPaddedCopyLength := getDataAndAdjustedBounds(code, uint64CodeOffset, length.Uint64())
if !contract.IsSystemCall {
Copy link
Copy Markdown
Contributor

@s1na s1na Feb 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like an alternative to #31114. Checking the conversation there it seems unresolved. Should we leave it out of this PR or do you think this is the correct fix for the issue?

statelessGas := interpreter.evm.AccessEvents.CodeChunksRangeGas(addr, copyOffset, nonPaddedCopyLength, uint64(len(contract.Code)), false)
if !scope.Contract.IsSystemCall {
statelessGas := interpreter.evm.AccessEvents.CodeChunksRangeGas(addr, copyOffset, nonPaddedCopyLength, uint64(len(code)), false)
if !scope.Contract.UseGas(statelessGas, interpreter.evm.Config.Tracer, tracing.GasChangeUnspecified) {
scope.Contract.Gas = 0
return nil, ErrOutOfGas
Expand Down
Loading