refactor(starknet_batcher): rename GetProposalResultError and remove the ProposalNotFound variant

[dafnamatsry]

No description provided.

[reviewable-StarkWare]

This change is 

[dafnamatsry]

• refactor(starknet_batcher): add tests and refactor batcher_test #2640
• refactor(starknet_batcher): delete the proposal manager #2618
• refactor(starknet_batcher): move around some types and utils to a common utils file #2622
• refactor(starknet_batcher): validate flow refactor #2441
• refactor(starknet_batcher): rename GetProposalResultError and remove the ProposalNotFound variant #2483 👈 (View in Graphite)
• refactor(starknet_batcher): add getters to the proposal manger #2480
• main

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

[codecov]

Codecov Report

Attention: Patch coverage is 33.33333% with 10 lines in your changes missing coverage. Please review.

Project coverage is 57.15%. Comparing base (e3165c4) to head (372a1c5).
Report is 759 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/starknet_batcher/src/proposal_manager.rs	33.33%	5 Missing and 1 partial ⚠️
crates/starknet_batcher/src/batcher.rs	33.33%	4 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2483       +/-   ##
===========================================
+ Coverage   40.10%   57.15%   +17.05%     
===========================================
  Files          26      156      +130     
  Lines        1895    18309    +16414     
  Branches     1895    18309    +16414     
===========================================
+ Hits          760    10465     +9705     
- Misses       1100     7378     +6278     
- Partials       35      466      +431     

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

[Yael-Starkware]

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @alonh5 and @dafnamatsry)

---

crates/starknet_batcher/src/batcher.rs line 288 at r1 (raw file):

                _ => return Err(BatcherError::InternalError),
            },
            Some(Err(ProposalError::Aborted)) => return Err(BatcherError::ProposalAborted),

I think that if the proposal was aborted, the function close_input_transaction_stream would return an error.
so this match arm would never be reached.

Code quote:

Some(Err(ProposalError::Aborted)) => return Err(BatcherError::ProposalAborted),

---

crates/starknet_batcher/src/batcher_test.rs line 560 at r1 (raw file):

        .times(1)
        .with(eq(PROPOSAL_ID))
        .return_once(|_| async move { None }.boxed());

I think that move is not required here,
probably in other places too.
non blocking for this PR, but we should fix separately.

Code quote:

move

[dafnamatsry]

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

---

crates/starknet_batcher/src/batcher.rs line 288 at r1 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

I think that if the proposal was aborted, the function close_input_transaction_stream would return an error.
so this match arm would never be reached.

We still need to cover it, and I think returning an error is better than panic in this case.
Also, in the following PR, I added a check at the beginning of send_proposal_content, so if the proposal does not exist/aborted/etc. it wouldn't reach this function at all.

---

crates/starknet_batcher/src/batcher_test.rs line 560 at r1 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

I think that move is not required here,
probably in other places too.
non blocking for this PR, but we should fix separately.

Right.
The tests are going to be changed a lot when the mock proposal manager will be deleted (and specifically this and almost all other expectations will be removed) . I'll keep this in mind.

[Yael-Starkware]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @alonh5)

[Yael-Starkware]

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

[alonh5]

Reviewed 3 of 4 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dafnamatsry)

[dafnamatsry]

Merge activity

• Dec 9, 6:47 AM EST: A user started a stack merge that includes this pull request via Graphite.
• Dec 9, 6:49 AM EST: Graphite rebased this pull request as part of a merge.
• Dec 9, 7:01 AM EST: A user merged this pull request with Graphite.