Support single arch builds on macOS

[Enzime]

This is useful for building ZeroTier in Nixpkgs as we only want to build one architecture at once

[dariusc93]

Hey! Nice catch on the possible overflow. While I am not against having it fixed to a specific value if it does overflow, I am curious on if we should also have this apart of libp2p spec as well so it would possibly be more consistent in such cases where there is a overflow.

[elenaf9]

If that overflow is due to a malformed or malicious packet do we even want to handle the content? IMO it's also reasonable to just filter out a peer in a response if the ttl is invalid.

[momoshell]

Hey guys, thanks for the prompt responses, really appreciate it. I agree, the extract_discovered function already does the filtering and seems like the perfect place to add a filter combinator.

Without defining fallback TTL values to compare to, would something like this be a suitable change for the response handling?

pub(crate) fn extract_discovered(
    &self,
    now: Instant,
    local_peer_id: PeerId,
) -> impl Iterator<Item = (PeerId, Multiaddr, Instant)> + '_ {
    self.discovered_peers()
        .filter(move |peer| peer.id() != &local_peer_id)
        // Filter out peers whose TTL would cause overflow
        .filter(move |peer| now.checked_add(peer.ttl()).is_some())
        .flat_map(move |peer| {
            let observed = self.observed_address();
            // Safe to use direct addition since we've filtered out overflow cases
            let new_expiration = now + peer.ttl();

            peer.addresses().iter().filter_map(move |address| {
                let new_addr = _address_translation(address, &observed)?;
                let new_addr = new_addr.with_p2p(*peer.id()).ok()?;

                Some((*peer.id(), new_addr, new_expiration))
            })
        })
}

[elenaf9]

Yes, that's what I was thinking too. I think we can simply do:

self.discovered_peers()
        .filter_map(|peer| {
            if peer.id() != &local_peer_id {
                return None;
            }
            let new_expiration = now.checked_add(peer.ttl())?;
            Some((peer, new_expiration))
        }
        .flat_map(|(peer, new_expiration)| {
             let observed = self.observed_address();
             ...
        }

[momoshell]

Hey hey, the initial changes were meant to keep things minimal, but I’m totally on board with simplifying the code. Just a heads up though, the latest one has a pretty critical logic error. If I'm correct the difference being is:

1. In the INCORRECT version:

• if peer.id() != &local_peer_id { return None; }
• This means "if the peer is NOT the local peer, return None"
• This is BACKWARDS from the intent
• It would KEEP the local peer and DISCARD all other peers
• The exact opposite of what we want!

2. In the CORRECT version:

• .filter(|peer| peer.id() != &local_peer_id)
• This explicitly removes the local peer
• Keeps all peers EXCEPT the local peer
• This matches the original function's logic

maan, I hate negations ! 🗡

Looking at the possible filter_map combinations now, the one with just the added filter feels like the cleanest and easiest to follow.

[elenaf9]

Ah yes, that was a typo, I meant if peer.id() == &local_peer_id { return None; }.

Anyway, let's discuss this on the PR :)

[momoshell]

Awesome, really appreciate all your help guys. Will create a PR shortly

[elenaf9]

Reopening the issue just to track properly how/ when it is resolved.