Get rid of HexQuantityStr and HexDataStr usage

Both types are not safe and require validation/conversion
from rpc implementer.
This PR change it to safer types and delegate the conversion
and validation to the rpc library.

fluffy/rpc/rpc_eth_api.nim

@@ -10,7 +10,7 @@
 import
   std/[times, sequtils, strutils, typetraits],
   json_rpc/[rpcproxy, rpcserver], stew/byteutils,
-  web3/[conversions, ethhexstrings], # sigh, for FixedBytes marshalling
+  web3/[conversions], # sigh, for FixedBytes marshalling
   eth/[common/eth_types, rlp],
   beacon_chain/spec/forks,
   ../../nimbus/rpc/[rpc_types, filters],
@@ -218,8 +218,10 @@ proc installEthApiHandlers*(
     return some(BlockObject.init(header, body, fullTransactions))
 
   rpcServerWithProxy.rpc("eth_getBlockByNumber") do(
-      quantityTag: string, fullTransactions: bool) -> Option[BlockObject]:
+      quantityTag: BlockTag, fullTransactions: bool) -> Option[BlockObject]:
-    let tag = quantityTag.toLowerAscii
+
+    if quantityTag.kind == bidAlias:
+      let tag = quantityTag.alias.toLowerAscii
       case tag
       of "latest":
         # TODO:
@@ -262,11 +264,10 @@ proc installEthApiHandlers*(
       of "pending":
         raise newException(ValueError, "Pending tag not yet implemented")
       else:
-      if not validate(quantityTag.HexQuantityStr):
+        raise newException(ValueError, "Unsupported block tag " & tag)
-        raise newException(ValueError, "Provided block number is not a hex number")
+    else:
-
       let
-        blockNumber = fromHex(UInt256, quantityTag)
+        blockNumber = quantityTag.number.toBlockNumber
         maybeBlock = (await historyNetwork.getBlock(blockNumber)).valueOr:
           raise newException(ValueError, error)
 

nimbus/evm/async/data_sources/json_rpc_data_source.nim

@@ -136,7 +136,6 @@ type
 proc fetchAccountAndSlots*(rpcClient: RpcClient, address: EthAddress, slots: seq[UInt256], blockNumber: common.BlockNumber): Future[(Account, AccountProof, seq[StorageProof])] {.async.} =
   let t0 = now()
   debug "Got to fetchAccountAndSlots", address=address, slots=slots, blockNumber=blockNumber
-  #let blockNumberHexStr: HexQuantityStr = encodeQuantity(blockNumber)
   let blockNumberUint64 = blockNumber.truncate(uint64)
   let a = web3.Address(address)
   let bid = blockId(blockNumber.truncate(uint64))

nimbus/rpc/debug.nim

@@ -9,7 +9,9 @@
 
 import
   std/json,
-  json_rpc/rpcserver, rpc_utils,
+  json_rpc/rpcserver, 
+  ./rpc_utils,
+  ./rpc_types,
   ../tracer, ../vm_types,
   ../common/common,
   ../beacon/web3_eth_conv,
@@ -63,7 +65,7 @@ proc setupDebugRpc*(com: CommonRef, rpcsrv: RpcServer) =
 
     result = traceTransaction(com, blockHeader, blockBody, txDetails.index, flags)
 
-  rpcsrv.rpc("debug_dumpBlockStateByNumber") do(quantityTag: string) -> JsonNode:
+  rpcsrv.rpc("debug_dumpBlockStateByNumber") do(quantityTag: BlockTag) -> JsonNode:
     ## Retrieves the state that corresponds to the block number and returns
     ## a list of accounts (including storage and code).
     ##
@@ -89,7 +91,7 @@ proc setupDebugRpc*(com: CommonRef, rpcsrv: RpcServer) =
 
     result = dumpBlockState(com, header, body)
 
-  rpcsrv.rpc("debug_traceBlockByNumber") do(quantityTag: string, options: Option[TraceOptions]) -> JsonNode:
+  rpcsrv.rpc("debug_traceBlockByNumber") do(quantityTag: BlockTag, options: Option[TraceOptions]) -> JsonNode:
     ## The traceBlock method will return a full stack trace of all invoked opcodes of all transaction
     ## that were included included in this block.
     ##
@@ -119,7 +121,7 @@ proc setupDebugRpc*(com: CommonRef, rpcsrv: RpcServer) =
 
     result = traceBlock(com, header, body, flags)
 
-  rpcsrv.rpc("debug_setHead") do(quantityTag: string) -> bool:
+  rpcsrv.rpc("debug_setHead") do(quantityTag: BlockTag) -> bool:
     ## Sets the current head of the local chain by block number.
     ## Note, this is a destructive action and may severely damage your chain.
     ## Use with extreme caution.

nimbus/rpc/p2p.nim

@@ -25,15 +25,6 @@ import
   ../beacon/web3_eth_conv,
   ./filters
 
-#[
-  Note:
-    * Hexstring types (HexQuantitySt, HexDataStr, Web3Address, Web3Hash)
-      are parsed to check format before the RPC blocks are executed and will
-      raise an exception if invalid.
-    * Many of the RPC calls do not validate hex string types when output, only
-      type cast to avoid extra processing.
-]#
-
 type
   BlockHeader = eth_types.BlockHeader
   Hash256 = eth_types.Hash256
