Transaction: EVMC fix, `CREATE2` salt is a 256-bit blob not a number

This changes fixes a bug in `CREATE2` ops when used with EVMC.
Because it changes the salt type, it affects non-EVMC code as well.

The salt was passed through EVMC with the wrong byte order, although this went
unnoticed as the Nimbus host flipped the byte order before using it.

This was found when running Nimbus with third-party EVM,
["evmone"](https://github.com/ethereum/evmone).

There are different ways to remedy this.

If treated as a number, Nimbus EVM would byte-flip the value when calling EVMC,
then Nimbus host would flip the received value.  Finally, it would be flipped a
third time when generating the address in `generateSafeAddress`.  The first two
flips can be eliminated by negotiation (like other numbers), but there would
always be one flip.

As a bit pattern, Nimbus EVM would flip the same way it does when dealing with
hashes on the stack (e.g. with `getBlockHash`).  Nimbus host wouldn't flip at
all - and when using third-party EVMs there would be no flips in Nimbus.

Because this value is not for arithmetic, any bit pattern is valid, and there
shouldn't be any flips when using a third-party EVM, the bit-pattern
interpretation is favoured.  The only flip is done in Nimbus EVM (and might be
eliminated in an optimised version).

As suggested, we'll define a new "opaque 256 bits" type to hold this value.
(Similar to `Hash256`, but the salt isn't necessarily a hash.)

Signed-off-by: Jamie Lokier <jamie@shareable.org>

nimbus/transaction/host_call_nested.nim

@@ -11,7 +11,7 @@
 import
   sets, stint, chronicles, stew/ranges/ptr_arith,
   eth/common/eth_types,
-  ".."/[vm_types, vm_computation],
+  ".."/[vm_types, vm_computation, utils],
   ./host_types
 
 proc evmcResultRelease(res: var EvmcResult) {.cdecl, gcsafe.} =
@@ -28,8 +28,8 @@ proc beforeExecCreateEvmcNested(host: TransactionHost,
     value: m.value.fromEvmc,
     data: @(makeOpenArray(m.inputData, m.inputSize.int))
   )
-  # TODO: Salt should maybe change endianness when called via EVMC glue.
-  return newComputation(host.vmState, childMsg, m.create2_salt.fromEvmc)
+  return newComputation(host.vmState, childMsg,
+                        cast[ContractSalt](m.create2_salt))
 
 proc afterExecCreateEvmcNested(host: TransactionHost, child: Computation,
                                res: var EvmcResult) {.inline.} =

nimbus/utils.nim

@@ -22,7 +22,13 @@ func keccakHash*(value: openarray[byte]): Hash256 {.inline.} =
 func generateAddress*(address: EthAddress, nonce: AccountNonce): EthAddress =
   result[0..19] = keccakHash(rlp.encodeList(address, nonce)).data.toOpenArray(12, 31)
 
-func generateSafeAddress*(address: EthAddress, salt: Uint256, data: openArray[byte]): EthAddress =
+type ContractSalt* = object
+  bytes*: array[32, byte]
+
+const ZERO_CONTRACTSALT* = default(ContractSalt)
+
+func generateSafeAddress*(address: EthAddress, salt: ContractSalt,
+                          data: openArray[byte]): EthAddress =
   const prefix = [0xff.byte]
   let dataHash = keccakHash(data)
   var hashResult: Hash256
@@ -31,7 +37,7 @@ func generateSafeAddress*(address: EthAddress, salt: Uint256, data: openArray[by
   ctx.init()
   ctx.update(prefix)
   ctx.update(address)
-  ctx.update(salt.toByteArrayBE())
+  ctx.update(salt.bytes)
   ctx.update(dataHash.data)
   ctx.finish hashResult.data
   ctx.clear()

nimbus/vm/computation.nim

@@ -136,7 +136,7 @@ template getCode*(c: Computation, address: EthAddress): seq[byte] =
   else:
     c.vmState.readOnlyStateDB.getCode(address)
 
-proc generateContractAddress(c: Computation, salt: Uint256): EthAddress =
+proc generateContractAddress(c: Computation, salt: ContractSalt): EthAddress =
   if c.msg.kind == evmcCreate:
     let creationNonce = c.vmState.readOnlyStateDb().getNonce(c.msg.sender)
     result = generateAddress(c.msg.sender, creationNonce)
@@ -145,7 +145,8 @@ proc generateContractAddress(c: Computation, salt: Uint256): EthAddress =
 
 import stew/byteutils
 
-proc newComputation*(vmState: BaseVMState, message: Message, salt= 0.u256): Computation =
+proc newComputation*(vmState: BaseVMState, message: Message,
+                     salt: ContractSalt = ZERO_CONTRACTSALT): Computation =
   new result
   result.vmState = vmState
   result.msg = message

nimbus/vm/evmc_helpers.nim

@@ -1,4 +1,4 @@
-import eth/common, stint, evmc/evmc
+import eth/common, stint, evmc/evmc, ../utils
 
 const
   evmc_native* {.booldefine.} = true
@@ -6,7 +6,8 @@ const
 func toEvmc*(a: EthAddress): evmc_address {.inline.} =
   cast[evmc_address](a)
 
-func toEvmc*(h: Hash256): evmc_bytes32 {.inline.} =
+func toEvmc*(h: Hash256 | ContractSalt): evmc_bytes32 {.inline.} =
+  doAssert sizeof(h) == sizeof(evmc_bytes32)
   cast[evmc_bytes32](h)
 
 func toEvmc*(n: Uint256): evmc_uint256be {.inline.} =
@@ -16,8 +17,9 @@ func toEvmc*(n: Uint256): evmc_uint256be {.inline.} =
     cast[evmc_uint256be](n.toByteArrayBE)
 
 func fromEvmc*(T: type, n: evmc_bytes32): T {.inline.} =
-  when T is Hash256:
-    cast[Hash256](n)
+  when T is Hash256 | ContractSalt:
+    doAssert sizeof(n) == sizeof(T)
+    cast[T](n)
   elif T is Uint256:
     when evmc_native:
       cast[Uint256](n)
@@ -30,7 +32,7 @@ func fromEvmc*(a: evmc_address): EthAddress {.inline.} =
   cast[EthAddress](a)
 
 when isMainModule:
-  import constants
+  import ../constants
   var a: evmc_address
   a.bytes[19] = 3.byte
   var na = fromEvmc(a)
@@ -41,3 +43,6 @@ when isMainModule:
   var h = EMPTY_SHA3
   var eh = toEvmc(h)
   assert(h == fromEvmc(Hash256, eh))
+  var s = cast[ContractSalt](BLANK_ROOT_HASH)
+  var es = toEvmc(s)
+  assert(s == fromEvmc(ContractSalt, es))

nimbus/vm/evmc_host.nim

@@ -134,7 +134,8 @@ proc enterCreateImpl(c: Computation, m: nimbus_message): Computation =
     value: Uint256.fromEvmc(m.value),
     data: @(makeOpenArray(m.inputData, m.inputSize.int))
     )
-  return newComputation(c.vmState, childMsg, Uint256.fromEvmc(m.create2_salt))
+  return newComputation(c.vmState, childMsg,
+                        ContractSalt.fromEvmc(m.create2_salt))
 
 template leaveCreateImpl(c, child: Computation, res: nimbus_result) =
   if not child.shouldBurnGas:

nimbus/vm/interpreter/opcodes_impl.nim

@@ -10,8 +10,8 @@ import
   chronicles, stint, nimcrypto, stew/ranges/ptr_arith, eth/common,
   ./utils/[macros_procs_opcodes, utils_numeric],
   ./gas_meter, ./gas_costs, ./opcode_values,
-  ../memory, ../stack, ../code_stream, ../computation, ../state, ../types,
-  ../../errors, ../../constants, ../../forks,
+  ".."/[memory, stack, code_stream, computation, state, types],
+  ".."/../[errors, constants, forks, utils],
   ../../db/[db_chain, accounts_cache]
 
 when defined(evmc_enabled):
@@ -612,11 +612,11 @@ template genCreate(callName: untyped, opCode: Op): untyped =
     when opCode == Create:
       const callKind = evmcCreate
       let memLen {.inject.} = c.stack.peekInt().safeInt
-      let salt = 0.u256
+      const salt: ContractSalt = ZERO_CONTRACTSALT
     else:
       const callKind = evmcCreate2
       let memLen {.inject.} = c.stack.popInt().safeInt
-      let salt = c.stack.peekInt()
+      let salt = ContractSalt(bytes: c.stack.peekInt().toBytesBE)
 
     c.stack.top(0)
 
@@ -659,7 +659,7 @@ template genCreate(callName: untyped, opCode: Op): untyped =
         input_data: c.memory.readPtr(memPos),
         input_size: memLen.uint,
         value: toEvmc(endowment),
-        create2_salt: toEvmc(salt)
+        create2_salt: toEvmc(salt),
       )
 
       c.chainTo(msg):