Skip to content

fix(AutoNAT v2): Client should reset address status #6203

@momoshell

Description

@momoshell

Summary

The AutoNAT v2 Client behaviour never resets the address status when the external address is flagged as expired (FromSwarm::ExternalAddrExpired).

Once an address is tested successfully, it stays in TestStatus::Received state forever in the behaviour instance.
Thus, expired addresses are disabled from being retested again.

Expected behavior

The issue is the stale state in the client behaviour, specifically:

  • address_candidates: HashMap<Multiaddr, AddressInfo>
  • with status: status: TestStatus::Received(nonce)

Handling the FromSwarm::ExternalAddrExpired should remove the address key from the address_candidates map.
As similarly done in V1:

fn on_swarm_event(&mut self, event: FromSwarm) {
        self.listen_addresses.on_swarm_event(&event);
        self.inner.on_swarm_event(event);

        match event {
            FromSwarm::ConnectionEstablished(e) => self.on_connection_established(e),
            FromSwarm::ConnectionClosed(e) => self.on_connection_closed(e),
            FromSwarm::DialFailure(e) => self.on_dial_failure(e),
            FromSwarm::AddressChange(e) => self.on_address_change(e),
            FromSwarm::NewListenAddr(_) => {
                self.as_client().on_new_address();
            }
            FromSwarm::ExpiredListenAddr(e) => {
                self.as_client().on_expired_address(e.addr);
            }
            FromSwarm::ExternalAddrExpired(e) => {
                self.as_client().on_expired_address(e.addr);
            }
            FromSwarm::NewExternalAddrCandidate(e) => {
                self.probe_address(e.addr.to_owned());
            }
            _ => {}
        }
    }
pub(crate) fn on_expired_address(&mut self, addr: &Multiaddr) {
        if let NatStatus::Public(public_address) = self.nat_status {
            if public_address == addr {
                *self.confidence = 0;
                *self.nat_status = NatStatus::Unknown;
                self.schedule_next_probe(Duration::ZERO);
            }
        }
    }

Actual behavior

The expiry event is never handled:

fn on_swarm_event(&mut self, event: FromSwarm) {
        match event {
            FromSwarm::NewExternalAddrCandidate(NewExternalAddrCandidate { addr }) => {
                self.address_candidates
                    .entry(addr.clone())
                    .or_default()
                    .score += 1;
            }
            FromSwarm::ConnectionEstablished(ConnectionEstablished {
                peer_id,
                connection_id,
                endpoint: _,
                ..
            }) => {
                self.peer_info.insert(
                    connection_id,
                    ConnectionInfo {
                        peer_id,
                        supports_autonat: false,
                    },
                );
            }
            FromSwarm::ConnectionClosed(ConnectionClosed {
                peer_id,
                connection_id,
                ..
            }) => {
                let info = self
                    .peer_info
                    .remove(&connection_id)
                    .expect("inconsistent state");

                if info.supports_autonat {
                    tracing::debug!(%peer_id, "Disconnected from AutoNAT server");
                }
            } // <- SHOULD BE HANDLED HERE
            _ => {}
        }
    }

Version

libp2p-v0.56.0

Would you like to work on fixing this bug?

Yes

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions