cmd/swarm, p2p, swarm: Enable ENR in binary/execadapter

[nolash]

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 simulatioñ (done)
2. Embed ENR in binary (this PR)

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

[acud]

@nolash build is broken however I am unsure if this is related to the content of the PR

[nolash]

@justelad I would say not, since the fails are using sim inproc, and the changes only concern sim exec and swarm binary.

[frncmx]

Change request: handle error of Config.Init() in test cases. Others are optional.

That would be nice if you could increase the abstraction level of Config.Init() by extracting low level functionality into higher level methods. That the Init() would become easier to understand and shorter.

General style:
I strongly prefer of using the if err := f(); err != nil format. It comes as a builtin language feature for a reason. By using the proposed format - where possible - the reader can immediately know the scope of the error - thus less code to understand - and the surrounding environment does not get polluted.

[nolash]

@frncmx

That would be nice if you could increase the abstraction level of Config.Init() by extracting low level functionality into higher level methods. That the Init() would become easier to understand and shorter.

Sorry, I disagree. I can add a couple of comments if you want. But I don't think it will be clearer by splitting it up.

[nolash]

@frncmx on second thoughts, I believe the Config.Init() is executing code that is done more or less identically somewhere else (where the ENR bzz key is added in sims). So in this case I agree let's make resuable code for that. Since you're already on it, would you like to implement?

[frncmx]

@nolash I would rather not take up on a refactoring task just because I just wanted to prove one specific function could be cleaner.

I came up with the following code:

• I think the code below is more readable than the previous version, just by adding 2 additional methods. What do you think?

func (c *Config) Init(privateKey *ecdsa.PrivateKey) (err error) {
	c.privateKey = privateKey

	c.Path, err = c.createRootDir()
	if err != nil {
		return
	}

	bzzkeybytes := network.PrivateKeyToBzzKey(c.privateKey)
	c.BzzKey = hexutil.Encode(bzzkeybytes)

	publicKey := crypto.FromECDSAPub(&c.privateKey.PublicKey)
	c.PublicKey = common.ToHex(publicKey)

	c.Enode, err = c.createEnode()
	if err != nil {
		return
	}

	if c.SwapEnabled {
		c.Swap.Init(c.Contract, c.privateKey)
	}

	c.LocalStoreParams.Init(c.Path, common.FromHex(c.BzzKey))
	// TODO: nil check on PSS
	c.Pss = c.Pss.WithPrivateKey(c.privateKey)
	return nil
}

[frncmx]

@nolash I can post you the patch if you would like to apply my changes.

Just looking at the code I moved around I also noticed: we use both hexutil.Encode() and common.ToHex() to achieve the same thing. Note: the latter is deprecated in favor of the 1st.

// TODO: nil check on PSS: I added because I think we can potentially panic there. But as Init() is so far only called after a constructor call that does not cause a problem.

[frncmx]

Just one more quick iteration, so that we really "pick one level of abstraction per method and stick with it":

func (c *Config) Init(privateKey *ecdsa.PrivateKey) error {
	c.privateKey = privateKey
	
	c.setBzzKey()
	c.setPublicKey()

	if err := c.createRootDirAndUpdatePath(); err != nil {
		return err
	}

	if err := c.initEnode(); err != nil {
		return err
	}

	if c.SwapEnabled {
		c.initSwap()
	}

	c.initLocalStoreParams()
	c.initPss()
	return nil
}

The above code makes it easy to read it through in a few seconds and get a high level understanding of what is going on without bothered by details.

[nolash]

I would rather not take up on a refactoring task just because I just wanted to prove one specific function could be cleaner.

Right, I didn't mean to push anything on you. I merely thought since you said you were writing it anyway, that it literally would have been replace code with that new function call you were making in two places.

Don't worry. I can take care of merging this in. Thanks for the contribution.

[nolash]

we use both hexutil.Encode() and common.ToHex() to achieve the same thing.

Yeah common.ToHex() calls were used before. The code just hasn't been updated in some places. All pss tests will be rewritten to use the swarm sim framework, so those details will be attended to then.

[frncmx]

Thank you! I like Config.Init() much better now. 👍

[zelig]

this is always there whenever you call config.Init() could we not just put the nodkey generation into Init?

[nolash]

@zelig no because we want to generate it with node/Config.NodeKey() when in cmd/swarm

[nolash]

Thanks for reviews @frncmx @zelig

submitted upstream ethereum/go-ethereum#19309

[nolash]

Merged as ethereum/go-ethereum#19309