setup and persist private key (#292)

* setup and persist private key

* return dht record spr

* helper to remap multiaddr ip and port

* set/update discovery and announce addrs

* add nat and discovery IPs

* allow for announce and DHT addresses separatelly

* update tests

* check for nat or discoveryIp

* fix integration tests

* misc align

* don't share data dirs and and set bootstrap node

* add log scope

* remap announceAddrs after node start

* simplify discovery initialization

* make nat and disc-ip required

* add log scope don't init dht spr in constructor

* bump dht

* dissallow `0.0.0.0` for `--nat`

codex.nim

@@ -14,14 +14,13 @@ import pkg/libp2p
 
 import ./codex/conf
 import ./codex/codex
+import ./codex/utils/keyutils
 
 export codex, conf, libp2p, chronos, chronicles
 
 when isMainModule:
   import std/os
-
   import pkg/confutils/defs
-
   import ./codex/utils/fileutils
 
   logScope:
@@ -39,6 +38,13 @@ when isMainModule:
   case config.cmd:
   of StartUpCommand.noCommand:
 
+    if config.nat == ValidIpAddress.init(IPv4_any()):
+      error "`--nat` cannot be set to the any (`0.0.0.0`) address"
+      quit QuitFailure
+
+    if config.nat == ValidIpAddress.init("127.0.0.1"):
+      warn "`--nat` is set to local loopback, your node wont be properly announce over the DHT"
+
     if not(checkAndCreateDataDir((config.dataDir).string)):
       # We are unable to access/create data folder or data folder's
       # permissions are insecure.
@@ -53,7 +59,15 @@ when isMainModule:
 
     trace "Repo dir initialized", dir = config.dataDir / "repo"
 
-    let server = CodexServer.new(config)
+    let
+      keyPath =
+        if isAbsolute(string config.netPrivKeyFile):
+          string config.netPrivKeyFile
+        else:
+          string config.dataDir / string config.netPrivKeyFile
+
+      privateKey = setupKey(keyPath).expect("Should setup private key!")
+      server = CodexServer.new(config, privateKey)
 
     ## Ctrl+C handling
     proc controlCHandler() {.noconv.} =

codex/codex.nim

@@ -31,6 +31,11 @@ import ./utils/fileutils
 import ./erasure
 import ./discovery
 import ./contracts
+import ./utils/keyutils
+import ./utils/addrutils
+
+logScope:
+  topics = "codex node"
 
 type
   CodexServer* = ref object
@@ -39,10 +44,37 @@ type
     restServer: RestServerRef
     codexNode: CodexNodeRef
 
+  CodexPrivateKey* = libp2p.PrivateKey # alias
+
 proc start*(s: CodexServer) {.async.} =
   s.restServer.start()
   await s.codexNode.start()
 
+  let
+    # TODO: Can't define this as constants, pity
+    natIpPart = MultiAddress.init("/ip4/" & $s.config.nat & "/")
+      .expect("Should create multiaddress")
+    anyAddrIp = MultiAddress.init("/ip4/0.0.0.0/")
+      .expect("Should create multiaddress")
+    loopBackAddrIp = MultiAddress.init("/ip4/127.0.0.1/")
+      .expect("Should create multiaddress")
+
+    # announce addresses should be set to bound addresses,
+    # but the IP should be mapped to the provided nat ip
+    announceAddrs = s.codexNode.switch.peerInfo.addrs.mapIt:
+      block:
+        let
+          listenIPPart = it[multiCodec("ip4")].expect("Should get IP")
+
+        if listenIPPart == anyAddrIp or
+          (listenIPPart == loopBackAddrIp and natIpPart != loopBackAddrIp):
+          it.remapAddr(s.config.nat.some)
+        else:
+          it
+
+  s.codexNode.discovery.updateAnnounceRecord(announceAddrs)
+  s.codexNode.discovery.updateDhtRecord(s.config.nat, s.config.discoveryPort)
+
   s.runHandle = newFuture[void]()
   await s.runHandle
 
@@ -67,39 +99,9 @@ proc new(_: type ContractInteractions, config: CodexConf): ?ContractInteractions
   else:
     ContractInteractions.new(config.ethProvider, account)
 
-proc new*(T: type CodexServer, config: CodexConf): T =
+proc new*(T: type CodexServer, config: CodexConf, privateKey: CodexPrivateKey): T =
 
-  const SafePermissions = {UserRead, UserWrite}
   let
-    privateKey =
-      if config.netPrivKeyFile == "random":
-        PrivateKey.random(Rng.instance()[]).get()
-      else:
-        let path =
-          if config.netPrivKeyFile.isAbsolute:
-            config.netPrivKeyFile
-          else:
-            config.dataDir / config.netPrivKeyFile
-
-        if path.fileAccessible({AccessFlags.Find}):
-          info "Found a network private key"
-
-          if path.getPermissionsSet().get() != SafePermissions:
-            warn "The network private key file is not safe, aborting"
-            quit QuitFailure
-
-          PrivateKey.init(path.readAllBytes().expect("accessible private key file")).
-            expect("valid private key file")
-        else:
-          info "Creating a private key and saving it"
-          let
-            res = PrivateKey.random(Rng.instance()[]).get()
-            bytes = res.getBytes().get()
-
-          path.writeFile(bytes, SafePermissions.toInt()).expect("writing private key file")
-
-          PrivateKey.init(bytes).expect("valid key bytes")
-
     switch = SwitchBuilder
     .new()
     .withPrivateKey(privateKey)
@@ -124,16 +126,11 @@ proc new*(T: type CodexServer, config: CodexConf): T =
       config.dataDir / "dht")
       .expect("Should not fail!"))
 
