feat: Blake2s implementation #821
Conversation
- Implemented Blake2s and Blake2sArray functions for hashing felt.Felt pointers.
…lake hash - Introduced CompiledClassHashV2 for using the Blake2s hash function to generate Compiled Clash Hashes - Make a compiledClassHash function that accepts a hash function as a parameter, and use it for the CompiledClassHash and CompiledClassHashV2 functions.
…aram - Adds a new hashFunc param to make the function work with any hash function specified. - Adjusted return values in hashCasmClassByteCode to return the computed hash instead of a function.
- Introduced the UseBlake2sHash option in TxnOptions to allow selection of the Blake2s hash function for compiled class hash calculation. - Updated BuildDeclareTxn to conditionally use Blake2s or the default hash function based on the new option. - Added unit tests to validate the behavior of BuildDeclareTxn with both hash options.
…hods - Added the shouldUseBlake2sHash function to determine if the Blake2s hash function should be used for compiled class hashes based on the Starknet version. - Updated BuildAndSendDeclareTxn and other transaction methods to utilize the new hash selection logic. - Introduced unit tests for BuildAndSendDeclareTxn to validate behavior with different Starknet versions and their corresponding compiled class hashes.
… selection - Updated TxnOptions to accept a new UseBlake2sHash field, allowing users to specify whether to use the Blake2s hash function for compiled class hash calculation. - Modified the logic to determine the hash function based on the provided options or default to fetching the Starknet version. - Enhanced unit tests to cover various scenarios for hash selection based on Starknet versions and TxnOptions.
| UseQueryBit: opts.UseQueryBit, | ||
| Tip: tip, | ||
| UseQueryBit: opts.UseQueryBit, | ||
| UseBlake2sHash: false, |
There was a problem hiding this comment.
This option doesn't make sense in the context of an invoke transaction right? It shouldn't be here
There was a problem hiding this comment.
The linter complains about omitted fields in structs
There was a problem hiding this comment.
And it makes sense, the comment is not about initializing the field or not, is about why does this field exist in the context of an invoke transaction.
There was a problem hiding this comment.
Because currently, the options used by all three BuildAndSend... methods are the same.
Since this is temporary, the easiest and simplest way I found was to add this new flag in the existing TxnOptions struct. Even though the "semantic" is not perfect, there's a comprehensive description. This solution brings the benefit of zero breaking changes, and at the same time, it allows high flexibility in its use.
| nonce, | ||
| callData, | ||
| makeResourceBoundsMapWithZeroValues(), | ||
| &utils.TxnOptions{ |
There was a problem hiding this comment.
Why there is a TxnOptions defined here and another defined in the utils package?
There was a problem hiding this comment.
Because they're different, with different options, used by different functions.
utils.TxnOptions is used by the utils.Build... functions, while account.TxnOptions is used by the account.BuildAndSend... methods.
Even though some field names are the same, their use and documentation in the code are different.
There was a problem hiding this comment.
My initial question remains unanswered. Why there are two transaction options? Why there are two different methods to interact with transaction options?
| // If omitted (nil), Starknet.go will automatically fetch the current Starknet | ||
| // version and decide whether to use the Blake2s hash function. | ||
| UseBlake2sHash *bool | ||
| // TODO: remove this field after the Starknet v0.14.1 upgrade |
There was a problem hiding this comment.
Is the intention is to represent three states, let's use an enum. Which will be hash:
- "Auto"
- "Blake2s"
- "Poseidon"
I think it is more descriptive code
There was a problem hiding this comment.
I liked the idea, but since this is temporary (will be removed after the v0.14.1 mainnet upgrade), what about leaving it as is? Less code, and it will be simple to implement in older starknet.go versions as I will need to create another release for rpcv08 users
There was a problem hiding this comment.
I don't agree with this logic. Let's revise this.
Is this code only used for invoking transactions / hence it only mean they work on the tip of the chain?
Right?
- Modified hash function implementations in curve.go to return pointers to the computed hashes. - Updated references in hash.go to utilize the new curve functions for Poseidon and Pedersen hashes, ensuring consistency across the codebase.
TODO:
From Starknet v0.14.1 onward, the hash function to calculate the compiled class hash will be Blake2s instead of Poseidon.
This PR implements the Blake2s hash function in Starknet.go, and a feature to automatically decide whether to use it or not to calculate the compiled class hash.
Key changes:
UseBlake2sHashparam in theTxnOptionstruct to be used by theBuildAndSendDeclareTxnmethod. It's a pointer to a bool value, so it can be: true, false, or nil. True or false is to determine whether or not to use the blake hash, and if nil (the default value if not manually set by the user), Starknet.go will automatically fetch the current Starknet version and decide whether to use the Blake2s hash function.Blake2sandBlake2sArrayfunctions.getByteCodeSegmentHasher,hashCasmClassEntryPointByType(renamed tohashCasmEntryPoints), andhashCasmClassByteCodefunctions to accept a custom hash function as a parameter, making them able to be used with any provided hash function instead of using thePoseidonArrayfunction hardcoded inside them.CompiledClassHashfunction: it was converted to an internalcompiledClassHashfunction, accepting a custom hash function to be used. Now we have two functions to calculate the compiled class hash:CompiledClassHash, which calls thecompiledClassHashfunction with thePoseidonArrayhash functionCompiledClassHashV2, which calls thecompiledClassHashfunction with theBlake2shash functionUseBlake2sHashparam in theTxnOptionstruct to be used by theBuildDeclareTxnfunction.