Client: bootstrap() extraction

[holgerd77]

@ryanio could you please have a look into this and see how this can be done better? 😄

[codecov]

Codecov Report

Merging #1222 (b1b5560) into master (64fbffc) will increase coverage by 1.79%.
The diff coverage is 84.84%.

Flag	Coverage Δ
block	80.17% <ø> (ø)
blockchain	84.23% <ø> (ø)
client	83.78% <84.84%> (-0.28%)	⬇️
common	87.39% <ø> (+0.47%)	⬆️
devp2p	83.67% <ø> (-0.37%)	⬇️
ethash	82.08% <ø> (ø)
tx	82.26% <ø> (ø)
vm	86.57% <ø> (+5.70%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

[ryanio]

would this achieve the same?

Suggested change

	// eslint-disable-next-line @typescript-eslint/no-floating-promises
	Promise.all(this.services.map((s) => s.start()))
	.then(() => {
	return Promise.all(this.config.servers.map((s) => s.start()))
	})
	.then(() => {
	// eslint-disable-next-line @typescript-eslint/no-floating-promises
	Promise.all(this.config.servers.map((s) => s.bootstrap()))
	})
	await Promise.all(this.services.map((s) => s.start()))
	await Promise.all(this.config.servers.map((s) => s.start()))
	await Promise.all(this.config.servers.map((s) => s.bootstrap()))

[ryanio]

i believe libp2p runs its own bootstrap, so if the bootstrap() method is only available for RlpxServer, we could add a simple guard to the last line, something like: if('bootstrap' in s) { return s.bootstrap() }

[holgerd77]

No, this is just not working, this was my naive (is it?) approach as well on first try. You can test this I guess now also on master with Aleut. If you use this clean await chaining version of yours and do a DEBUG=devp2p:* npm run client:start -- --network=aleut run you should see no Send GET_BLOCK_HEADERS response from our side. If you run with this bad hacky code from my test fix you should see a response together with some follow-up processing (going wrong with "could not decode message BlockHeaders"):

My Promise-feeling unfortunately is so bad that it seems that I don't get this right. Or maybe this just needs some stronger sequentiality with some artificial delays with a timer or so due to the networking stuff? Not sure if this makes sense but we can also try.

Anyhow: I am a bit stuck how to fix this.

[ryanio]

thanks for the info, ok I'll keep looking into it and report back, I am curious now too :)

[ryanio]

I found the problem, we needed to await the Devp2pRLPx initialization, fixed in 0297fd5

[holgerd77]

@ryanio thanks for having a look here! 😄 👍

Just pulled the branch and tested with DEBUG=devp2p:* npm run client:start -- --network=aleut and there is still no answer from our side on the GET_BLOCK_HEADERS request, are you sure this fixes the problem?

Apart from this test: I am generally wondering what this Devp2pRLPx initialization does or changes (just for my understanding) since I am not seeing any internal async calls? Or is this due to the code in the event handlers?? (does this make sense? This should be independent I guess?)

[ryanio]

@ryanio thanks for having a look here! 😄 👍

Just pulled the branch and tested with DEBUG=devp2p:* npm run client:start -- --network=aleut and there is still no answer from our side on the GET_BLOCK_HEADERS request, are you sure this fixes the problem?

Hmmm, I am seeing it on my side with the current branch, does this run look correct?

Apart from this test: I am generally wondering what this Devp2pRLPx initialization does or changes (just for my understanding) since I am not seeing any internal async calls? Or is this due to the code in the event handlers?? (does this make sense? This should be independent I guess?)

All I did was wrap RlpxServer.initRlpx() with a Promise that resolves when Devp2pRLPx emits listening. Otherwise it would continue without being ready for connections.

[holgerd77]

@ryanio I'll do a new bootstrap from my local root folder and then report back here, maybe there is something wrong on my side.

[holgerd77]

This (a bootstrap run) indeed has solved the problem, impressive! 😄

Will do another admin-merge here, again: reviewed by @ryanio by continuing the work respectively I've now reviewed the continued work from Ryan.