apollo_batcher: add revert task

[yoavGrs]

No description provided.

[reviewable-StarkWare]

This change is 

[yoavGrs]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• apollo_batcher: add revert task #11451 : 2 dependent PRs (#11496 , #11516 ) 👈 (View in Graphite)
• apollo_batcher: define custom input task type #11595
• apollo_batcher: wait for revert result #11484
• apollo_batcher: perform commitment tasks #11442
• main-v0.14.1-committer

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

[amosStarkware]

@amosStarkware reviewed 2 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @yoavGrs).

---

crates/apollo_batcher/src/commitment_manager/commitment_manager_impl.rs line 268 at r1 (raw file):

    // Associated functions.

    pub(crate) async fn add_revert_task(

please decrease task offset. and please add a unit test for this method

Code quote:

pub(crate) async fn add_revert_task(

---

crates/apollo_batcher/src/commitment_manager/commitment_manager_impl.rs line 280 at r1 (raw file):

                actual: height,
                state_diff_commitment: None,
            });

This error is specific to commitment task (see its message) - please rename this error and add a variant for reverts.

Code quote:

            return Err(CommitmentManagerError::WrongTaskHeight {
                expected: expected_height,
                actual: height,
                state_diff_commitment: None,
            });

[yoavGrs]

@yoavGrs made 2 comments.
Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @amosStarkware).

---

crates/apollo_batcher/src/commitment_manager/commitment_manager_impl.rs line 268 at r1 (raw file):

Previously, amosStarkware wrote…

please decrease task offset. and please add a unit test for this method

IMO, we update the offset when we are ready for the next block.
In revert, we decrease when we have a valid result.

---

crates/apollo_batcher/src/commitment_manager/commitment_manager_impl.rs line 280 at r1 (raw file):

Previously, amosStarkware wrote…

This error is specific to commitment task (see its message) - please rename this error and add a variant for reverts.

Done.

[amosStarkware]

@amosStarkware reviewed 3 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @yoavGrs).

---

crates/apollo_batcher/src/commitment_manager/commitment_manager_impl.rs line 268 at r1 (raw file):

Previously, yoavGrs wrote…

IMO, we update the offset when we are ready for the next block.
In revert, we decrease when we have a valid result.

We currently update the offset when we add a commitment task - not sure what you mean by ready for the next block

[yoavGrs]

@yoavGrs made 1 comment.
Reviewable status: 2 of 3 files reviewed, 3 unresolved discussions (waiting on @amosStarkware and @dorimedini-starkware).

---

crates/apollo_batcher/src/commitment_manager/commitment_manager_impl.rs line 268 at r1 (raw file):

Previously, amosStarkware wrote…

The commitment task offset is the batch ID of the next task the commitment manager expects - So if a commitment task was added, it is immediately increased. and when a revert task is added, the next task the commitment manager expects is (offset -1), so IMO it should be immediately decreased.
Note that this offset is only in memory - it is distinct from the global root offset, which lives in storage, and is only updated once the results are written to storage (or, in case of revert - deleted from storage).

@dorimedini-starkware
https://reviewable.io/reviews/starkware-libs/sequencer/11496

[dorimedini-starkware]

@dorimedini-starkware reviewed 1 file and all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @amosStarkware and @yoavGrs).

[yoavGrs]

@yoavGrs made 2 comments.
Reviewable status: 2 of 3 files reviewed, 3 unresolved discussions (waiting on @amosStarkware and @dorimedini-starkware).

---

crates/apollo_batcher/src/commitment_manager/commitment_manager_impl.rs line 113 at r2 (raw file):

Previously, dorimedini-starkware wrote…

what data is in task_details that cannot be derived from task_input?
why not just impl Display for CommitterTaskInput?

Done.

---

crates/apollo_batcher/src/commitment_manager/commitment_manager_test.rs line 214 at r2 (raw file):

Previously, dorimedini-starkware wrote…

block height is no longer part of the panic message?

It is part of the error message:

Commit block 4, state diff commitment: Some(StateDiffCommitment(PoseidonHash(0x0))).. The channel is full. channel size: 1. Consider increasing the channel size or enabling waiting in the config.

I don't want to include the state root in the expected message.

[dorimedini-starkware]

@dorimedini-starkware reviewed 1 file and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @amosStarkware and @yoavGrs).

---

crates/apollo_batcher/src/commitment_manager/commitment_manager_impl.rs line 113 at r2 (raw file):

Previously, yoavGrs wrote…

Done.

did you push? don't see it here

[yoavGrs]

@yoavGrs made 2 comments.
Reviewable status: 2 of 3 files reviewed, 2 unresolved discussions (waiting on @amosStarkware and @dorimedini-starkware).

---

crates/apollo_batcher/src/commitment_manager/commitment_manager_impl.rs line 268 at r1 (raw file):

Previously, yoavGrs wrote…

@dorimedini-starkware
https://reviewable.io/reviews/starkware-libs/sequencer/11496

Added.

---

crates/apollo_batcher/src/commitment_manager/commitment_manager_impl.rs line 113 at r2 (raw file):

Previously, dorimedini-starkware wrote…

did you push? don't see it here

Sorry, now.

[dorimedini-starkware]

@dorimedini-starkware reviewed 1 file and all commit messages, and resolved 1 discussion.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @amosStarkware).

[dorimedini-starkware]

@dorimedini-starkware reviewed 1 file and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @amosStarkware).

[dorimedini-starkware]

@dorimedini-starkware reviewed 2 files and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @amosStarkware).

[amosStarkware]

@amosStarkware reviewed 2 files and all commit messages, and resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @yoavGrs).