feat(papyrus_base_layer): start implementing eth events api

[giladchase]

• Currently only implementing the simplest event to parse, the rest are coming up.
• removed two unused error variants: FeltParseError, Serde

[reviewable-StarkWare]

This change is 

[github-actions]

Artifacts upload workflows:

• Started at 2024-12-20T13:40:18Z
• Started at 2024-12-20T13:44:41Z
• Started at 2024-12-20T16:19:28Z
• Started at 2024-12-20T16:28:22Z
• Started at 2024-12-20T16:32:29Z
• Started at 2024-12-20T16:57:19Z
• Started at 2024-12-20T23:26:12Z
• Started at 2024-12-24T11:12:33Z
• Started at 2024-12-24T11:38:57Z
• Started at 2024-12-24T13:38:37Z
• Started at 2024-12-25T07:34:05Z
• Started at 2024-12-25T08:01:02Z
• Started at 2024-12-25T11:05:28Z
• Started at 2024-12-29T15:37:23Z

[elintul]

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

---

crates/papyrus_base_layer/src/ethereum_base_layer_contract.rs line 120 at r2 (raw file):

    StarknetApiParsingError(StarknetApiError),
    #[error("{0:?}")]
    UnhandledL1Event(alloy_primitives::Log),

Can you shortly explain the scenario in which this variant is returned?

Code quote:

UnhandledL1Event(alloy_primitives::Log)

---

crates/papyrus_base_layer/src/eth_events.rs line 76 at r2 (raw file):

    })
}

Don't we have the utils below somewhere in the codebase?
Add a TODO to reuse if you prefer not to find/move them now.

[elintul]

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

[giladchase]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @elintul, @graphite-app[bot], and @matan-starkware)

---

crates/papyrus_base_layer/src/eth_events.rs line 76 at r2 (raw file):

Previously, elintul (Elin) wrote…

Don't we have the utils below somewhere in the codebase?
Add a TODO to reuse if you prefer not to find/move them now.

It's cause EthereumContractAddress and U256 are types from the alloy crate for communicating with L1, which probably shouldn't be a dependency anywhere else outside this module, I think

---

crates/papyrus_base_layer/src/ethereum_base_layer_contract.rs line 120 at r2 (raw file):

Previously, elintul (Elin) wrote…

Can you shortly explain the scenario in which this variant is returned?

< Hofer >
Maybe i should have used unimplemented!(), but it's a bit aggresive.
Basically if someone calls EthereumBaseLayer's events with the arg event_identifiers including legal Starknet L1 events that are not the four events I'm currently supporting (L1Message, the 2 cancelation events and the consumed event), the match in eth_events.rs:25 above will get to the Unhandled branch.

That is, at some point this base_layer module will support all StarknetEvents, then I can remove this variant because the match will cover all options.

I wasn't sure about returning error or calling unimplemnted! because technically the user gave legal input (so returning error isn't cool), but it simply is not supported yet (which typically calls for unimplemented!).
< /Hofer >

WDYT?

[elintul]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @giladchase, @graphite-app[bot], and @matan-starkware)

---

crates/papyrus_base_layer/src/ethereum_base_layer_contract.rs line 120 at r2 (raw file):

Previously, giladchase wrote…

< Hofer >
Maybe i should have used unimplemented!(), but it's a bit aggresive.
Basically if someone calls EthereumBaseLayer's events with the arg event_identifiers including legal Starknet L1 events that are not the four events I'm currently supporting (L1Message, the 2 cancelation events and the consumed event), the match in eth_events.rs:25 above will get to the Unhandled branch.

That is, at some point this base_layer module will support all StarknetEvents, then I can remove this variant because the match will cover all options.

I wasn't sure about returning error or calling unimplemnted! because technically the user gave legal input (so returning error isn't cool), but it simply is not supported yet (which typically calls for unimplemented!).
< /Hofer >

WDYT?

Go with the option that won't get forgotten longterm. 😅

[elintul]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @giladchase, @graphite-app[bot], and @matan-starkware)

[giladchase]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @elintul, @graphite-app[bot], and @matan-starkware)

---

crates/papyrus_base_layer/src/ethereum_base_layer_contract.rs line 120 at r2 (raw file):

Previously, elintul (Elin) wrote…

