[EC-310] Implement EIP-684

src/ets/scala/io/iohk/ethereum/ets/blockchain/BlockchainSuite.scala

@@ -11,14 +11,7 @@ class BlockchainSuite extends FreeSpec with Matchers with Logger {
   val supportedNetworks = Set("EIP150", "Frontier", "FrontierToHomesteadAt5", "Homestead", "HomesteadToEIP150At5", "HomesteadToDaoAt5", "EIP158")
 
   //Map of ignored tests, empty set of ignored names means cancellation of whole group
-  val ignoredTests: Map[String, Set[String]] = Map(
-    // they are failing on older version and success on fresh on. Blockchain test suite should be updated
-    // after introduction of EIP684.
-    "TransitionTests/bcHomesteadToDao/DaoTransactions" -> Set.empty,
-    "TransitionTests/bcHomesteadToDao/DaoTransactions_EmptyTransactionAndForkBlocksAhead" -> Set.empty,
-    "TransitionTests/bcHomesteadToDao/DaoTransactions_UncleExtradata" -> Set.empty,
-    "TransitionTests/bcHomesteadToDao/DaoTransactions_XBlockm1" -> Set.empty
-  )
+  val ignoredTests: Map[String, Set[String]] = Map()
 
   override def run(testName: Option[String], args: Args): Status = {
     val options = TestOptions(args.configMap)

src/ets/scala/io/iohk/ethereum/ets/vm/VMSuite.scala

@@ -1,10 +1,14 @@
 package io.iohk.ethereum.ets.vm
 
+import akka.util.ByteString
 import io.iohk.ethereum.domain.TxLogEntry
 import io.iohk.ethereum.ets.common.TestOptions
+import io.iohk.ethereum.network.p2p.messages.PV63.TxLogEntryImplicits._
+import io.iohk.ethereum.rlp._
 import io.iohk.ethereum.utils.Logger
 import io.iohk.ethereum.vm.MockWorldState._
 import io.iohk.ethereum.vm._
+import io.iohk.ethereum.crypto.kec256
 import org.scalatest._
 
 class VMSuite extends FreeSpec with Matchers with Logger {
@@ -65,9 +69,8 @@ class VMSuite extends FreeSpec with Matchers with Logger {
       deadAccounts.foreach(addr => result.world.isAccountDead(addr) shouldBe true)
     }
 
-    scenario.logs.foreach { logs =>
-      val expectedLogs = logs.map(l => TxLogEntry(l.address, l.topics, l.data))
-      result.logs shouldEqual expectedLogs
+    scenario.logs.foreach { expectedLogHash =>
+      hashLogs(result.logs) shouldEqual expectedLogHash
     }
 
     scenario.callcreates.foreach { callcreates =>
@@ -91,4 +94,9 @@ class VMSuite extends FreeSpec with Matchers with Logger {
     result.copy(world = worldAfterDel)
   }
 
+  private def hashLogs(logs: Seq[TxLogEntry]): ByteString = {
+    val rlpLogs = RLPList(logs.map(_.toRLPEncodable): _*)
+    ByteString(kec256(encode(rlpLogs)))
+  }
+
 }

src/ets/scala/io/iohk/ethereum/ets/vm/scenario.scala

@@ -10,7 +10,7 @@ case class VMScenario(
   callcreates: Option[List[CallCreate]],
   pre: Map[Address, AccountState],
   post: Option[Map[Address, AccountState]],
-  logs: Option[List[LogEntry]],
+  logs: Option[ByteString],
   gas: Option[BigInt],
   out: Option[ByteString]
 )
@@ -42,9 +42,3 @@ case class CallCreate(
   value: BigInt
 )
 
-case class LogEntry(
-  address: Address,
-  data: ByteString,
-  topics: List[ByteString]
-)
-

src/main/scala/io/iohk/ethereum/domain/Account.scala

@@ -42,15 +42,18 @@ case class Account(
   def withStorage(storageRoot: ByteString): Account =
     copy(storageRoot = storageRoot)
 
-  def resetAccountPreservingBalance(startNonce: UInt256 = UInt256.Zero): Account =
-    copy(nonce = startNonce, storageRoot = Account.EmptyStorageRootHash, codeHash = Account.EmptyCodeHash)
-
   /**
     * According to EIP161: An account is considered empty when it has no code and zero nonce and zero balance.
     * An account's storage is not relevant when determining emptiness.
     */
-  def isEmpty: Boolean =
-    nonce == UInt256.Zero && balance == UInt256.Zero && codeHash == Account.EmptyCodeHash
+  def isEmpty(startNonce: UInt256 = UInt256.Zero): Boolean =
+    nonce == startNonce && balance == UInt256.Zero && codeHash == Account.EmptyCodeHash
+
+  /**
+    * Under EIP-684 if this evaluates to true then we have a conflict when creating a new account
+    */
+  def nonEmptyCodeOrNonce(startNonce: UInt256 = UInt256.Zero): Boolean =
+    nonce != startNonce || codeHash != Account.EmptyCodeHash
 
   override def toString: String =
     s"Account(nonce: $nonce, balance: $balance, " +

src/main/scala/io/iohk/ethereum/ledger/InMemoryWorldStateProxy.scala

@@ -140,7 +140,7 @@ class InMemoryWorldStateProxy private[ledger](
   // Account's code by Address
   val accountCodes: Map[Address, Code],
   val getBlockByNumber: (BigInt) => Option[ByteString],
-  accountStartNonce: UInt256,
+  override val accountStartNonce: UInt256,
   // touchedAccounts and noEmptyAccountsCond are introduced by EIP161 to track accounts touched during the transaction
   // execution. Touched account are only added to Set if noEmptyAccountsCond == true, otherwise all other operations
   // operate on empty set.

src/main/scala/io/iohk/ethereum/ledger/Ledger.scala

@@ -411,7 +411,7 @@ class LedgerImpl(
 
       validatedStx match {
         case Right(_) =>
-          val TxResult(newWorld, gasUsed, logs, _) = executeTransaction(stx, blockHeader, worldForTx)
+          val TxResult(newWorld, gasUsed, logs, _, _) = executeTransaction(stx, blockHeader, worldForTx)
 
           val receipt = Receipt(
             postTransactionStateHash = newWorld.stateRootHash,
@@ -447,7 +447,7 @@ class LedgerImpl(
 
     val totalGasToRefund = calcTotalGasToRefund(stx, result)
 
-    TxResult(result.world, gasLimit - totalGasToRefund, result.logs, result.returnData)
+    TxResult(result.world, gasLimit - totalGasToRefund, result.logs, result.returnData, result.error)
   }
 
   private[ledger] def executeTransaction(stx: SignedTransaction, blockHeader: BlockHeader, world: InMemoryWorldStateProxy): TxResult = {
@@ -461,7 +461,7 @@ class LedgerImpl(
     val result = runVM(stx, context, config)
 
     val resultWithErrorHandling: PR =
-      if(result.error.isDefined) {
+      if (result.error.isDefined) {
         //Rollback to the world before transfer was done if an error happened
         result.copy(world = checkpointWorldState, addressesToDelete = Set.empty, logs = Nil)
       } else
@@ -487,7 +487,7 @@ class LedgerImpl(
          | - Total Gas to Refund: $totalGasToRefund
          | - Execution gas paid to miner: $executionGasToPayToMiner""".stripMargin)
 
-    TxResult(world2, executionGasToPayToMiner, resultWithErrorHandling.logs, result.returnData)
+    TxResult(world2, executionGasToPayToMiner, resultWithErrorHandling.logs, result.returnData, result.error)
   }
 
   private def validateBlockBeforeExecution(block: Block): Either[BlockExecutionError, Unit] = {
@@ -584,15 +584,21 @@ class LedgerImpl(
     worldStateProxy.saveAccount(senderAddress, account.increaseBalance(-calculateUpfrontGas(stx.tx)).increaseNonce())
   }
 
-  private def prepareProgramContext(stx: SignedTransaction, blockHeader: BlockHeader, worldStateProxy: InMemoryWorldStateProxy,
-                                            config: EvmConfig): PC = {
+  private def prepareProgramContext(stx: SignedTransaction, blockHeader: BlockHeader, world: InMemoryWorldStateProxy,
+      config: EvmConfig): PC = {
     stx.tx.receivingAddress match {
       case None =>
-        val address = worldStateProxy.createAddress(creatorAddr = stx.senderAddress)
-        val worldAfterInitialisation = worldStateProxy.initialiseAccount(stx.senderAddress, address, UInt256(stx.tx.value))
-        ProgramContext(stx, address,  Program(stx.tx.payload), blockHeader, worldAfterInitialisation, config)
+        val address = world.createAddress(creatorAddr = stx.senderAddress)
+
+        // EIP-684
+        val conflict = world.nonEmptyCodeOrNonceAccount(address)
+        val code = if (conflict) ByteString(INVALID.code) else stx.tx.payload
+
+        val world1 = world.initialiseAccount(address).transfer(stx.senderAddress, address, UInt256(stx.tx.value))
+        ProgramContext(stx, address,  Program(code), blockHeader, world1, config)
+
       case Some(txReceivingAddress) =>
-        val world1 = worldStateProxy.transfer(stx.senderAddress, txReceivingAddress, UInt256(stx.tx.value))
+        val world1 = world.transfer(stx.senderAddress, txReceivingAddress, UInt256(stx.tx.value))
         ProgramContext(stx, txReceivingAddress, Program(world1.getCode(txReceivingAddress)), blockHeader, world1, config)
     }
   }
@@ -702,7 +708,7 @@ class LedgerImpl(
     */
   private[ledger] def deleteEmptyTouchedAccounts(world: InMemoryWorldStateProxy): InMemoryWorldStateProxy = {
     def deleteEmptyAccount(world: InMemoryWorldStateProxy, address: Address) = {
-      if (world.getAccount(address).exists(_.isEmpty))
+      if (world.getAccount(address).exists(_.isEmpty(blockchainConfig.accountStartNonce)))
         world.deleteAccount(address)
       else
         world
@@ -720,7 +726,8 @@ object Ledger {
 
   case class BlockResult(worldState: InMemoryWorldStateProxy, gasUsed: BigInt = 0, receipts: Seq[Receipt] = Nil)
   case class BlockPreparationResult(block: Block, blockResult: BlockResult, stateRootHash: ByteString)
-  case class TxResult(worldState: InMemoryWorldStateProxy, gasUsed: BigInt, logs: Seq[TxLogEntry], vmReturnData: ByteString)
+  case class TxResult(worldState: InMemoryWorldStateProxy, gasUsed: BigInt, logs: Seq[TxLogEntry],
+    vmReturnData: ByteString, vmError: Option[ProgramError])
 }
 
 sealed trait BlockExecutionError{

src/main/scala/io/iohk/ethereum/vm/OpCode.scala

@@ -682,14 +682,17 @@ abstract class CreateOp extends OpCode(0xf0, 3, 1, _.G_create) {
 
       val (initCode, memory1) = state.memory.load(inOffset, inSize)
       val (newAddress, world1) = state.world.createAddressWithOpCode(state.env.ownerAddr)
+      val world2 = world1.initialiseAccount(newAddress).transfer(state.env.ownerAddr, newAddress, endowment)
 
-      val worldAfterInitialisation = world1.initialiseAccount(state.env.ownerAddr, newAddress, endowment)
+      // EIP-684
+      val conflict = state.world.nonEmptyCodeOrNonceAccount(newAddress)
+      val code = if (conflict) ByteString(INVALID.code) else initCode
 
       val newEnv = state.env.copy(
         callerAddr = state.env.ownerAddr,
         ownerAddr = newAddress,
         value = endowment,
-        program = Program(initCode),
+        program = Program(code),
         inputData = ByteString.empty,
         callDepth = state.env.callDepth + 1
       )
@@ -699,7 +702,7 @@ abstract class CreateOp extends OpCode(0xf0, 3, 1, _.G_create) {
       val availableGas = state.gas - (constGasFn(state.config.feeSchedule) + varGas(state))
       val startGas = state.config.gasCap(availableGas)
 
-      val context = ProgramContext[W, S](newEnv, newAddress, startGas, worldAfterInitialisation, state.config, state.addressesToDelete)
+      val context = ProgramContext[W, S](newEnv, newAddress, startGas, world2, state.config, state.addressesToDelete)
       val result = VM.run(context)
 
       val contractCode = result.returnData

src/main/scala/io/iohk/ethereum/vm/WorldStateProxy.scala

@@ -23,7 +23,10 @@ trait WorldStateProxy[WS <: WorldStateProxy[WS, S], S <: Storage[S]] { self: WS
   protected def touchAccounts(addresses: Address*): WS
   protected def clearTouchedAccounts: WS
   protected def noEmptyAccounts: Boolean
+  protected def accountStartNonce: UInt256 = UInt256.Zero
+
   def combineTouchedAccounts(world: WS): WS
+
   /**
     * In certain situation an account is guaranteed to exist, e.g. the account that executes the code, the account that
     * transfer value to another. There could be no input to our application that would cause this fail, so we shouldn't
@@ -53,6 +56,7 @@ trait WorldStateProxy[WS <: WorldStateProxy[WS, S], S <: Storage[S]] { self: WS
     if (from == to ||  isZeroValueTransferToNonExistentAccount(to, value))
       touchAccounts(from)
     else
+      // perhaps as an optimisation we could avoid touching accounts having non-zero nonce or non-empty code
       guaranteedTransfer(from, to, value).touchAccounts(from, to)
   }
 
@@ -63,16 +67,17 @@ trait WorldStateProxy[WS <: WorldStateProxy[WS, S], S <: Storage[S]] { self: WS
   }
 
   /**
-    * Method for creating new account and transferring value to it, that handles possible address collisions.
+    * IF EIP-161 is in effect this sets new contract's account initial nonce to 1 over the default value
+    * for the given network (usually zero)
+    * Otherwise it's no-op
     */
-  def initialiseAccount(creatorAddress: Address, newAddress: Address, value: UInt256): WS = {
-    val nonceOffset = if (noEmptyAccounts) 1 else 0
-
-    val creatorAccount = getGuaranteedAccount(creatorAddress).increaseBalance(-value)
-    val newAccount = getAccount(newAddress).getOrElse(getEmptyAccount)
-      .resetAccountPreservingBalance().increaseBalance(value)
-      .increaseNonce(nonceOffset)
-    saveAccount(creatorAddress,creatorAccount).saveAccount(newAddress, newAccount).touchAccounts(creatorAddress, newAddress)
+  def initialiseAccount(newAddress: Address): WS = {
+    if (!noEmptyAccounts)
+      this
+    else {
+      val newAccount = getAccount(newAddress).getOrElse(getEmptyAccount).copy(nonce = accountStartNonce + 1)
+      saveAccount(newAddress, newAccount)
+    }
   }
 
   /**
@@ -117,9 +122,12 @@ trait WorldStateProxy[WS <: WorldStateProxy[WS, S], S <: Storage[S]] { self: WS
     * @return true if account is dead, false otherwise
     */
   def isAccountDead(address: Address): Boolean = {
-    getAccount(address).forall(_.isEmpty)
+    getAccount(address).forall(_.isEmpty(accountStartNonce))
   }
 
+  def nonEmptyCodeOrNonceAccount(address: Address): Boolean =
+    getAccount(address).exists(_.nonEmptyCodeOrNonce(accountStartNonce))
+
   def isZeroValueTransferToNonExistentAccount(address: Address, value: UInt256): Boolean =
     noEmptyAccounts && value == UInt256(0) && !accountExists(address)
 }

src/test/scala/io/iohk/ethereum/jsonrpc/EthServiceSpec.scala

@@ -405,7 +405,7 @@ class EthServiceSpec extends FlatSpec with Matchers with ScalaFutures with MockF
     blockchain.save(blockToRequest)
     (appStateStorage.getBestBlockNumber _).expects().returning(blockToRequest.header.number)
 
-    val txResult = TxResult(BlockchainImpl(storagesInstance.storages).getWorldStateProxy(-1, UInt256.Zero, None), 123, Nil, ByteString("return_value"))
+    val txResult = TxResult(BlockchainImpl(storagesInstance.storages).getWorldStateProxy(-1, UInt256.Zero, None), 123, Nil, ByteString("return_value"), None)
     (ledger.simulateTransaction _).expects(*, *).returning(txResult)
 
     val tx = CallTx(
@@ -421,7 +421,7 @@ class EthServiceSpec extends FlatSpec with Matchers with ScalaFutures with MockF
     blockchain.save(blockToRequest)
     (appStateStorage.getBestBlockNumber _).expects().returning(blockToRequest.header.number)
 
-    val txResult = TxResult(BlockchainImpl(storagesInstance.storages).getWorldStateProxy(-1, UInt256.Zero, None), 123, Nil, ByteString("return_value"))
+    val txResult = TxResult(BlockchainImpl(storagesInstance.storages).getWorldStateProxy(-1, UInt256.Zero, None), 123, Nil, ByteString("return_value"), None)
     (ledger.simulateTransaction _).expects(*, *).returning(txResult)
 
     val tx = CallTx(