Storage and edge case fixes (#28)

* Applied storage fixes and updates

* Minor execution refactor

core/execution.go

@@ -45,7 +45,6 @@ var (
 // the necessary steps to create accounts and reverses the state in case of an
 // execution error or failed value transfer.
 func Call(env vm.Environment, caller vm.ContractRef, addr common.Address, input []byte, gas, gasPrice, value *big.Int) (ret []byte, err error) {
-	evm := env.Vm()
 	// Depth check execution. Fail if we're trying to execute above the limit.
 	if env.Depth() > callCreateDepthMax {
 		caller.ReturnGas(gas, gasPrice)
@@ -86,7 +85,7 @@ func Call(env vm.Environment, caller vm.ContractRef, addr common.Address, input
 	defer contract.Finalise()
 
 	// Even if the account has no code, we need to continue because it might be a precompile
-	ret, err = evm.Run(contract, input, false)
+	ret, err = env.Vm().Run(contract, input, false)
 
 	// When an error was returned by the EVM or when setting the creation code
 	// above we revert to the snapshot and consume any gas remaining. Additionally
@@ -102,7 +101,6 @@ func Call(env vm.Environment, caller vm.ContractRef, addr common.Address, input
 
 // CallCode executes the given address' code as the given contract address
 func CallCode(env vm.Environment, caller vm.ContractRef, addr common.Address, input []byte, gas, gasPrice, value *big.Int) (ret []byte, err error) {
-	evm := env.Vm()
 	// Depth check execution. Fail if we're trying to execute above the limit.
 	if env.Depth() > callCreateDepthMax {
 		caller.ReturnGas(gas, gasPrice)
@@ -127,7 +125,7 @@ func CallCode(env vm.Environment, caller vm.ContractRef, addr common.Address, in
 	defer contract.Finalise()
 
 	// Even if the account has no code, we need to continue because it might be a precompile
-	ret, err = evm.Run(contract, input, false)
+	ret, err = env.Vm().Run(contract, input, false)
 
 	if err != nil {
 		env.RevertToSnapshot(snapshot)
@@ -140,7 +138,6 @@ func CallCode(env vm.Environment, caller vm.ContractRef, addr common.Address, in
 
 // DelegateCall is equivalent to CallCode except that sender and value propagates from parent scope to child scope
 func DelegateCall(env vm.Environment, caller vm.ContractRef, addr common.Address, input []byte, gas, gasPrice *big.Int) (ret []byte, err error) {
-	evm := env.Vm()
 	// Depth check execution. Fail if we're trying to execute above the limit.
 	if env.Depth() > callCreateDepthMax {
 		caller.ReturnGas(gas, gasPrice)
@@ -165,7 +162,7 @@ func DelegateCall(env vm.Environment, caller vm.ContractRef, addr common.Address
 	defer contract.Finalise()
 
 	// Even if the account has no code, we need to continue because it might be a precompile
-	ret, err = evm.Run(contract, input, false)
+	ret, err = env.Vm().Run(contract, input, false)
 
 	// When an error was returned by the EVM or when setting the creation code
 	// above we revert to the snapshot and consume any gas remaining. Additionally
@@ -181,7 +178,6 @@ func DelegateCall(env vm.Environment, caller vm.ContractRef, addr common.Address
 
 // StaticCall executes within the given contract and throws exception if state is attempted to be changed
 func StaticCall(env vm.Environment, caller vm.ContractRef, addr common.Address, input []byte, gas, gasPrice *big.Int) (ret []byte, err error) {
-	evm := env.Vm()
 	// Depth check execution. Fail if we're trying to execute above the limit.
 	if env.Depth() > callCreateDepthMax {
 		caller.ReturnGas(gas, gasPrice)
@@ -211,7 +207,7 @@ func StaticCall(env vm.Environment, caller vm.ContractRef, addr common.Address,
 	env.Db().AddBalance(addr, big.NewInt(0))
 
 	// Even if the account has no code, we need to continue because it might be a precompile
-	ret, err = evm.Run(contract, input, true)
+	ret, err = env.Vm().Run(contract, input, true)
 
 	// When an error was returned by the EVM or when setting the creation code
 	// above we revert to the snapshot and consume any gas remaining. Additionally
@@ -227,20 +223,6 @@ func StaticCall(env vm.Environment, caller vm.ContractRef, addr common.Address,
 
 // Create creates a new contract with the given code
 func Create(env vm.Environment, caller vm.ContractRef, code []byte, gas, gasPrice, value *big.Int) (ret []byte, address common.Address, err error) {
-	ret, address, err = create(env, caller, crypto.Keccak256Hash(code), code, gas, gasPrice, value)
-	// Here we get an error if we run into maximum stack depth,
-	// See: https://github.com/ethereum/yellowpaper/pull/131
-	// and YP definitions for CREATE
-
-	//if there's an error we return nothing
-	if err != nil && err != vm.ErrRevert {
-		return nil, address, err
-	}
-	return ret, address, err
-}
-
-func create(env vm.Environment, caller vm.ContractRef, codeHash common.Hash, code []byte, gas, gasPrice, value *big.Int) (ret []byte, address common.Address, err error) {
-	evm := env.Vm()
 	// Depth check execution. Fail if we're trying to execute above the limit.
 	if env.Depth() > callCreateDepthMax {
 		caller.ReturnGas(gas, gasPrice)
@@ -281,10 +263,10 @@ func create(env vm.Environment, caller vm.ContractRef, codeHash common.Hash, cod
 	// EVM. The contract is a scoped environment for this execution context
 	// only.
 	contract := vm.NewContract(caller, to, value, gas, gasPrice)
-	contract.SetCallCode(nil, codeHash, code)
+	contract.SetCallCode(nil, crypto.Keccak256Hash(code), code)
 	defer contract.Finalise()
 
-	ret, err = evm.Run(contract, nil, false)
+	ret, err = env.Vm().Run(contract, nil, false)
 
 	maxCodeSizeExceeded := len(ret) > maxCodeSize && env.RuleSet().IsAtlantis(env.BlockNumber())
 	// if the contract creation ran successfully and no errors were returned
@@ -317,6 +299,10 @@ func create(env vm.Environment, caller vm.ContractRef, codeHash common.Hash, cod
 		err = errMaxCodeSizeExceeded
 	}
 
+	//if there's an error we return nothing
+	if err != nil && err != vm.ErrRevert {
+		return nil, address, err
+	}
 	return ret, address, err
 }
 

core/state/state_object.go

@@ -386,18 +386,27 @@ func (self *StateObject) Value() *big.Int {
 	panic("Value on StateObject should never be called")
 }
 
-func (self *StateObject) ForEachStorage(cb func(key, value common.Hash) bool) {
-	// When iterating over the storage check the cache first
-	for h, value := range self.cachedStorage {
-		cb(h, value)
-	}
-
-	it := trie.NewIterator(self.getTrie(self.db.db).NodeIterator(nil))
+func (s *StateObject) ForEachStorage(cb func(key, value common.Hash) bool) error {
+	it := trie.NewIterator(s.getTrie(s.db.db).NodeIterator(nil))
 	for it.Next() {
 		// ignore cached values
-		key := common.BytesToHash(self.trie.GetKey(it.Key))
-		if _, ok := self.cachedStorage[key]; !ok {
-			cb(key, common.BytesToHash(it.Value))
+		key := common.BytesToHash(s.trie.GetKey(it.Key))
+		if value, dirty := s.dirtyStorage[key]; dirty {
+			if cb(key, value) {
+				return nil
+			}
+			continue
+		}
+
+		if len(it.Value) > 0 {
+			_, content, _, err := rlp.Split(it.Value)
+			if err != nil {
+				return err
+			}
+			if !cb(key, common.BytesToHash(content)) {
+				return nil
+			}
 		}
 	}
+	return nil
 }

core/state/statedb.go

@@ -481,8 +481,12 @@ func (self *StateDB) Copy() *StateDB {
 	}
 
 	for hash, logs := range self.logs {
-		state.logs[hash] = make(vm.Logs, len(logs))
-		copy(state.logs[hash], logs)
+		cpy := make([]*vm.Log, len(logs))
+		for i, l := range logs {
+			cpy[i] = new(vm.Log)
+			*cpy[i] = *l
+		}
+		state.logs[hash] = cpy
 	}
 	for hash, preimage := range self.preimages {
 		state.preimages[hash] = preimage
@@ -558,27 +562,6 @@ func (s *StateDB) Finalise(deleteEmptyObjects bool) {
 	s.clearJournalAndRefund()
 }
 
-// DeleteSuicides flags the suicided objects for deletion so that it
-// won't be referenced again when called / queried up on.
-//
-// DeleteSuicides should not be used for consensus related updates
-// under any circumstances.
-func (s *StateDB) DeleteSuicides() {
-	// Reset refund so that any used-gas calculations can use this method.
-	s.clearJournalAndRefund()
-
-	for addr := range s.stateObjectsDirty {
-		stateObject := s.stateObjects[addr]
-
-		// If the object has been removed by a suicide
-		// flag the object as deleted.
-		if stateObject.suicided {
-			stateObject.deleted = true
-		}
-		delete(s.stateObjectsDirty, addr)
-	}
-}
-
 func (s *StateDB) clearJournalAndRefund() {
 	s.journal = newJournal()
 	s.validRevisions = s.validRevisions[:0]
@@ -624,23 +607,32 @@ func (s *StateDB) CommitTo(dbw trie.DatabaseWriter, deleteEmptyObjects bool) (ro
 	return root, err
 }
 
-func (db *StateDB) ForEachStorage(addr common.Address, cb func(key, value common.Hash) bool) {
+func (db *StateDB) ForEachStorage(addr common.Address, cb func(key, value common.Hash) bool) error {
 	so := db.getStateObject(addr)
 	if so == nil {
-		return
-	}
-
-	// When iterating over the storage check the cache first
-	for h, value := range so.cachedStorage {
-		cb(h, value)
+		return nil
 	}
 
 	it := trie.NewIterator(so.getTrie(db.db).NodeIterator(nil))
 	for it.Next() {
 		// ignore cached values
 		key := common.BytesToHash(db.trie.GetKey(it.Key))
-		if _, ok := so.cachedStorage[key]; !ok {
-			cb(key, common.BytesToHash(it.Value))
+		if value, dirty := so.dirtyStorage[key]; dirty {
+			if cb(key, value) {
+				return nil
+			}
+			continue
+		}
+
+		if len(it.Value) > 0 {
+			_, content, _, err := rlp.Split(it.Value)
+			if err != nil {
+				return err
+			}
+			if !cb(key, common.BytesToHash(content)) {
+				return nil
+			}
 		}
 	}
+	return nil
 }

core/state_processor.go

@@ -87,15 +87,15 @@ func (p *StateProcessor) Process(block *types.Block, statedb *state.StateDB) (ty
 		if UseSputnikVM != "true" {
 			receipt, logs, _, err := ApplyTransaction(p.config, p.bc, gp, statedb, header, tx, totalUsedGas)
 			if err != nil {
-				return nil, nil, totalUsedGas, err
+				return nil, nil, nil, err
 			}
 			receipts = append(receipts, receipt)
 			allLogs = append(allLogs, logs...)
 			continue
 		}
 		receipt, logs, _, err := ApplyMultiVmTransaction(p.config, p.bc, gp, statedb, header, tx, totalUsedGas)
 		if err != nil {
-			return nil, nil, totalUsedGas, err
+			return nil, nil, nil, err
 		}
 		receipts = append(receipts, receipt)
 		allLogs = append(allLogs, logs...)

core/tx_pool.go

@@ -297,7 +297,7 @@ func (pool *TxPool) validateTx(tx *types.Transaction) (e error) {
 		return
 	}
 
-	intrGas := IntrinsicGas(tx.Data(), MessageCreatesContract(tx), pool.homestead)
+	intrGas := IntrinsicGas(tx.Data(), tx.To() == nil, pool.homestead)
 	if tx.Gas().Cmp(intrGas) < 0 {
 		e = ErrIntrinsicGas
 		return

core/vm/contract.go

@@ -28,7 +28,7 @@ type ContractRef interface {
 	Address() common.Address
 	Value() *big.Int
 	SetCode(common.Hash, []byte)
-	ForEachStorage(callback func(key, value common.Hash) bool)
+	ForEachStorage(callback func(key, value common.Hash) bool) error
 }
 
 // Contract represents an ethereum contract in the state database. It contains
@@ -159,8 +159,8 @@ func (self *Contract) SetCallCode(addr *common.Address, hash common.Hash, code [
 
 // EachStorage iterates the contract's storage and calls a method for every key
 // value pair.
-func (self *Contract) ForEachStorage(cb func(key, value common.Hash) bool) {
-	self.caller.ForEachStorage(cb)
+func (self *Contract) ForEachStorage(cb func(key, value common.Hash) bool) error {
+	return self.caller.ForEachStorage(cb)
 }
 
 func (c *Contract) isValidJump(pc *uint64, to *big.Int) bool {

core/vm/environment.go

@@ -134,6 +134,6 @@ type Account interface {
 	Address() common.Address
 	ReturnGas(*big.Int, *big.Int)
 	SetCode(common.Hash, []byte)
-	ForEachStorage(cb func(key, value common.Hash) bool)
+	ForEachStorage(cb func(key, value common.Hash) bool) error
 	Value() *big.Int
 }

eth/api.go

@@ -568,14 +568,14 @@ type PublicBlockChainAPI struct {
 // NewPublicBlockChainAPI creates a new Etheruem blockchain API.
 func NewPublicBlockChainAPI(config *core.ChainConfig, bc *core.BlockChain, m *miner.Miner, chainDb ethdb.Database, gpo *GasPriceOracle, eventMux *event.TypeMux, am *accounts.Manager) *PublicBlockChainAPI {
 	api := &PublicBlockChainAPI{
-		config:   config,
-		bc:       bc,
-		miner:    m,
-		chainDb:  chainDb,
-		eventMux: eventMux,
-		am:       am,
+		config:                config,
+		bc:                    bc,
+		miner:                 m,
+		chainDb:               chainDb,
+		eventMux:              eventMux,
+		am:                    am,
 		newBlockSubscriptions: make(map[string]func(core.ChainEvent) error),
-		gpo: gpo,
+		gpo:                   gpo,
 	}
 
 	go api.subscriptionLoop()
@@ -2154,7 +2154,7 @@ func (s *PublicDebugAPI) computeTxEnv(blockHash common.Hash, txIndex int) (core.
 		if err != nil {
 			return nil, nil, fmt.Errorf("tx %x failed: %v", tx.Hash(), err)
 		}
-		statedb.DeleteSuicides()
+		statedb.Finalise(true)
 	}
 	return nil, nil, fmt.Errorf("tx index %d out of range for block %x", txIndex, blockHash)
 }