Go with the option that won't get forgotten longterm. 😅

Oh neither would, someone will hit either an error or a panic when they try to add support for a new event.
I'll just keep it as an error then.

[matan-starkware]

Reviewed 1 of 6 files at r1, 2 of 5 files at r2, 4 of 4 files at r3, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @giladchase and @graphite-app[bot])

---

crates/papyrus_base_layer/src/ethereum_base_layer_contract.rs line 101 at r3 (raw file):

            .into_iter()
            .map(TryInto::try_into)
            .collect()

Can you explain to me how this call works?

Code quote:

        // Get all logs from the latest block that match the filter.
        self.contract
            .provider()
            .get_logs(&filter)
            .await?
            .into_iter()
            .map(TryInto::try_into)
            .collect()

---

crates/papyrus_base_layer/src/eth_events.rs line 55 at r3 (raw file):

        )
    }
}

So this is here because we don't directly have this type in rust? I saw the sol! macro.

Code quote:

impl TryFrom<Starknet::MessageToL2Canceled> for EventData {
    type Error = EthereumBaseLayerError;

    fn try_from(event: Starknet::MessageToL2Canceled) -> EthereumBaseLayerResult<Self> {
        create_l1_event_data(
            event.fromAddress,
            event.toAddress,
            event.selector,
            &event.payload,
            event.nonce,
        )
    }
}

---

crates/papyrus_base_layer/src/eth_events.rs line 79 at r3 (raw file):

pub fn felt_from_eth_address(address: EthereumContractAddress) -> Felt {
    Felt::from_bytes_be_slice(address.0.as_slice())
}

Are we going to use this in other places? If not can we just inline this?

Code quote:

pub fn felt_from_eth_address(address: EthereumContractAddress) -> Felt {
    Felt::from_bytes_be_slice(address.0.as_slice())
}

[github-actions]

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [34.096 ms 34.141 ms 34.195 ms]
change: [-3.9894% -2.3997% -1.0070%] (p = 0.00 < 0.05)
Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
6 (6.00%) high mild
1 (1.00%) high severe

[elintul]

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @giladchase and @graphite-app[bot])

[giladchase]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @graphite-app[bot] and @matan-starkware)

---

crates/papyrus_base_layer/src/eth_events.rs line 55 at r3 (raw file):

Previously, matan-starkware wrote…

So this is here because we don't directly have this type in rust? I saw the sol! macro.

We have it, Starknet::MessageToL2Canceled is generated from sol! and has all the fields seen here.
Thing is, I can't leak that type out of this crate, because users should all use the BaseLayerContract trait, which is agnostic to ethereum and to alloy in particular. Therefore, I can't export sol! types, since they derive from alloy 😔 .

Another alternative could have been to newType or re-export them, but for now I wanted to do the simplest solution possible in order to push this through.

---

crates/papyrus_base_layer/src/eth_events.rs line 79 at r3 (raw file):

Previously, matan-starkware wrote…

Are we going to use this in other places? If not can we just inline this?

Thanks! Inlined.
It had previous uses previously, but they went away during the review.

---

crates/papyrus_base_layer/src/ethereum_base_layer_contract.rs line 101 at r3 (raw file):

Previously, matan-starkware wrote…

Can you explain to me how this call works?

Sure, i split it into two lines to improve readability.

The first line gets provider(), which is the actual L1 client that was initialized through alloy.
Then calls get_logs(filter), which is an alloy API (from its L1 client) that fetches all L1 events that match the given filter, which for us is {from,to}_block and a whitelist of events to fetch, where the white list is a list of &str such that each &str is keccak(<signature_of_event_in_l1>), so for example: keccak("LogMessageToL1(uint256, address, uint256[])").

The second line casts the vector of events from the first line, which all have the inner alloy event type called Log, into the Sequencer event type we called Eventdata, using the casts from eth_events.rs.

Since these cast can throw an error, we then cast the vector of results into a result using collect.

[matan-starkware]

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @giladchase and @graphite-app[bot])

---

crates/papyrus_base_layer/src/ethereum_base_layer_contract.rs line 101 at r3 (raw file):

Previously, giladchase wrote…

Sure, i split it into two lines to improve readability.