-    announceAddrs =
+    discovery = Discovery.new(
-      if config.announceAddrs.len <= 0:
-          config.announceAddrs
-        else:
-          config.listenAddrs
-
-    blockDiscovery = Discovery.new(
       switch.peerInfo.privateKey,
-      announceAddrs = config.announceAddrs,
+      announceAddrs = config.listenAddrs,
-      discoveryPort = config.discoveryPort,
+      bindIp = config.discoveryIp,
+      bindPort = config.discoveryPort,
       bootstrapNodes = config.bootstrapNodes,
       store = discoveryStore)
 
@@ -150,18 +147,18 @@ proc new*(T: type CodexServer, config: CodexConf): T =
     localStore = FSStore.new(repoDir, cache = cache)
     peerStore = PeerCtxStore.new()
     pendingBlocks = PendingBlocksManager.new()
-    discovery = DiscoveryEngine.new(localStore, peerStore, network, blockDiscovery, pendingBlocks)
+    blockDiscovery = DiscoveryEngine.new(localStore, peerStore, network, discovery, pendingBlocks)
-    engine = BlockExcEngine.new(localStore, wallet, network, discovery, peerStore, pendingBlocks)
+    engine = BlockExcEngine.new(localStore, wallet, network, blockDiscovery, peerStore, pendingBlocks)
     store = NetworkStore.new(engine, localStore)
     erasure = Erasure.new(store, leoEncoderProvider, leoDecoderProvider)
     contracts = ContractInteractions.new(config)
-    codexNode = CodexNodeRef.new(switch, store, engine, erasure, blockDiscovery, contracts)
+    codexNode = CodexNodeRef.new(switch, store, engine, erasure, discovery, contracts)
     restServer = RestServerRef.new(
       codexNode.initRestApi(config),
       initTAddress("127.0.0.1" , config.apiPort),
       bufferSize = (1024 * 64),
       maxRequestBodySize = int.high)
-      .tryGet()
+      .expect("Should start rest server!")
 
   switch.mount(network)
   T(

codex/conf.nim

@@ -95,22 +95,29 @@ type
         abbr: "i"
         name: "listen-addrs" }: seq[MultiAddress]
 
-      announceAddrs* {.
+      nat* {.
-        desc: "Multi Addresses to announce behind a NAT"
+        # TODO: change this once we integrate nat support
-        defaultValue: @[]
+        desc: "IP Addresses to announce behind a NAT"
-        defaultValueDesc: ""
+        defaultValue: ValidIpAddress.init("127.0.0.1")
+        defaultValueDesc: "127.0.0.1"
         abbr: "a"
-        name: "announce-addrs" }: seq[MultiAddress]
+        name: "nat" }: ValidIpAddress
+
+      discoveryIp* {.
+        desc: "Discovery listen address"
+        defaultValue: ValidIpAddress.init(IPv4_any())
+        defaultValueDesc: "0.0.0.0"
+        name: "disc-ip" }: ValidIpAddress
 
       discoveryPort* {.
-        desc: "Specify the discovery (UDP) port"
+        desc: "Discovery (UDP) port"
         defaultValue: Port(8090)
         defaultValueDesc: "8090"
-        name: "udp-port" }: Port
+        name: "disc-port" }: Port
 
       netPrivKeyFile* {.
-        desc: "Source of network (secp256k1) private key file (random|<path>)"
+        desc: "Source of network (secp256k1) private key file path or name"
-        defaultValue: "random"
+        defaultValue: "key"
         name: "net-privkey" }: string
 
       bootstrapNodes* {.
@@ -183,7 +190,6 @@ const
     "Codex build " & codexVersion & "\p" &
     nimBanner
 
-
 proc defaultDataDir*(): string =
   let dataDir = when defined(windows):
     "AppData" / "Roaming" / "Codex"

codex/discovery.nim

@@ -22,7 +22,6 @@ import pkg/libp2pdht/discv5/protocol as discv5
 
 import ./rng
 import ./errors
-import ./formats
 
 export discv5
 
@@ -30,12 +29,18 @@ export discv5
 # deprecated, this could have been implemented
 # much more elegantly.
 
+logScope:
+  topics = "codex discovery"
+
 type
   Discovery* = ref object of RootObj
-    protocol: discv5.Protocol
+    protocol: discv5.Protocol           # dht protocol
-    key: PrivateKey
+    key: PrivateKey                     # private key
-    announceAddrs: seq[MultiAddress]
+    peerId: PeerId                      # the peer id of the local node
-    record: SignedPeerRecord
+    announceAddrs: seq[MultiAddress]    # addresses announced as part of the provider records
+    providerRecord*: ?SignedPeerRecord  # record to advertice node connection information, this carry any
+                                        # address that the node can be connected on
+    dhtRecord*: ?SignedPeerRecord       # record to advertice DHT connection information
 
 proc toNodeId*(cid: Cid): NodeId =
   ## Cid to discovery id
@@ -57,9 +62,9 @@ proc findPeer*(
 
   return
     if node.isSome():
-      some(node.get().record.data)
+      node.get().record.data.some
     else:
-      none(PeerRecord)
+      PeerRecord.none
 
 method find*(
   d: Discovery,
@@ -81,7 +86,7 @@ method provide*(d: Discovery, cid: Cid) {.async, base.} =
   trace "Providing block", cid
   let
     nodes = await d.protocol.addProvider(
-      cid.toNodeId(), d.record)
+      cid.toNodeId(), d.providerRecord.get)
 
   if nodes.len <= 0:
     trace "Couldn't provide to any nodes!"
@@ -116,7 +121,7 @@ method provide*(d: Discovery, host: ca.Address) {.async, base.} =
   trace "Providing host", host = $host
   let
     nodes = await d.protocol.addProvider(
-    host.toNodeId(), d.record)
+      host.toNodeId(), d.providerRecord.get)
   if nodes.len > 0:
     trace "Provided to nodes", nodes = nodes.len
 
@@ -127,20 +132,32 @@ method removeProvider*(d: Discovery, peerId: PeerId): Future[void] {.base.} =
   trace "Removing provider", peerId
   d.protocol.removeProvidersLocal(peerId)
 
-proc updateRecord*(d: Discovery, addrs: openArray[MultiAddress]) =
+proc updateAnnounceRecord*(d: Discovery, addrs: openArray[MultiAddress]) =
   ## Update providers record
   ##
 
   d.announceAddrs = @addrs
-  d.record = SignedPeerRecord.init(
+
-    d.key,
+  trace "Updating announce record", addrs = d.announceAddrs
-    PeerRecord.init(
+  d.providerRecord = SignedPeerRecord.init(
-      PeerId.init(d.key).expect("Should construct PeerId"),
+    d.key, PeerRecord.init(d.peerId, d.announceAddrs))
-      d.announceAddrs)).expect("Should construct signed record")
+      .expect("Should construct signed record").some
 
   if not d.protocol.isNil:
-    d.protocol.updateRecord(d.record.some)
+    d.protocol.updateRecord(d.providerRecord)
-      .expect("should update SPR")
+      .expect("Should update SPR")
+
+proc updateDhtRecord*(d: Discovery, ip: ValidIpAddress, port: Port) =
+  ## Update providers record
+  ##
+
+  trace "Updating Dht record", ip, port = $port
+  d.dhtRecord = SignedPeerRecord.init(
+    d.key, PeerRecord.init(d.peerId, @[
+      MultiAddress.init(
+        ip,
+        IpTransportProtocol.udpProtocol,
+        port)])).expect("Should construct signed record").some
 
 proc start*(d: Discovery) {.async.} =
   d.protocol.open()
@@ -152,34 +169,25 @@ proc stop*(d: Discovery) {.async.} =
 proc new*(
   T: type Discovery,
   key: PrivateKey,
-  discoveryIp = IPv4_any(),
+  bindIp = ValidIpAddress.init(IPv4_any()),
-  discoveryPort = 0.Port,
+  bindPort = 0.Port,
-  announceAddrs: openArray[MultiAddress] = [],
+  announceAddrs: openArray[MultiAddress],
   bootstrapNodes: openArray[SignedPeerRecord] = [],
   store: Datastore = SQLiteDatastore.new(Memory)
-  .expect("Should not fail!")): T =
+    .expect("Should not fail!")): T =
-
-  let
-    announceAddrs =
-      if announceAddrs.len <= 0:
-        @[
-          MultiAddress.init(
-            ValidIpAddress.init(discoveryIp),
-            IpTransportProtocol.tcpProtocol,
-            discoveryPort)]
-      else:
-        @announceAddrs
 
   var