@@ -49,9 +40,9 @@ proc setupEthRpc*(
     let ac = newAccountStateDB(chainDB, header.stateRoot, com.pruneTrie)
     result = ReadOnlyStateDB(ac)
 
-  proc stateDBFromTag(tag: string, readOnly = true): ReadOnlyStateDB
+  proc stateDBFromTag(quantityTag: BlockTag, readOnly = true): ReadOnlyStateDB
       {.gcsafe, raises: [CatchableError].} =
-    result = getStateDB(chainDB.headerFromTag(tag))
+    result = getStateDB(chainDB.headerFromTag(quantityTag))
 
   server.rpc("eth_protocolVersion") do() -> Option[string]:
     # Old Ethereum wiki documents this as returning a decimal string.
@@ -112,7 +103,7 @@ proc setupEthRpc*(
     ## Returns integer of the current block number the client is on.
     result = w3Qty(chainDB.getCanonicalHead().blockNumber)
 
-  server.rpc("eth_getBalance") do(data: Web3Address, quantityTag: string) -> UInt256:
+  server.rpc("eth_getBalance") do(data: Web3Address, quantityTag: BlockTag) -> UInt256:
     ## Returns the balance of the account of given address.
     ##
     ## data: address to check for balance.
@@ -123,7 +114,7 @@ proc setupEthRpc*(
       address = data.ethAddr
     result = accDB.getBalance(address)
 
-  server.rpc("eth_getStorageAt") do(data: Web3Address, slot: UInt256, quantityTag: string) -> UInt256:
+  server.rpc("eth_getStorageAt") do(data: Web3Address, slot: UInt256, quantityTag: BlockTag) -> UInt256:
     ## Returns the value from a storage position at a given address.
     ##
     ## data: address of the storage.
@@ -135,7 +126,7 @@ proc setupEthRpc*(
       address = data.ethAddr
     result = accDB.getStorage(address, slot)[0]
 
-  server.rpc("eth_getTransactionCount") do(data: Web3Address, quantityTag: string) -> Web3Quantity:
+  server.rpc("eth_getTransactionCount") do(data: Web3Address, quantityTag: BlockTag) -> Web3Quantity:
     ## Returns the number of transactions sent from an address.
     ##
     ## data: address.
@@ -157,7 +148,7 @@ proc setupEthRpc*(
       txCount   = chainDB.getTransactionCount(header.txRoot)
     result = w3Qty(txCount)
 
-  server.rpc("eth_getBlockTransactionCountByNumber") do(quantityTag: string) -> Web3Quantity:
+  server.rpc("eth_getBlockTransactionCountByNumber") do(quantityTag: BlockTag) -> Web3Quantity:
     ## Returns the number of transactions in a block matching the given block number.
     ##
     ## data: integer of a block number, or the string "earliest", "latest" or "pending", as in the default block parameter.
@@ -178,7 +169,7 @@ proc setupEthRpc*(
       unclesCount = chainDB.getUnclesCount(header.ommersHash)
     result = w3Qty(unclesCount)
 
-  server.rpc("eth_getUncleCountByBlockNumber") do(quantityTag: string) -> Web3Quantity:
+  server.rpc("eth_getUncleCountByBlockNumber") do(quantityTag: BlockTag) -> Web3Quantity:
     ## Returns the number of uncles in a block from a block matching the given block number.
     ##
     ## quantityTag: integer of a block number, or the string "latest", "earliest" or "pending", see the default block parameter.
@@ -188,7 +179,7 @@ proc setupEthRpc*(
       unclesCount = chainDB.getUnclesCount(header.ommersHash)
     result = w3Qty(unclesCount.uint)
 
-  server.rpc("eth_getCode") do(data: Web3Address, quantityTag: string) -> seq[byte]:
+  server.rpc("eth_getCode") do(data: Web3Address, quantityTag: BlockTag) -> seq[byte]:
     ## Returns code at a given address.
     ##
     ## data: address
@@ -232,7 +223,7 @@ proc setupEthRpc*(
       raise newException(ValueError, "Account locked, please unlock it first")
 
     let
-      accDB    = stateDBFromTag("latest")
+      accDB    = stateDBFromTag(blockId("latest"))
       tx       = unsignedTx(data, chainDB, accDB.getNonce(address) + 1)
       eip155   = com.isEIP155(com.syncCurrent)
       signedTx = signTransaction(tx, acc.privateKey, com.chainId, eip155)
@@ -252,7 +243,7 @@ proc setupEthRpc*(
       raise newException(ValueError, "Account locked, please unlock it first")
 
     let
-      accDB    = stateDBFromTag("latest")
+      accDB    = stateDBFromTag(blockId("latest"))
       tx       = unsignedTx(data, chainDB, accDB.getNonce(address) + 1)
       eip155   = com.isEIP155(com.syncCurrent)
       signedTx = signTransaction(tx, acc.privateKey, com.chainId, eip155)
@@ -276,7 +267,7 @@ proc setupEthRpc*(
       raise newException(ValueError, res.error)
     result = txHash.w3Hash
 
-  server.rpc("eth_call") do(call: EthCall, quantityTag: string) -> seq[byte]:
+  server.rpc("eth_call") do(call: EthCall, quantityTag: BlockTag) -> seq[byte]:
     ## Executes a new message call immediately without creating a transaction on the block chain.
     ##
     ## call: the transaction call object.
@@ -288,7 +279,7 @@ proc setupEthRpc*(
       res      = rpcCallEvm(callData, header, com)
     result = res.output
 
-  server.rpc("eth_estimateGas") do(call: EthCall, quantityTag: string) -> Web3Quantity:
+  server.rpc("eth_estimateGas") do(call: EthCall, quantityTag: BlockTag) -> Web3Quantity:
     ## Generates and returns an estimate of how much gas is necessary to allow the transaction to complete.
     ## The transaction will not be added to the blockchain. Note that the estimate may be significantly more than
     ## the amount of gas actually used by the transaction, for a variety of reasons including EVM mechanics and node performance.
@@ -318,7 +309,7 @@ proc setupEthRpc*(
     else:
       result = nil
 
-  server.rpc("eth_getBlockByNumber") do(quantityTag: string, fullTransactions: bool) -> BlockObject:
+  server.rpc("eth_getBlockByNumber") do(quantityTag: BlockTag, fullTransactions: bool) -> BlockObject:
     ## Returns information about a block by block number.
     ##
     ## quantityTag: integer of a block number, or the string "earliest", "latest" or "pending", as in the default block parameter.
@@ -366,7 +357,7 @@ proc setupEthRpc*(
     else:
       result = nil
 
-  server.rpc("eth_getTransactionByBlockNumberAndIndex") do(quantityTag: string, quantity: Web3Quantity) -> TransactionObject:
+  server.rpc("eth_getTransactionByBlockNumberAndIndex") do(quantityTag: BlockTag, quantity: Web3Quantity) -> TransactionObject:
     ## Returns information about a transaction by block number and transaction index position.
     ##
     ## quantityTag: a block number, or the string "earliest", "latest" or "pending", as in the default block parameter.
@@ -425,7 +416,7 @@ proc setupEthRpc*(
     result = populateBlockObject(uncles[index], chainDB, false, true)
     result.totalDifficulty = chainDB.getScore(header.hash)
 
-  server.rpc("eth_getUncleByBlockNumberAndIndex") do(quantityTag: string, quantity: Web3Quantity) -> BlockObject:
+  server.rpc("eth_getUncleByBlockNumberAndIndex") do(quantityTag: BlockTag, quantity: Web3Quantity) -> BlockObject:
     # Returns information about a uncle of a block by number and uncle index position.
     ##
     ## quantityTag: a block number, or the string "earliest", "latest" or "pending", as in the default block parameter.

nimbus/rpc/rpc_types.nim

@@ -18,3 +18,7 @@ export
 
 type
   FilterLog* = eth_api_types.LogObject
+
+  # BlockTag instead of BlockId:
+  # prevent type clash with eth2 BlockId in fluffy/verified_proxy
+  BlockTag* = eth_api_types.RtBlockIdentifier

nimbus/rpc/rpc_utils.nim

@@ -13,7 +13,6 @@ import
   std/[strutils, algorithm, options],
   ./rpc_types,
   eth/[common, keys],
-  web3/ethhexstrings,
   ../db/core_db,
   ../constants, stint,
   ../utils/utils,
@@ -23,14 +22,16 @@ import
   ../beacon/web3_eth_conv
 
 const
-  defaultTag = "latest"
+  defaultTag = blockId("latest")
 
 type
   BlockHeader = common.BlockHeader
 
-proc headerFromTag*(chain: CoreDbRef, blockTag: string): BlockHeader
+proc headerFromTag*(chain: CoreDbRef, blockId: BlockTag): BlockHeader
     {.gcsafe, raises: [CatchableError].} =
-  let tag = blockTag.toLowerAscii
+
+  if blockId.kind == bidAlias:
+    let tag = blockId.alias.toLowerAscii
     case tag
     of "latest": result = chain.getCanonicalHead()
     of "earliest": result = chain.getBlockHeader(GENESIS_BLOCK_NUMBER)
@@ -40,21 +41,15 @@ proc headerFromTag*(chain: CoreDbRef, blockTag: string): BlockHeader
       #TODO: Implement get pending block
       raise newException(ValueError, "Pending tag not yet implemented")
     else:
-    if not validate(tag.HexQuantityStr):
+      raise newException(ValueError, "Unsupported block tag " & tag)
-      raise newException(ValueError, "Invalid hex of blockTag")
+  else:
-    let blockNum = stint.fromHex(UInt256, tag)
+    let blockNum = blockId.number.toBlockNumber
-    result = chain.getBlockHeader(blockNum.toBlockNumber)
+    result = chain.getBlockHeader(blockNum)
 
-proc headerFromTag*(chain: CoreDbRef, blockTag: Option[RtBlockIdentifier]): BlockHeader
+proc headerFromTag*(chain: CoreDbRef, blockTag: Option[BlockTag]): BlockHeader
     {.gcsafe, raises: [CatchableError].} =
-  if blockTag.isSome():
+  let blockId = blockTag.get(defaultTag)
-    let blockId = blockTag.get
+  chain.headerFromTag(blockId)
-    if blockId.kind == bidAlias:
-      return chain.headerFromTag(blockId.alias)
-    else:
-      return chain.getBlockHeader(blockId.number.toBlockNumber)
-  else:
-    return chain.headerFromTag(defaultTag)
 
 proc calculateMedianGasPrice*(chain: CoreDbRef): GasInt
     {.gcsafe, raises: [CatchableError].} =

nimbus_verified_proxy/rpc/rpc_eth_api.nim

@@ -8,14 +8,14 @@
 {.push raises: [].}
 
 import
-  std/strutils,
+  std/[strutils, typetraits],
   stint,
   stew/[byteutils, results],
   chronicles,
   json_rpc/[rpcproxy, rpcserver, rpcclient],
   eth/common/eth_types as etypes,
   web3,
-  web3/[ethhexstrings, primitives],
+  web3/[primitives, eth_api_types],
   beacon_chain/el/el_manager,
   beacon_chain/networking/network_metadata,
   beacon_chain/spec/forks,
@@ -30,18 +30,6 @@ logScope:
 
 proc `==`(x, y: Quantity): bool {.borrow, noSideEffect.}
 
-template encodeQuantity(value: UInt256): HexQuantityStr =
-  hexQuantityStr("0x" & value.toHex())
-
-template encodeHexData(value: UInt256): HexDataStr =
-  hexDataStr("0x" & toBytesBE(value).toHex)
-
-template bytesToHex(bytes: seq[byte]): HexDataStr =
-  hexDataStr("0x" & toHex(bytes))
-
-template encodeQuantity(value: Quantity): HexQuantityStr =
-  hexQuantityStr(encodeQuantity(value.uint64))
-
 type
   VerifiedRpcProxy* = ref object
     proxy: RpcProxy
@@ -51,6 +39,8 @@ type
   QuantityTagKind = enum
     LatestBlock, BlockNumber
 
+  BlockTag = eth_api_types.RtBlockIdentifier
+
   QuantityTag = object
     case kind: QuantityTagKind
     of LatestBlock:
@@ -58,27 +48,16 @@ type
     of BlockNumber:
       blockNumber: Quantity
 
-func parseHexIntResult(tag: string): Result[uint64, string] =
+func parseQuantityTag(blockTag: BlockTag): Result[QuantityTag, string] =
-  try:
+  if blockTag.kind == bidAlias:
-    ok(parseHexInt(tag).uint64)
+    let tag = blockTag.alias.toLowerAscii
-  except ValueError as e:
-    err(e.msg)
-
-func parseHexQuantity(tag: string): Result[Quantity, string] =
-  let hexQuantity = hexQuantityStr(tag)
-  if validate(hexQuantity):
-    let parsed = ? parseHexIntResult(tag)
-    return ok(Quantity(parsed))
-  else:
-    return err("Invalid hex quantity.")
-
-func parseQuantityTag(blockTag: string): Result[QuantityTag, string] =
-  let tag = blockTag.toLowerAscii
     case tag
     of "latest":
       return ok(QuantityTag(kind: LatestBlock))
     else:
-    let quantity = ? parseHexQuantity(tag)
+      return err("Unsupported blockTag: " & tag)
+  else:
+    let quantity = blockTag.number.Quantity
     return ok(QuantityTag(kind: BlockNumber, blockNumber: quantity))
 
 template checkPreconditions(proxy: VerifiedRpcProxy) =
@@ -90,7 +69,7 @@ template rpcClient(lcProxy: VerifiedRpcProxy): RpcClient =
 
 proc getPayloadByTag(
     proxy: VerifiedRpcProxy,
-    quantityTag: string):
+    quantityTag: BlockTag):
     results.Opt[ExecutionData] {.raises: [ValueError].} =
   checkPreconditions(proxy)
 
@@ -110,27 +89,27 @@ proc getPayloadByTag(
 
 proc getPayloadByTagOrThrow(
     proxy: VerifiedRpcProxy,
-    quantityTag: string): ExecutionData {.raises: [ValueError].} =
+    quantityTag: BlockTag): ExecutionData {.raises: [ValueError].} =
 
   let tagResult = getPayloadByTag(proxy, quantityTag)
 
   if tagResult.isErr:
-    raise newException(ValueError, "No block stored for given tag " & quantityTag)
+    raise newException(ValueError, "No block stored for given tag " & $quantityTag)
 
   return tagResult.get()
 
 proc installEthApiHandlers*(lcProxy: VerifiedRpcProxy) =
-  lcProxy.proxy.rpc("eth_chainId") do() -> HexQuantityStr:
+  lcProxy.proxy.rpc("eth_chainId") do() -> Quantity:
-    return encodeQuantity(lcProxy.chainId)
+    return lcProxy.chainId
 
-  lcProxy.proxy.rpc("eth_blockNumber") do() -> HexQuantityStr:
+  lcProxy.proxy.rpc("eth_blockNumber") do() -> Quantity:
     ## Returns the number of the most recent block.
     checkPreconditions(lcProxy)
 
-    return encodeQuantity(lcProxy.blockCache.latest.get.blockNumber)
+    return lcProxy.blockCache.latest.get.blockNumber
 
   lcProxy.proxy.rpc("eth_getBalance") do(
-      address: Address, quantityTag: string) -> HexQuantityStr:
+      address: Address, quantityTag: BlockTag) -> UInt256:
     # When requesting state for `latest` block number, we need to translate
     # `latest` to actual block number as `latest` on proxy and on data provider
     # can mean different blocks and ultimatly piece received piece of state
@@ -155,32 +134,31 @@ proc installEthApiHandlers*(lcProxy: VerifiedRpcProxy) =
     )
 
     if accountResult.isOk():
-      return encodeQuantity(accountResult.get.balance)
+      return accountResult.get.balance
     else:
       raise newException(ValueError, accountResult.error)
 
   lcProxy.proxy.rpc("eth_getStorageAt") do(
-      address: Address, slot: HexDataStr, quantityTag: string) -> HexDataStr:
+      address: Address, slot: UInt256, quantityTag: BlockTag) -> UInt256:
     let
       executionPayload = lcProxy.getPayloadByTagOrThrow(quantityTag)
-      uslot = UInt256.fromHex(slot.string)
       blockNumber = executionPayload.blockNumber.uint64
 
     info "Forwarding eth_getStorageAt", blockNumber
 
     let proof = await lcProxy.rpcClient.eth_getProof(
-      address, @[uslot], blockId(blockNumber))
+      address, @[slot], blockId(blockNumber))
 
-    let dataResult = getStorageData(executionPayload.stateRoot, uslot, proof)
+    let dataResult = getStorageData(executionPayload.stateRoot, slot, proof)
 
     if dataResult.isOk():
       let slotValue = dataResult.get()
-      return encodeHexData(slotValue)
+      return slotValue
     else:
       raise newException(ValueError, dataResult.error)
 
   lcProxy.proxy.rpc("eth_getTransactionCount") do(
-      address: Address, quantityTag: string) -> HexQuantityStr:
+      address: Address, quantityTag: BlockTag) -> Quantity:
     let
       executionPayload = lcProxy.getPayloadByTagOrThrow(quantityTag)
       blockNumber = executionPayload.blockNumber.uint64
@@ -201,12 +179,12 @@ proc installEthApiHandlers*(lcProxy: VerifiedRpcProxy) =
     )
 
     if accountResult.isOk():
-      return hexQuantityStr(encodeQuantity(accountResult.get.nonce))
+      return Quantity(accountResult.get.nonce)
     else:
       raise newException(ValueError, accountResult.error)
 
   lcProxy.proxy.rpc("eth_getCode") do(
-      address: Address, quantityTag: string) -> HexDataStr:
+      address: Address, quantityTag: BlockTag) -> seq[byte]:
     let
       executionPayload = lcProxy.getPayloadByTagOrThrow(quantityTag)
       blockNumber = executionPayload.blockNumber.uint64
@@ -231,7 +209,7 @@ proc installEthApiHandlers*(lcProxy: VerifiedRpcProxy) =
 
     if account.codeHash == etypes.EMPTY_CODE_HASH:
       # account does not have any code, return empty hex data
-      return hexDataStr("0x")
+      return @[]
 
     info "Forwarding eth_getCode", blockNumber
 
@@ -241,7 +219,7 @@ proc installEthApiHandlers*(lcProxy: VerifiedRpcProxy) =
     )
 
     if isValidCode(account, code):
-      return bytesToHex(code)
+      return code
     else:
       raise newException(ValueError,
         "Received code which does not match the account code hash")
@@ -257,7 +235,7 @@ proc installEthApiHandlers*(lcProxy: VerifiedRpcProxy) =
   # TODO currently we do not handle fullTransactions flag. It require updates on
   # nim-web3 side
   lcProxy.proxy.rpc("eth_getBlockByNumber") do(
-      quantityTag: string, fullTransactions: bool) -> Option[BlockObject]:
+      quantityTag: BlockTag, fullTransactions: bool) -> Option[BlockObject]:
     let executionPayload = lcProxy.getPayloadByTag(quantityTag)
 
     if executionPayload.isErr:

premix/parser.nim

@@ -14,7 +14,17 @@ import
   stint, stew/byteutils
 
 import ../nimbus/transaction, ../nimbus/utils/ec_recover
-from web3/ethhexstrings import encodeQuantity
+
+template stripLeadingZeros(value: string): string =
+  var cidx = 0
+  # ignore the last character so we retain '0' on zero value
+  while cidx < value.len - 1 and value[cidx] == '0':
+    cidx.inc
+  value[cidx .. ^1]
+
+func encodeQuantity(value: SomeUnsignedInt): string =
+  var hValue = value.toHex.stripLeadingZeros
+  result = "0x" & hValue.toLowerAscii
 
 func hexToInt*(s: string, T: typedesc[SomeInteger]): T =
   var i = 0