The first line gets provider(), which is the actual L1 client that was initialized through alloy.
Then calls get_logs(filter), which is an alloy API (from its L1 client) that fetches all L1 events that match the given filter, which for us is {from,to}_block and a whitelist of events to fetch, where the white list is a list of &str such that each &str is keccak(<signature_of_event_in_l1>), so for example: keccak("LogMessageToL1(uint256, address, uint256[])").

The second line casts the vector of events from the first line, which all have the inner alloy event type called Log, into the Sequencer event type we called Eventdata, using the casts from eth_events.rs.

Since these cast can throw an error, we then cast the vector of results into a result using collect.

Thanks

---

crates/papyrus_base_layer/src/ethereum_base_layer_contract.rs line 94 at r4 (raw file):

            .to_block(until_block);

        // Get all logs from the latest block that match the filter.

This is a little confusing, since the trait has the comment

    /// Get specific events from the Starknet base contract between two L1 block numbers.

That comment makes it sound like we get events for all blocks in the range provided. The comment here makes it sound like we only get events from that last block in the range provided.

Suggestion:

        // Get all logs from the latest block that matches the filter.

---

crates/papyrus_base_layer/src/ethereum_base_layer_contract.rs line 95 at r4 (raw file):

        // Get all logs from the latest block that match the filter.
        let l1_events_that_match_the_filter = self.contract.provider().get_logs(&filter).await?;

?

Suggestion:

        let matching_logs = self.contract.provider().get_logs(&filter).await?;

---

crates/papyrus_base_layer/src/ethereum_base_layer_contract.rs line 96 at r4 (raw file):

        // Get all logs from the latest block that match the filter.
        let l1_events_that_match_the_filter = self.contract.provider().get_logs(&filter).await?;
        // Try to convert all events to L1Events and return an error if one of them failed.

Mention the try and error stuff it basic rust (we can see TryInto).

Suggestion:

        // Convert logs to L1Events.

[giladchase]

Reviewable status: 6 of 7 files reviewed, 3 unresolved discussions (waiting on @elintul, @graphite-app[bot], and @matan-starkware)

---

crates/papyrus_base_layer/src/ethereum_base_layer_contract.rs line 94 at r4 (raw file):

Previously, matan-starkware wrote…

This is a little confusing, since the trait has the comment

    /// Get specific events from the Starknet base contract between two L1 block numbers.

That comment makes it sound like we get events for all blocks in the range provided. The comment here makes it sound like we only get events from that last block in the range provided.

Ya the comment isn't right, removed, the comment in the trait is sufficient I think.

---

crates/papyrus_base_layer/src/ethereum_base_layer_contract.rs line 96 at r4 (raw file):

Previously, matan-starkware wrote…

Mention the try and error stuff it basic rust (we can see TryInto).

Done.

[elintul]

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @giladchase, @graphite-app[bot], and @matan-starkware)

---

crates/papyrus_base_layer/src/ethereum_base_layer_contract.rs line 95 at r5 (raw file):

        let matching_logs = self.contract.provider().get_logs(&filter).await?;
        // Convert logs to L1Events.

I'd refrain from using struct/enum names in doc.; it's a false-positive when searching the codebase.

Suggestion:

L1 events

[giladchase]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @elintul, @graphite-app[bot], and @matan-starkware)

---

crates/papyrus_base_layer/src/ethereum_base_layer_contract.rs line 95 at r5 (raw file):

Previously, elintul (Elin) wrote…

I'd refrain from using struct/enum names in doc.; it's a false-positive when searching the codebase.

Yr right, done.

[elintul]

Reviewed 2 of 2 files at r6, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @graphite-app[bot] and @matan-starkware)

[matan-starkware]

Reviewed 2 of 2 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @graphite-app[bot])

[matan-starkware]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @giladchase and @graphite-app[bot])

---

crates/papyrus_base_layer/src/ethereum_base_layer_contract.rs line 96 at r6 (raw file):

        let matching_logs = self.contract.provider().get_logs(&filter).await?;
        // Convert logs to L1 events.
        matching_logs.into_iter().map(TryInto::try_into).collect()

Honestly, given the variable name and the function return type idk if we need any comment.

Suggestion:

        matching_logs.into_iter().map(TryInto::try_into).collect()