refactor(consensus): move the sync check into run_height

[matan-starkware]

The goal is to remove the nested selects to make cancellation logic simpler.

[reviewable-StarkWare]

This change is 

[matan-starkware]

• refactor(consensus): move code from select to handle_proposal #2620
• refactor(consensus): move the sync check into run_height #2499 👈 (View in Graphite)
• refactor(consensus): lower test timeouts and fix test #2498
• refactor(consensus): remove next_message to ensure cancellation safety #2497
• refactor(consensus): create start_height function in the manager #2496
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 9.65%. Comparing base (af0e37d) to head (270de19).

Additional details and impacted files

@@                           Coverage Diff                           @@
##           matan/consensus/manager_test_timeout   #2499      +/-   ##
=======================================================================
- Coverage                                  9.76%   9.65%   -0.12%     
=======================================================================
  Files                                        87      87              
  Lines                                     10491   10476      -15     
  Branches                                  10491   10476      -15     
=======================================================================
- Hits                                       1024    1011      -13     
  Misses                                     9430    9430              
+ Partials                                     37      35       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[asmaastarkware]

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dan-starkware)

[dan-starkware]

crates/sequencing/papyrus_consensus/src/manager.rs line 185 at r1 (raw file):

                sync_height = sync_height(height, sync_receiver) => {
                    return Ok(RunHeightRes::Sync(sync_height?));
                }

Should we move the logic into an habdle_ function and have only the next() here?

Code quote:

                sync_height = sync_height(height, sync_receiver) => {
                    return Ok(RunHeightRes::Sync(sync_height?));
                }

[matan-starkware]

Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @asmaastarkware and @dan-starkware)

---

crates/sequencing/papyrus_consensus/src/manager.rs line 185 at r1 (raw file):

Previously, dan-starkware wrote…

Should we move the logic into an habdle_ function and have only the next() here?

Well we can't actually put this neatly into a handle_ function since I want to continue, but I can just put the logic inline since it is fairly straightforward.

I think that they benefit of simplifying the cancellation logic is more important than avoiding code in the macro.

[dan-starkware]

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @matan-starkware)

[matan-starkware]

Reviewed 1 of 2 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @matan-starkware)

[matan-starkware]

Merge activity

• Dec 15, 3:44 AM EST: A user started a stack merge that includes this pull request via Graphite.
• Dec 15, 4:12 AM EST: Graphite couldn't merge this PR because it was not satisfying all requirements (Failed CI: 'merge-gatekeeper').
• Dec 15, 4:54 AM EST: A user started a stack merge that includes this pull request via Graphite.
• Dec 15, 4:55 AM EST: A user merged this pull request with Graphite.