Implement starknet_core_types::curve module

[pefontana]

Implement starknet_core_types::curve module

• Move the Felt implementation to starknet_core_types
• Implement Implement starknet_core_types::curve module:
  • affine_point
  • projective_point
  • curve_errors

Does this introduce a breaking change?

Yes

[nils-mathieu]

I'm not sure whether moving the Felt type to the core crate is a good idea. If we have starknet-types-rpc depend on it, it will bring a whole range of types that regular Starknet clients just don't care about.

Felt might not be the only type for which this matters though.

The question to be asked is whether the "rpc types" crate should depend on the "core types" crate. It's not clear to me.

Obvious pro is simplicity. Less crates to worry about = less time auditing dependencies and thinking about dependency trees. Obvious con is bringing a whole range of types that people might not need. Just using feature flags a bit aggressively might suffice to solve the issue though.

I don't have a strong opinion on that.

[pefontana]

I'm not sure whether moving the Felt type to the core crate is a good idea. If we have starknet-types-rpc depend on it, it will bring a whole range of types that regular Starknet clients just don't care about.

Felt might not be the only type for which this matters though.

The question to be asked is whether the "rpc types" crate should depend on the "core types" crate. It's not clear to me.

Obvious pro is simplicity. Less crates to worry about = less time auditing dependencies and thinking about dependency trees. Obvious con is bringing a whole range of types that people might not need. Just using feature flags a bit aggressively might suffice to solve the issue though.

I don't have a strong opinion on that.

great @nils-mathieu
As we talked on telegram, I added a feature flag with the curve implementation
15bed72

I think it is okay to leave these implementations inside the starknet-types-core crate since I dont think there are plans to add many more structs inside the crates. In case there is any problem I can move all to a new starknet-arithmetic

[nils-mathieu]

Of note, if we do end up using unsafe code to return references, I'd say we should create a private function on those lines:

#[repr(transparent)]
struct Felt(FieldElement< /* ... */ >);

impl Felt {
    // no-op function to turn a field element reference into a `Felt`
    #[inline(always)]
    fn wrap(inner: &FieldElement< /* ... */ >) -> &Self {
        // SAFETY:
        //  `Felt` is a `#[repr(transparent)]` wrapper around `FieldElement< /* ... */ >`,
        //  which ensures that its memory layout is always the same as its content.
        unsafe { &*(inner as *const _ as *const Self) }
    }
}

to avoid putting the same bit of unsafe code everywhere

[0xLucqs]

Can you add a curve feature flag plz

[tdelabro]

Even if we don't do the unsafe stuff, I think we should still add #[repr(transparent)] to the wrappers types.

[pefontana]

Can you add a curve feature flag plz

Hi @LucasLvy !
The feature flag is already added
https://github.com/starknet-io/types-rs/pull/8/files#diff-5fc4f24b82d50a8341933bd608f0a88b530790f6a4a38d9c8d0583646bf5762fR2

[pefontana]

df43f6c

Sure @tdelabro
Done df43f6c

[nils-mathieu]

Looks good to me.