feat: impl From<EthAddress> for StarkFelt

[tdelabro]

This change is 

[giladchase]

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

---

src/core.rs line 271 at r1 (raw file):

        // Padding H160 with zeros to 32 bytes (big endian)
        bytes[12..32].copy_from_slice(value.0.as_bytes());
        StarkFelt::new_unchecked(bytes)

I think the risk of adding this unsafe constructor is warranted for this usage.
WDYT?

Suggestion:

        StarkFelt::new(bytes).expect("EthAddress should be within StarkFelt bounds")

---

src/hash.rs line 60 at r1 (raw file):

    }

    pub const fn new_unchecked(bytes: [u8; 32]) -> StarkFelt {

See comment above please.

[tdelabro]

The unchecked constructor is used there, but it won't be the only place. We added it to our fork as it is useful in multiple places.
The main thing is that it is const while new cannot be const because the check returns an error that allocates memory:

        Err(StarknetApiError::OutOfRange { string: hex_str_from_bytes::<32, true>(bytes) })

So there is now a way to create StarkFelt in a const environment, which is useful/required.

[giladchase]

Fair enough, so please add a docstring to this method that sufficiently scares off users against abusing it.
In other words, when a StarkFelt is created this way for an out of bounds number, here be dragons...

Reviewable status: 0 of 3 files reviewed, all discussions resolved

[tdelabro]

Fair enough, so please add a docstring to this method that sufficiently scares off users against abusing it.
In other words, when a StarkFelt is created this way for an out of bounds number, here be dragons...

@giladchase Done

[giladchase]

Reviewed 2 of 3 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @tdelabro)

---

src/hash.rs line 68 at r2 (raw file):

    /// The `StarkFelt::new` method make sure that you can't initialize a `StarkFelt` greater than
    /// it's specification maximum. This method does not not. It's your responsability make sure
    /// it's okay to call this method.

For brevity, also to avoid duplicated instructions.

Suggestion:

/// To avoid undefined behavior, refer to [`StarkFelt`] struct's docstring for the required constraints on the `bytes` argument, or use [`StarkFelt::new`] instead of this method.

---

src/hash.rs line 75 at r2 (raw file):

    /// - creating instances of `StarkFelt` at compile time
    /// - implementing `From<T> for StarkFelt` on types that have a smaller binary representation
    ///   than `StarkFelt`

Suggestion:

    /// Most of the time you should use `new` instead, but it comes in handy for a few cases:
    /// - creating instances of `StarkFelt` at compile time
    /// - implementing `From<T> for StarkFelt` for types that have a smaller binary representation
    ///   than `StarkFelt`

[giladchase]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @tdelabro)

[giladchase]

Plz rebase and i'll merge it.

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @tdelabro)

[tdelabro]

Plz rebase and i'll merge it.

Just did :)

[giladchase]

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @tdelabro)