From 24132d71294afe3b22b2792601be6dc1ff3441db Mon Sep 17 00:00:00 2001 From: Dmitriy Ryajov Date: Sat, 22 May 2021 12:27:30 -0600 Subject: [PATCH] More raises cleanup (#575) * use toException to map errors * don't initialize address twice * better error messages * allow LPError to escape * allow LPError to escape --- libp2p/builders.nim | 15 +++++++++------ libp2p/daemon/daemonapi.nim | 14 +++++++++++--- libp2p/peerinfo.nim | 32 +++++++++++++++++++++---------- libp2p/protocols/secure/noise.nim | 16 +++++++++------- libp2p/protocols/secure/secio.nim | 2 +- 5 files changed, 52 insertions(+), 27 deletions(-) diff --git a/libp2p/builders.nim b/libp2p/builders.nim index 0bf097f..a9ac32f 100644 --- a/libp2p/builders.nim +++ b/libp2p/builders.nim @@ -50,7 +50,7 @@ proc new*(T: type[SwitchBuilder]): T = let address = MultiAddress .init("/ip4/127.0.0.1/tcp/0") - .expect("address should initialize to default") + .expect("Should initialize to default") SwitchBuilder( privKey: none(PrivateKey), @@ -123,13 +123,15 @@ proc withAgentVersion*(b: SwitchBuilder, agentVersion: string): SwitchBuilder = b.agentVersion = agentVersion b -proc build*(b: SwitchBuilder): Switch = +proc build*(b: SwitchBuilder): Switch + {.raises: [Defect, LPError].} = + if b.rng == nil: # newRng could fail raise newException(Defect, "Cannot initialize RNG") let pkRes = PrivateKey.random(b.rng[]) let - seckey = b.privKey.get(otherwise = pkRes.expect("Should supply a valid RNG")) + seckey = b.privKey.get(otherwise = pkRes.expect("Expected default Private Key")) var secureManagerInstances: seq[Secure] @@ -183,7 +185,7 @@ proc build*(b: SwitchBuilder): Switch = return switch proc newStandardSwitch*(privKey = none(PrivateKey), - address = MultiAddress.init("/ip4/127.0.0.1/tcp/0").tryGet(), + address = MultiAddress.init("/ip4/127.0.0.1/tcp/0"), secureManagers: openarray[SecureProtocol] = [ SecureProtocol.Noise, ], @@ -194,13 +196,14 @@ proc newStandardSwitch*(privKey = none(PrivateKey), maxConnections = MaxConnections, maxIn = -1, maxOut = -1, - maxConnsPerPeer = MaxConnectionsPerPeer): Switch = + maxConnsPerPeer = MaxConnectionsPerPeer): Switch + {.raises: [Defect, LPError].} = if SecureProtocol.Secio in secureManagers: quit("Secio is deprecated!") # use of secio is unsafe var b = SwitchBuilder .new() - .withAddress(address) + .withAddress(address.expect("Should have been initialized with default")) .withRng(rng) .withMaxConnections(maxConnections) .withMaxIn(maxIn) diff --git a/libp2p/daemon/daemonapi.nim b/libp2p/daemon/daemonapi.nim index 3ecc16b..f27c446 100644 --- a/libp2p/daemon/daemonapi.nim +++ b/libp2p/daemon/daemonapi.nim @@ -11,12 +11,13 @@ ## This module implementes API for `go-libp2p-daemon`. import std/[os, osproc, strutils, tables, strtabs] -import chronos, chronicles +import pkg/[chronos, chronicles] import ../varint, ../multiaddress, ../multicodec, ../cid, ../peerid import ../wire, ../multihash, ../protobuf/minprotobuf import ../crypto/crypto -export peerid, multiaddress, multicodec, multihash, cid, crypto, wire +export + peerid, multiaddress, multicodec, multihash, cid, crypto, wire when not defined(windows): import posix @@ -154,6 +155,9 @@ type ticket: PubsubTicket, message: PubSubMessage): Future[bool] {.gcsafe.} + # TODO: would be nice to be able to map other errors to + # this types with `Result.toException`, but it doesn't work + # in this module DaemonRemoteError* = object of CatchableError DaemonLocalError* = object of CatchableError @@ -833,15 +837,19 @@ proc getPeerInfo(pb: var ProtoBuffer): PeerInfo result.addresses = newSeq[MultiAddress]() if pb.getValue(1, result.peer) == -1: raise newException(DaemonLocalError, "Missing required field `peer`!") + var address = newSeq[byte]() while pb.getBytes(2, address) != -1: if len(address) != 0: var copyaddr = address let addrRes = MultiAddress.init(copyaddr) + + # TODO: for some reason `toException` doesn't + # work for this module if addrRes.isErr: raise newException(DaemonLocalError, addrRes.error) - result.addresses.add(MultiAddress.init(copyaddr).get()) + result.addresses.add(addrRes.get()) address.setLen(0) proc identity*(api: DaemonAPI): Future[PeerInfo] {.async.} = diff --git a/libp2p/peerinfo.nim b/libp2p/peerinfo.nim index 81b41bd..c7f946b 100644 --- a/libp2p/peerinfo.nim +++ b/libp2p/peerinfo.nim @@ -9,11 +9,11 @@ {.push raises: [Defect].} -import options, sequtils, hashes -import chronos, chronicles -import peerid, multiaddress, crypto/crypto +import std/[options, sequtils, hashes] +import pkg/[chronos, chronicles, stew/results] +import peerid, multiaddress, crypto/crypto, errors -export peerid, multiaddress, crypto +export peerid, multiaddress, crypto, errors, results ## A peer can be constructed in one of tree ways: ## 1) A local peer with a private key @@ -28,6 +28,8 @@ type HasPrivate, HasPublic + PeerInfoError* = LPError + PeerInfo* = ref object of RootObj peerId*: PeerID addrs*: seq[MultiAddress] @@ -51,6 +53,12 @@ func shortLog*(p: PeerInfo): auto = ) chronicles.formatIt(PeerInfo): shortLog(it) +func toException*(e: string): ref PeerInfoError = + (ref PeerInfoError)(msg: e) + +func toException*(e: cstring): ref PeerInfoError = + (ref PeerInfoError)(msg: $e) + template postInit(peerinfo: PeerInfo, addrs: openarray[MultiAddress], protocols: openarray[string]) = @@ -65,10 +73,12 @@ proc init*( addrs: openarray[MultiAddress] = [], protocols: openarray[string] = [], protoVersion: string = "", - agentVersion: string = ""): PeerInfo = + agentVersion: string = ""): PeerInfo + {.raises: [Defect, PeerInfoError].} = + let peerInfo = PeerInfo( keyType: HasPrivate, - peerId: PeerID.init(key).expect("Unable to create peer id from key"), + peerId: PeerID.init(key).tryGet(), privateKey: key, protoVersion: protoVersion, agentVersion: agentVersion) @@ -98,11 +108,12 @@ proc init*( addrs: openarray[MultiAddress] = [], protocols: openarray[string] = [], protoVersion: string = "", - agentVersion: string = ""): PeerInfo = + agentVersion: string = ""): PeerInfo + {.raises: [Defect, PeerInfoError].} = let peerInfo = PeerInfo( keyType: HasPublic, - peerId: PeerID.init(peerId).expect("Unable to create peer id from string"), + peerId: PeerID.init(peerId).tryGet(), protoVersion: protoVersion, agentVersion: agentVersion) @@ -115,11 +126,12 @@ proc init*( addrs: openarray[MultiAddress] = [], protocols: openarray[string] = [], protoVersion: string = "", - agentVersion: string = ""): PeerInfo = + agentVersion: string = ""): PeerInfo + {.raises: [Defect, PeerInfoError].} = let peerInfo = PeerInfo( keyType: HasPublic, - peerId: PeerID.init(key).expect("Unable to create peer id from public key"), + peerId: PeerID.init(key).tryGet(), key: some(key), protoVersion: protoVersion, agentVersion: agentVersion) diff --git a/libp2p/protocols/secure/noise.nim b/libp2p/protocols/secure/noise.nim index c5af536..cec4283 100644 --- a/libp2p/protocols/secure/noise.nim +++ b/libp2p/protocols/secure/noise.nim @@ -189,7 +189,7 @@ proc mixKey(ss: var SymmetricState, ikm: ChaChaPolyKey) = ss.cs = CipherState(k: temp_keys[1]) trace "mixKey", key = ss.cs.k.shortLog -proc mixHash(ss: var SymmetricState; data: openArray[byte]) = +proc mixHash(ss: var SymmetricState, data: openArray[byte]) = var ctx: sha256 ctx.init() ctx.update(ss.h.data) @@ -198,7 +198,7 @@ proc mixHash(ss: var SymmetricState; data: openArray[byte]) = trace "mixHash", hash = ss.h.data.shortLog # We might use this for other handshake patterns/tokens -proc mixKeyAndHash(ss: var SymmetricState; ikm: openArray[byte]) {.used.} = +proc mixKeyAndHash(ss: var SymmetricState, ikm: openArray[byte]) {.used.} = var temp_keys: array[3, ChaChaPolyKey] sha256.hkdf(ss.ck, ikm, [], temp_keys) @@ -597,14 +597,16 @@ method init*(p: Noise) {.gcsafe.} = proc newNoise*( rng: ref BrHmacDrbgContext, - privateKey: PrivateKey; - outgoing: bool = true; + privateKey: PrivateKey, + outgoing: bool = true, commonPrologue: seq[byte] = @[]): Noise = - let pkBytes = privateKey.getKey() - .expect("Expected valid private key") + let pkBytes = privateKey + .getKey() + .expect("Expected valid Private Key") .getBytes() - .expect("Couldn't get key bytes") + .expect("Couldn't get Private Key bytes") + var noise = Noise( rng: rng, outgoing: outgoing, diff --git a/libp2p/protocols/secure/secio.nim b/libp2p/protocols/secure/secio.nim index c4211ab..d89b6c7 100644 --- a/libp2p/protocols/secure/secio.nim +++ b/libp2p/protocols/secure/secio.nim @@ -259,7 +259,7 @@ proc newSecioConn(conn: Connection, secrets: Secret, order: int, remotePubKey: PublicKey): SecioConn - {.raises: [Defect, SecioError].} = + {.raises: [Defect, LPError].} = ## Create new secure stream/lpstream, using specified hash algorithm ``hash``, ## cipher algorithm ``cipher``, stretched keys ``secrets`` and order ## ``order``.