From 7ed2403c5c6abb2c3179b856c3786683fa08f31a Mon Sep 17 00:00:00 2001 From: coffeepots Date: Mon, 3 Sep 2018 19:24:55 +0100 Subject: [PATCH 01/16] Add hasCodeOrNonce --- nimbus/db/state_db.nim | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nimbus/db/state_db.nim b/nimbus/db/state_db.nim index 915a3a941..fefaf70b8 100644 --- a/nimbus/db/state_db.nim +++ b/nimbus/db/state_db.nim @@ -133,3 +133,5 @@ proc getCode*(db: AccountStateDB, address: EthAddress): ByteRange = let codeHash = db.getCodeHash(address) result = db.trie.get(codeHash.toByteRange_Unnecessary) +proc hasCodeOrNonce*(account: AccountStateDB, address: EthAddress): bool {.inline.} = + account.getNonce(address) != 0 or account.getCodeHash(address) != EMPTY_SHA3 \ No newline at end of file From a7c1168b3aec570f0a0eabe0bf991b281ce99280 Mon Sep 17 00:00:00 2001 From: coffeepots Date: Mon, 3 Sep 2018 19:30:27 +0100 Subject: [PATCH 02/16] Create Op now generates contract addresses & checks for collisions --- nimbus/vm/interpreter/opcodes_impl.nim | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/nimbus/vm/interpreter/opcodes_impl.nim b/nimbus/vm/interpreter/opcodes_impl.nim index dace995f4..7a6e9d99a 100644 --- a/nimbus/vm/interpreter/opcodes_impl.nim +++ b/nimbus/vm/interpreter/opcodes_impl.nim @@ -546,13 +546,16 @@ op create, inline = false, value, startPosition, size: ## TODO dynamic gas that depends on remaining gas - ##### getNonce type error: expression 'db' is of type: proc (vmState: untyped, readOnly: untyped, handler: untyped): untyped{.noSideEffect, gcsafe, locks: .} - # computation.vmState.db(readOnly=true): - # let creationNonce = db.getNonce(computation.msg.storageAddress) - # db.incrementNonce(computation.msg.storageAddress) - let contractAddress = ZERO_ADDRESS # generateContractAddress(computation.msg.storageAddress, creationNonce) + var + contractAddress: EthAddress + isCollision: bool - let isCollision = false # TODO: db.accountHasCodeOrNonce ... + computation.vmState.mutateStateDB: + let creationNonce = db.getNonce(computation.msg.storageAddress) + db.setNonce(computation.msg.storageAddress, creationNonce + 1) + + contractAddress = generateAddress(computation.msg.storageAddress, creationNonce) + isCollision = db.hasCodeOrNonce(contractAddress) if isCollision: debug("Address collision while creating contract", address = contractAddress.toHex) From a28ef962b327109de9cce72eda8399b311efee6c Mon Sep 17 00:00:00 2001 From: coffeepots Date: Tue, 4 Sep 2018 12:23:47 +0100 Subject: [PATCH 03/16] Add generateAddress utility --- nimbus/utils/addresses.nim | 4 ++++ nimbus/vm/interpreter/opcodes_impl.nim | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 nimbus/utils/addresses.nim diff --git a/nimbus/utils/addresses.nim b/nimbus/utils/addresses.nim new file mode 100644 index 000000000..fbb55f5b6 --- /dev/null +++ b/nimbus/utils/addresses.nim @@ -0,0 +1,4 @@ +import nimcrypto, eth_common, rlp + +proc generateAddress*(address: EthAddress, nonce: AccountNonce): EthAddress = + result[0..19] = keccak256.digest(rlp.encodeList(address, nonce).toOpenArray).data[12..31] diff --git a/nimbus/vm/interpreter/opcodes_impl.nim b/nimbus/vm/interpreter/opcodes_impl.nim index 7a6e9d99a..cc841ddca 100644 --- a/nimbus/vm/interpreter/opcodes_impl.nim +++ b/nimbus/vm/interpreter/opcodes_impl.nim @@ -12,7 +12,7 @@ import ./gas_meter, ./gas_costs, ./opcode_values, ./vm_forks, ../memory, ../message, ../stack, ../code_stream, ../computation, ../../vm_state, ../../errors, ../../constants, ../../vm_types, - ../../db/[db_chain, state_db] + ../../db/[db_chain, state_db], ../../utils/addresses # ################################## # Syntactic sugar From c3c84782a55a385252b918b7a750ae12815db1d3 Mon Sep 17 00:00:00 2001 From: coffeepots Date: Tue, 4 Sep 2018 12:27:07 +0100 Subject: [PATCH 04/16] Make generateAddress a func --- nimbus/utils/addresses.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nimbus/utils/addresses.nim b/nimbus/utils/addresses.nim index fbb55f5b6..4e702f995 100644 --- a/nimbus/utils/addresses.nim +++ b/nimbus/utils/addresses.nim @@ -1,4 +1,4 @@ import nimcrypto, eth_common, rlp -proc generateAddress*(address: EthAddress, nonce: AccountNonce): EthAddress = +func generateAddress*(address: EthAddress, nonce: AccountNonce): EthAddress = result[0..19] = keccak256.digest(rlp.encodeList(address, nonce).toOpenArray).data[12..31] From f3fb306a339618712abff7a8faef29456f881b8c Mon Sep 17 00:00:00 2001 From: coffeepots Date: Tue, 4 Sep 2018 16:35:20 +0100 Subject: [PATCH 05/16] Comment reference to issue regarding address collisions --- nimbus/vm/interpreter/opcodes_impl.nim | 1 + 1 file changed, 1 insertion(+) diff --git a/nimbus/vm/interpreter/opcodes_impl.nim b/nimbus/vm/interpreter/opcodes_impl.nim index cc841ddca..8a760c4d8 100644 --- a/nimbus/vm/interpreter/opcodes_impl.nim +++ b/nimbus/vm/interpreter/opcodes_impl.nim @@ -555,6 +555,7 @@ op create, inline = false, value, startPosition, size: db.setNonce(computation.msg.storageAddress, creationNonce + 1) contractAddress = generateAddress(computation.msg.storageAddress, creationNonce) + # Regarding collisions, see: https://github.com/status-im/nimbus/issues/133 isCollision = db.hasCodeOrNonce(contractAddress) if isCollision: From bd9f732b4e0de90aaede0e42b619b648eb726125 Mon Sep 17 00:00:00 2001 From: coffeepots Date: Tue, 4 Sep 2018 17:15:46 +0100 Subject: [PATCH 06/16] Don't touch nonce until after address in-use check --- nimbus/vm/interpreter/opcodes_impl.nim | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/nimbus/vm/interpreter/opcodes_impl.nim b/nimbus/vm/interpreter/opcodes_impl.nim index 8a760c4d8..4f160ffca 100644 --- a/nimbus/vm/interpreter/opcodes_impl.nim +++ b/nimbus/vm/interpreter/opcodes_impl.nim @@ -546,22 +546,21 @@ op create, inline = false, value, startPosition, size: ## TODO dynamic gas that depends on remaining gas - var - contractAddress: EthAddress - isCollision: bool - - computation.vmState.mutateStateDB: - let creationNonce = db.getNonce(computation.msg.storageAddress) - db.setNonce(computation.msg.storageAddress, creationNonce + 1) - + let + readOnlyState = computation.vmState.readOnlyStateDB + creationNonce = readOnlyState.getNonce(computation.msg.storageAddress) contractAddress = generateAddress(computation.msg.storageAddress, creationNonce) - # Regarding collisions, see: https://github.com/status-im/nimbus/issues/133 - isCollision = db.hasCodeOrNonce(contractAddress) + isCollision = readOnlyState.hasCodeOrNonce(contractAddress) - if isCollision: + # Regarding collisions, see: https://github.com/status-im/nimbus/issues/133 + # See: https://github.com/ethereum/EIPs/issues/684 + if not isCollision: + computation.vmState.mutateStateDB: + db.setNonce(computation.msg.storageAddress, creationNonce + 1) + else: debug("Address collision while creating contract", address = contractAddress.toHex) push: 0 - return + raise newException(ValidationError, "Contract address already exists and is in use") let childMsg = prepareChildMessage( computation, From 485781e6ade522bbb6537432e7f1e43b9bbf1327 Mon Sep 17 00:00:00 2001 From: coffeepots Date: Thu, 6 Sep 2018 10:58:35 +0100 Subject: [PATCH 07/16] Always update account nonce even if there's an contract address collision --- nimbus/vm/interpreter/opcodes_impl.nim | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/nimbus/vm/interpreter/opcodes_impl.nim b/nimbus/vm/interpreter/opcodes_impl.nim index 4f160ffca..2fb641f61 100644 --- a/nimbus/vm/interpreter/opcodes_impl.nim +++ b/nimbus/vm/interpreter/opcodes_impl.nim @@ -545,22 +545,23 @@ op create, inline = false, value, startPosition, size: let callData = computation.memory.read(memPos, len) ## TODO dynamic gas that depends on remaining gas + var + contractAddress: EthAddress + isCollision: bool - let - readOnlyState = computation.vmState.readOnlyStateDB - creationNonce = readOnlyState.getNonce(computation.msg.storageAddress) + computation.vmState.mutateStateDB: + # Regarding collisions, see: https://github.com/status-im/nimbus/issues/133 + # See: https://github.com/ethereum/EIPs/issues/684 + let creationNonce = db.getNonce(computation.msg.storageAddress) + db.setNonce(computation.msg.storageAddress, creationNonce + 1) + contractAddress = generateAddress(computation.msg.storageAddress, creationNonce) - isCollision = readOnlyState.hasCodeOrNonce(contractAddress) + isCollision = db.hasCodeOrNonce(contractAddress) - # Regarding collisions, see: https://github.com/status-im/nimbus/issues/133 - # See: https://github.com/ethereum/EIPs/issues/684 - if not isCollision: - computation.vmState.mutateStateDB: - db.setNonce(computation.msg.storageAddress, creationNonce + 1) - else: + if isCollision: debug("Address collision while creating contract", address = contractAddress.toHex) push: 0 - raise newException(ValidationError, "Contract address already exists and is in use") + raise newException(ValidationError, "Contract creation failed, address already in use") let childMsg = prepareChildMessage( computation, From 69befaf4fd28bb44604e2465365deb5fb0031d7f Mon Sep 17 00:00:00 2001 From: coffeepots Date: Thu, 6 Sep 2018 13:48:06 +0100 Subject: [PATCH 08/16] Use toOpenArray in generateAddress --- nimbus/utils/addresses.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nimbus/utils/addresses.nim b/nimbus/utils/addresses.nim index 4e702f995..850952ca8 100644 --- a/nimbus/utils/addresses.nim +++ b/nimbus/utils/addresses.nim @@ -1,4 +1,4 @@ import nimcrypto, eth_common, rlp func generateAddress*(address: EthAddress, nonce: AccountNonce): EthAddress = - result[0..19] = keccak256.digest(rlp.encodeList(address, nonce).toOpenArray).data[12..31] + result[0..19] = keccak256.digest(rlp.encodeList(address, nonce).toOpenArray).data.toOpenArray(12, 31) From 350bf7e6728c5a3a5900e87e960c5588f2b7ac99 Mon Sep 17 00:00:00 2001 From: coffeepots Date: Mon, 3 Sep 2018 19:24:55 +0100 Subject: [PATCH 09/16] Add hasCodeOrNonce --- nimbus/db/state_db.nim | 3 +++ 1 file changed, 3 insertions(+) diff --git a/nimbus/db/state_db.nim b/nimbus/db/state_db.nim index bb9ded52e..9782fa916 100644 --- a/nimbus/db/state_db.nim +++ b/nimbus/db/state_db.nim @@ -138,3 +138,6 @@ proc getCode*(db: AccountStateDB, address: EthAddress): ByteRange = proc dumpAccount*(db: AccountStateDB, addressS: string): string = let address = addressS.parseAddress return fmt"{addressS}: Storage: {db.getStorage(address, 0.u256)}; getAccount: {db.getAccount address}" + +proc hasCodeOrNonce*(account: AccountStateDB, address: EthAddress): bool {.inline.} = + account.getNonce(address) != 0 or account.getCodeHash(address) != EMPTY_SHA3 From 0e2068c99ebe2901908b7d6b554ac5af030ea65e Mon Sep 17 00:00:00 2001 From: coffeepots Date: Mon, 3 Sep 2018 19:30:27 +0100 Subject: [PATCH 10/16] Create Op now generates contract addresses & checks for collisions --- nimbus/vm/interpreter/opcodes_impl.nim | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/nimbus/vm/interpreter/opcodes_impl.nim b/nimbus/vm/interpreter/opcodes_impl.nim index dace995f4..7a6e9d99a 100644 --- a/nimbus/vm/interpreter/opcodes_impl.nim +++ b/nimbus/vm/interpreter/opcodes_impl.nim @@ -546,13 +546,16 @@ op create, inline = false, value, startPosition, size: ## TODO dynamic gas that depends on remaining gas - ##### getNonce type error: expression 'db' is of type: proc (vmState: untyped, readOnly: untyped, handler: untyped): untyped{.noSideEffect, gcsafe, locks: .} - # computation.vmState.db(readOnly=true): - # let creationNonce = db.getNonce(computation.msg.storageAddress) - # db.incrementNonce(computation.msg.storageAddress) - let contractAddress = ZERO_ADDRESS # generateContractAddress(computation.msg.storageAddress, creationNonce) + var + contractAddress: EthAddress + isCollision: bool - let isCollision = false # TODO: db.accountHasCodeOrNonce ... + computation.vmState.mutateStateDB: + let creationNonce = db.getNonce(computation.msg.storageAddress) + db.setNonce(computation.msg.storageAddress, creationNonce + 1) + + contractAddress = generateAddress(computation.msg.storageAddress, creationNonce) + isCollision = db.hasCodeOrNonce(contractAddress) if isCollision: debug("Address collision while creating contract", address = contractAddress.toHex) From 7a5a43f50f90a71842a2e27d611ce0164d73336a Mon Sep 17 00:00:00 2001 From: coffeepots Date: Tue, 4 Sep 2018 12:23:47 +0100 Subject: [PATCH 11/16] Add generateAddress utility --- nimbus/utils/addresses.nim | 4 ++++ nimbus/vm/interpreter/opcodes_impl.nim | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 nimbus/utils/addresses.nim diff --git a/nimbus/utils/addresses.nim b/nimbus/utils/addresses.nim new file mode 100644 index 000000000..fbb55f5b6 --- /dev/null +++ b/nimbus/utils/addresses.nim @@ -0,0 +1,4 @@ +import nimcrypto, eth_common, rlp + +proc generateAddress*(address: EthAddress, nonce: AccountNonce): EthAddress = + result[0..19] = keccak256.digest(rlp.encodeList(address, nonce).toOpenArray).data[12..31] diff --git a/nimbus/vm/interpreter/opcodes_impl.nim b/nimbus/vm/interpreter/opcodes_impl.nim index 7a6e9d99a..cc841ddca 100644 --- a/nimbus/vm/interpreter/opcodes_impl.nim +++ b/nimbus/vm/interpreter/opcodes_impl.nim @@ -12,7 +12,7 @@ import ./gas_meter, ./gas_costs, ./opcode_values, ./vm_forks, ../memory, ../message, ../stack, ../code_stream, ../computation, ../../vm_state, ../../errors, ../../constants, ../../vm_types, - ../../db/[db_chain, state_db] + ../../db/[db_chain, state_db], ../../utils/addresses # ################################## # Syntactic sugar From 9f1027102c08b757c48d4bd214e2873916547649 Mon Sep 17 00:00:00 2001 From: coffeepots Date: Tue, 4 Sep 2018 12:27:07 +0100 Subject: [PATCH 12/16] Make generateAddress a func --- nimbus/utils/addresses.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nimbus/utils/addresses.nim b/nimbus/utils/addresses.nim index fbb55f5b6..4e702f995 100644 --- a/nimbus/utils/addresses.nim +++ b/nimbus/utils/addresses.nim @@ -1,4 +1,4 @@ import nimcrypto, eth_common, rlp -proc generateAddress*(address: EthAddress, nonce: AccountNonce): EthAddress = +func generateAddress*(address: EthAddress, nonce: AccountNonce): EthAddress = result[0..19] = keccak256.digest(rlp.encodeList(address, nonce).toOpenArray).data[12..31] From 555687ba97bcada13f15798571c55a67994375af Mon Sep 17 00:00:00 2001 From: coffeepots Date: Tue, 4 Sep 2018 16:35:20 +0100 Subject: [PATCH 13/16] Comment reference to issue regarding address collisions --- nimbus/vm/interpreter/opcodes_impl.nim | 1 + 1 file changed, 1 insertion(+) diff --git a/nimbus/vm/interpreter/opcodes_impl.nim b/nimbus/vm/interpreter/opcodes_impl.nim index cc841ddca..8a760c4d8 100644 --- a/nimbus/vm/interpreter/opcodes_impl.nim +++ b/nimbus/vm/interpreter/opcodes_impl.nim @@ -555,6 +555,7 @@ op create, inline = false, value, startPosition, size: db.setNonce(computation.msg.storageAddress, creationNonce + 1) contractAddress = generateAddress(computation.msg.storageAddress, creationNonce) + # Regarding collisions, see: https://github.com/status-im/nimbus/issues/133 isCollision = db.hasCodeOrNonce(contractAddress) if isCollision: From ca6eb0cb3891c7310d1bc2b721d8eb76c7c7d303 Mon Sep 17 00:00:00 2001 From: coffeepots Date: Tue, 4 Sep 2018 17:15:46 +0100 Subject: [PATCH 14/16] Don't touch nonce until after address in-use check --- nimbus/vm/interpreter/opcodes_impl.nim | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/nimbus/vm/interpreter/opcodes_impl.nim b/nimbus/vm/interpreter/opcodes_impl.nim index 8a760c4d8..4f160ffca 100644 --- a/nimbus/vm/interpreter/opcodes_impl.nim +++ b/nimbus/vm/interpreter/opcodes_impl.nim @@ -546,22 +546,21 @@ op create, inline = false, value, startPosition, size: ## TODO dynamic gas that depends on remaining gas - var - contractAddress: EthAddress - isCollision: bool - - computation.vmState.mutateStateDB: - let creationNonce = db.getNonce(computation.msg.storageAddress) - db.setNonce(computation.msg.storageAddress, creationNonce + 1) - + let + readOnlyState = computation.vmState.readOnlyStateDB + creationNonce = readOnlyState.getNonce(computation.msg.storageAddress) contractAddress = generateAddress(computation.msg.storageAddress, creationNonce) - # Regarding collisions, see: https://github.com/status-im/nimbus/issues/133 - isCollision = db.hasCodeOrNonce(contractAddress) + isCollision = readOnlyState.hasCodeOrNonce(contractAddress) - if isCollision: + # Regarding collisions, see: https://github.com/status-im/nimbus/issues/133 + # See: https://github.com/ethereum/EIPs/issues/684 + if not isCollision: + computation.vmState.mutateStateDB: + db.setNonce(computation.msg.storageAddress, creationNonce + 1) + else: debug("Address collision while creating contract", address = contractAddress.toHex) push: 0 - return + raise newException(ValidationError, "Contract address already exists and is in use") let childMsg = prepareChildMessage( computation, From 085880758bbdfd94f4859312f409f958e5c3d04d Mon Sep 17 00:00:00 2001 From: coffeepots Date: Thu, 6 Sep 2018 10:58:35 +0100 Subject: [PATCH 15/16] Always update account nonce even if there's an contract address collision --- nimbus/vm/interpreter/opcodes_impl.nim | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/nimbus/vm/interpreter/opcodes_impl.nim b/nimbus/vm/interpreter/opcodes_impl.nim index 4f160ffca..2fb641f61 100644 --- a/nimbus/vm/interpreter/opcodes_impl.nim +++ b/nimbus/vm/interpreter/opcodes_impl.nim @@ -545,22 +545,23 @@ op create, inline = false, value, startPosition, size: let callData = computation.memory.read(memPos, len) ## TODO dynamic gas that depends on remaining gas + var + contractAddress: EthAddress + isCollision: bool - let - readOnlyState = computation.vmState.readOnlyStateDB - creationNonce = readOnlyState.getNonce(computation.msg.storageAddress) + computation.vmState.mutateStateDB: + # Regarding collisions, see: https://github.com/status-im/nimbus/issues/133 + # See: https://github.com/ethereum/EIPs/issues/684 + let creationNonce = db.getNonce(computation.msg.storageAddress) + db.setNonce(computation.msg.storageAddress, creationNonce + 1) + contractAddress = generateAddress(computation.msg.storageAddress, creationNonce) - isCollision = readOnlyState.hasCodeOrNonce(contractAddress) + isCollision = db.hasCodeOrNonce(contractAddress) - # Regarding collisions, see: https://github.com/status-im/nimbus/issues/133 - # See: https://github.com/ethereum/EIPs/issues/684 - if not isCollision: - computation.vmState.mutateStateDB: - db.setNonce(computation.msg.storageAddress, creationNonce + 1) - else: + if isCollision: debug("Address collision while creating contract", address = contractAddress.toHex) push: 0 - raise newException(ValidationError, "Contract address already exists and is in use") + raise newException(ValidationError, "Contract creation failed, address already in use") let childMsg = prepareChildMessage( computation, From 7e02eedea058acbc3ef63001d7957ff46dbab1a6 Mon Sep 17 00:00:00 2001 From: coffeepots Date: Thu, 6 Sep 2018 13:48:06 +0100 Subject: [PATCH 16/16] Use toOpenArray in generateAddress --- nimbus/utils/addresses.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nimbus/utils/addresses.nim b/nimbus/utils/addresses.nim index 4e702f995..850952ca8 100644 --- a/nimbus/utils/addresses.nim +++ b/nimbus/utils/addresses.nim @@ -1,4 +1,4 @@ import nimcrypto, eth_common, rlp func generateAddress*(address: EthAddress, nonce: AccountNonce): EthAddress = - result[0..19] = keccak256.digest(rlp.encodeList(address, nonce).toOpenArray).data[12..31] + result[0..19] = keccak256.digest(rlp.encodeList(address, nonce).toOpenArray).data.toOpenArray(12, 31)