Skip to content

Commit

Permalink
core: remove stray account creations in state transition
Browse files Browse the repository at this point in the history
The 'from' and 'to' methods on StateTransitions are reader methods and
shouldn't have inadvertent side effects on state.

It is safe to remove the check in 'from' because account existence is
implicitly checked by the nonce and balance checks. If the account has
non-zero balance or nonce, it must exist. Even if the sender account has
nonce zero at the start of the state transition or no balance, the nonce
is incremented before execution and the account will be created at that
time.

It is safe to remove the check in 'to' because the EVM creates the
account if necessary.

Fixes #15119
  • Loading branch information
fjl committed Apr 10, 2018
1 parent 39f4c80 commit e8c30e8
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 43 deletions.
58 changes: 16 additions & 42 deletions core/state_transition.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,28 +132,12 @@ func ApplyMessage(evm *vm.EVM, msg Message, gp *GasPool) ([]byte, uint64, bool,
return NewStateTransition(evm, msg, gp).TransitionDb()
}

func (st *StateTransition) from() vm.AccountRef {
f := st.msg.From()
if !st.state.Exist(f) {
st.state.CreateAccount(f)
// to returns the recipient of the message.
func (st *StateTransition) to() common.Address {
if st.msg == nil || st.msg.To() == nil /* contract creation */ {
return common.Address{}
}
return vm.AccountRef(f)
}

func (st *StateTransition) to() vm.AccountRef {
if st.msg == nil {
return vm.AccountRef{}
}
to := st.msg.To()
if to == nil {
return vm.AccountRef{} // contract creation
}

reference := vm.AccountRef(*to)
if !st.state.Exist(*to) {
st.state.CreateAccount(*to)
}
return reference
return *st.msg.To()
}

func (st *StateTransition) useGas(amount uint64) error {
Expand All @@ -166,12 +150,8 @@ func (st *StateTransition) useGas(amount uint64) error {
}

func (st *StateTransition) buyGas() error {
var (
state = st.state
sender = st.from()
)
mgval := new(big.Int).Mul(new(big.Int).SetUint64(st.msg.Gas()), st.gasPrice)
if state.GetBalance(sender.Address()).Cmp(mgval) < 0 {
if st.state.GetBalance(st.msg.From()).Cmp(mgval) < 0 {
return errInsufficientBalanceForGas
}
if err := st.gp.SubGas(st.msg.Gas()); err != nil {
Expand All @@ -180,20 +160,17 @@ func (st *StateTransition) buyGas() error {
st.gas += st.msg.Gas()

st.initialGas = st.msg.Gas()
state.SubBalance(sender.Address(), mgval)
st.state.SubBalance(st.msg.From(), mgval)
return nil
}

func (st *StateTransition) preCheck() error {
msg := st.msg
sender := st.from()

// Make sure this transaction's nonce is correct
if msg.CheckNonce() {
nonce := st.state.GetNonce(sender.Address())
if nonce < msg.Nonce() {
// Make sure this transaction's nonce is correct.
if st.msg.CheckNonce() {
nonce := st.state.GetNonce(st.msg.From())
if nonce < st.msg.Nonce() {
return ErrNonceTooHigh
} else if nonce > msg.Nonce() {
} else if nonce > st.msg.Nonce() {
return ErrNonceTooLow
}
}
Expand All @@ -208,8 +185,7 @@ func (st *StateTransition) TransitionDb() (ret []byte, usedGas uint64, failed bo
return
}
msg := st.msg
sender := st.from() // err checked in preCheck

sender := vm.AccountRef(msg.From())
homestead := st.evm.ChainConfig().IsHomestead(st.evm.BlockNumber)
contractCreation := msg.To() == nil

Expand All @@ -233,8 +209,8 @@ func (st *StateTransition) TransitionDb() (ret []byte, usedGas uint64, failed bo
ret, _, st.gas, vmerr = evm.Create(sender, st.data, st.gas, st.value)
} else {
// Increment the nonce for the next transaction
st.state.SetNonce(sender.Address(), st.state.GetNonce(sender.Address())+1)
ret, st.gas, vmerr = evm.Call(sender, st.to().Address(), st.data, st.gas, st.value)
st.state.SetNonce(msg.From(), st.state.GetNonce(sender.Address())+1)
ret, st.gas, vmerr = evm.Call(sender, st.to(), st.data, st.gas, st.value)
}
if vmerr != nil {
log.Debug("VM returned with error", "err", vmerr)
Expand All @@ -260,10 +236,8 @@ func (st *StateTransition) refundGas() {
st.gas += refund

// Return ETH for remaining gas, exchanged at the original rate.
sender := st.from()

remaining := new(big.Int).Mul(new(big.Int).SetUint64(st.gas), st.gasPrice)
st.state.AddBalance(sender.Address(), remaining)
st.state.AddBalance(st.msg.From(), remaining)

// Also return remaining gas to the block gas counter so it is
// available for the next transaction.
Expand Down
1 change: 0 additions & 1 deletion tests/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ func TestState(t *testing.T) {
// Expected failures:
st.fails(`^stRevertTest/RevertPrecompiledTouch\.json/EIP158`, "bug in test")
st.fails(`^stRevertTest/RevertPrecompiledTouch\.json/Byzantium`, "bug in test")
st.fails(`^stRandom2/randomStatetest64[45]\.json/(EIP150|Frontier|Homestead)/.*`, "known bug #15119")

st.walk(t, stateTestDir, func(t *testing.T, name string, test *StateTest) {
for _, subtest := range test.Subtests() {
Expand Down

0 comments on commit e8c30e8

Please sign in to comment.