-
Notifications
You must be signed in to change notification settings - Fork 138
feat: Blake2s implementation #821
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
Changes from all commits
6a1629d
f7b84e7
e7aae92
17b2b7d
dc9e8bb
f30c9e3
0de732a
8746904
ea9c5ad
88e4c2e
4173d4d
63d1693
f99b24a
62c8e11
d681926
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ import ( | |
| "fmt" | ||
| "time" | ||
|
|
||
| "github.com/Masterminds/semver/v3" | ||
| "github.com/NethermindEth/juno/core/felt" | ||
| "github.com/NethermindEth/starknet.go/client/rpcerr" | ||
| "github.com/NethermindEth/starknet.go/contracts" | ||
|
|
@@ -59,8 +60,9 @@ func (account *Account) BuildAndSendInvokeTxn( | |
| callData, | ||
| makeResourceBoundsMapWithZeroValues(), | ||
| &utils.TxnOptions{ | ||
| Tip: tip, | ||
| UseQueryBit: opts.UseQueryBit, | ||
| Tip: tip, | ||
| UseQueryBit: opts.UseQueryBit, | ||
| UseBlake2sHash: false, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This option doesn't make sense in the context of an invoke transaction right? It shouldn't be here
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The linter complains about omitted fields in structs
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because currently, the options used by all three Since this is temporary, the easiest and simplest way I found was to add this new flag in the existing |
||
| }, | ||
| ) | ||
|
|
||
|
|
@@ -138,6 +140,16 @@ func (account *Account) BuildAndSendDeclareTxn( | |
| return response, err | ||
| } | ||
|
|
||
| var useBlake2sHash bool | ||
| if opts.UseBlake2sHash == nil { | ||
| useBlake2sHash, err = shouldUseBlake2sHash(ctx, account.Provider) | ||
| if err != nil { | ||
| return response, fmt.Errorf("failed to check whether to use Blake2s hash: %w", err) | ||
| } | ||
| } else { | ||
| useBlake2sHash = *opts.UseBlake2sHash | ||
| } | ||
|
|
||
| // building and signing the txn, as it needs a signature to estimate the fee | ||
| broadcastDeclareTxnV3, err := utils.BuildDeclareTxn( | ||
| account.Address, | ||
|
|
@@ -146,8 +158,9 @@ func (account *Account) BuildAndSendDeclareTxn( | |
| nonce, | ||
| makeResourceBoundsMapWithZeroValues(), | ||
| &utils.TxnOptions{ | ||
| Tip: tip, | ||
| UseQueryBit: opts.UseQueryBit, | ||
| Tip: tip, | ||
| UseQueryBit: opts.UseQueryBit, | ||
| UseBlake2sHash: useBlake2sHash, | ||
| }, | ||
| ) | ||
| if err != nil { | ||
|
|
@@ -239,8 +252,9 @@ func (account *Account) BuildAndEstimateDeployAccountTxn( | |
| classHash, | ||
| makeResourceBoundsMapWithZeroValues(), | ||
| &utils.TxnOptions{ | ||
| Tip: tip, | ||
| UseQueryBit: opts.UseQueryBit, | ||
| Tip: tip, | ||
| UseQueryBit: opts.UseQueryBit, | ||
| UseBlake2sHash: false, | ||
| }, | ||
| ) | ||
|
|
||
|
|
@@ -465,3 +479,35 @@ func (account *Account) WaitForTransactionReceipt( | |
| } | ||
| } | ||
| } | ||
|
|
||
| // shouldUseBlake2sHash determines whether to use the Blake2s hash function for the | ||
| // compiled class hash. | ||
| // Starknet v0.14.1 upgrade will deprecate the Poseidon hash function for the compiled class hash. | ||
| // | ||
| // Parameters: | ||
| // - ctx: The context | ||
| // - provider: The provider | ||
| // | ||
| // Returns: | ||
| // - bool: whether to use the Blake2s hash function for the compiled class hash | ||
| // - error: an error if any | ||
| func shouldUseBlake2sHash(ctx context.Context, provider rpc.RPCProvider) (bool, error) { | ||
| block, err := provider.BlockWithTxHashes(ctx, rpc.WithBlockTag(rpc.BlockTagLatest)) | ||
rodrigo-pino marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if err != nil { | ||
| return false, fmt.Errorf("failed to get block with tx hashes: %w", err) | ||
| } | ||
|
|
||
| blockTxHashes, ok := block.(*rpc.BlockTxHashes) | ||
| if !ok { | ||
| return false, fmt.Errorf("block is not a BlockTxHashes: %T", block) | ||
| } | ||
|
|
||
| upgradeVersion := semver.MustParse("0.14.1") | ||
|
|
||
| currentVersion, err := semver.NewVersion(blockTxHashes.StarknetVersion) | ||
| if err != nil { | ||
| return false, fmt.Errorf("failed to parse block's starknet version: %w", err) | ||
| } | ||
|
|
||
| return currentVersion.Compare(upgradeVersion) >= 0, nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,6 +45,14 @@ type TxnOptions struct { | |
| UseLatest bool | ||
| // The simulation flag to be used when estimating fees. Default: none. | ||
| SimulationFlag rpc.SimulationFlag | ||
|
|
||
| // ONLY FOR THE `BuildAndSendDeclareTxn` METHOD: A pointer to a boolean flag | ||
| // indicating whether to use the Blake2s hash function to calculate the compiled | ||
| // class hash. | ||
| // 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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the intention is to represent three states, let's use an enum. Which will be hash:
I think it is more descriptive code
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? |
||
| } | ||
|
|
||
| // BlockID returns the block ID for fee estimation based on the UseLatest flag. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why there is a TxnOptions defined here and another defined in the
utilspackage?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because they're different, with different options, used by different functions.
utils.TxnOptionsis used by theutils.Build...functions, whileaccount.TxnOptionsis used by theaccount.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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My initial question remains unanswered. Why there are two transaction options? Why there are two different methods to interact with transaction options?