feat!: add circuitv2 relay tests #83

ckousik · 2023-01-31T16:31:16Z

Adds circuitv2 relay tests.

ckousik · 2023-01-31T16:34:12Z

src/connect.ts

@@ -64,11 +64,11 @@ function runConnectTests (name: string, factory: DaemonFactory, optionsA: SpawnO

      // verify connected peers
      const knownPeersAfterConnect1 = await daemons[0].client.listPeers()
-      expect(knownPeersAfterConnect1).to.have.lengthOf(1)
+      expect(knownPeersAfterConnect1).to.have.length.greaterThanOrEqual(1)


listPeers was returning two peers with the same peer ID. On further inspection, this was the same peer with two different local addresses. eg. Peer A has addresses /ip4/127.0.0.1/tcp/36543 and /ip4/192.168.1.2/tcp/36543

ckousik · 2023-01-31T16:36:36Z

src/pubsub/floodsub.ts

@@ -12,8 +13,9 @@ export function floodsubTests (factory: DaemonFactory) {
    for (const typeB of nodeTypes) {
      runFloodsubTests(
        factory,
-        { type: typeA, pubsub: true, pubsubRouter: 'floodsub' },
-        { type: typeB, pubsub: true, pubsubRouter: 'floodsub' }
+        // RSA key ensures the `key` field is set in the generated signed message


libp2p/js-libp2p-pubsub#118 (comment)

Since libp2p/js-libp2p-pubsub#118 has been merged can all the pubsub related changes can be reverted, or are they still necessary?

It seems like tests are still failing with:

libp2p:pubsub starting +8ms libp2p:pubsub started +5ms libp2p:pubsub connected 12D3KooWBaiBJ1ngvSyYJYYnKuEQNWx2j4b3s2z8c3z5shGLRM8U +60ms libp2p:pubsub new peer 12D3KooWBaiBJ1ngvSyYJYYnKuEQNWx2j4b3s2z8c3z5shGLRM8U +48ms libp2p:pubsub rpc from 12D3KooWBaiBJ1ngvSyYJYYnKuEQNWx2j4b3s2z8c3z5shGLRM8U +1ms libp2p:pubsub subscribe to topic: test-topic +3ms libp2p:pubsub rpc from 12D3KooWBaiBJ1ngvSyYJYYnKuEQNWx2j4b3s2z8c3z5shGLRM8U +802ms libp2p:pubsub messages from 12D3KooWBaiBJ1ngvSyYJYYnKuEQNWx2j4b3s2z8c3z5shGLRM8U +1ms libp2p:pubsub:error CodeError: Need key when signature policy is StrictSign but it was missing libp2p:pubsub:error at EventTarget.getMsgId (file:///home/ckousik/projects/js-libp2p/node_modules/@libp2p/pubsub/src/index.ts:458:17) libp2p:pubsub:error at EventTarget.processMessage (file:///home/ckousik/projects/js-libp2p/node_modules/@libp2p/floodsub/src/index.ts:76:31) libp2p:pubsub:error at file:///home/ckousik/projects/js-libp2p/node_modules/@libp2p/pubsub/src/index.ts:376:22 libp2p:pubsub:error at processTicksAndRejections (node:internal/process/task_queues:96:5) libp2p:pubsub:error at file:///home/ckousik/projects/js-libp2p/node_modules/p-queue/dist/index.js:118:36 +0ms ^C libp2p:pubsub stopping +2s libp2p:pubsub delete peer 12D3KooWBaiBJ1ngvSyYJYYnKuEQNWx2j4b3s2z8c3z5shGLRM8U +0ms libp2p:pubsub stopped +0ms

Could we merge this and open an issue for updating libp2p/pubsub and reverting this change?

ckousik · 2023-01-31T16:41:49Z

depends on libp2p/js-libp2p-pubsub#118

src/index.ts

src/pubsub/floodsub.ts

src/relay/index.ts

MarcoPolo · 2023-02-03T03:59:34Z

@ckousik please request a review when this is ready again. Thank you!

src/relay/index.ts

p-shahi · 2023-02-21T00:15:12Z

It seems like this PR is ready for review again cc @MarcoPolo

There is one unresolved question here from @achingbrain though: #83 (comment)

MarcoPolo · 2023-02-21T18:58:37Z

src/relay/index.ts

+      // without this the socket can close before we receive a response
+      const responseReceived = defer()
+      const input = [uint8ArrayFromString('test')]
+      const output = await pipe(


I don't doubt that this is correct because you tested this. But do you feel like this is confusing? or is it just me? Not saying you could have written this better, just that the iterable + pipe abstraction is confusing to me.

Yeah, so this is the same issue that is observed in the stream tests. Please refer to https://github.com/libp2p/interop/blob/master/src/streams/echo.ts#L77-L94

Oh I see. Latest versions of go-libp2p shouldn't have this issue with ping so you can probably just use the ping protocol.

Fine to not change.

ckousik · 2023-02-22T16:16:02Z

@achingbrain The latest change causes a failure when dialing over js.

  js to go over relay go
    ✔ connects (869ms)
  libp2p:circuit:error Circuit relay dial failed Error: failed to connect via relay with status NO_RESERVATION
    at Circuit.connectV2 (file:///home/ckousik/projects/js-libp2p/src/circuit/transport.ts:260:27)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at Circuit.dial (file:///home/ckousik/projects/js-libp2p/src/circuit/transport.ts:224:14)
    at EventTarget.dial (file:///home/ckousik/projects/js-libp2p/src/transport-manager.ts:122:14)
    at DialRequest.dialAction (file:///home/ckousik/projects/js-libp2p/src/connection-manager/dialer/index.ts:311:14)
    at file:///home/ckousik/projects/js-libp2p/src/connection-manager/dialer/dial-request.ts:100:18
    at async Promise.any (index 0)
    at DialRequest.run (file:///home/ckousik/projects/js-libp2p/src/connection-manager/dialer/dial-request.ts:85:14)
    at DefaultDialer.dial (file:///home/ckousik/projects/js-libp2p/src/connection-manager/dialer/index.ts:204:26)
    at EventTarget.openConnection (file:///home/ckousik/projects/js-libp2p/src/connection-manager/index.ts:505:26)
    at EventTarget.dial (file:///home/ckousik/projects/js-libp2p/src/libp2p.ts:367:12)
    at Server.connect (file:///home/ckousik/projects/js-libp2p/node_modules/@libp2p/daemon-server/src/index.ts:108:12)
    at file:///home/ckousik/projects/js-libp2p/node_modules/@libp2p/daemon-server/src/index.ts:398:19
    at file:///home/ckousik/projects/js-libp2p/node_modules/@libp2p/daemon-server/src/index.ts:520:26 {
  code: 'ERR_HOP_REQUEST_FAILED'
} +0ms
  libp2p:circuit:error Circuit relay dial failed Error: failed to connect via relay with status NO_RESERVATION
    at Circuit.connectV2 (file:///home/ckousik/projects/js-libp2p/src/circuit/transport.ts:260:27)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at Circuit.dial (file:///home/ckousik/projects/js-libp2p/src/circuit/transport.ts:224:14)
    at EventTarget.dial (file:///home/ckousik/projects/js-libp2p/src/transport-manager.ts:122:14)
    at DialRequest.dialAction (file:///home/ckousik/projects/js-libp2p/src/connection-manager/dialer/index.ts:311:14)
    at file:///home/ckousik/projects/js-libp2p/src/connection-manager/dialer/dial-request.ts:100:18
    at async Promise.any (index 0)
    at DialRequest.run (file:///home/ckousik/projects/js-libp2p/src/connection-manager/dialer/dial-request.ts:85:14)
    at DefaultDialer.dial (file:///home/ckousik/projects/js-libp2p/src/connection-manager/dialer/index.ts:204:26)
    at EventTarget.openConnection (file:///home/ckousik/projects/js-libp2p/src/connection-manager/index.ts:505:26)
    at EventTarget.dial (file:///home/ckousik/projects/js-libp2p/src/libp2p.ts:367:12)
    at Server.connect (file:///home/ckousik/projects/js-libp2p/node_modules/@libp2p/daemon-server/src/index.ts:108:12)
    at file:///home/ckousik/projects/js-libp2p/node_modules/@libp2p/daemon-server/src/index.ts:398:19
    at file:///home/ckousik/projects/js-libp2p/node_modules/@libp2p/daemon-server/src/index.ts:520:26 {
  code: 'ERR_HOP_REQUEST_FAILED'
} +37ms
    1) fails to connect without a reservation


  9 passing (6s)
  1 failing

  1) js to go over relay go
       fails to connect without a reservation:
     AssertionError: expected 'All promises were rejected' to match /NO_RESERVATION/
      at /home/ckousik/projects/libp2p/interop/node_modules/chai-as-promised/lib/chai-as-promised.js:302:22
      at runMicrotasks (<anonymous>)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)
      at Context.<anonymous> (file:///home/ckousik/projects/libp2p/interop/src/relay/index.ts:72:7)

The internal error is correct, but it is hidden by https://github.com/libp2p/js-libp2p/blob/master/src/connection-manager/dialer/dial-request.ts#L85 . Should we skip matching the error for js?

achingbrain · 2023-02-22T21:32:46Z

@ckousik this will be fixed by a push to the libp2p branch updating the versions, give me a few minutes.

## [6.0.0](v5.0.0...v6.0.0) (2023-02-22) ### ⚠ BREAKING CHANGES * add circuitv2 relay tests (#83) ### Features * add circuitv2 relay tests ([#83](#83)) ([0512b15](0512b15))

ckousik commented Jan 31, 2023

View reviewed changes

ckousik marked this pull request as ready for review January 31, 2023 16:41

p-shahi requested a review from achingbrain January 31, 2023 17:30

ckousik commented Jan 31, 2023

View reviewed changes

src/index.ts Outdated Show resolved Hide resolved

MarcoPolo reviewed Feb 2, 2023

View reviewed changes

src/pubsub/floodsub.ts Outdated Show resolved Hide resolved

src/relay/index.ts Outdated Show resolved Hide resolved

src/relay/index.ts Outdated Show resolved Hide resolved

src/relay/index.ts Show resolved Hide resolved

p-shahi mentioned this pull request Feb 2, 2023

feat: implement circuit v2 libp2p/js-libp2p#1533

Merged

2 tasks

p-shahi added this to the M2: WebRTC browser-to-browser milestone Feb 2, 2023

ckousik requested review from MarcoPolo and removed request for achingbrain February 3, 2023 04:15

ckousik added 7 commits February 3, 2023 10:11

feat: add circuitv2 relay tests

76992d5

cleanup

1fc3875

remove external relay function

e3f29f5

address review

aa2b0c7

fix await

1ef4c80

fix lint reverts await

1fa8b0b

rebase fixes

2336330

ckousik force-pushed the ckousik/relay-tests branch from 95e5d46 to 2336330 Compare February 3, 2023 04:47

fix lint and dep-check

54dcd7c

ckousik mentioned this pull request Feb 3, 2023

fix: allow using transient connections libp2p/go-libp2p-daemon#276

Merged

achingbrain mentioned this pull request Feb 3, 2023

Change @typescript-eslint/return-await to in-try-catch? ipfs/eslint-config-ipfs#130

Closed

achingbrain changed the title ~~feat: add circuitv2 relay tests~~ feat!: add circuitv2 relay tests Feb 7, 2023

achingbrain reviewed Feb 7, 2023

View reviewed changes

src/relay/index.ts Outdated Show resolved Hide resolved

remove commented code

b1bff97

ckousik requested review from achingbrain and removed request for MarcoPolo February 21, 2023 12:19

MarcoPolo approved these changes Feb 21, 2023

View reviewed changes

chore: simplify tests and add failure test

c421311

achingbrain added 2 commits February 22, 2023 18:14

chore: downgrade aegir until libp2p is upgraded

76dd031

chore: remove aegir version

dae6e07

achingbrain merged commit 0512b15 into libp2p:master Feb 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: add circuitv2 relay tests #83

feat!: add circuitv2 relay tests #83

ckousik commented Jan 31, 2023

ckousik Jan 31, 2023

ckousik Jan 31, 2023

achingbrain Feb 7, 2023

ckousik Feb 21, 2023

ckousik commented Jan 31, 2023

MarcoPolo commented Feb 3, 2023

p-shahi commented Feb 21, 2023

MarcoPolo Feb 21, 2023 •

edited

Loading

ckousik Feb 21, 2023

MarcoPolo Feb 22, 2023

ckousik commented Feb 22, 2023

achingbrain commented Feb 22, 2023

feat!: add circuitv2 relay tests #83

feat!: add circuitv2 relay tests #83

Conversation

ckousik commented Jan 31, 2023

ckousik Jan 31, 2023

Choose a reason for hiding this comment

ckousik Jan 31, 2023

Choose a reason for hiding this comment

achingbrain Feb 7, 2023

Choose a reason for hiding this comment

ckousik Feb 21, 2023

Choose a reason for hiding this comment

ckousik commented Jan 31, 2023

MarcoPolo commented Feb 3, 2023

p-shahi commented Feb 21, 2023

MarcoPolo Feb 21, 2023 • edited Loading

Choose a reason for hiding this comment

ckousik Feb 21, 2023

Choose a reason for hiding this comment

MarcoPolo Feb 22, 2023

Choose a reason for hiding this comment

ckousik commented Feb 22, 2023

achingbrain commented Feb 22, 2023

MarcoPolo Feb 21, 2023 •

edited

Loading