Skip to content
This repository has been archived by the owner on May 26, 2022. It is now read-only.

remove incorrect call to InterceptAddrDial #260

Merged
merged 1 commit into from
May 30, 2021

Conversation

marten-seemann
Copy link
Contributor

addConn is called both when we add a dialed and an accepted connection to the swarm. InterceptAddrDial is only supposed to intercept outgoing connections though. When dialing, we already call InterceptAddrDial when we compose the
list of dialable addresses:

func (s *Swarm) filterKnownUndialables(p peer.ID, addrs []ma.Multiaddr) []ma.Multiaddr {
lisAddrs, _ := s.InterfaceListenAddresses()
var ourAddrs []ma.Multiaddr
for _, addr := range lisAddrs {
protos := addr.Protocols()
// we're only sure about filtering out /ip4 and /ip6 addresses, so far
if protos[0].Code == ma.P_IP4 || protos[0].Code == ma.P_IP6 {
ourAddrs = append(ourAddrs, addr)
}
}
return addrutil.FilterAddrs(addrs,
addrutil.SubtractFilter(ourAddrs...),
s.canDial,
// TODO: Consider allowing link-local addresses
addrutil.AddrOverNonLocalIP,
func(addr ma.Multiaddr) bool {
return s.gater == nil || s.gater.InterceptAddrDial(p, addr)
},
)
}

Copy link
Member

@raulk raulk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fair, and intercepting outbound dials is what the contract says, so if users are using this to intercept inbounds, it's wrong usage so no need to do change management.

However, we should add an integration test here swarm <=> conngater. It makes me uneasy that this was uncaught and there are no tests here.

addConn is called both when we add a dialed and an accepted connection to the
swarm. InterceptAddrDial is only supposed to intercept outgoing connections
though. When dialing, we already call InterceptAddrDial when we compose the
list of dialable addresses.
@marten-seemann marten-seemann merged commit e6dd8fb into master May 30, 2021
@marten-seemann marten-seemann deleted the remove-incorrect-gating branch July 7, 2021 19:34
@aschmahmann aschmahmann mentioned this pull request Aug 23, 2021
62 tasks
@prestonvanloon
Copy link

prestonvanloon commented Nov 3, 2021

Hey guys, we have been reviewing recent updates to libp2p for prysmaticlabs/prysm#9772 and this PR seems to modify expected behavior of the ConnectionGater.

Please correct me if I am misunderstanding this change. Does this change modify the behavior such that a ConnectionGater will no longer actively reject inbound and outbound connections/dials? We are following the description in the upstream godoc here: https://github.com/libp2p/go-libp2p/blob/v0.15.1/options.go#L329-L333

@marten-seemann
Copy link
Contributor Author

marten-seemann commented Nov 10, 2021

InterceptAddrDial was (incorrectly) called for incoming connections. This PR fixes that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants