refactor(cli): Remove babe-lead flag

[dimartiro]

Changes

• Removes babe-lead flag that prevents a node to create blocks until it receive the first one

Tests

make tests

• manual tests running nodes locally checking that all nodes are producing blocks using the same canonical chain
• also tested using zombienet

Issues

#3184

Primary Reviewer

@timwu20

[codecov]

Codecov Report

Merging #3193 (9b7f93d) into development (91b748f) will decrease coverage by 0.08%.
The diff coverage is n/a.

Additional details and impacted files

@@               Coverage Diff               @@
##           development    #3193      +/-   ##
===============================================
- Coverage        51.62%   51.55%   -0.08%     
===============================================
  Files              221      221              
  Lines            28283    28236      -47     
===============================================
- Hits             14601    14556      -45     
+ Misses           12364    12362       -2     
  Partials          1318     1318              

[noot]

the reason the --babe-lead flag exists is because the start slot of the network is based on the timestamp of block 1. if there are two conflicting block 1s, then the network will partition due to some nodes having a different start slot than others. I'd ensure this issue is resolved before merging this

the fork choice rule should pick the same block 1 on every node, but I believe the start slot was being set on first validation which was the problem

[dimartiro]

the reason the --babe-lead flag exists is because the start slot of the network is based on the timestamp of block 1. if there are two conflicting block 1s, then the network will partition due to some nodes having a different start slot than others. I'd ensure this issue is resolved before merging this

the fork choice rule should pick the same block 1 on every node, but I believe the start slot was being set on first validation which was the problem

Thanks for this comment Elizabeth.

We've made a lot of changes since this flag was added, so I think that's why this issue should be resolved by now.
I did some tests locally and it seems to be working good selecting the right canonical chain but I gonna be doing more tests today to be sure.
I'm also working on use zombienet #3192 to automatize this tests
What do you think is the best way to test this change? If you have any recommendation will be very helpful
Also, do you know how is this solved in substrate?

[noot]

okay awesome, I just wasn't sure if context was needed for why it was there in the first place lol. if it's working with multiple block producing nodes and multiple block 1s being produced then I think you're good 👍

edit: to force multiple block 1s you can modify the babe threshold to make it more likely btw

[qdm12]

Looking nice 👍

[jimjbrettj]

This is not a blocker I don't think, but do you know why two different blocks are built with the same number here?

To clarify, you did not introduce this, just an interesting thing I observed that probably should be looked into even if its not in this PR

[jimjbrettj]

Great work!

[dimartiro]

This is not a blocker I don't think, but do you know why two different blocks are built with the same number here?

To clarify, you did not introduce this, just an interesting thing I observed that probably should be looked into even if its not in this PR

Very good catch @jimjbrettj ! 👏 I executed the tests in development and it has the same behavior so it is not introduced by my change.

[github-actions]

🎉 This PR is included in version 0.8.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