From b9ada212db5ae6fae323b6e12a76bfc0ce0e89eb Mon Sep 17 00:00:00 2001 From: Radek Tkaczyk Date: Mon, 9 Oct 2017 13:42:12 +0200 Subject: [PATCH 1/3] Implement EIP-684 --- .../ets/blockchain/BlockchainSuite.scala | 9 +- .../io/iohk/ethereum/domain/Account.scala | 13 +- .../ledger/InMemoryWorldStateProxy.scala | 2 +- .../io/iohk/ethereum/ledger/Ledger.scala | 31 +++-- .../scala/io/iohk/ethereum/vm/OpCode.scala | 9 +- .../io/iohk/ethereum/vm/WorldStateProxy.scala | 28 ++-- .../ethereum/jsonrpc/EthServiceSpec.scala | 4 +- .../ledger/ContractCreationSpec.scala | 120 +++++++++++++++++- .../ledger/DeleteTouchedAccountsSpec.scala | 37 +----- .../ledger/InMemoryWorldStateProxySpec.scala | 90 +++---------- .../iohk/ethereum/vm/CreateOpcodeSpec.scala | 49 +++++++ 11 files changed, 239 insertions(+), 153 deletions(-) diff --git a/src/ets/scala/io/iohk/ethereum/ets/blockchain/BlockchainSuite.scala b/src/ets/scala/io/iohk/ethereum/ets/blockchain/BlockchainSuite.scala index 702869028f..ff795609c0 100644 --- a/src/ets/scala/io/iohk/ethereum/ets/blockchain/BlockchainSuite.scala +++ b/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) diff --git a/src/main/scala/io/iohk/ethereum/domain/Account.scala b/src/main/scala/io/iohk/ethereum/domain/Account.scala index 411aec6762..e49fae37b7 100644 --- a/src/main/scala/io/iohk/ethereum/domain/Account.scala +++ b/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, " + diff --git a/src/main/scala/io/iohk/ethereum/ledger/InMemoryWorldStateProxy.scala b/src/main/scala/io/iohk/ethereum/ledger/InMemoryWorldStateProxy.scala index d5d775f0c5..380235a94b 100644 --- a/src/main/scala/io/iohk/ethereum/ledger/InMemoryWorldStateProxy.scala +++ b/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. diff --git a/src/main/scala/io/iohk/ethereum/ledger/Ledger.scala b/src/main/scala/io/iohk/ethereum/ledger/Ledger.scala index 802e24cddf..667c397b00 100644 --- a/src/main/scala/io/iohk/ethereum/ledger/Ledger.scala +++ b/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{ diff --git a/src/main/scala/io/iohk/ethereum/vm/OpCode.scala b/src/main/scala/io/iohk/ethereum/vm/OpCode.scala index 94424e58a2..73a0b70ea7 100644 --- a/src/main/scala/io/iohk/ethereum/vm/OpCode.scala +++ b/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 diff --git a/src/main/scala/io/iohk/ethereum/vm/WorldStateProxy.scala b/src/main/scala/io/iohk/ethereum/vm/WorldStateProxy.scala index 6907be40d9..056324ca3d 100644 --- a/src/main/scala/io/iohk/ethereum/vm/WorldStateProxy.scala +++ b/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) } diff --git a/src/test/scala/io/iohk/ethereum/jsonrpc/EthServiceSpec.scala b/src/test/scala/io/iohk/ethereum/jsonrpc/EthServiceSpec.scala index d3908a6098..960887cb8b 100644 --- a/src/test/scala/io/iohk/ethereum/jsonrpc/EthServiceSpec.scala +++ b/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( diff --git a/src/test/scala/io/iohk/ethereum/ledger/ContractCreationSpec.scala b/src/test/scala/io/iohk/ethereum/ledger/ContractCreationSpec.scala index 30e61e964d..46fa72f5a5 100644 --- a/src/test/scala/io/iohk/ethereum/ledger/ContractCreationSpec.scala +++ b/src/test/scala/io/iohk/ethereum/ledger/ContractCreationSpec.scala @@ -6,7 +6,7 @@ import io.iohk.ethereum.Mocks import io.iohk.ethereum.Mocks.MockVM import io.iohk.ethereum.blockchain.sync.EphemBlockchainTestSetup import io.iohk.ethereum.crypto.{generateKeyPair, kec256} -import io.iohk.ethereum.domain.{Address, BlockchainImpl, TxLogEntry, UInt256} +import io.iohk.ethereum.domain._ import io.iohk.ethereum.ledger.Ledger.PR import io.iohk.ethereum.nodebuilder.SecureRandomBuilder import io.iohk.ethereum.utils.Config.SyncConfig @@ -38,9 +38,9 @@ class ContractCreationSpec extends FlatSpec with PropertyChecks with Matchers { error = error ) - it should "return an error if it's size is larger than the limit" in new TestSetup { + "Ledger" should "return an error if new contract's code size is larger than the limit" in new TestSetup { val longContractCode = ByteString(Array.fill(codeSizeLimit + 1)(1.toByte)) - val resultBeforeSaving = createResult(emptyWorld, gasUsed = defaultGasLimit / 2, + val resultBeforeSaving = createResult(emptyWorld(), gasUsed = defaultGasLimit / 2, gasLimit = defaultGasLimit, gasRefund = 0, error = None, returnData = longContractCode) val ledger = new LedgerImpl(new MockVM(), blockchain, blockchainConfig, syncConfig, Mocks.MockValidatorsAlwaysSucceed) @@ -48,9 +48,9 @@ class ContractCreationSpec extends FlatSpec with PropertyChecks with Matchers { resultAfterSaving.error shouldBe Some(OutOfGas) } - it should "not return an error if it's size is smaller than the limit" in new TestSetup { + it should "not return an error if new contract's code size is smaller than the limit" in new TestSetup { val shortContractCode = ByteString(Array.fill(codeSizeLimit - 1)(1.toByte)) - val resultBeforeSaving = createResult(emptyWorld, gasUsed = defaultGasLimit / 2, + val resultBeforeSaving = createResult(emptyWorld(), gasUsed = defaultGasLimit / 2, gasLimit = defaultGasLimit, gasRefund = 0, error = None, returnData = shortContractCode) val ledger = new LedgerImpl(new MockVM(), blockchain, blockchainConfig, syncConfig, Mocks.MockValidatorsAlwaysSucceed) @@ -58,6 +58,68 @@ class ContractCreationSpec extends FlatSpec with PropertyChecks with Matchers { resultAfterSaving.error shouldBe None } + it should "fail to execute contract creation code in case of address conflict (non-empty code)" in + new TestSetup with ContractCreatingTx { + val ledger = new LedgerImpl(VM, blockchain, blockchainConfig, syncConfig, validators) + val world = worldWithCreator.saveAccount(newAddress, accountNonEmptyCode) + val result = ledger.executeTransaction(stx, blockHeader, world) + + result.vmError shouldEqual Some(InvalidOpCode(INVALID.code)) + result.worldState.getGuaranteedAccount(newAddress) shouldEqual accountNonEmptyCode + } + + it should "fail to execute contract creation code in case of address conflict (non-zero nonce)" in + new TestSetup with ContractCreatingTx { + val ledger = new LedgerImpl(VM, blockchain, blockchainConfig, syncConfig, validators) + val world = worldWithCreator.saveAccount(newAddress, accountNonZeroNonce) + val result = ledger.executeTransaction(stx, blockHeader, world) + + result.vmError shouldEqual Some(InvalidOpCode(INVALID.code)) + result.worldState.getGuaranteedAccount(newAddress) shouldEqual accountNonZeroNonce + } + + it should "succeed in creating a contract if the account already has some balance, but zero nonce and empty code" in + new TestSetup with ContractCreatingTx { + val ledger = new LedgerImpl(VM, blockchain, blockchainConfig, syncConfig, validators) + val world = worldWithCreator.saveAccount(newAddress, accountNonZeroBalance) + val result = ledger.executeTransaction(stx, blockHeader, world) + + result.vmError shouldEqual None + + val newContract = result.worldState.getGuaranteedAccount(newAddress) + newContract.balance shouldEqual (transferValue + accountNonZeroBalance.balance) + newContract.nonce shouldEqual accountNonZeroBalance.nonce + newContract.codeHash should not equal Account.EmptyCodeHash + + result.worldState.getCode(newAddress) shouldEqual ByteString(STOP.code) + } + + it should "initialise a new contract account with zero nonce before EIP-161" in + new TestSetup with ContractCreatingTx { + val ledger = new LedgerImpl(VM, blockchain, blockchainConfig, syncConfig, validators) + val result = ledger.executeTransaction(stx, blockHeader, worldWithCreator) + + result.vmError shouldEqual None + + val newContract = result.worldState.getGuaranteedAccount(newAddress) + newContract.balance shouldEqual transferValue + newContract.nonce shouldEqual UInt256.Zero + result.worldState.getCode(newAddress) shouldEqual ByteString(STOP.code) + } + + it should "initialise a new contract account with incremented nonce after EIP-161" in + new TestSetup with ContractCreatingTx { + val ledger = new LedgerImpl(VM, blockchain, blockchainConfig, syncConfig, validators) + val result = ledger.executeTransaction(stx, blockHeader, worldWithCreatorAfterEip161) + + result.vmError shouldEqual None + + val newContract = result.worldState.getGuaranteedAccount(newAddress) + newContract.balance shouldEqual transferValue + newContract.nonce shouldEqual UInt256.One + result.worldState.getCode(newAddress) shouldEqual ByteString(STOP.code) + } + trait TestSetup extends EphemBlockchainTestSetup with SecureRandomBuilder { val keyPair: AsymmetricCipherKeyPair = generateKeyPair(secureRandom) val contractAddress = Address(kec256(keyPair.getPublic.asInstanceOf[ECPublicKeyParameters].getQ.getEncoded(false).tail)) @@ -66,7 +128,8 @@ class ContractCreationSpec extends FlatSpec with PropertyChecks with Matchers { val defaultGasLimit = 5000 val config = EvmConfig.FrontierConfigBuilder(None) - val emptyWorld = BlockchainImpl(storagesInstance.storages).getWorldStateProxy(-1, UInt256.Zero, None) + def emptyWorld(noEmptyAccounts: Boolean = false) = + BlockchainImpl(storagesInstance.storages).getWorldStateProxy(-1, UInt256.Zero, None, noEmptyAccounts) val defaultBlockchainConfig = BlockchainConfig(Config.config) val blockchainConfig = new BlockchainConfig { @@ -91,5 +154,50 @@ class ContractCreationSpec extends FlatSpec with PropertyChecks with Matchers { } val syncConfig = SyncConfig(Config.config) + + val validators = Mocks.MockValidatorsAlwaysSucceed + } + + trait ContractCreatingTx { self: TestSetup => + + val blockHeader = BlockHeader( + parentHash = bEmpty, + ommersHash = bEmpty, + beneficiary = bEmpty, + stateRoot = bEmpty, + transactionsRoot = bEmpty, + receiptsRoot = bEmpty, + logsBloom = bEmpty, + difficulty = 1000000, + number = blockchainConfig.homesteadBlockNumber + 1, + gasLimit = 10000000, + gasUsed = 0, + unixTimestamp = 0, + extraData = bEmpty, + mixHash = bEmpty, + nonce = bEmpty + ) + + // code returns a single STOP instruction + val initialisingCode = Assembly( + PUSH1, 1, + PUSH1, 0, + RETURN + ).code + + val transferValue: BigInt = 100 + val tx = Transaction(0, 1, 100000, None, transferValue, initialisingCode) + + val stx = SignedTransaction.sign(tx, keyPair, None) + + val accountNonZeroNonce = Account(nonce = 1) + val accountNonEmptyCode = Account(codeHash = ByteString("abc")) + val accountNonZeroBalance = Account(balance = 1) + + val creatorAccount = Account(nonce = 1, balance = 1000000) + val creatorAddr = stx.senderAddress + val worldWithCreator = emptyWorld().saveAccount(creatorAddr, creatorAccount) + val worldWithCreatorAfterEip161 = emptyWorld(noEmptyAccounts = true).saveAccount(creatorAddr, creatorAccount) + val newAddress = worldWithCreator.saveAccount(creatorAddr, creatorAccount.increaseNonce()).createAddress(creatorAddr) } } diff --git a/src/test/scala/io/iohk/ethereum/ledger/DeleteTouchedAccountsSpec.scala b/src/test/scala/io/iohk/ethereum/ledger/DeleteTouchedAccountsSpec.scala index 3f1c52ffd5..7bc4372f09 100644 --- a/src/test/scala/io/iohk/ethereum/ledger/DeleteTouchedAccountsSpec.scala +++ b/src/test/scala/io/iohk/ethereum/ledger/DeleteTouchedAccountsSpec.scala @@ -85,39 +85,14 @@ class DeleteTouchedAccountsSpec extends FlatSpec with Matchers with MockFactory newWorld.touchedAccounts.size shouldEqual 0 } - it should "delete multiple touched empty accounts more operations" in new TestSetup { - val worldAfterTransfer = worldStatePostEIP161.transfer(validAccountAddress3, validEmptyAccountAddress, zeroTransferBalance) + it should "not delete touched new account resulting from contract creation (initialised)" in new TestSetup { + val worldAfterInitAndTransfer = + worldStatePostEIP161.initialiseAccount(validCreatedAccountAddress) + .transfer(validAccountAddress, validCreatedAccountAddress, zeroTransferBalance) - worldAfterTransfer.touchedAccounts.size shouldEqual 2 - - val worldAfterPayingToMiner = ledger.pay(validEmptyAccountAddress1, zeroTransferBalance)(worldAfterTransfer) - - worldAfterPayingToMiner.touchedAccounts.size shouldEqual 3 + worldAfterInitAndTransfer.touchedAccounts.size shouldEqual 2 - val worldafterInitialisation = - worldAfterPayingToMiner.initialiseAccount(validAccountAddress, validCreatedAccountAddress, validAccountBalance) - - worldafterInitialisation.touchedAccounts.size shouldEqual 5 - - val newWorld = InMemoryWorldStateProxy.persistState(ledger.deleteEmptyTouchedAccounts(worldafterInitialisation)) - - (accountAddresses -- Set(validEmptyAccountAddress, validEmptyAccountAddress1, validAccountAddress) + validCreatedAccountAddress) - .foreach{ a => assert(newWorld.getAccount(a).isDefined) } - - newWorld.getAccount(validEmptyAccountAddress) shouldBe None - newWorld.getAccount(validEmptyAccountAddress1) shouldBe None - newWorld.getAccount(validAccountAddress) shouldBe None - newWorld.touchedAccounts.size shouldEqual 0 - } - - - it should "not delete touched account created by message transaction or createOp" in new TestSetup { - val worldAfterTransfer = - worldStatePostEIP161.initialiseAccount(validAccountAddress, validCreatedAccountAddress, zeroTransferBalance) - - worldAfterTransfer.touchedAccounts.size shouldEqual 2 - - val newWorld = InMemoryWorldStateProxy.persistState(ledger.deleteEmptyTouchedAccounts(worldAfterTransfer)) + val newWorld = InMemoryWorldStateProxy.persistState(ledger.deleteEmptyTouchedAccounts(worldAfterInitAndTransfer)) (accountAddresses + validCreatedAccountAddress).foreach{ a => assert(newWorld.getAccount(a).isDefined) } newWorld.touchedAccounts.size shouldEqual 0 diff --git a/src/test/scala/io/iohk/ethereum/ledger/InMemoryWorldStateProxySpec.scala b/src/test/scala/io/iohk/ethereum/ledger/InMemoryWorldStateProxySpec.scala index cc77b99cbb..966566c26c 100644 --- a/src/test/scala/io/iohk/ethereum/ledger/InMemoryWorldStateProxySpec.scala +++ b/src/test/scala/io/iohk/ethereum/ledger/InMemoryWorldStateProxySpec.scala @@ -13,12 +13,12 @@ class InMemoryWorldStateProxySpec extends FlatSpec with Matchers { worldState.newEmptyAccount(address1).accountExists(address1) shouldBe true } - "InMemoryWorldStateProxy" should "allow to save and retrieve code" in new TestSetup { + it should "allow to save and retrieve code" in new TestSetup { val code = Generators.getByteStringGen(1, 100).sample.get worldState.saveCode(address1, code).getCode(address1) shouldEqual code } - "InMemoryWorldStateProxy" should "allow to save and get storage" in new TestSetup { + it should "allow to save and get storage" in new TestSetup { val addr = Generators.getUInt256Gen().sample.getOrElse(UInt256.MaxValue) val value = Generators.getUInt256Gen().sample.getOrElse(UInt256.MaxValue) @@ -29,7 +29,7 @@ class InMemoryWorldStateProxySpec extends FlatSpec with Matchers { worldState.saveStorage(address1, storage).getStorage(address1).load(addr) shouldEqual value } - "InMemoryWorldStateProxy" should "allow to transfer value to other address" in new TestSetup { + it should "allow to transfer value to other address" in new TestSetup { val account = Account(0, 100) val toTransfer = account.balance - 20 val finalWorldState = worldState @@ -41,7 +41,7 @@ class InMemoryWorldStateProxySpec extends FlatSpec with Matchers { finalWorldState.getGuaranteedAccount(address2).balance shouldEqual toTransfer } - "InMemoryWorldStateProxy" should "not store within contract store if value is zero" in new TestSetup { + it should "not store within contract store if value is zero" in new TestSetup { val account = Account(0, 100) val worldStateWithAnAccount = worldState.saveAccount(address1, account) val persistedWorldStateWithAnAccount = InMemoryWorldStateProxy.persistState(worldStateWithAnAccount) @@ -55,7 +55,7 @@ class InMemoryWorldStateProxySpec extends FlatSpec with Matchers { persistedWorldStateWithAnAccount.stateRootHash shouldEqual persistedWithContractStorageValue.stateRootHash } - "InMemoryWorldStateProxy" should "storing a zero on a contract store position should remove it from the underlying tree" in new TestSetup { + it should "storing a zero on a contract store position should remove it from the underlying tree" in new TestSetup { val account = Account(0, 100) val worldStateWithAnAccount = worldState.saveAccount(address1, account) val persistedWorldStateWithAnAccount = InMemoryWorldStateProxy.persistState(worldStateWithAnAccount) @@ -78,7 +78,7 @@ class InMemoryWorldStateProxySpec extends FlatSpec with Matchers { persistedWorldStateWithAnAccount.stateRootHash shouldEqual persistedWithZero.stateRootHash } - "InMemoryWorldStateProxy" should "be able to persist changes and continue working after that" in new TestSetup { + it should "be able to persist changes and continue working after that" in new TestSetup { val account = Account(0, 100) val addr = UInt256.Zero @@ -139,7 +139,7 @@ class InMemoryWorldStateProxySpec extends FlatSpec with Matchers { finalWorldState.getGuaranteedAccount(address1).balance shouldEqual account.balance } - "InMemoryWorldStateProxy" should "not allow transfer to create empty accounts post EIP161" in new TestSetup { + it should "not allow transfer to create empty accounts post EIP161" in new TestSetup { val account = Account(0, 100) val zeroTransfer = UInt256.Zero val nonZeroTransfer = account.balance - 20 @@ -160,34 +160,7 @@ class InMemoryWorldStateProxySpec extends FlatSpec with Matchers { secondAccount.nonce shouldEqual UInt256.Zero } - "InMemoryWorldStateProxy" should "should be able to initialise account with correct nonce" in new TestSetup { - val account = Account(0, 100) - val zeroTransfer = UInt256.Zero - val nonZeroTransfer = account.balance - 80 - - // POST EIP161 - val worldStateAfterFirstTransfer = postEIP161WorldState - .saveAccount(address1, account) - .initialiseAccount(address1, address2, zeroTransfer) - - worldStateAfterFirstTransfer.getGuaranteedAccount(address1).balance shouldEqual account.balance - - val secondAccountPostEIP161 = worldStateAfterFirstTransfer.getAccount(address2) - secondAccountPostEIP161 shouldBe defined - secondAccountPostEIP161.get.nonce shouldEqual UInt256.One - - //PRE EIP161 - val worldStateAfterSecondTransfer = worldState - .saveAccount(address1, account) - .initialiseAccount(address1, address3, nonZeroTransfer) - - val thirdAccountPreEIP161 = worldStateAfterSecondTransfer.getAccount(address3) - thirdAccountPreEIP161 shouldBe defined - thirdAccountPreEIP161.get.nonce shouldEqual UInt256.Zero - thirdAccountPreEIP161.get.balance shouldEqual nonZeroTransfer - } - - "InMemoryWorldStateProxy" should "should correctly mark touched accounts post EIP161" in new TestSetup { + it should "correctly mark touched accounts post EIP161" in new TestSetup { val account = Account(0, 100) val zeroTransfer = UInt256.Zero val nonZeroTransfer = account.balance - 80 @@ -197,16 +170,15 @@ class InMemoryWorldStateProxySpec extends FlatSpec with Matchers { .transfer(address1, address1, nonZeroTransfer) val worldStateAfterFirstTransfer = worldAfterSelfTransfer - .saveAccount(address1, account) - .initialiseAccount(address1, address2, zeroTransfer) + .transfer(address1, address2, zeroTransfer) val worldStateAfterSecondTransfer = worldStateAfterFirstTransfer .transfer(address1, address3, nonZeroTransfer) - worldStateAfterSecondTransfer.touchedAccounts should contain theSameElementsAs Set(address1, address2, address3) + worldStateAfterSecondTransfer.touchedAccounts should contain theSameElementsAs Set(address1, address3) } - "InMemoryWorldStateProxy" should "should update touched account based on oldWorld" in new TestSetup { + it should "update touched accounts using combineTouchedAccounts method" in new TestSetup { val account = Account(0, 100) val zeroTransfer = UInt256.Zero val nonZeroTransfer = account.balance - 80 @@ -217,20 +189,17 @@ class InMemoryWorldStateProxySpec extends FlatSpec with Matchers { val worldStateAfterFirstTransfer = worldAfterSelfTransfer .saveAccount(address1, account) - .initialiseAccount(address1, address2, zeroTransfer) + .transfer(address1, address2, zeroTransfer) val worldStateAfterSecondTransfer = worldStateAfterFirstTransfer .transfer(address1, address3, nonZeroTransfer) val postEip161UpdatedWorld = postEIP161WorldState.combineTouchedAccounts(worldStateAfterSecondTransfer) - val preEip161UpdatedWorld = worldState.combineTouchedAccounts(worldStateAfterSecondTransfer) - - postEip161UpdatedWorld.touchedAccounts should contain theSameElementsAs Set(address1, address2, address3) - preEip161UpdatedWorld.touchedAccounts.size shouldEqual 3 + postEip161UpdatedWorld.touchedAccounts should contain theSameElementsAs Set(address1, address3) } - "InMemoryWorldStateProxy" should "should correctly determine if account is dead" in new TestSetup { + it should "correctly determine if account is dead" in new TestSetup { val emptyAccountWorld = worldState.newEmptyAccount(address1) emptyAccountWorld.accountExists(address1) shouldBe true @@ -240,36 +209,7 @@ class InMemoryWorldStateProxySpec extends FlatSpec with Matchers { emptyAccountWorld.isAccountDead(address2) shouldBe true } - "InMemoryWorldStateProxy" should "initialise new account and handle address collision correctly" in new TestSetup { - val startValue = 100 - val transferValue = 50 - val balanceAfterTransfer = startValue + transferValue - - val account = Account(UInt256.One, startValue) - val addr = UInt256.One - val value = UInt256.MaxValue - val code = ByteString(Hex.decode("deadbeefdeadbeefdeadbeef")) - - val initialWorld = InMemoryWorldStateProxy.persistState(worldState.saveAccount(address1, account)) - - val worldWithTwoAccounts = InMemoryWorldStateProxy.persistState( - initialWorld - .saveAccount(address2, account) - .saveCode(address2,code) - .saveStorage(address2, worldState.getStorage(address2).store(addr,value)) - ) - - val worldAfterInitialisation = worldWithTwoAccounts.initialiseAccount(address1, address2, transferValue) - - val acc2 = worldAfterInitialisation.getGuaranteedAccount(address2) - - acc2.nonce shouldEqual UInt256.Zero - acc2.balance shouldEqual balanceAfterTransfer - acc2.codeHash shouldEqual Account.EmptyCodeHash - acc2.storageRoot shouldEqual Account.EmptyStorageRootHash - } - - "InMemoryWorldStateProxy" should "remove all ether from existing account" in new TestSetup { + it should "remove all ether from existing account" in new TestSetup { val startValue = 100 val account = Account(UInt256.One, startValue) diff --git a/src/test/scala/io/iohk/ethereum/vm/CreateOpcodeSpec.scala b/src/test/scala/io/iohk/ethereum/vm/CreateOpcodeSpec.scala index 78671a7505..bbe6737194 100644 --- a/src/test/scala/io/iohk/ethereum/vm/CreateOpcodeSpec.scala +++ b/src/test/scala/io/iohk/ethereum/vm/CreateOpcodeSpec.scala @@ -262,4 +262,53 @@ class CreateOpcodeSpec extends WordSpec with Matchers { } } + + "account with non-empty code already exists" should { + + "fail to create contract" in { + val accountNonEmptyCode = Account(codeHash = ByteString("abc")) + + val world = fxt.initWorld.saveAccount(fxt.newAddr, accountNonEmptyCode) + val context: PC = fxt.context.copy(world = world) + val result = CreateResult(context = context) + + result.returnValue shouldEqual UInt256.Zero + result.world.getGuaranteedAccount(fxt.newAddr) shouldEqual accountNonEmptyCode + result.world.getCode(fxt.newAddr) shouldEqual ByteString.empty + } + } + + "account with non-zero nonce already exists" should { + + "fail to create contract" in { + val accountNonZeroNonce = Account(nonce = 1) + + val world = fxt.initWorld.saveAccount(fxt.newAddr, accountNonZeroNonce) + val context: PC = fxt.context.copy(world = world) + val result = CreateResult(context = context) + + result.returnValue shouldEqual UInt256.Zero + result.world.getGuaranteedAccount(fxt.newAddr) shouldEqual accountNonZeroNonce + result.world.getCode(fxt.newAddr) shouldEqual ByteString.empty + } + } + + "account with non-zero balance, but empty code and zero nonce, already exists" should { + + "succeed in creating new contract" in { + val accountNonZeroBalance = Account(balance = 1) + + val world = fxt.initWorld.saveAccount(fxt.newAddr, accountNonZeroBalance) + val context: PC = fxt.context.copy(world = world) + val result = CreateResult(context = context) + + result.returnValue shouldEqual fxt.newAddr.toUInt256 + + val newContract = result.world.getGuaranteedAccount(fxt.newAddr) + newContract.balance shouldEqual (accountNonZeroBalance.balance + fxt.endowment) + newContract.nonce shouldEqual accountNonZeroBalance.nonce + + result.world.getCode(fxt.newAddr) shouldEqual fxt.contractCode.code + } + } } From 53ef7268ab88ec0a376875f5959b8d08d804208d Mon Sep 17 00:00:00 2001 From: Radek Tkaczyk Date: Thu, 19 Oct 2017 13:57:19 +0200 Subject: [PATCH 2/3] updated ETS tests --- src/ets/resources/ets | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ets/resources/ets b/src/ets/resources/ets index 815151e4ce..f22fe81db5 160000 --- a/src/ets/resources/ets +++ b/src/ets/resources/ets @@ -1 +1 @@ -Subproject commit 815151e4cea4e73328f8586b4e61c3d7e1e9e543 +Subproject commit f22fe81db56c10c01004787c787a3f8cc575644d From 9339d196face54df6dcd5039eb025aa8a927356b Mon Sep 17 00:00:00 2001 From: Radek Tkaczyk Date: Fri, 20 Oct 2017 13:23:04 +0200 Subject: [PATCH 3/3] updated VMSuite to match latest ETS files - log entries replaced with hashes --- .../scala/io/iohk/ethereum/ets/vm/VMSuite.scala | 14 +++++++++++--- .../scala/io/iohk/ethereum/ets/vm/scenario.scala | 8 +------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/ets/scala/io/iohk/ethereum/ets/vm/VMSuite.scala b/src/ets/scala/io/iohk/ethereum/ets/vm/VMSuite.scala index 3a6d97c72a..77f2bc8dc8 100644 --- a/src/ets/scala/io/iohk/ethereum/ets/vm/VMSuite.scala +++ b/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))) + } + } diff --git a/src/ets/scala/io/iohk/ethereum/ets/vm/scenario.scala b/src/ets/scala/io/iohk/ethereum/ets/vm/scenario.scala index acaa96cbd5..2b9bc29685 100644 --- a/src/ets/scala/io/iohk/ethereum/ets/vm/scenario.scala +++ b/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] -) -