cmd/swarm/swarm-snapshot: Swarm snapshot binary

[acud]

No description provided.

[nolash]

This is not an exhaustive review. For example, I have not reviewed the tests, since I have several questions about the implementation, which may cause the premise of the tests to change.

[acud]

@nolash the puritan approach of trying to scope the usage of components to be mainly from the p2p package is biting me in the axx. the swarm simulations package stitches additional abstractions over the p2p ones. notably:
https://github.com/ethersphere/go-ethereum/blob/9e9fc87e70accf2b81be8772ab2ab0c914e95666/swarm/network/simulation/simulation.go#L66

https://github.com/ethersphere/go-ethereum/blob/9e9fc87e70accf2b81be8772ab2ab0c914e95666/p2p/simulations/adapters/types.go#L231

right now i'm using the swarm simulations package just to wait for the network health (basically just creating an empty simulation and injecting the network object into it, then calling the health function).

I think that this really isn't necessary to have this method with a simulation as a receiver, but rather a function that just takes a Network as an argument.

[zelig]

in general i am very doubtful about this entire PR.
it is a lot of code for something that already existed plus stuff that is not needed.
lets discuss

[acud]

@zelig,
I can understand your concerns with this PR.
My view on this is that maybe this is not really obviously necessary, but I personally don't like the fact that snapshot generation is being handled inside an e2e test.
Therefore, I think that splitting this to a separate binary is indeed the way to go. Also, I have to say that working through the swarm simulations and p2p simulations helps a lot to see where we need to clean up and where are our current weaknesses in the way we use the simulations framework. Speaking of duplicate code, there's a lot of it there and there's still room to make it cleaner and better so we can really finish the work on this piece of the puzzle. Speaking for myself, I think we still can't fully reason about the simulations FW, and hopefully cleaning stuff up will help resolve the flakiness that we have every now and then.

That being said, there's certain boilerplate code that is indeed replicated and unnecessary, and maybe we should think about having just one toolbox binary that includes, for example, smoke tests, snapshot generation, snapshot verification and any other need that might arise further down the road for that matter.

[zelig]

linter fails https://travis-ci.org/ethersphere/go-ethereum/jobs/477430239#L513 @janos

[janos]

This PR is updated to reflect changes in comments and to simplify a few implementations.

• All options for topology are removed.
• Snapshot create test now is extended to test various options and values in generated snapshot.
• Global variables for flags and cli app are removed and cli app initialization is refactored.
• Function triggerChecks in p2p/simulations/network.go is removed as it was not used in that form in that package and was causing compilation errors.
• Changes in swarm/network and swarm/network/simulations/discovery are left in this PR even though they do not have anything to do with current swarm-snapshot cmd implementation.

This implementations is using WaitTIllHealthy which may cause flakiness in tests and actual usage. I noticed that sometimes test command execution timeouts on 5 seconds which may or may not be the consequence of that, but I raised the hardcoded timeout from 5 to 15s https://github.com/ethersphere/go-ethereum/pull/1077/files#diff-ffa4082d3e02ea59f31c2e4220dfe321R230. This should probably be reverted if the actual reason for timeout is WaitTillHealthy.

I am not sure if this PR should be merged before WaitTillHealthy is fixed.

[acud]

@nolash @zelig @janos i removed the verification command from the binary and the snapshot json. verification will be relied upon testing.