chore(starknet_l1_provider): implement event fetch

[giladchase]

No description provided.

[reviewable-StarkWare]

This change is 

[github-actions]

Artifacts upload workflows:

• Started at 2024-12-20T21:04:08Z
• Started at 2024-12-24T13:52:10Z

[github-actions]

Benchmark movements:
tree_computation_flow performance regressed!
tree_computation_flow time: [34.511 ms 34.544 ms 34.584 ms]
change: [+1.0304% +1.4044% +1.7001%] (p = 0.00 < 0.05)
Performance has regressed.
Found 7 outliers among 100 measurements (7.00%)
2 (2.00%) high mild
5 (5.00%) high severe

[elintul]

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

---

crates/starknet_l1_provider/src/l1_scraper.rs line 57 at r1 (raw file):

            error!("Failed to get latest L1 block number, finality too high.");
            return Ok(());
        };

In case this code is repeated, consider a function.

Code quote:

        let latest_l1_block_number = self
            .base_layer
            .latest_l1_block_number(self.config.finality)
            .await
            .map_err(L1ScraperError::BaseLayer)?;

        let Some(latest_l1_block_number) = latest_l1_block_number else {
            error!("Failed to get latest L1 block number, finality too high.");
            return Ok(());
        };

---

crates/starknet_l1_provider/src/l1_scraper.rs line 72 at r1 (raw file):

            .into_iter()
            .map(|event| self.event_from_raw_l1_event(event))
            .collect::<L1ScraperResult<Vec<_>, _>>()?;

Do you prefer turbofish over type annotations?
Also, WDYM by collecting to a Result?

Code quote:

::<L1ScraperResult<Vec<_>, _>>

---

crates/starknet_l1_provider/src/l1_scraper.rs line 86 at r1 (raw file):

    }

    fn event_from_raw_l1_event(&self, l1_event: L1Event) -> L1ScraperResult<Event, B> {

Can this be Event::TryFrom<L1Event> method?

Code quote:

event_from_raw_l1_event

---

crates/starknet_l1_provider/src/l1_scraper.rs line 92 at r1 (raw file):

                ExecutableL1HandlerTransaction::create(tx, chain_id, fee)
                    .map(Event::L1HandlerTransaction)
                    .map_err(L1ScraperError::HashCalculationError)

Not very readable, IMO. Any alternative?

Code quote:

                ExecutableL1HandlerTransaction::create(tx, chain_id, fee)
                    .map(Event::L1HandlerTransaction)
                    .map_err(L1ScraperError::HashCalculationError)

---

crates/starknet_l1_provider/src/l1_scraper.rs line 124 at r1 (raw file):

#[derive(Error, Debug)]
pub enum L1ScraperError<T: BaseLayerContract + Send + Sync> {

Is this necessary? What exactly is generic here - the L1, or the contract we're interacting with?

Code quote:

T: BaseLayerContract

[matan-starkware]

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @elintul and @giladchase)

---

crates/starknet_l1_provider/src/l1_scraper.rs line 72 at r1 (raw file):

Previously, elintul (Elin) wrote…

Do you prefer turbofish over type annotations?
Also, WDYM by collecting to a Result?

"Also, WDYM by collecting to a Result?" This tells Rust to iterate over Result<T> elements and upon encountering a single error exit, returning that error. If no errors are encountered, it produces Vec<T>.

---

crates/starknet_l1_provider/src/l1_scraper.rs line 92 at r1 (raw file):

Previously, elintul (Elin) wrote…

Not very readable, IMO. Any alternative?

Agreed. If you update the error type to HashCalculationError(#[from] StarknetApiError), Then you could do

                let tx = ExecutableL1HandlerTransaction::create(tx, &self.config.chain_id, fee)?;
                Ok(Event::L1HandlerTransaction(tx))

---

crates/starknet_l1_provider/src/l1_scraper.rs line 55 at r1 (raw file):

        let Some(latest_l1_block_number) = latest_l1_block_number else {
            error!("Failed to get latest L1 block number, finality too high.");

This means that there is no finalized block on ethereum? (dependent upon our config)

Code quote:

            error!("Failed to get latest L1 block number, finality too high.");

---

crates/starknet_l1_provider/src/l1_scraper.rs line 74 at r1 (raw file):

            .collect::<L1ScraperResult<Vec<_>, _>>()?;

        self.last_block_number_processed = latest_l1_block_number + 1;

Is BaseLayerContract::until_block inclusive on until_block? I think that should be explicit in the trait doc.

Code quote:

        self.last_block_number_processed = latest_l1_block_number + 1;

---

crates/starknet_l1_provider/src/l1_scraper.rs line 128 at r1 (raw file):

    BaseLayer(T::Error),
    #[error("Failed to calculate hash: {0}")]
    HashCalculationError(StarknetApiError),

Allow for easy conversions

Suggestion:

    HashCalculationError(#[from] StarknetApiError),

[elintul]

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

---

crates/starknet_l1_provider/src/l1_scraper.rs line 72 at r1 (raw file):

Previously, matan-starkware wrote…

"Also, WDYM by collecting to a Result?" This tells Rust to iterate over Result<T> elements and upon encountering a single error exit, returning that error. If no errors are encountered, it produces Vec<T>.

Thanks!