-    self = T(key: key)
+    self = T(
+      key: key,
+      peerId: PeerId.init(key).expect("Should construct PeerId"))
 
-  self.updateRecord(announceAddrs)
+  self.updateAnnounceRecord(announceAddrs)
 
   self.protocol = newProtocol(
     key,
-    bindIp = discoveryIp,
+    bindIp = bindIp.toNormalIp,
-    bindPort = discoveryPort,
+    bindPort = bindPort,
-    record = self.record,
+    record = self.providerRecord.get,
     bootstrapRecords = bootstrapNodes,
     rng = Rng.instance(),
     providers = ProvidersManager.new(store))

codex/rest/api.nim

@@ -195,9 +195,9 @@ proc initRestApi*(node: CodexNodeRef, conf: CodexConf): RestRouter =
     "/api/codex/v1/storage/request/{cid}") do (cid: Cid) -> RestApiResponse:
       ## Create a request for storage
       ##
-      ## cid            - the cid of a previously uploaded dataset
+      ## cid      - the cid of a previously uploaded dataset
-      ## duration       - the duration of the contract
+      ## duration - the duration of the contract
-      ## reward       - the maximum price the client is willing to pay
+      ## reward   - the maximum price the client is willing to pay
 
       without cid =? cid.tryGet.catch, error:
         return RestApiResponse.error(Http400, error.msg)
