Skip to content
This repository was archived by the owner on Aug 2, 2021. It is now read-only.

Conversation

nolash
Copy link
Contributor

@nolash nolash commented Mar 12, 2019

This PR contains some preliminary changes to allow for using ENR to transmit the swarm overlay address between peers.

Currently, ENR is not being sent over the p2p handshake. The necessary changes for this to happen is pending an EIP submission (and approval, and implementation). This is described in this PR by @fjl which incidentally also contains a hack by him that provides a functional demonstration on how ENR record will be available through PeerInfo and on the p2p.Peerobject:

ethereum/go-ethereum#19220

In the meantime, the aim is to include the maximum amount of changes here that can be made towards the goal without relying on the implementation of the EIP. Some of the changes are cherry-picked from an experimental branch https://github.com/ethersphere/go-ethereum/tree/enr-bzz-poc building on the in the above mentioned PR.

In the experimental branch all handshake related code has been removed, and includes a temporary POC test swarm/network/simulation/protocol_new_test.go:TestENR that demonstrates a successful WaitTillHealthy() only using ENR.


My proposed steps towards the basic but sound transition to handshake-free Swarm are:

BEFORE EIP

  1. Preliminary changes in swarm/network and embed ENR in simulation (this PR)
  2. Embed ENR in binary

AFTER EIP

  1. Remove handshake from swarm/network
  2. Eliminate swarm/network:BzzAddr
  3. Merge swarm/network:BzzPeer and swarm/network:Peer
  4. Remove handshake from p2p/protocols
  5. Move p2p/protocols to swarm package

@nolash nolash self-assigned this Mar 12, 2019
@nolash nolash requested review from janos and zelig March 12, 2019 22:49
@nolash nolash requested review from acud and nonsense as code owners March 12, 2019 22:49
@nolash nolash changed the title swarm, p2p: Prerequisites for handshak… swarm, p2p: Prerequities for ENR replacing handshake Mar 12, 2019
Copy link
Member

@janos janos left a comment

Choose a reason for hiding this comment

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

Some calls to p2ptest.NewProtocolTester needs to be updated:

p2p/protocols/protocol_test.go:264:38:warning: unused variable or constant cannot use conf.ID (variable of type enode.ID) as *ecdsa.PrivateKey value in argument to p2ptest.NewProtocolTester (varcheck)
p2p/protocols/protocol_test.go:148:35:warning: unused variable or constant cannot use conf.ID (variable of type enode.ID) as *ecdsa.PrivateKey value in argument to p2ptest.NewProtocolTester (varcheck)

}
if err := net.Start(peers[i].ID); err != nil {
panic(fmt.Sprintf("error starting peer %v: %v", peers[i].ID, err))
panic(err.Error())
Copy link
Member

Choose a reason for hiding this comment

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

The second panic impossible to happen.

@@ -0,0 +1,70 @@
package network
Copy link
Member

Choose a reason for hiding this comment

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

Why to move this code to file named types.go? This utility names should be avoided. I propose name network.go instead types.go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

Copy link
Member

@janos janos left a comment

Choose a reason for hiding this comment

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

Thank @nolash.

pubkey := crypto.FromECDSAPub(&prvKey.PublicKey)
pubkeyhex := common.ToHex(pubkey)
keyhex := crypto.Keccak256Hash(pubkey).Hex()
//keyhex := crypto.Keccak256Hash(pubkey).Hex()
Copy link
Member

Choose a reason for hiding this comment

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

remove commented line

@nolash
Copy link
Contributor Author

nolash commented Mar 15, 2019

submitted upstream ethereum/go-ethereum#19275

Thank you @janos and @zelig for reviews.

@nolash
Copy link
Contributor Author

nolash commented Mar 15, 2019

upstream merged

@nolash nolash closed this Mar 15, 2019
@nolash nolash deleted the enr-bzz-preparations branch June 5, 2019 14:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants