feat: implement event hash

[dorimedini-starkware]

This change is 

[dorimedini-starkware]

Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @noaov1 and @Yoni-Starkware)

---

src/block_hash.rs line 6 at r1 (raw file):

use starknet_types_core::felt::Felt;
use starknet_types_core::hash::{Pedersen, StarkHash};

(blocking self) replace this crate's pedersen_hash directly (in a previous PR; maybe need to implement hash_array as well)

Code quote:

use starknet_types_core::hash::{Pedersen, StarkHash};

[Yoni-Starkware]

Note that the old block hash should be implemented somewhere in papyrus

Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @noaov1)

[Yoni-Starkware]

Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @noaov1 and @ShahakShama)

---

src/block_hash.rs line 6 at r1 (raw file):

Previously, dorimedini-starkware wrote…

(blocking self) replace this crate's pedersen_hash directly (in a previous PR; maybe need to implement hash_array as well)

I think we should use Poseidon whenever we can now. Right @ShahakShama?

[ShahakShama]

Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware and @noaov1)

---

src/block_hash.rs line 6 at r1 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

I think we should use Poseidon whenever we can now. Right @ShahakShama?

It depends if the goal of this PR is to implement the new hash that will appear from 0.13.2 or the current hash

[ShahakShama]

Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware and @noaov1)

---

src/block_hash.rs line 6 at r1 (raw file):

Previously, ShahakShama wrote…

It depends if the goal of this PR is to implement the new hash that will appear from 0.13.2 or the current hash

If it's the former, then that hash is not final yet, and we should advance finalizing it if we want to implement it already

[ShahakShama]

Reviewed 5 of 5 files at r2.
Reviewable status: all files reviewed (commit messages unreviewed), 4 unresolved discussions (waiting on @dorimedini-starkware and @noaov1)

---

src/block_hash.rs line 1 at r2 (raw file):

#[cfg(test)]

Consider making block_hash a directory with submodule for each subcomponent that has a hash function (event, transaction, class, state_diff)

---

src/block_hash.rs line 10 at r2 (raw file):

use crate::transaction::{Event, TransactionHash};

