[Merged by Bors] - Log ttd

[pawanjay176]

Issue Addressed

Resolves #3249

Proposed Changes

Log merge related parameters and EE status in the beacon notifier before the merge.

[pawanjay176]

I would love some feedback on the actual logging here. This currently logs only if the chain is in the bellatrix fork and the chain hasn't hit ttd. Currently, if you have everything setup correctly, this should log something like

...
Jul 19 00:45:29.010 INFO Bellatrix merge parameters              params: Terminal total difficulty 6400, service: slot_notifier
Jul 19 00:45:29.010 INFO Execution layer status                  current_difficulty: 2500, synced: true, service: slot_notifier
...

If the execution layer is not synced, it will log

...
Jul 19 00:45:29.010 INFO Bellatrix merge parameters              params: Terminal total difficulty 6400, service: slot_notifier
Jul 19 00:45:29.010 WARN Execution layer not synced              service: slot_notifier
...

If the execution layer isn't configured with execution-endpoints, it will log

...
Jul 19 00:45:29.010 INFO Bellatrix merge parameters              params: Terminal total difficulty 6400, service: slot_notifier
Jul 19 00:45:29.010 WARN Execution layer not configured       action: configure an execution endpoint immediately to be merge ready, service: slot_notifier
...

Still needs testing with more edge cases, but want to get feedback on changing the wording or adding additional parameters to the logs.

[paulhauner]

Very cool! The only suggestion I have regarding the log output is to rename the params field to total_terminal_difficulty. It might be a little verbose in the code due to the way slog macros work, but I think it would look nice.

On another note, I was looking at #3260 and it talks about an API endpoint that displays similar info to this. I have a sketch of how we might be able to compute such info on the beacon chain and then use it in the client notifier and in the HTTP API:

paulhauner@9182210

It's pretty rough, but maybe you could integrate your TTD stuff into something like that and we could reuse it to resolve #3260 as well (I'm open to that being in another PR if you like).

[pawanjay176]

Ohh didn't realise we are looking to expose merge readiness over an http endpoint as well. I like your approach better in that case. Will refactor this one with your approach and then make a separate PR for the http api.

[pawanjay176]

Another thing to consider is if we should log something for terminal_block_hash and terminal_block_hash_activation_epoch. In the happy case, we only have to care about ttd. But in case we need to make overrides, I think logging the other parameters might end up being useful as well.

[paulhauner]

Cool, I think it'll be super easy to add the HTTP endpoints with this format.

I've made some suggestions over in https://github.com/paulhauner/lighthouse/tree/log-ttd-paul:

• Start logging preparation warnings 1 week before Bellatrix (we might want to lengthen this for mainnet, but Geth isn't ready for Prater yet)
• Fixed a duplicate Display message (my fault).
• Modify the logging format a bit.

Examples from my testing

Ready for the merge

Jul 20 06:00:30.002 INFO Ready for the merge                     current_difficulty: 50443911550974788, terminal_total_difficulty: 50000000000000000, service: slot_notifier

I had to hack a Ropsten node to display this, that's why the TTD is high.

No --execution-endpoints flag

Jul 20 05:13:54.002 WARN Not ready for merge                     info: The --execution-endpoint flag is not specified, this is a requirement for the merge, service: slot_notifier

EL is syncing

Jul 20 04:56:18.001 WARN Not ready for merge                     info: The execution endpoint is connected and configured, however it is not synced, service: slot_notifier

TTD mismatch with EL

Jul 20 04:03:54.001 ERRO Not ready for merge                     info: Could not confirm the transition configuration with the execution endpoint: EngineError(Api { error: ServerMessage { code: -32000, message: "invalid ttd: execution 50000000000000000 consensus 0x64" } }), service: slot_notifier

Terminal block hash/epoch mismatch with EL

Jul 20 04:12:30.003 ERRO Not ready for merge                     info: Could not confirm the transition configuration with the execution endpoint: EngineError(Api { error: ServerMessage { code: -32000, message: "invalid terminal block hash" } }), service: slot_notifier

Could not connect to EL

Jul 20 04:00:18.001 ERRO Not ready for merge                     info: Could not confirm the transition configuration with the execution endpoint: EngineError(Api { error: Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("localhost")), port: Some(8551), path: "/", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 111, kind: ConnectionRefused, message: "Connection refused" })) }) }), service: slot_notifier

[pawanjay176]

@paulhauner Pulled in your changes as is. Couldn't fault it 👌

[paulhauner]

Yay! Let's merge it!

bors r+

[bors]

Pull request successfully merged into unstable.

Build succeeded:

• arbitrary-check
• beacon-chain-tests
• cargo-audit
• cargo-fmt
• cargo-udeps
• cargo-vendor
• check-benchmarks
• check-consensus
• check-msrv
• clippy
• debug-tests-ubuntu
• dockerfile-ubuntu
• doppelganger-protection-test
• ef-tests-ubuntu
• eth1-simulator-ubuntu
• execution-engine-integration-ubuntu
• merge-transition-ubuntu
• no-eth1-simulator-ubuntu
• op-pool-tests
• release-tests-ubuntu
• release-tests-windows
• state-transition-vectors-ubuntu