@@ -265,12 +265,17 @@ proc initRestApi*(node: CodexNodeRef, conf: CodexConf): RestRouter =
       ## Print rudimentary node information
       ##
 
-      let json = %*{
+      let
-        "id": $node.switch.peerInfo.peerId,
+        json = %*{
-        "addrs": node.switch.peerInfo.addrs.mapIt( $it ),
+          "id": $node.switch.peerInfo.peerId,
-        "repo": $conf.dataDir,
+          "addrs": node.switch.peerInfo.addrs.mapIt( $it ),
-        "spr": node.switch.peerInfo.signedPeerRecord.toURI
+          "repo": $conf.dataDir,
-      }
+          "spr":
+            if node.discovery.dhtRecord.isSome:
+              node.discovery.dhtRecord.get.toURI
+            else:
+              ""
+        }
 
       return RestApiResponse.response($json)
 

codex/utils/addrutils.nim

@@ -0,0 +1,40 @@
+## Nim-Codex
+## Copyright (c) 2022 Status Research & Development GmbH
+## Licensed under either of
+##  * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE))
+##  * MIT license ([LICENSE-MIT](LICENSE-MIT))
+## at your option.
+## This file may not be copied, modified, or distributed except according to
+## those terms.
+
+import pkg/upraises
+push: {.upraises: [].}
+
+import std/strutils
+import std/options
+
+import pkg/libp2p
+import pkg/stew/shims/net
+
+func remapAddr*(
+  address: MultiAddress,
+  ip: Option[ValidIpAddress] = ValidIpAddress.none,
+  port: Option[Port] = Port.none): MultiAddress =
+  ## Remap addresses to new IP and/or Port
+  ##
+
+  var
+    parts = ($address).split("/")
+
+  parts[2] = if ip.isSome:
+      $ip.get
+    else:
+      parts[2]
+
+  parts[4] = if port.isSome:
+      $port.get
+    else:
+      parts[4]
+
+  MultiAddress.init(parts.join("/"))
+    .expect("Should construct multiaddress")

