chore(starknet_consensus_manager): add proposal init into validate proposal input

[ArniStarkware]

No description provided.

[reviewable-StarkWare]

This change is 

[ArniStarkware]

• chore(starknet_consensus_manager): add proposal init into validate proposal input #2408 👈 (View in Graphite)
• chore(consensus): use non zero validator id in tests #2550
• chore(consensus): use non zero validator id in consensus config #2549
• main

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

[codecov]

Codecov Report

Attention: Patch coverage is 74.07407% with 7 lines in your changes missing coverage. Please review.

Project coverage is 32.26%. Comparing base (e3165c4) to head (79d57f2).
Report is 809 commits behind head on main.

Files with missing lines	Patch %	Lines
...nsus_orchestrator/src/papyrus_consensus_context.rs	53.33%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2408      +/-   ##
==========================================
- Coverage   40.10%   32.26%   -7.85%     
==========================================
  Files          26      119      +93     
  Lines        1895    14085   +12190     
  Branches     1895    14085   +12190     
==========================================
+ Hits          760     4544    +3784     
- Misses       1100     8989    +7889     
- Partials       35      552     +517     

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

[ArniStarkware]

Reviewable status: 9 of 11 files reviewed, 1 unresolved discussion (waiting on @alonh5 and @matan-starkware)

---

crates/sequencing/papyrus_consensus_orchestrator/src/papyrus_consensus_context_test.rs line 75 at r1 (raw file):

Previously, matan-starkware wrote…

1. Let's avoid ValidatorId::default() since this is 0 which is an illegal address.
2. Don't set valid_round explicitly because: (a) validate_proposal doesn't care about the field, (b) we'll add signature field soon which also should be ignored and default makes that seamless.

Please do this for all tests

I did that for all the tests I added in this PR.
Done.

[ArniStarkware]

crates/sequencing/papyrus_consensus_orchestrator/src/papyrus_consensus_context_test.rs line 75 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

I did that for all the tests I added in this PR.
Done.

Also, it is based on the change to the validator ID.

[alonh5]

Reviewed 2 of 11 files at r1, 9 of 9 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ArniStarkware and @matan-starkware)

---

crates/sequencing/papyrus_consensus_orchestrator/src/papyrus_consensus_context.rs line 184 at r3 (raw file):

                // the block in storage and to getting the transaction was a revert
                // this flow will fail.
                wait_for_block(&storage_reader, proposal_init.height)

Maybe it's worth adding let height = proposal_init.height?

Code quote:

proposal_init.height

[ArniStarkware]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @alonh5 and @matan-starkware)

---

crates/sequencing/papyrus_consensus_orchestrator/src/papyrus_consensus_context.rs line 184 at r3 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Maybe it's worth adding let height = proposal_init.height?

This is consistent with how this variable is handled in a similar function in this file:build_proposal

[alonh5]

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ArniStarkware and @matan-starkware)

---

crates/sequencing/papyrus_consensus_orchestrator/src/papyrus_consensus_context.rs line 184 at r3 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

This is consistent with how this variable is handled in a similar function in this file:build_proposal

I didn't understand your response. I'm saying proposal_init.height is used like 7 times in this function so it's probably defining a shorter variable name for it. Same goes for build_proposal in that case.

[ArniStarkware]

Reviewable status: 10 of 11 files reviewed, 2 unresolved discussions (waiting on @alonh5 and @matan-starkware)

---

crates/sequencing/papyrus_consensus_orchestrator/src/papyrus_consensus_context.rs line 184 at r3 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

I didn't understand your response. I'm saying proposal_init.height is used like 7 times in this function so it's probably defining a shorter variable name for it. Same goes for build_proposal in that case.

Done.

[alonh5]

Reviewed 5 of 5 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @matan-starkware)