Migrate to types-rs Felt #231

maciejka · 2024-04-12T11:01:50Z

This change is

src/core.rs

src/crypto.rs

src/hash.rs

src/transaction_hash.rs

src/core.rs

src/transaction_hash.rs

src/crypto.rs

tdelabro · 2024-05-13T11:37:37Z

@noaov1 can you give it a look too?

Yoni-Starkware

Reviewable status: 0 of 25 files reviewed, 12 unresolved discussions (waiting on @LucasLvy, @maciejka, and @tdelabro)

src/external_transaction_test.rs line 33 at r8 (raw file):

        signature: TransactionSignature(vec![StarkFelt::ONE, StarkFelt::TWO]),
        nonce: Nonce(stark_felt!("0x1")),
        compiled_class_hash: CompiledClassHash(stark_felt!("0x2")),

Can you define and use a similar macro?

Code quote:

stark_felt!

tdelabro

Reviewable status: 0 of 25 files reviewed, 12 unresolved discussions (waiting on @LucasLvy, @maciejka, and @Yoni-Starkware)

src/external_transaction_test.rs line 33 at r8 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Can you define and use a similar macro?

For values like 0, 1, 2, 3, that are defined as const, isn't using StarkFelt::ONE, StarkFelt::TWO, and StarkFelt::THREE better?

Yoni-Starkware

Reviewable status: 0 of 25 files reviewed, 12 unresolved discussions (waiting on @LucasLvy, @maciejka, and @tdelabro)

src/external_transaction_test.rs line 33 at r8 (raw file):

Previously, tdelabro (Timothée Delabrouille) wrote…

For values like 0, 1, 2, 3, that are defined as const, isn't using StarkFelt::ONE, StarkFelt::TWO, and StarkFelt::THREE better?

Right, I meant for general hex values. We use it a lot in the blockifier

most changes done cleanup hash.rs fmt clippy fmt remove Lazy where possible fixes changes after the rebase fix clippy replace small felts with constants stark_felt macro fmt

noaov1

Reviewed 3 of 15 files at r1, 1 of 17 files at r8, 4 of 22 files at r9, all commit messages.
Reviewable status: 8 of 32 files reviewed, 15 unresolved discussions (waiting on @LucasLvy, @maciejka, @tdelabro, and @Yoni-Starkware)

src/core.rs line 117 at r9 (raw file):

pub const CONTRACT_ADDRESS_PREFIX: &str = "STARKNET_CONTRACT_ADDRESS";
/// The size of the contract address domain.
pub const CONTRACT_ADDRESS_DOMAIN_SIZE: Felt = Felt::from_hex_unchecked(PATRICIA_KEY_UPPER_BOUND);

Why is it called unchecked? Is it because it can panic?

Code quote:

from_hex_unchecked

src/core.rs line 122 at r9 (raw file):

    NonZeroFelt::from_felt_unchecked(
        CONTRACT_ADDRESS_DOMAIN_SIZE - Felt::from(MAX_STORAGE_ITEM_SIZE),
    )

Consider using the try_from implementation, and panic in case of an error.

Code quote:

pub static L2_ADDRESS_UPPER_BOUND: Lazy<NonZeroFelt> = Lazy::new(|| {
    NonZeroFelt::from_felt_unchecked(
        CONTRACT_ADDRESS_DOMAIN_SIZE - Felt::from(MAX_STORAGE_ITEM_SIZE),
    )

src/core.rs line 388 at r9 (raw file):

        const COMPLIMENT_OF_H160: usize = std::mem::size_of::<Felt>() - H160::len_bytes();

        let bytes = felt.to_bytes_be();

Out of curiousity- why use be and not le?

Code quote:

        let bytes = felt.to_bytes_be();

dorimedini-starkware

Reviewed 2 of 15 files at r1, 4 of 17 files at r8, 18 of 22 files at r9, all commit messages.
Reviewable status: all files reviewed, 21 unresolved discussions (waiting on @LucasLvy, @maciejka, @tdelabro, and @Yoni-Starkware)

src/core.rs line 364 at r9 (raw file):

        ClassHash(StarkHash::from_hex($s).unwrap())
    };
}