codex/utils/keyutils.nim

@@ -0,0 +1,50 @@
+
+## Nim-Codex
+## Copyright (c) 2022 Status Research & Development GmbH
+## Licensed under either of
+##  * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE))
+##  * MIT license ([LICENSE-MIT](LICENSE-MIT))
+## at your option.
+## This file may not be copied, modified, or distributed except according to
+## those terms.
+
+import pkg/upraises
+push: {.upraises: [].}
+
+import std/os
+
+import pkg/chronicles
+import pkg/questionable/results
+import pkg/libp2p
+
+import ./fileutils
+import ../conf
+import ../errors
+import ../rng
+
+const
+  SafePermissions = {UserRead, UserWrite}
+
+type
+  CodexKeyError = object of CodexError
+  CodexKeyUnsafeError = object of CodexKeyError
+
+proc setupKey*(path: string): ?!PrivateKey =
+  if not path.fileAccessible({AccessFlags.Find}):
+    info "Creating a private key and saving it"
+    let
+      res = ? PrivateKey.random(Rng.instance()[]).mapFailure(CodexKeyError)
+      bytes = ? res.getBytes().mapFailure(CodexKeyError)
+
+    ? path.writeFile(bytes, SafePermissions.toInt()).mapFailure(CodexKeyError)
+    return PrivateKey.init(bytes).mapFailure(CodexKeyError)
+
+  info "Found a network private key"
+  if path.getPermissionsSet().get() != SafePermissions:
+    warn "The network private key file is not safe, aborting"
+    return failure newException(
+      CodexKeyUnsafeError, "The network private key file is not safe")
+
+  return PrivateKey.init(
+    ? path.readAllBytes().mapFailure(CodexKeyError))
+    .mapFailure(CodexKeyError)

tests/codex/helpers/nodeutils.nim

