-
Notifications
You must be signed in to change notification settings - Fork 804
Add force sign hash wallet option #4337
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
base: master
Are you sure you want to change the base?
Conversation
This commit introduces a wallet option to enable force sign hash functionality across P-chain and X-chain transaction signing. Changes: - Add ForceSignHash option to common wallet options - Update P-chain and X-chain signers to support options parameter - Implement forceSignHash field in visitor structs - Create WithOptions wrappers for both P-chain and X-chain signers - Fix wallet WithOptions implementations to return signers with options applied - Add force sign hash logic to use SignHash() vs Sign() based on option The force sign hash option can now be used by passing common.WithForceSignHash() to any wallet transaction method.
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.
Pull Request Overview
This PR introduces a ForceSignHash
wallet option that enables hash-based transaction signing instead of full transaction signing for both P-chain and X-chain wallets. This feature provides testing flexibility and network compatibility for scenarios where CubeSigner API lacks network options beyond mainnet/fuji.
- Adds a new
ForceSignHash
option to the common options structure - Updates both P-chain and X-chain signers to support the new signing mode
- Implements conditional signing logic that chooses between hash-based and full transaction signing
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
File | Description |
---|---|
wallet/subnet/primary/common/options.go | Adds forceSignHash field and WithForceSignHash option |
wallet/chain/x/wallet_with_options.go | Updates Signer method to propagate options |
wallet/chain/x/wallet.go | Passes options to SignUnsigned function |
wallet/chain/x/signer/with_options.go | Creates new wrapper for signers with default options |
wallet/chain/x/signer/visitor.go | Implements conditional signing logic for X-chain |
wallet/chain/x/signer/signer.go | Updates Sign interface to accept options |
wallet/chain/p/wallet/with_options.go | Updates Signer method to propagate options |
wallet/chain/p/signer/with_options.go | Creates new wrapper for signers with default options |
wallet/chain/p/signer/visitor.go | Updates all transaction types to use forceSignHash |
wallet/chain/p/signer/signer.go | Updates Sign interface to accept options |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Why this should be merged
This PR introduces a ForceSignHash wallet option that enables hash-based transaction signing instead of full transaction signing. This feature is essential for:
The feature is implemented as an optional wallet parameter, ensuring existing functionality remains unchanged while providing the flexibility needed for specializeduse cases.
How this works
The implementation adds a new common.WithForceSignHash() option that can be passed to any wallet transaction method:
Technical Implementation:
Signing Logic:
How this was tested
Need to be documented in RELEASES.md?