these macros are used with unsigned integer inputs as well

Code quote:

#[macro_export]
macro_rules! patricia_key {
    ($s:expr) => {
        PatriciaKey::try_from(StarkHash::from_hex($s).unwrap()).unwrap()
    };
}

/// A utility macro to create a [`ClassHash`] from a hex string / unsigned integer representation.
#[cfg(any(feature = "testing", test))]
#[macro_export]
macro_rules! class_hash {
    ($s:expr) => {
        ClassHash(StarkHash::from_hex($s).unwrap())
    };
}

src/external_transaction_test.rs line 33 at r8 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Right, I meant for general hex values. We use it a lot in the blockifier

not just hex... see my comments on the macros

src/hash.rs line 29 at r9 (raw file):

        Felt::from_hex_unchecked($s)
    };
}

the docstring on this macro was wrong before: this macro is used in tests to convert values to starkfelt in hex or unsigned integer format (or any type T that implements TryFrom for StarkFelt).
This will break many usages as it will only work for string types.
consider adding impl From<T> for StarkFelt blocks for unsigned int / string T types, gated behind #[cfg(any(feature = "testing", test))]

Code quote:

/// A utility macro to create a [`starknet_types_core::felt::Felt`] from a hex string
/// representation.
#[cfg(any(feature = "testing", test))]
#[macro_export]
macro_rules! stark_felt {
    ($s:expr) => {
        Felt::from_hex_unchecked($s)
    };
}

src/hash.rs line 196 at r9 (raw file):

        res.write_all(&self.0[first_index + 1..])?;
        Ok(())
    }

how does this implementation differ from the Felt's implementation of the Serialize trait?
I see the first four bit are used here for felt size, something which is used by papyrus.
@ShahakShama can this logic be moved to papyrus?

Code quote:

    pub fn serialize(&self, res: &mut impl std::io::Write) -> Result<(), Error> {
        // We use the fact that bytes[0] < 0x10 and encode the size of the felt in the 4 most
        // significant bits of the serialization, which we call `chooser`. We assume that 128 bit
        // felts are prevalent (because of how uint256 is encoded in felts).

        // The first i for which nibbles 2i+1, 2i+2 are nonzero. Note that the first nibble is
        // always 0.
        let mut first_index = 31;
        for i in 0..32 {
            let value = self.0[i];
            if value == 0 {
                continue;
            } else if value < 16 {
                // Can encode the chooser and the value on a single byte.
                first_index = i;
            } else {
                // The chooser is encoded with the first nibble of the value.
                first_index = i - 1;
            }
            break;
        }
        let chooser = if first_index < 15 {
            // For 34 up to 63 nibble felts: chooser == 15, serialize using 32 bytes.
            first_index = 0;
            CHOOSER_FULL
        } else if first_index < 18 {
            // For 28 up to 33 nibble felts: chooser == 14, serialize using 17 bytes.
            first_index = 15;
            CHOOSER_HALF
        } else {
            // For up to 27 nibble felts: serialize the lower 1 + (chooser * 2) nibbles of the felt
            // using chooser + 1 bytes.
            (31 - first_index) as u8
        };
        res.write_all(&[(chooser << 4) | self.0[first_index]])?;
        res.write_all(&self.0[first_index + 1..])?;
        Ok(())
    }

src/hash.rs line 218 at r9 (raw file):

        bytes.read_exact(&mut res[first_index + 1..]).ok()?;
        Some(Self(res))
    }

ditto on the serialize comment above @ShahakShama

Code quote:

    pub fn deserialize(bytes: &mut impl std::io::Read) -> Option<Self> {
        let mut res = [0u8; 32];

        bytes.read_exact(&mut res[..1]).ok()?;
        let first = res[0];
        let chooser: u8 = first >> 4;
        let first = first & 0x0f;

        let first_index = if chooser == CHOOSER_FULL {
            0
        } else if chooser == CHOOSER_HALF {
            15
        } else {
            (31 - chooser) as usize
        };
        res[0] = 0;
        res[first_index] = first;
        bytes.read_exact(&mut res[first_index + 1..]).ok()?;
        Some(Self(res))
    }

