fix(apollo_batcher): batcher returns error when l1_provider fail while commiting a block

[itamar-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[itamar-starkware]

• fix(apollo_batcher): batcher returns error when l1_provider fail while commiting a block #6359 👈 (View in Graphite)
• main

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

[alonh5]

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @itamar-starkware)

---

crates/apollo_batcher/src/batcher.rs line 611 at r1 (raw file):

            // Rollback the state diff in the storage.
            self.storage_writer.revert_block(height);
            STORAGE_HEIGHT.decrement(1);

Instead of incrementing an decrementing you can only increment at the end of the function once you can't fail anymore.

Code quote:

STORAGE_HEIGHT.decrement(1);

---

crates/apollo_batcher/src/batcher.rs line 612 at r1 (raw file):

            self.storage_writer.revert_block(height);
            STORAGE_HEIGHT.decrement(1);
            REVERTED_BLOCKS.increment(1);

No need to increment this, it's only for when blocks were reverted in Starknet. Here we a re just reverting in this node because of an error.

Code quote:

REVERTED_BLOCKS.increment(1);

---

crates/apollo_batcher/src/batcher_test.rs line 1132 at r1 (raw file):

    mock_dependencies.storage_writer.expect_revert_block().returning(|_| ());

    // let expected_artifacts = BlockExecutionArtifacts::create_for_testing();

Remove?

Code quote:

// let expected_artifacts = BlockExecutionArtifacts::create_for_testing();

[itamar-starkware]

crates/apollo_batcher/src/batcher_test.rs line 1132 at r1 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Remove?

Yes, I was considering using this variable like that:

Code snippet:

    mock_create_builder_for_propose_block(
        &mut mock_dependencies.block_builder_factory,
        vec![],
        Ok(expected_artifacts),
    );
What do you think?

[itamar-starkware]

Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @alonh5)

---

crates/apollo_batcher/src/batcher.rs line 611 at r1 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Instead of incrementing an decrementing you can only increment at the end of the function once you can't fail anymore.

Done.

---

crates/apollo_batcher/src/batcher.rs line 612 at r1 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

No need to increment this, it's only for when blocks were reverted in Starknet. Here we a re just reverting in this node because of an error.

Done.

[alonh5]

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

---

crates/apollo_batcher/src/batcher_test.rs line 1132 at r1 (raw file):

Previously, itamar-starkware wrote…

Yes, I was considering using this variable like that:

It's okay like this