@@ -29,7 +29,10 @@ proc generateNodes*(
   for i in 0..<num:
     let
       switch = newStandardSwitch(transportFlags = {ServerFlags.ReuseAddr})
-      discovery = Discovery.new(switch.peerInfo.privateKey)
+      discovery = Discovery.new(
+        switch.peerInfo.privateKey,
+        announceAddrs = @[MultiAddress.init("/ip4/127.0.0.1/tcp/0")
+          .expect("Should return multiaddress")])
       wallet = WalletRef.example
       network = BlockExcNetwork.new(switch)
       localStore = CacheStore.new(blocks.mapIt( it ))

tests/codex/storageproofs/testnetwork.nim

@@ -81,8 +81,8 @@ suite "Storage Proofs Network":
     switch1 = newStandardSwitch()
     switch2 = newStandardSwitch()
 
-    discovery1 = MockDiscovery.new(switch1.peerInfo.privateKey)
+    discovery1 = MockDiscovery.new()
-    discovery2 = MockDiscovery.new(switch2.peerInfo.privateKey)
+    discovery2 = MockDiscovery.new()
 
     stpNetwork1 = StpNetwork.new(switch1, discovery1)
     stpNetwork2 = StpNetwork.new(switch2, discovery2)

tests/codex/testnode.nim

@@ -80,7 +80,10 @@ suite "Test Node":
     wallet = WalletRef.new(EthPrivateKey.random())
     network = BlockExcNetwork.new(switch)
     localStore = CacheStore.new()
-    blockDiscovery = Discovery.new(switch.peerInfo.privateKey)
+    blockDiscovery = Discovery.new(
+      switch.peerInfo.privateKey,
+      announceAddrs = @[MultiAddress.init("/ip4/127.0.0.1/tcp/0")
+        .expect("Should return multiaddress")])
     peerStore = PeerCtxStore.new()
     pendingBlocks = PendingBlocksManager.new()
     discovery = DiscoveryEngine.new(localStore, peerStore, network, blockDiscovery, pendingBlocks)

tests/testIntegration.nim

@@ -1,6 +1,9 @@
 import std/osproc
+import std/os
 import std/httpclient
 import std/json
+import std/strutils
+
 import pkg/chronos
 import ./ethertest
 import ./contracts/time
@@ -13,31 +16,47 @@ ethersuite "Integration tests":
   var baseurl1, baseurl2: string
   var client: HttpClient
 
+  let dataDir1 = getTempDir() / "Codex1"
+  let dataDir2 = getTempDir() / "Codex2"
+
   setup:
     await provider.getSigner(accounts[0]).mint()
     await provider.getSigner(accounts[1]).mint()
     await provider.getSigner(accounts[1]).deposit()
-    node1 = startNode [
+
-      "--api-port=8080",
-      "--udp-port=8090",
-      "--persistence",
-      "--eth-account=" & $accounts[0]
-    ]
-    node2 = startNode [
-      "--api-port=8081",
-      "--udp-port=8091",
-      "--persistence",
-      "--eth-account=" & $accounts[1]
-    ]
     baseurl1 = "http://localhost:8080/api/codex/v1"
     baseurl2 = "http://localhost:8081/api/codex/v1"
     client = newHttpClient()
 
+    node1 = startNode([
+      "--api-port=8080",
+      "--data-dir=" & dataDir1,
+      "--nat=127.0.0.1",
+      "--disc-ip=127.0.0.1",
+      "--disc-port=8090",
+      "--persistence",
+      "--eth-account=" & $accounts[0]
+    ], debug = false)
+
+    node2 = startNode([
+      "--api-port=8081",
+      "--data-dir=" & dataDir2,
+      "--nat=127.0.0.1",
+      "--disc-ip=127.0.0.1",
+      "--disc-port=8091",
+      "--bootstrap-node=" & strip($(parseJson(client.get(baseurl1 & "/info").body)["spr"]), chars = {'"'}),
+      "--persistence",
+      "--eth-account=" & $accounts[1]
+    ], debug = false)
+
   teardown:
     client.close()
     node1.stop()
     node2.stop()
 
+    dataDir1.removeDir()
+    dataDir2.removeDir()
+
   test "nodes can print their peer information":
     let info1 = client.get(baseurl1 & "/info").body
     let info2 = client.get(baseurl2 & "/info").body

vendor/nim-libp2p-dht

@@ -1 +1 @@
-Subproject commit 08928e57d82736ab9524010a3bd59a707f2e0769
+Subproject commit d6d255b4b5d6a4fa56db0eb6677ed7391cbb4897