From 73ad6e86f834b6a3439dc12071862d3aaa629cbc Mon Sep 17 00:00:00 2001 From: Kim De Mey Date: Thu, 24 Nov 2022 14:41:00 +0100 Subject: [PATCH] Several clean-ups in nimbus_verified_proxy (#1324) - clean-up config a bit and remove deprecated log-file - style fixes - a bit of error msg & logging clean-up and add missing log - ... --- nimbus_verified_proxy/block_cache.nim | 5 +- .../nimbus_verified_proxy.nim | 7 +- .../nimbus_verified_proxy_conf.nim | 66 +++++++++-------- nimbus_verified_proxy/rpc/rpc_eth_api.nim | 74 ++++++++++++------- nimbus_verified_proxy/rpc/rpc_utils.nim | 18 +++-- nimbus_verified_proxy/validate_proof.nim | 12 ++- 6 files changed, 110 insertions(+), 72 deletions(-) diff --git a/nimbus_verified_proxy/block_cache.nim b/nimbus_verified_proxy/block_cache.nim index d38ac9ec7..9c49782e0 100644 --- a/nimbus_verified_proxy/block_cache.nim +++ b/nimbus_verified_proxy/block_cache.nim @@ -5,7 +5,10 @@ # * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). # at your option. This file may not be copied, modified, or distributed except according to those terms. -{.push raises: [Defect].} +when (NimMajor, NimMinor) < (1, 4): + {.push raises: [Defect].} +else: + {.push raises: [].} import std/tables, diff --git a/nimbus_verified_proxy/nimbus_verified_proxy.nim b/nimbus_verified_proxy/nimbus_verified_proxy.nim index 76b64e1b3..2ffca3a1c 100644 --- a/nimbus_verified_proxy/nimbus_verified_proxy.nim +++ b/nimbus_verified_proxy/nimbus_verified_proxy.nim @@ -5,7 +5,10 @@ # * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). # at your option. This file may not be copied, modified, or distributed except according to those terms. -{.push raises: [Defect].} +when (NimMajor, NimMinor) < (1, 4): + {.push raises: [Defect].} +else: + {.push raises: [].} import std/[os, strutils], @@ -50,7 +53,7 @@ proc run() {.raises: [Exception, Defect].} = # Required as both Eth2Node and LightClient requires correct config type var lcConfig = config.asLightClientConf() - setupLogging(config.logLevel, config.logStdout, config.logFile) + setupLogging(config.logLevel, config.logStdout, none(OutFile)) notice "Launching Nimbus verified proxy", version = fullVersionStr, cmdParams = commandLineParams(), config diff --git a/nimbus_verified_proxy/nimbus_verified_proxy_conf.nim b/nimbus_verified_proxy/nimbus_verified_proxy_conf.nim index 756f037fd..d1651f5b1 100644 --- a/nimbus_verified_proxy/nimbus_verified_proxy_conf.nim +++ b/nimbus_verified_proxy/nimbus_verified_proxy_conf.nim @@ -19,7 +19,7 @@ import export net, conf -proc defaultLCPDataDir*(): string = +proc defaultVerifiedProxyDataDir*(): string = let dataDir = when defined(windows): "AppData" / "Roaming" / "NimbusVerifiedProxy" elif defined(macosx): @@ -30,7 +30,7 @@ proc defaultLCPDataDir*(): string = getHomeDir() / dataDir const - defaultDataLCPDirDesc* = defaultLCPDataDir() + defaultDataVerifiedProxyDirDesc* = defaultVerifiedProxyDataDir() type Web3UrlKind* = enum @@ -59,15 +59,11 @@ type VerifiedProxyConf* = object defaultValue: StdoutLogKind.Auto name: "log-format" .}: StdoutLogKind - logFile* {. - desc: "Specifies a path for the written Json log file (deprecated)" - name: "log-file" .}: Option[OutFile] - # Storage dataDir* {. - desc: "The directory where nimbus will store all blockchain data" - defaultValue: defaultLCPDataDir() - defaultValueDesc: $defaultDataLCPDirDesc + desc: "The directory where nimbus_verified_proxy will store all data" + defaultValue: defaultVerifiedProxyDataDir() + defaultValueDesc: $defaultDataVerifiedProxyDirDesc abbr: "d" name: "data-dir" .}: OutDir @@ -77,6 +73,30 @@ type VerifiedProxyConf* = object defaultValueDesc: "mainnet" name: "network" .}: Option[string] + # Consensus light sync + # No default - Needs to be provided by the user + trustedBlockRoot* {. + desc: "Recent trusted finalized block root to initialize the consensus light client from" + name: "trusted-block-root" .}: Eth2Digest + + # (Untrusted) web3 provider + # No default - Needs to be provided by the user + web3url* {. + desc: "URL of the web3 data provider" + name: "web3-url" .}: ValidatedWeb3Url + + # Local JSON-RPC server + rpcAddress* {. + desc: "Listening address of the JSON-RPC server" + defaultValue: defaultAdminListenAddress + defaultValueDesc: $defaultAdminListenAddressDesc + name: "rpc-address" .}: ValidIpAddress + + rpcPort* {. + desc: "Listening port of the JSON-RPC server" + defaultValue: 8545 + name: "rpc-port" .}: Port + # Libp2p bootstrapNodes* {. desc: "Specifies one or more bootstrap nodes to use when connecting to the network" @@ -106,9 +126,10 @@ type VerifiedProxyConf* = object defaultValueDesc: $defaultEth2TcpPortDesc name: "udp-port" .}: Port + # TODO: Select a lower amount of peers. maxPeers* {. desc: "The target number of peers to connect to" - defaultValue: 160 # 5 (fanout) * 64 (subnets) / 2 (subs) for a heathy mesh + defaultValue: 160 # 5 (fanout) * 64 (subnets) / 2 (subs) for a healthy mesh name: "max-peers" .}: int hardMaxPeers* {. @@ -131,7 +152,7 @@ type VerifiedProxyConf* = object agentString* {. defaultValue: "nimbus", - desc: "Node agent string which is used as identifier in network" + desc: "Node agent string which is used as identifier in the LibP2P network" name: "agent-string" .}: string discv5Enabled* {. @@ -144,29 +165,10 @@ type VerifiedProxyConf* = object "maintain the connection to, this requires a not random netkey-file." & "In the complete multiaddress format like:" & "/ip4/
/tcp//p2p/." & - "Peering agreements are established out of band and must be reciprocal." + "Peering agreements are established out of band and must be reciprocal" name: "direct-peer" .}: seq[string] - rpcPort* {. - desc: "HTTP port for the JSON-RPC server" - defaultValue: 8545 - name: "rpc-port" .}: Port - rpcAddress* {. - desc: "Listening address of the RPC server" - defaultValue: defaultAdminListenAddress - defaultValueDesc: $defaultAdminListenAddressDesc - name: "rpc-address" .}: ValidIpAddress - - # No default - Needs to be provided by the user - trustedBlockRoot* {. - desc: "Recent trusted finalized block root to initialize the consensus light client from" - name: "trusted-block-root" .}: Eth2Digest - - # No default - Needs to be provided by the user - web3url* {. - desc: "url of web3 data provider" - name: "web3-url" .}: ValidatedWeb3Url proc parseCmdArg*(T: type ValidatedWeb3Url, p: TaintedString): T {.raises: [Defect, ConfigurationError].} = @@ -189,7 +191,7 @@ func asLightClientConf*(pc: VerifiedProxyConf): LightClientConf = configFile: pc.configFile, logLevel: pc.logLevel, logStdout: pc.logStdout, - logFile: pc.logFile, + logFile: none(OutFile), dataDir: pc.dataDir, eth2Network: pc.eth2Network, bootstrapNodes: pc.bootstrapNodes, diff --git a/nimbus_verified_proxy/rpc/rpc_eth_api.nim b/nimbus_verified_proxy/rpc/rpc_eth_api.nim index 790baab73..f2b4655b7 100644 --- a/nimbus_verified_proxy/rpc/rpc_eth_api.nim +++ b/nimbus_verified_proxy/rpc/rpc_eth_api.nim @@ -5,7 +5,10 @@ # * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). # at your option. This file may not be copied, modified, or distributed except according to those terms. -{.push raises: [Defect].} +when (NimMajor, NimMinor) < (1, 4): + {.push raises: [Defect].} +else: + {.push raises: [].} import std/strutils, @@ -70,7 +73,7 @@ func parseHexQuantity(tag: string): Result[Quantity, string] = let parsed = ? parseHexIntResult(tag) return ok(Quantity(parsed)) else: - return err("Invalid Etheruem Hex quantity.") + return err("Invalid hex quantity.") func parseQuantityTag(blockTag: string): Result[QuantityTag, string] = let tag = blockTag.toLowerAscii @@ -85,11 +88,13 @@ template checkPreconditions(proxy: VerifiedRpcProxy) = if proxy.blockCache.isEmpty(): raise newException(ValueError, "Syncing") -template rpcClient(lcProxy: VerifiedRpcProxy): RpcClient = lcProxy.proxy.getClient() +template rpcClient(lcProxy: VerifiedRpcProxy): RpcClient = + lcProxy.proxy.getClient() proc getPayloadByTag( proxy: VerifiedRpcProxy, - quantityTag: string): results.Opt[ExecutionPayloadV1] {.raises: [ValueError, Defect].} = + quantityTag: string): + results.Opt[ExecutionPayloadV1] {.raises: [ValueError, Defect].} = checkPreconditions(proxy) let tagResult = parseQuantityTag(quantityTag) @@ -122,12 +127,13 @@ proc installEthApiHandlers*(lcProxy: VerifiedRpcProxy) = return encodeQuantity(lcProxy.chainId) lcProxy.proxy.rpc("eth_blockNumber") do() -> HexQuantityStr: - ## Returns the number of most recent block. + ## Returns the number of the most recent block. checkPreconditions(lcProxy) return encodeQuantity(lcProxy.blockCache.latest.get.blockNumber) - lcProxy.proxy.rpc("eth_getBalance") do(address: Address, quantityTag: string) -> HexQuantityStr: + lcProxy.proxy.rpc("eth_getBalance") do( + address: Address, quantityTag: string) -> HexQuantityStr: # 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 @@ -136,9 +142,10 @@ proc installEthApiHandlers*(lcProxy: VerifiedRpcProxy) = executionPayload = lcProxy.getPayloadByTagOrThrow(quantityTag) blockNumber = executionPayload.blockNumber.uint64 - info "Forwarding get_Balance", executionBn = blockNumber + info "Forwarding eth_getBalance call", blockNumber - let proof = await lcProxy.rpcClient.eth_getProof(address, @[], blockId(blockNumber)) + let proof = await lcProxy.rpcClient.eth_getProof( + address, @[], blockId(blockNumber)) let accountResult = getAccountFromProof( executionPayload.stateRoot, @@ -155,15 +162,17 @@ proc installEthApiHandlers*(lcProxy: VerifiedRpcProxy) = else: raise newException(ValueError, accountResult.error) - lcProxy.proxy.rpc("eth_getStorageAt") do(address: Address, slot: HexDataStr, quantityTag: string) -> HexDataStr: + lcProxy.proxy.rpc("eth_getStorageAt") do( + address: Address, slot: HexDataStr, quantityTag: string) -> HexDataStr: let executionPayload = lcProxy.getPayloadByTagOrThrow(quantityTag) uslot = UInt256.fromHex(slot.string) blockNumber = executionPayload.blockNumber.uint64 - info "Forwarding eth_getStorageAt", executionBn = blockNumber + info "Forwarding eth_getStorageAt", blockNumber - let proof = await lcProxy.rpcClient.eth_getProof(address, @[uslot], blockId(blockNumber)) + let proof = await lcProxy.rpcClient.eth_getProof( + address, @[uslot], blockId(blockNumber)) let dataResult = getStorageData(executionPayload.stateRoot, uslot, proof) @@ -173,14 +182,16 @@ proc installEthApiHandlers*(lcProxy: VerifiedRpcProxy) = else: raise newException(ValueError, dataResult.error) - lcProxy.proxy.rpc("eth_getTransactionCount") do(address: Address, quantityTag: string) -> HexQuantityStr: + lcProxy.proxy.rpc("eth_getTransactionCount") do( + address: Address, quantityTag: string) -> HexQuantityStr: let executionPayload = lcProxy.getPayloadByTagOrThrow(quantityTag) blockNumber = executionPayload.blockNumber.uint64 - info "Forwarding eth_getTransactionCount", executionBn = blockNumber + info "Forwarding eth_getTransactionCount", blockNumber - let proof = await lcProxy.rpcClient.eth_getProof(address, @[], blockId(blockNumber)) + let proof = await lcProxy.rpcClient.eth_getProof( + address, @[], blockId(blockNumber)) let accountResult = getAccountFromProof( executionPayload.stateRoot, @@ -197,13 +208,15 @@ proc installEthApiHandlers*(lcProxy: VerifiedRpcProxy) = else: raise newException(ValueError, accountResult.error) - lcProxy.proxy.rpc("eth_getCode") do(address: Address, quantityTag: string) -> HexDataStr: + lcProxy.proxy.rpc("eth_getCode") do( + address: Address, quantityTag: string) -> HexDataStr: let executionPayload = lcProxy.getPayloadByTagOrThrow(quantityTag) blockNumber = executionPayload.blockNumber.uint64 let - proof = await lcProxy.rpcClient.eth_getProof(address, @[], blockId(blockNumber)) + proof = await lcProxy.rpcClient.eth_getProof( + address, @[], blockId(blockNumber)) accountResult = getAccountFromProof( executionPayload.stateRoot, proof.address, @@ -223,6 +236,8 @@ proc installEthApiHandlers*(lcProxy: VerifiedRpcProxy) = # account does not have any code, return empty hex data return hexDataStr("0x") + info "Forwarding eth_getCode", blockNumber + let code = await lcProxy.rpcClient.eth_getCode( address, blockId(blockNumber) @@ -231,10 +246,12 @@ proc installEthApiHandlers*(lcProxy: VerifiedRpcProxy) = if isValidCode(account, code): return bytesToHex(code) else: - raise newException(ValueError, "received code which does not match account code hash") + raise newException(ValueError, + "Received code which does not match the account code hash") - # TODO This methods are forwarded directly to provider therefore thay are not - # validated in any way + # TODO: + # Following methods are forwarded directly to the web3 provider and therefore + # are not validated in any way. lcProxy.proxy.registerProxyMethod("net_version") lcProxy.proxy.registerProxyMethod("eth_call") lcProxy.proxy.registerProxyMethod("eth_sendRawTransaction") @@ -242,7 +259,8 @@ 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]: + lcProxy.proxy.rpc("eth_getBlockByNumber") do( + quantityTag: string, fullTransactions: bool) -> Option[BlockObject]: let executionPayload = lcProxy.getPayloadByTag(quantityTag) if executionPayload.isErr: @@ -250,7 +268,8 @@ proc installEthApiHandlers*(lcProxy: VerifiedRpcProxy) = return some(asBlockObject(executionPayload.get())) - lcProxy.proxy.rpc("eth_getBlockByHash") do(blockHash: BlockHash, fullTransactions: bool) -> Option[BlockObject]: + lcProxy.proxy.rpc("eth_getBlockByHash") do( + blockHash: BlockHash, fullTransactions: bool) -> Option[BlockObject]: let executionPayload = lcProxy.blockCache.getPayloadByHash(blockHash) if executionPayload.isErr: @@ -263,17 +282,15 @@ proc new*( proxy: RpcProxy, blockCache: BlockCache, chainId: Quantity): T = - - return VerifiedRpcProxy( + VerifiedRpcProxy( proxy: proxy, blockCache: blockCache, - chainId: chainId - ) + chainId: chainId) proc verifyChaindId*(p: VerifiedRpcProxy): Future[void] {.async.} = let localId = p.chainId - # retry 2 times, if the data provider will fail despite re-tries, propagate + # retry 2 times, if the data provider fails despite the re-tries, propagate # exception to the caller. let providerId = awaitWithRetries( p.rpcClient.eth_chainId(), @@ -281,8 +298,9 @@ proc verifyChaindId*(p: VerifiedRpcProxy): Future[void] {.async.} = timeout = seconds(30) ) - # this configuration error, in theory we could allow proxy to chung on, but - # it would only mislead the user. It is better to fail fast here. + # This is a chain/network mismatch error between the Nimbus verified proxy and + # the application using it. Fail fast to avoid misusage. The user must fix + # the configuration. if localId != providerId: fatal "The specified data provider serves data for a different chain", expectedChain = distinctBase(localId), diff --git a/nimbus_verified_proxy/rpc/rpc_utils.nim b/nimbus_verified_proxy/rpc/rpc_utils.nim index ece1525b4..f4e68428d 100644 --- a/nimbus_verified_proxy/rpc/rpc_utils.nim +++ b/nimbus_verified_proxy/rpc/rpc_utils.nim @@ -5,6 +5,11 @@ # * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). # at your option. This file may not be copied, modified, or distributed except according to those terms. +when (NimMajor, NimMinor) < (1, 4): + {.push raises: [Defect].} +else: + {.push raises: [].} + import std/typetraits, eth/common/eth_types as etypes, @@ -22,7 +27,8 @@ template asEthHash(hash: BlockHash): Hash256 = Hash256(data: distinctBase(hash)) proc calculateTransactionData( - items: openArray[TypedTransaction]): (etypes.Hash256, seq[TxHash], uint64) = + items: openArray[TypedTransaction]): + (etypes.Hash256, seq[TxHash], uint64) {.raises: [Defect, RlpError].} = ## returns tuple composed of ## - root of transactions trie ## - list of transactions hashes @@ -37,7 +43,8 @@ proc calculateTransactionData( txHashes.add(toFixedBytes(keccakHash(tx))) return (tr.rootHash(), txHashes, txSize) -func blockHeaderSize(payload: ExecutionPayloadV1, txRoot: etypes.Hash256): uint64 = +func blockHeaderSize( + payload: ExecutionPayloadV1, txRoot: etypes.Hash256): uint64 = let bh = etypes.BlockHeader( parentHash : payload.parentHash.asEthHash, ommersHash : etypes.EMPTY_UNCLE_HASH, @@ -58,8 +65,9 @@ func blockHeaderSize(payload: ExecutionPayloadV1, txRoot: etypes.Hash256): uint6 ) return uint64(len(rlp.encode(bh))) -proc asBlockObject*(p: ExecutionPayloadV1): BlockObject = - # TODO currently we always calculate txHashes as BlockObject does not have +proc asBlockObject*( + p: ExecutionPayloadV1): BlockObject {.raises: [Defect, RlpError].} = + # TODO: currently we always calculate txHashes as BlockObject does not have # option of returning full transactions. It needs fixing at nim-web3 library # level let (txRoot, txHashes, txSize) = calculateTransactionData(p.transactions) @@ -82,7 +90,7 @@ proc asBlockObject*(p: ExecutionPayloadV1): BlockObject = timestamp: p.timestamp, nonce: some(default(FixedBytes[8])), size: Quantity(blockSize), - # TODO It does not matter what we put here in after merge blocks. + # TODO: It does not matter what we put here after merge blocks. # Other projects like `helios` return `0`, data providers like alchemy return # transition difficulty. For now retruning `0` as this is a bit easier to do. totalDifficulty: UInt256.zero, diff --git a/nimbus_verified_proxy/validate_proof.nim b/nimbus_verified_proxy/validate_proof.nim index 0e705712e..06736b2ba 100644 --- a/nimbus_verified_proxy/validate_proof.nim +++ b/nimbus_verified_proxy/validate_proof.nim @@ -5,7 +5,10 @@ # * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). # at your option. This file may not be copied, modified, or distributed except according to those terms. -{.push raises: [Defect].} +when (NimMajor, NimMinor) < (1, 4): + {.push raises: [Defect].} +else: + {.push raises: [].} import std/[sequtils, typetraits, options], @@ -36,8 +39,8 @@ proc isValidProof( key, value: seq[byte]): bool = try: # TODO: Investigate if this handles proof of non-existence. - # Probably not as bool is not expressive enough to say if proof is valid, but - # key actually does not exists in MPT + # Probably not as bool is not expressive enough to say if proof is valid, + # but key actually does not exists in MPT return isValidBranch(branch, rootHash, key, value) except RlpError: return false @@ -85,7 +88,8 @@ proc getStorageData( storageMptNodes = storageProof.proof.mapIt(distinctBase(it)) key = toSeq(keccakHash(toBytesBE(storageProof.key)).data) encodedValue = rlp.encode(storageProof.value) - proofResult = verifyMptProof(storageMptNodes, account.storageRoot, key, encodedValue) + proofResult = verifyMptProof( + storageMptNodes, account.storageRoot, key, encodedValue) case proofResult.kind of MissingKey: