Skip to content
This repository has been archived by the owner on Nov 30, 2021. It is now read-only.

In the "newEVM" function, the member variable "GetHash" is not initialized when "vm.Context" is initialized #618

Closed
summerpro opened this issue Nov 25, 2020 · 3 comments · Fixed by #620
Assignees

Comments

@summerpro
Copy link
Contributor

summerpro commented Nov 25, 2020

System info:

  • in branch "development"

Expected behavior: [What you expected to happen]

  • function newEVM
func (st StateTransition) newEVM(ctx sdk.Context, csdb *CommitStateDB, gasLimit uint64, gasPrice *big.Int, config ChainConfig) *vm.EVM {
	// Create context for evm
	context := vm.Context{
		CanTransfer: core.CanTransfer,
		Transfer:    core.Transfer,
                 GetHash: FUNCTION_GET_HASH,
		Origin:      st.Sender,
		Coinbase:    common.Address{}, // there's no benefitiary since we're not mining
		BlockNumber: big.NewInt(ctx.BlockHeight()),
		Time:        big.NewInt(ctx.BlockHeader().Time.Unix()),
		Difficulty:  big.NewInt(0), // unused. Only required in PoW context
		GasLimit:    gasLimit,
		GasPrice:    gasPrice,
	}

	return vm.NewEVM(context, csdb, config.EthereumConfig(st.ChainID), vm.Config{})
}

Actual behavior: [What actually happened]

  • function newEVM
func (st StateTransition) newEVM(ctx sdk.Context, csdb *CommitStateDB, gasLimit uint64, gasPrice *big.Int, config ChainConfig) *vm.EVM {
	// Create context for evm
	context := vm.Context{
		CanTransfer: core.CanTransfer,
		Transfer:    core.Transfer,
		Origin:      st.Sender,
		Coinbase:    common.Address{}, // there's no benefitiary since we're not mining
		BlockNumber: big.NewInt(ctx.BlockHeight()),
		Time:        big.NewInt(ctx.BlockHeader().Time.Unix()),
		Difficulty:  big.NewInt(0), // unused. Only required in PoW context
		GasLimit:    gasLimit,
		GasPrice:    gasPrice,
	}

	return vm.NewEVM(context, csdb, config.EthereumConfig(st.ChainID), vm.Config{})
}

Additional info: [Include gist of relevant config, logs, etc.]

  • the struct "vm.Context" have a member variable called "GetHash"
// Context provides the EVM with auxiliary information. Once provided
// it shouldn't be modified.
type Context struct {
	// CanTransfer returns whether the account contains
	// sufficient ether to transfer the value
	CanTransfer CanTransferFunc
	// Transfer transfers ether from one account to the other
	Transfer TransferFunc
	// GetHash returns the hash corresponding to n
	GetHash GetHashFunc

	// Message information
	Origin   common.Address // Provides information for ORIGIN
	GasPrice *big.Int       // Provides information for GASPRICE

	// Block information
	Coinbase    common.Address // Provides information for COINBASE
	GasLimit    uint64         // Provides information for GASLIMIT
	BlockNumber *big.Int       // Provides information for NUMBER
	Time        *big.Int       // Provides information for TIME
	Difficulty  *big.Int       // Provides information for DIFFICULTY
}
  • function "GetHash" from "vm.Context" is used in opCode of evm
func opBlockhash(pc *uint64, interpreter *EVMInterpreter, callContext *callCtx) ([]byte, error) {
	num := callContext.stack.peek()
	num64, overflow := num.Uint64WithOverflow()
	if overflow {
		num.Clear()
		return nil, nil
	}
	var upper, lower uint64
	upper = interpreter.evm.BlockNumber.Uint64()
	if upper < 257 {
		lower = 0
	} else {
		lower = upper - 256
	}
	if num64 >= lower && num64 < upper {
		num.SetBytes(interpreter.evm.GetHash(num64).Bytes())
	} else {
		num.Clear()
	}
	return nil, nil
}
  • If the method opBlockhash is called when the contract is executed, it may panic because GetHash is nil
@fedekunze fedekunze self-assigned this Nov 25, 2020
@summerpro
Copy link
Contributor Author

hi @fedekunze , how do you plan to deal with this problem

@fedekunze
Copy link
Contributor

Hi @summerpro, I have a WIP pull request here: #620. I still need to write a few more tests but it should be ready for an early review if you want to take a look. Please make comments if you think something is not clear enough

@summerpro
Copy link
Contributor Author

Hi @summerpro, I have a WIP pull request here: #620. I still need to write a few more tests but it should be ready for an early review if you want to take a look. Please make comments if you think something is not clear enough

ok,I just left a comment

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants