Compare chain id env variable with rpc on startup

[kimpers]

Implements #171

[fabioberger]

Move this into a case statement in the above select block.

[fabioberger]

Can we move the actual logic in this go routine into a helper method with a good name? Having it in-lined here feels like the wrong level of abstraction when compared with the rest of it's parent's function logic.

[fabioberger]

Use innerCtx instead of context.Background() here.

[albrow]

@kimpers @fabioberger do you mind if I take over this PR to push it over the finish line? This check would actually be really useful and solve a common problem, especially in the browser.

[kimpers]

@kimpers @fabioberger do you mind if I take over this PR to push it over the finish line? This check would actually be really useful and solve a common problem, especially in the browser.

Feel free 👍 Beware that there seems to be a strange race condition with the test, locally on my machine it passes some times and fails other times.

When you figure out what the underlying issue is I'd be very interested to know what causes it.

[albrow]

@kimpers the race condition here was definitely a tricky one. It doesn't directly stem from the code you added, moreso this new code made an existing bug more apparent.

Inside of core.App.Start there is a select statement where we wait for several different goroutines to send an error through their respective error channels (e.g. p2pErrChan, orderWatcherErrChan, or the new chainIDMismatchErrChan which you added in this PR):

select {
		case err := <-p2pErrChan:
			if err != nil {
				log.WithError(err).Error("p2p node exited with error")
				cancel()
				return err
			}
		case err := <-orderWatcherErrChan:
			if err != nil {
				log.WithError(err).Error("order watcher exited with error")
				cancel()
				return err
			}

		// etc...
}

There was an implicit assumption here that if one of the goroutines sent an error through their respective channels, it was time to shut the app down. But that assumption was wrong. There are some cases where a goroutine may send nil through the channel to indicate that it has exited without an error, and in this case we don't shut down the application. The most straightforward example of this is that the Ethereum RPC rate limiter sends nil through ethRPCRateLimiterErrChan immediately if Ethereum RPC rate limiting has been disabled. In the old code this would cause us to exit the select statement, and therefore not receive errors from any of the other (still running) goroutines.

I fixed this by wrapping the select statement in a for loop. This way if one of the goroutines exits and sends nil through its respective error channel, we keep waiting to receive errors on the other channels and then handle them appropriately.

[albrow]

@fabio This PR is now ready for review. I changed it to no longer be a draft.