src/transaction_hash.rs line 156 at r9 (raw file):

// TODO: should be part of core::Felt
pub fn ascii_as_felt(ascii_str: &str) -> Result<Felt, StarknetApiError> {

why was the (crate) dropped?

Code quote:

pub

src/crypto/utils.rs line 20 at r9 (raw file):

#[derive(thiserror::Error, Clone, Debug)]
#[allow(clippy::explicit_auto_deref)]

why are these now needed? curious, non-blocking

Code quote:

#[allow(clippy::explicit_auto_deref)]

ShahakShama

Reviewed all commit messages.
Reviewable status: all files reviewed, 25 unresolved discussions (waiting on @dan-starkware, @dorimedini-starkware, @DvirYo-starkware, @LucasLvy, @maciejka, @noaov1, @tdelabro, @yair-starkware, and @Yoni-Starkware)

.gitignore line 6 at r9 (raw file):

/Cargo.lock
*.DS_Store
.idea/*

We usually don't put "specific develop environment" stuff here. You can configure your own local gitignore by editing the file ~/.gitignore_global

src/core.rs line 12 at r9 (raw file):

use primitive_types::H160;
use serde::{Deserialize, Deserializer, Serialize, Serializer};
use starknet_types_core::felt::{Felt, NonZeroFelt};

Maybe we want to add a type alias type StarkFelt = Felt in order to not break api?

src/core.rs line 364 at r9 (raw file):

Previously, dorimedini-starkware wrote…

these macros are used with unsigned integer inputs as well

Where? I think they shouldn't (I don't think we do that in papyrus)

src/core.rs line 388 at r9 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Out of curiousity- why use be and not le?

I don't think there was any specific reason

src/hash.rs line 196 at r9 (raw file):

Previously, dorimedini-starkware wrote…

how does this implementation differ from the Felt's implementation of the Serialize trait?
I see the first four bit are used here for felt size, something which is used by papyrus.
@ShahakShama can this logic be moved to papyrus?

We can define our own StarkHash in the rpc crate if needed
@dan-starkware @yair-starkware @DvirYo-starkware WDYT

src/hash.rs line 218 at r9 (raw file):

Previously, dorimedini-starkware wrote…

ditto on the serialize comment above @ShahakShama

Same, but for the client crate

src/block_hash/transaction_commitment.rs line 2 at r9 (raw file):

use starknet_types_core::felt::Felt;
use starknet_types_core::hash::StarkHash;

This is very confusing because there's a StarkHash in sn api. Could you rename this to CoreStarkHash?

src/crypto/patricia_hash.rs line 29 at r9 (raw file):

use bitvec::prelude::{BitArray, Msb0};
use starknet_types_core::felt::Felt;
use starknet_types_core::hash::StarkHash;

Same, rename this.

dorimedini-starkware

Reviewable status: all files reviewed, 23 unresolved discussions (waiting on @LucasLvy, @maciejka, @noaov1, @ShahakShama, @tdelabro, and @Yoni-Starkware)

src/core.rs line 12 at r9 (raw file):

Previously, ShahakShama wrote…

Maybe we want to add a type alias type StarkFelt = Felt in order to not break api?

I prefer not to do that - I would rather the compiler complain everywhere I used StarkFelt, it is now a completely different type.

src/core.rs line 364 at r9 (raw file):

Previously, ShahakShama wrote…

Where? I think they shouldn't (I don't think we do that in papyrus)

search for stark_felt!, patricia_key! and class_hash! in the blockifier, you will see usages with strings, unsigned integers and even StarkFelts.
this is a useful test util, I see no reason why we shouldn't use it to reduce boilerplate.

src/hash.rs line 196 at r9 (raw file):

Previously, ShahakShama wrote…

We can define our own StarkHash in the rpc crate if needed
@dan-starkware @yair-starkware @DvirYo-starkware WDYT

I'll unblock this now; @ShahakShama please make sure this code doesn't get lost and is ported to papyrus?

tdelabro

Reviewable status: all files reviewed, 23 unresolved discussions (waiting on @LucasLvy, @maciejka, @noaov1, @ShahakShama, and @Yoni-Starkware)

src/block_hash/transaction_commitment.rs line 2 at r9 (raw file):

Previously, ShahakShama wrote…

This is very confusing because there's a StarkHash in sn api. Could you rename this to CoreStarkHash?

starknet_types_core being the new reference for Felt types. Wouldn't it be better to start aliasing the sn-api type rather than this one.
I have been told sn-api is to be deprecated in some time anyway

maciejka

Reviewable status: all files reviewed, 22 unresolved discussions (waiting on @dorimedini-starkware, @LucasLvy, @noaov1, @ShahakShama, @tdelabro, and @Yoni-Starkware)

.gitignore line 6 at r9 (raw file):

Previously, ShahakShama wrote…

We usually don't put "specific develop environment" stuff here. You can configure your own local gitignore by editing the file ~/.gitignore_global

Done.

src/core.rs line 117 at r9 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Why is it called unchecked? Is it because it can panic?

Yes

src/core.rs line 388 at r9 (raw file):

Previously, ShahakShama wrote…

I don't think there was any specific reason

H160::from_slice expects be representation

src/hash.rs line 29 at r9 (raw file):

Previously, dorimedini-starkware wrote…

the docstring on this macro was wrong before: this macro is used in tests to convert values to starkfelt in hex or unsigned integer format (or any type T that implements TryFrom for StarkFelt).
This will break many usages as it will only work for string types.
consider adding impl From<T> for StarkFelt blocks for unsigned int / string T types, gated behind #[cfg(any(feature = "testing", test))]

Ok. Do we want to keep the name? stark_felt does not make sense since the type is now called just Felt.

src/transaction_hash.rs line 35 at r1 (raw file):

Previously, LucasLvy (Lucas @ StarkWare) wrote…

doesn't need to be in lazy static, use from_hex_unchecked

Done.

src/transaction_hash.rs line 34 at r3 (raw file):

Previously, LucasLvy (Lucas @ StarkWare) wrote…

const

Done.

src/transaction_hash.rs line 156 at r9 (raw file):

Previously, dorimedini-starkware wrote…

why was the (crate) dropped?

My mistake. Fixed.

src/block_hash/transaction_commitment.rs line 2 at r9 (raw file):

Previously, tdelabro (Timothée Delabrouille) wrote…

starknet_types_core being the new reference for Felt types. Wouldn't it be better to start aliasing the sn-api type rather than this one.
I have been told sn-api is to be deprecated in some time anyway

You mean use starknet_types_core::hash::StarkHash as CoreStarkHash; or a hard rename in the types-rs?

In general starknet_types_core::hash::StarkHash is a bad name for something that is not a hash value itself but a something that calculates a hash. It should be a Hasher or HashCalculator but it is a subject for another pr.

dorimedini-starkware

Reviewed 6 of 6 files at r10, all commit messages.
Reviewable status: all files reviewed, 20 unresolved discussions (waiting on @LucasLvy, @maciejka, @noaov1, @ShahakShama, @tdelabro, and @Yoni-Starkware)

src/crypto/utils.rs line 20 at r9 (raw file):

Previously, dorimedini-starkware wrote…

why are these now needed? curious, non-blocking

still curious but not as much lol
and please remove the newline between the #[derive] and the enum

dorimedini-starkware

Reviewable status: all files reviewed, 20 unresolved discussions (waiting on @LucasLvy, @maciejka, @noaov1, @ShahakShama, @tdelabro, and @Yoni-Starkware)

src/hash.rs line 29 at r9 (raw file):

Previously, maciejka (Maciej Kamiński @ StarkWare) wrote…

Ok. Do we want to keep the name? stark_felt does not make sense since the type is now called just Felt.

good point. maybe make_felt? or just felt?

ShahakShama

Reviewable status: all files reviewed, 21 unresolved discussions (waiting on @LucasLvy, @maciejka, @noaov1, @tdelabro, and @Yoni-Starkware)

src/hash.rs line 104 at r10 (raw file):

/// The StarkNet [field element](https://docs.starknet.io/documentation/architecture_and_concepts/Hashing/hash-functions/#domain_and_range).
#[derive(Copy, Clone, Eq, PartialEq, Default, Hash, Deserialize, Serialize, PartialOrd, Ord)]
#[serde(try_from = "PrefixedBytesAsHex<32_usize>", into = "PrefixedBytesAsHex<32_usize>")]

We need to add this to starknet-types-core

ShahakShama

Reviewable status: all files reviewed, 18 unresolved discussions (waiting on @LucasLvy, @maciejka, @noaov1, @tdelabro, and @Yoni-Starkware)

src/hash.rs line 196 at r9 (raw file):

Previously, dorimedini-starkware wrote…

I'll unblock this now; @ShahakShama please make sure this code doesn't get lost and is ported to papyrus?

We'll move it to papyrus

src/block_hash/transaction_commitment.rs line 2 at r9 (raw file):

You mean use starknet_types_core::hash::StarkHash as CoreStarkHash
yes

I

maciejka

Reviewable status: 14 of 32 files reviewed, 17 unresolved discussions (waiting on @dorimedini-starkware, @LucasLvy, @noaov1, @ShahakShama, @tdelabro, and @Yoni-Starkware)

src/core.rs line 72 at r3 (raw file):

Previously, LucasLvy (Lucas @ StarkWare) wrote…

change that to a const ?

Done.

src/hash.rs line 29 at r9 (raw file):

Previously, dorimedini-starkware wrote…

good point. maybe make_felt? or just felt?

I prefer just felt.
I made it more generic. The cost of implementing it at the starknet-api level and not types-rs are two extra imports:

use crate::hash::{FeltConverter, TryIntoFelt};

src/hash.rs line 104 at r10 (raw file):

Previously, ShahakShama wrote…

We need to add this to starknet-types-core

What is the use case? Why serde implementation that is already in the types-rs is not enough?

src/block_hash/transaction_commitment.rs line 2 at r9 (raw file):

Previously, ShahakShama wrote…

You mean use starknet_types_core::hash::StarkHash as CoreStarkHash
yes

I

Done.

src/crypto/patricia_hash.rs line 29 at r9 (raw file):

Previously, ShahakShama wrote…

Same, rename this.

Done.

src/crypto/utils.rs line 20 at r9 (raw file):

Previously, dorimedini-starkware wrote…

still curious but not as much lol
and please remove the newline between the #[derive] and the enum

Frankly not sure, why it was needed.

dorimedini-starkware

Reviewed 18 of 18 files at r11, 1 of 1 files at r12, all commit messages.
Reviewable status: all files reviewed, 14 unresolved discussions (waiting on @LucasLvy, @maciejka, @ShahakShama, @tdelabro, and @Yoni-Starkware)

maciejka

Reviewable status: all files reviewed, 14 unresolved discussions (waiting on @LucasLvy, @ShahakShama, @tdelabro, and @Yoni-Starkware)

src/core.rs line 358 at r3 (raw file):

Previously, LucasLvy (Lucas @ StarkWare) wrote…

you should be able to use from_byte_be_slice and remove that copy over

Done.

src/external_transaction_test.rs line 33 at r8 (raw file):

Previously, dorimedini-starkware wrote…

not just hex... see my comments on the macros

Done

dorimedini-starkware

Reviewed 9 of 9 files at r13, all commit messages.
Reviewable status: all files reviewed, 14 unresolved discussions (waiting on @LucasLvy, @maciejka, @ShahakShama, @tdelabro, and @Yoni-Starkware)

maciejka

Reviewable status: all files reviewed, 14 unresolved discussions (waiting on @LucasLvy, @ShahakShama, @tdelabro, and @Yoni-Starkware)

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

Previously, maciejka (Maciej Kamiński @ StarkWare) wrote…

True

Done

src/hash.rs line 104 at r10 (raw file):

Previously, maciejka (Maciej Kamiński @ StarkWare) wrote…

What is the use case? Why serde implementation that is already in the types-rs is not enough?

You mean you want felt! macro to be moved into types-rs?

src/crypto.rs line 19 at r5 (raw file):

Previously, maciejka (Maciej Kamiński @ StarkWare) wrote…

Fixed all CryptoError messages.

Done

src/crypto.rs line 40 at r7 (raw file):

Previously, tdelabro (Timothée Delabrouille) wrote…

I would rather use fmt::LowerHex instead. I know it's internally fmt::Display. But it's more logic

Done.

ShahakShama

Reviewed all commit messages.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @LucasLvy, @tdelabro, and @Yoni-Starkware)

dorimedini-starkware

Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @LucasLvy, @tdelabro, and @Yoni-Starkware)

0xLucqs

Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @tdelabro and @Yoni-Starkware)

dorimedini-starkware

Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @tdelabro and @Yoni-Starkware)

tdelabro

Reviewed 4 of 15 files at r1, 1 of 17 files at r8, 4 of 22 files at r9, 1 of 6 files at r10, 13 of 18 files at r11, 1 of 1 files at r12, 9 of 9 files at r13, all commit messages.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @Yoni-Starkware)

dorimedini-starkware

Reviewed 6 of 6 files at r14, all commit messages.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @LucasLvy, @tdelabro, and @Yoni-Starkware)

0xLucqs requested changes Apr 12, 2024

View reviewed changes

src/core.rs Outdated Show resolved Hide resolved

src/core.rs Outdated Show resolved Hide resolved

src/crypto.rs Outdated Show resolved Hide resolved

src/hash.rs Show resolved Hide resolved

src/transaction_hash.rs Outdated Show resolved Hide resolved

0xLucqs requested changes Apr 15, 2024

View reviewed changes

src/core.rs Outdated Show resolved Hide resolved

src/core.rs Outdated Show resolved Hide resolved

src/transaction_hash.rs Outdated Show resolved Hide resolved

maciejka requested a review from 0xLucqs April 15, 2024 20:01

tdelabro approved these changes Apr 16, 2024

View reviewed changes

src/transaction_hash.rs Show resolved Hide resolved

maciejka force-pushed the mk/migrate-to-types-rs branch 4 times, most recently from d7d849c to 766bbc4 Compare April 22, 2024 16:31

0xLucqs requested changes Apr 23, 2024

View reviewed changes

src/crypto.rs Outdated Show resolved Hide resolved

maciejka requested a review from 0xLucqs April 23, 2024 17:10

tdelabro suggested changes Apr 26, 2024

View reviewed changes

src/crypto.rs Outdated Show resolved Hide resolved

tdelabro approved these changes Apr 29, 2024

View reviewed changes

maciejka force-pushed the mk/migrate-to-types-rs branch from 66b6cf0 to b5fac83 Compare May 8, 2024 14:24

Yoni-Starkware reviewed May 15, 2024

View reviewed changes

tdelabro reviewed May 15, 2024

View reviewed changes

Yoni-Starkware reviewed May 15, 2024

View reviewed changes

wip

f5bedfd

most changes done cleanup hash.rs fmt clippy fmt remove Lazy where possible fixes changes after the rebase fix clippy replace small felts with constants stark_felt macro fmt

maciejka force-pushed the mk/migrate-to-types-rs branch from 9d442f5 to f5bedfd Compare May 20, 2024 14:41

fmt

2e410b9

maciejka requested review from tdelabro and Yoni-Starkware May 20, 2024 14:52

noaov1 reviewed May 21, 2024

View reviewed changes

dorimedini-starkware requested changes May 21, 2024

View reviewed changes

ShahakShama requested changes May 22, 2024

View reviewed changes

dorimedini-starkware requested changes May 22, 2024

View reviewed changes

tdelabro reviewed May 22, 2024

View reviewed changes

maciejka commented May 22, 2024

View reviewed changes

small fixes

144daac

dorimedini-starkware reviewed May 22, 2024

View reviewed changes

dorimedini-starkware requested changes May 22, 2024

View reviewed changes

ShahakShama requested changes May 22, 2024

View reviewed changes

maciejka added 2 commits May 22, 2024 15:15

make felt macro more generic

e8febe2

fmt

f250559

maciejka commented May 22, 2024

View reviewed changes

make TryIntoFelt public

0afd2ea

dorimedini-starkware previously approved these changes May 22, 2024

View reviewed changes

maciejka commented May 27, 2024

View reviewed changes

merge with main

29bd64e

maciejka dismissed dorimedini-starkware’s stale review via 29bd64e May 27, 2024 10:52

fmt

245b2cf

dorimedini-starkware approved these changes May 27, 2024

View reviewed changes

maciejka commented May 27, 2024

View reviewed changes

ShahakShama previously approved these changes May 27, 2024

View reviewed changes

0xLucqs approved these changes May 27, 2024

View reviewed changes

dorimedini-starkware approved these changes May 27, 2024

View reviewed changes

0xLucqs previously approved these changes May 27, 2024

View reviewed changes

dorimedini-starkware previously approved these changes May 27, 2024

View reviewed changes

tdelabro reviewed May 28, 2024

View reviewed changes

tdelabro mentioned this pull request May 30, 2024

chore: use type-rs felt #263

Closed

merge with main

a0e35f2

maciejka dismissed stale reviews from dorimedini-starkware, 0xLucqs, and ShahakShama via a0e35f2 June 3, 2024 08:21

dorimedini-starkware approved these changes Jun 3, 2024

View reviewed changes

dorimedini-starkware merged commit c9d67c0 into starkware-libs:main Jun 3, 2024
5 of 6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to types-rs Felt #231

Migrate to types-rs Felt #231

maciejka commented Apr 12, 2024 •

edited by alon-dotan-starkware

Loading

tdelabro commented May 13, 2024

Yoni-Starkware left a comment

tdelabro left a comment

Yoni-Starkware left a comment

noaov1 left a comment

dorimedini-starkware left a comment

ShahakShama left a comment

dorimedini-starkware left a comment

tdelabro left a comment

maciejka left a comment

dorimedini-starkware left a comment

dorimedini-starkware left a comment

ShahakShama left a comment

ShahakShama left a comment

maciejka left a comment

dorimedini-starkware left a comment

maciejka left a comment

dorimedini-starkware left a comment

maciejka left a comment

ShahakShama left a comment

dorimedini-starkware left a comment

0xLucqs left a comment

dorimedini-starkware left a comment

tdelabro left a comment

dorimedini-starkware left a comment

Migrate to types-rs Felt #231

Migrate to types-rs Felt #231

Conversation

maciejka commented Apr 12, 2024 • edited by alon-dotan-starkware Loading

tdelabro commented May 13, 2024

Yoni-Starkware left a comment

Choose a reason for hiding this comment

tdelabro left a comment

Choose a reason for hiding this comment

Yoni-Starkware left a comment

Choose a reason for hiding this comment

noaov1 left a comment

Choose a reason for hiding this comment

dorimedini-starkware left a comment

Choose a reason for hiding this comment

ShahakShama left a comment

Choose a reason for hiding this comment

dorimedini-starkware left a comment

Choose a reason for hiding this comment

tdelabro left a comment

Choose a reason for hiding this comment

maciejka left a comment

Choose a reason for hiding this comment

dorimedini-starkware left a comment

Choose a reason for hiding this comment

dorimedini-starkware left a comment

Choose a reason for hiding this comment

ShahakShama left a comment

Choose a reason for hiding this comment

ShahakShama left a comment

Choose a reason for hiding this comment

maciejka left a comment

Choose a reason for hiding this comment

dorimedini-starkware left a comment

Choose a reason for hiding this comment

maciejka left a comment

Choose a reason for hiding this comment

dorimedini-starkware left a comment

Choose a reason for hiding this comment

maciejka left a comment

Choose a reason for hiding this comment

ShahakShama left a comment

Choose a reason for hiding this comment

dorimedini-starkware left a comment

Choose a reason for hiding this comment

0xLucqs left a comment

Choose a reason for hiding this comment

dorimedini-starkware left a comment

Choose a reason for hiding this comment

tdelabro left a comment

Choose a reason for hiding this comment

dorimedini-starkware left a comment

Choose a reason for hiding this comment

maciejka commented Apr 12, 2024 •

edited by alon-dotan-starkware

Loading