Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev: improve poseidon API #107

Open
tdelabro opened this issue Dec 17, 2024 · 1 comment · May be fixed by #109
Open

dev: improve poseidon API #107

tdelabro opened this issue Dec 17, 2024 · 1 comment · May be fixed by #109
Labels
enhancement New feature or request

Comments

@tdelabro
Copy link
Collaborator

tdelabro commented Dec 17, 2024

crates/starknet-types-core/src/hash/poseidon.rs

So the poseidon hash is defined this way.

And our API is this way:

impl StarkHash for Poseidon {
    /// Computes the Poseidon hash of two Felts, as defined
    /// in <https://docs.starknet.io/documentation/architecture_and_concepts/Hashing/hash-functions/#poseidon_hash.>
    fn hash(felt_0: &Felt, felt_1: &Felt) -> Felt {
        Felt(PoseidonCairoStark252::hash(&felt_0.0, &felt_1.0))
    }
    /// Computes the Poseidon hash of an array of Felts, as defined
    /// in <https://docs.starknet.io/documentation/architecture_and_concepts/Cryptography/hash-functions/#poseidon_array_hash.>
    fn hash_array(felts: &[Felt]) -> Felt {
        // Non-copy but less dangerous than transmute
        // https://doc.rust-lang.org/std/mem/fn.transmute.html#alternatives
        Felt(PoseidonCairoStark252::hash_many(unsafe {
            core::slice::from_raw_parts(
                felts.as_ptr() as *const FieldElement<Stark252PrimeField>,
                felts.len(),
            )
        }))
    }
}

impl Poseidon {
    /// Computes the Hades permutation over a mutable state of 3 Felts, as defined
    /// in <https://docs.starknet.io/documentation/architecture_and_concepts/Cryptography/hash-functions/#poseidon_array_hash>
    pub fn hades_permutation(state: &mut [Felt; 3]) {
        let mut state_inner = [state[0].0, state[1].0, state[2].0];
        PoseidonCairoStark252::hades_permutation(&mut state_inner);
        for i in 0..3 {
            state[i] = Felt(state_inner[i]);
        }
    }
}

This means that if you want to hash many elements you use hash_array, two elements you use hash. and for 1 single element you have to understand by yourselves that, according to poseidon specification, you must call hades_permutation(my_value, 0, 1).
As you can probably guess people don't know that and just do crazy things instead.

The inner labmdawork API is way more useful for developers: https://github.com/lambdaclass/lambdaworks/blob/7b5a638d4ce81f380ea5f43a22be41ef9b2d7ff2/crypto/src/hash/poseidon/mod.rs#L17

I would strongly recommend the addition of a hash_single method to the trait in order to make it more dev friendly

@tdelabro tdelabro added the enhancement New feature or request label Dec 17, 2024
@varun-doshi
Copy link

I can take this one

@varun-doshi varun-doshi linked a pull request Dec 30, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants