Skip to content
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

swarm: enable p2p/discovery and disable dynamic dialling #19189

Merged

Conversation

nonsense
Copy link
Member

@nonsense nonsense commented Mar 1, 2019

This PR is re-enabling p2p discovery, but disabling the dynamic dialling functionality - the reason why we didn't want to have the p2p discovery in the first place.

The reason for this is - the hive protocol and the Kademlia table implementation on Swarm side does not update the underlay addresses of peers. More specifically Kademlia.Register does not update the underlay addresses ever, but Kademlia.On and Kademlia.Off do. This means that sometimes we are left with outdated destinations for nodes in our state store.

The p2p package handles that with:

    err := t.dial(srv, t.dest)
    if err != nil {
        log.Trace("Dial error", "task", t, "err", err)
        // Try resolving the ID of static nodes if dialing failed.
        if _, ok := err.(*dialError); ok && t.flags&staticDialedConn != 0 {
            if t.resolve(srv) {
                t.dial(srv, t.dest)
            }
        }
    }

Namely, if dialling fails, it tries to resolve an up-to-date destination for the node - something that is only done when discovery is enabled.

Bottom line: hive protocol relies heavily on the discovery functionality in the p2p package, so for now it is not wise to disable discovery on p2p.

Addresses: ethersphere/swarm#1268


I also considered solving this directly on Swarm side, but the hive protocol implementation is not straight-forward and it is not clear how to determine what an up-to-date / accurate node destination is. What happens is that when a new node starts up and loads an out-dated node destination, it might receive a hive message with a different destination for a given node, and it can't determine which one is correct, without trying them both. We don't have functionality to do that, and implementing this seems like a major change in the hive protocol.

@nonsense nonsense added this to the 1.9.0 milestone Mar 1, 2019
@nonsense nonsense requested review from janos, zelig and skylenet March 1, 2019 10:42
@nonsense nonsense merged commit 4e9230e into ethereum:master Mar 1, 2019
kiku-jw pushed a commit to kiku-jw/go-ethereum that referenced this pull request Mar 29, 2019
@nonsense nonsense deleted the enable-discovery-disable-dyncalls branch April 17, 2019 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants