From d5386df68478a71ac269acb2d00d36a7a5c9ebc5 Mon Sep 17 00:00:00 2001
From: Alex Potsides <alex@achingbrain.net>
Date: Wed, 18 May 2022 13:19:31 +0100
Subject: [PATCH] fix: do upnp hole punch after startup (#1217)

The transport manager configures it's addresses during the `start`
phase so access them during `afterStart` so they'll be ready for use.
---
 package.json                         |  2 +-
 src/nat-manager.ts                   | 10 +++++++---
 test/nat-manager/nat-manager.node.ts | 17 +++++++++--------
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/package.json b/package.json
index 2b3ffeae..028b34b3 100644
--- a/package.json
+++ b/package.json
@@ -92,7 +92,7 @@
     "test:interop": "aegir test -t node -f dist/test/interop.js"
   },
   "dependencies": {
-    "@achingbrain/nat-port-mapper": "^1.0.0",
+    "@achingbrain/nat-port-mapper": "^1.0.3",
     "@libp2p/connection": "^1.1.5",
     "@libp2p/crypto": "^0.22.11",
     "@libp2p/interfaces": "^1.3.31",
diff --git a/src/nat-manager.ts b/src/nat-manager.ts
index 0d53fdfe..8d229f07 100644
--- a/src/nat-manager.ts
+++ b/src/nat-manager.ts
@@ -94,10 +94,14 @@ export class NatManager implements Startable {
     return this.started
   }
 
+  start () {}
+
   /**
-   * Starts the NAT manager
+   * Attempt to use uPnP to configure port mapping using the current gateway.
+   *
+   * Run after start to ensure the transport manager has all addresses configured.
    */
-  start () {
+  afterStart () {
     if (isBrowser || !this.enabled || this.started) {
       return
     }
@@ -105,7 +109,7 @@ export class NatManager implements Startable {
     this.started = true
 
     // done async to not slow down startup
-    this._start().catch((err) => {
+    void this._start().catch((err) => {
       // hole punching errors are non-fatal
       log.error(err)
     })
diff --git a/test/nat-manager/nat-manager.node.ts b/test/nat-manager/nat-manager.node.ts
index 440cb295..30e251ee 100644
--- a/test/nat-manager/nat-manager.node.ts
+++ b/test/nat-manager/nat-manager.node.ts
@@ -13,6 +13,7 @@ import { createFromJSON } from '@libp2p/peer-id-factory'
 import { Components } from '@libp2p/interfaces/components'
 import type { NatAPI } from '@achingbrain/nat-port-mapper'
 import { StubbedInstance, stubInterface } from 'ts-sinon'
+import { start, stop } from '@libp2p/interfaces/startable'
 
 const DEFAULT_ADDRESSES = [
   '/ip4/127.0.0.1/tcp/0',
@@ -49,7 +50,7 @@ describe('Nat Manager (TCP)', () => {
     await components.getTransportManager().listen(components.getAddressManager().getListenAddrs())
 
     teardown.push(async () => {
-      await natManager.stop()
+      await stop(natManager)
       await components.getTransportManager().removeAll()
     })
 
@@ -78,7 +79,7 @@ describe('Nat Manager (TCP)', () => {
     let observed = components.getAddressManager().getObservedAddrs().map(ma => ma.toString())
     expect(observed).to.be.empty()
 
-    await natManager._start()
+    await start(natManager)
 
     observed = components.getAddressManager().getObservedAddrs().map(ma => ma.toString())
     expect(observed).to.not.be.empty()
@@ -127,7 +128,7 @@ describe('Nat Manager (TCP)', () => {
       enabled: false
     })
 
-    natManager.start()
+    await start(natManager)
 
     await delay(100)
 
@@ -146,7 +147,7 @@ describe('Nat Manager (TCP)', () => {
     let observed = components.getAddressManager().getObservedAddrs().map(ma => ma.toString())
     expect(observed).to.be.empty()
 
-    await natManager._start()
+    await start(natManager)
 
     observed = components.getAddressManager().getObservedAddrs().map(ma => ma.toString())
     expect(observed).to.be.empty()
@@ -163,7 +164,7 @@ describe('Nat Manager (TCP)', () => {
     let observed = components.getAddressManager().getObservedAddrs().map(ma => ma.toString())
     expect(observed).to.be.empty()
 
-    await natManager._start()
+    await start(natManager)
 
     observed = components.getAddressManager().getObservedAddrs().map(ma => ma.toString())
     expect(observed).to.be.empty()
@@ -180,7 +181,7 @@ describe('Nat Manager (TCP)', () => {
     let observed = components.getAddressManager().getObservedAddrs().map(ma => ma.toString())
     expect(observed).to.be.empty()
 
-    await natManager._start()
+    await start(natManager)
 
     observed = components.getAddressManager().getObservedAddrs().map(ma => ma.toString())
     expect(observed).to.be.empty()
@@ -197,7 +198,7 @@ describe('Nat Manager (TCP)', () => {
     let observed = components.getAddressManager().getObservedAddrs().map(ma => ma.toString())
     expect(observed).to.be.empty()
 
-    await natManager._start()
+    await start(natManager)
 
     observed = components.getAddressManager().getObservedAddrs().map(ma => ma.toString())
     expect(observed).to.be.empty()
@@ -214,7 +215,7 @@ describe('Nat Manager (TCP)', () => {
     let observed = components.getAddressManager().getObservedAddrs().map(ma => ma.toString())
     expect(observed).to.be.empty()
 
-    await natManager._start()
+    await start(natManager)
 
     observed = components.getAddressManager().getObservedAddrs().map(ma => ma.toString())
     expect(observed).to.be.empty()