pub fn calculate_event_hash(event: &Event, tx_hash: &TransactionHash) -> PoseidonHash {

tx_hash -> transaction_hash

---

src/block_hash.rs line 11 at r2 (raw file):

pub fn calculate_event_hash(event: &Event, tx_hash: &TransactionHash) -> PoseidonHash {
    // Poseidon(

Maybe put this above the function? It looks like an erased code that you want to leave around just in case

---

src/block_hash.rs line 18 at r2 (raw file):

    poseidon_hash_array(
        &[
            &[*event.from_address.0.key(), tx_hash.0, Felt::from(event.content.keys.len()).into()],

Consider adding a utility function for hashing an array that first puts the length and then the array itself

[Yoni-Starkware]

Reviewable status: all files reviewed (commit messages unreviewed), 6 unresolved discussions (waiting on @dorimedini-starkware and @noaov1)

---

src/block_hash.rs line 20 at r2 (raw file):

            &[*event.from_address.0.key(), tx_hash.0, Felt::from(event.content.keys.len()).into()],
            event.content.keys.iter().map(|k| k.0).collect::<Vec<_>>().as_slice(),
            &[Felt::from(event.content.data.0.len()).into()],

Can you define a utility for this? (converting len to Felt)
I guess we will see it a lot

Code quote:

Felt::from(event.content.data.0.len()).into()

---

src/block_hash.rs line 23 at r2 (raw file):

            event.content.data.0.as_slice(),
        ]
        .concat(),

Extract these out for readability

        let keys = event.content.keys.iter().map(|k| k.0).collect::<Vec<_>>();
        let data = event.content.data.0;

Code quote:

    poseidon_hash_array(
        &[
            &[*event.from_address.0.key(), tx_hash.0, Felt::from(event.content.keys.len()).into()],
            event.content.keys.iter().map(|k| k.0).collect::<Vec<_>>().as_slice(),
            &[Felt::from(event.content.data.0.len()).into()],
            event.content.data.0.as_slice(),
        ]
        .concat(),

[Yoni-Starkware]

Reviewable status: all files reviewed (commit messages unreviewed), 6 unresolved discussions (waiting on @dorimedini-starkware, @noaov1, and @ShahakShama)

---

src/block_hash.rs line 18 at r2 (raw file):

Previously, ShahakShama wrote…

Consider adding a utility function for hashing an array that first puts the length and then the array itself

In Poseidon you have to flatten everything in advance (it's not a hash chain)
So maybe a util the returns [len(arr), *arr]

[Yoni-Starkware]

Reviewable status: all files reviewed (commit messages unreviewed), 7 unresolved discussions (waiting on @dorimedini-starkware, @noaov1, and @ShahakShama)

---

src/block_hash_test.rs line 13 at r2 (raw file):

        content: EventContent {
            keys: [2_u8, 3].iter().map(|key| EventKey(stark_felt!(*key))).collect(),
            data: EventData([4_u8, 5, 6].into_iter().map(StarkFelt::from).collect()),

Why?

Suggestion:

        from_address: contract_address!(10),
        content: EventContent {
            keys: [2, 3].iter().map(|key| EventKey(stark_felt!(*key))).collect(),
            data: EventData([4, 5, 6].into_iter().map(StarkFelt::from).collect()),

[dorimedini-starkware]

Reviewable status: all files reviewed (commit messages unreviewed), 2 unresolved discussions (waiting on @noaov1 and @Yoni-Starkware)

---

src/block_hash.rs line 20 at r2 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Can you define a utility for this? (converting len to Felt)
I guess we will see it a lot

added impl From<usize> for StarkFelt

---

src/block_hash.rs line 23 at r2 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Extract these out for readability

        let keys = event.content.keys.iter().map(|k| k.0).collect::<Vec<_>>();
        let data = event.content.data.0;

Done.

---

src/block_hash_test.rs line 13 at r2 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Why?

no; compiler auto-deduces type as i32, and StarkFelt::from is not implemented for this type.
StarkFelt::from is implemented for all unsigned int types, so the compiler can't guess which integer type is correct...
by providing explicit type we mitigate these errors.

[Yoni-Starkware]

Reviewable status: 2 of 7 files reviewed, 2 unresolved discussions (waiting on @dorimedini-starkware, @noaov1, and @ShahakShama)

---

src/block_hash/utils.rs line 4 at r3 (raw file):

pub(crate) fn hash_array_preimage(array: &Vec<StarkFelt>) -> Vec<StarkFelt> {
    [vec![array.len().into()], array.iter().map(|f| *f).collect::<Vec<StarkFelt>>()].concat().into()

Can you do something simple like clone? (same complexity, right?)

Code quote:

array.iter().map(|f| *f).collect::<Vec<StarkFelt>>()

[Yoni-Starkware]

Reviewable status: 2 of 7 files reviewed, all discussions resolved (waiting on @noaov1 and @ShahakShama)

[ShahakShama]

Reviewed 4 of 5 files at r3, 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware and @noaov1)

---

src/block_hash/event_hash.rs line 15 at r5 (raw file):

/// ).
pub fn calculate_event_hash(event: &Event, transaction_hash: &TransactionHash) -> PoseidonHash {
    let keys = &event.content.keys.iter().map(|k| k.0).collect::<Vec<_>>();

Consider changing hash_array_preimage to receive anything that implements IntoIterator instead of Vec and not collecting here

[yoavGrs]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @noaov1 and @ShahakShama)

---

src/block_hash/event_hash.rs line 15 at r5 (raw file):

Previously, ShahakShama wrote…

Consider changing hash_array_preimage to receive anything that implements IntoIterator instead of Vec and not collecting here

If `changing hash_array_preimage' receives an iterator, it can't take its length first.