Skip to content

Certhash missing from appendAnnounce webrtc-direct multiaddrs #3080

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
PradaJoaquin opened this issue Apr 15, 2025 · 1 comment · Fixed by #3085 or #3086
Closed

Certhash missing from appendAnnounce webrtc-direct multiaddrs #3080

PradaJoaquin opened this issue Apr 15, 2025 · 1 comment · Fixed by #3085 or #3086
Labels
need/triage Needs initial labeling and prioritization

Comments

@PradaJoaquin
Copy link

  • Version:
    libp2p 2.8.3
    webRTC: 5.2.10

  • Platform:
    OS: Linux

Severity:

Medium

Description:

When using appendAnnounce to add a webrtc-direct multiaddress, I noticed that the certhash is not being included in the resulting multiaddr. However, the multiaddrs that are automatically added with my local IP do include the certhash.

Peer multiaddrs:
/ip4/127.0.0.1/udp/4001/webrtc-direct/certhash/uEiD2y_gB0kCrWddSmXSZy1QfKQXklbbgiMXnZqBjhOsQ9A/p2p/12D3KooWR3F8PgW9DdGmZfrxfha591EN1KGE9aT1bVGUTkMrYMuM
/ip4/192.168.0.124/udp/4001/webrtc-direct/certhash/uEiD2y_gB0kCrWddSmXSZy1QfKQXklbbgiMXnZqBjhOsQ9A/p2p/12D3KooWR3F8PgW9DdGmZfrxfha591EN1KGE9aT1bVGUTkMrYMuM
/ip4/<public-ip>/udp/4001/webrtc-direct/p2p/12D3KooWR3F8PgW9DdGmZfrxfha591EN1KGE9aT1bVGUTkMrYMuM

As shown above, the certhash is missing from the multiaddr corresponding to the public IP I manually added via appendAnnounce.

This behavior may be related to #2929.

Steps to Reproduce

  1. Replace in the code below with your public IPv4 address.
  2. Run the script and observe the output.

index.js

import { noise } from "@chainsafe/libp2p-noise";
import { yamux } from "@chainsafe/libp2p-yamux";
import { autoNAT } from "@libp2p/autonat";
import { identify, identifyPush } from "@libp2p/identify";
import { ping } from "@libp2p/ping";
import { uPnPNAT } from "@libp2p/upnp-nat";
import { webRTCDirect } from "@libp2p/webrtc";
import { createLibp2p } from "libp2p";

let libp2pOptions = {
    addresses: {
        listen: [
            "/ip4/0.0.0.0/udp/4001/webrtc-direct",
        ],
        appendAnnounce: [
            "/ip4/<public-ip>/udp/4001/webrtc-direct"
        ],
    },
    transports: [
        webRTCDirect(),
    ],
    connectionEncrypters: [
        noise(),
    ],
    streamMuxers: [yamux()],
    services: {
        autoNAT: autoNAT(),
        identify: identify(),
        identifyPush: identifyPush(),
        ping: ping(),
        upnp: uPnPNAT(),
    },
};

const libp2p = await createLibp2p(libp2pOptions);

console.log("Peer multiaddrs:");
let multiaddrs = libp2p.getMultiaddrs();
for (const ma of multiaddrs) {
    console.log(`${ma}`);
}

// Log the peer's multiaddrs whenever they change
let oldAddrs = [];
libp2p.addEventListener("self:peer:update", (evt) => {
    const newAddrs = libp2p.getMultiaddrs();
    if (JSON.stringify(oldAddrs) !== JSON.stringify(newAddrs)) {
        console.log("Peer multiaddrs changed:");
        for (const ma of newAddrs) {
            console.log(`${ma}`);
        }
        oldAddrs = newAddrs;
    }
});

package.json

{
  "name": "libp2p-test",
  "version": "1.0.0",
  "main": "index.js",
  "type": "module",
  "dependencies": {
    "@chainsafe/libp2p-noise": "^16.1.0",
    "@chainsafe/libp2p-yamux": "^7.0.1",
    "@libp2p/autonat": "^2.0.29",
    "@libp2p/identify": "^3.0.28",
    "@libp2p/ping": "^2.0.28",
    "@libp2p/upnp-nat": "^3.1.12",
    "@libp2p/webrtc": "^5.2.10",
    "libp2p": "^2.8.3"
  }
}

Any insights or guidance on this behavior would be appreciated.

@PradaJoaquin PradaJoaquin added the need/triage Needs initial labeling and prioritization label Apr 15, 2025
@PradaJoaquin PradaJoaquin changed the title WebRTC-Direct multiaddress does not upgrade on append announce Certhash missing from appendAnnounce webrtc-direct multiaddrs Apr 15, 2025
@achingbrain
Copy link
Member

Ah, yes - we allow transports to update announce addresses but not append announce addresses currently.

achingbrain added a commit that referenced this issue Apr 17, 2025
We allow transports to update announce addresses to add certhashes
and other data but updating append announce addresses was missed.

Fixes #3080
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/triage Needs initial labeling and prioritization
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants