Conversation
Agusx1211
commented
May 19, 2025
Member
- Update onchain config during transactions
- Transaction tests
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR updates onchain configuration logic and enhances transaction tests to validate wallet state transitions. Key changes include updating configuration retrieval and signature logic in transactions, refactoring getConfiguration to accept a wallet address directly, and adding new tests for error conditions and state updates.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/wallet/wdk/vitest.config.ts | Set fixed worker counts for test environment |
| packages/wallet/wdk/test/wallets.test.ts | Added assertions to check wallet existence after state changes |
| packages/wallet/wdk/test/setup.ts | Updated import list from 'ox' |
| packages/wallet/wdk/test/sessions.test.ts | Pass noConfigUpdate flag to avoid unintended config updates |
| packages/wallet/wdk/test/recovery.test.ts | Propagated noConfigUpdate flag in recovery tests |
| packages/wallet/wdk/src/sequence/wallets.ts | Updated getConfiguration signature to accept Address directly and adjusted signer resolution calls |
| packages/wallet/wdk/src/sequence/transactions.ts | Forwarded noConfigUpdate option and added cancellation of signature requests in deletions |
| packages/wallet/wdk/src/sequence/recovery.ts | Updated getConfiguration call to match the new signature |
| packages/wallet/wdk/src/sequence/manager.ts | Exposed onchain configuration methods and updated options interface in transaction requests |
| packages/wallet/primitives/src/constants.ts | Introduced a constant for updating image hash |
| packages/wallet/core/src/wallet.ts | Upgraded getStatus and prepareTransaction methods to support onchain config update logic |
Comments suppressed due to low confidence (1)
packages/wallet/core/src/wallet.ts:252
- [nitpick] The injection of a configuration update call into the transaction envelope is a critical side-effect. Please ensure that this behavior is clearly documented for API consumers so that its implications and usage are well understood.
if (!options?.noConfigUpdate) {
| imageHash, | ||
| pendingUpdates: [...updates].reverse(), | ||
| chainId, | ||
| onChainImageHash: onChainImageHash!, |
There was a problem hiding this comment.
Using a non-null assertion for onChainImageHash may hide potential runtime issues if the value is unexpectedly undefined. Consider adding an explicit check or error handling to ensure it is valid before using it.
Suggested change
| onChainImageHash: onChainImageHash!, | |
| onChainImageHash: (() => { | |
| if (onChainImageHash === undefined) { | |
| throw new Error(`onChainImageHash is undefined for ${this.address}`); | |
| } | |
| return onChainImageHash; | |
| })(), |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.