Feat/rf 1153 shapeshift snap#5
Open
RyukTheCoder wants to merge 801 commits intonextfrom
Open
Conversation
- [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] Affected packages: [email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected]
- [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] Affected packages: [email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected]
- [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] Affected packages: [email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected]
- [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] Affected packages: [email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected]
- [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] Affected packages: [email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected]
- [email protected] - [email protected] Affected packages: [email protected],[email protected]
- [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] Affected packages: [email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected]
- [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] Affected packages: [email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected]
- [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] Affected packages: [email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected]
- [email protected] Affected packages: [email protected]
- [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] Affected packages: [email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected]
- [email protected] Affected packages: [email protected]
|
Important Review skippedToo many files! This PR contains 299 files, which is 149 over the limit of 150. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (299)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Review Summary by QodoImplement ShapeShift Snap wallet provider and refactor transaction management system
WalkthroughsDescription• Added ShapeShift Snap wallet provider support with eager connection capability • Refactored transaction signing and event notification system in queue-manager with improved error handling and in-memory transaction tracking • Reorganized imports across multiple modules to separate type imports using type keyword for better code organization • Updated wallet type and network references from WalletType/Network to WalletTypes/Networks enums for consistency • Enhanced blockchain metadata with new configurations (STARKNET, ZKSYNC, KCC, STRIDE, MARS, TERRA 2.0, TELOS, BOBA variants, MAYA) and reorganized existing entries • Refactored mock data for blockchains and tokens to support multi-blockchain configurations and reduce redundancy • Removed deprecated wallet state management and WalletConnect provider compatibility functions from core helpers • Added comprehensive mock blockchain metadata with 56 blockchain configurations for testing • Created WalletModal component type definitions for improved type safety • Changed syncInterval type from NodeJS.Timer to number for browser compatibility in queue-manager • Improved code formatting and type safety across examples and helper modules Diagramflowchart LR
A["ShapeShift Snap<br/>Provider"] -->|"eager connect"| B["Wallet<br/>Integration"]
C["Transaction<br/>Signing"] -->|"refactored"| D["Event<br/>Notification"]
E["Mock Data<br/>Blockchain/Token"] -->|"enhanced"| F["Multi-blockchain<br/>Support"]
G["Wallet Types<br/>Enums"] -->|"updated"| H["Type Safety<br/>Improvements"]
B --> I["Queue Manager<br/>Core"]
D --> I
F --> I
H --> I
File Changes1. widget/embedded/src/hooks/usePrepareBlockchainList/usePrepareBlockchainList.mock.ts
|
Code Review by Qodo
1. EVM cleanup removes wrong handlers
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+59
to
+62
| const cleanup = () => { | ||
| instance?.off('chainChanged', handleAccountsChanged); | ||
| instance?.off('accountsChanged', handleChainChanged); | ||
| }; |
There was a problem hiding this comment.
1. Evm cleanup removes wrong handlers 🐞 Bug ☼ Reliability
subscribeToEvm() registers accountsChanged/chainChanged handlers but the returned cleanup function calls off() with swapped handler functions, so listeners are never removed and can be duplicated on re-subscribe.
Agent Prompt
### Issue description
`subscribeToEvm` registers `accountsChanged` and `chainChanged` listeners, but its `cleanup` uses mismatched event/handler pairs, so listeners are not removed (and can duplicate on remount).
### Issue Context
This cleanup is used by multiple EVM wallet providers (they call `cleanup()` on unmount). Incorrect unsubscription leads to duplicated wallet state updates and memory leaks.
### Fix Focus Areas
- wallets/shared/src/providers.ts[32-65]
### Implementation notes
- Change cleanup to:
- `instance?.off('accountsChanged', handleAccountsChanged)`
- `instance?.off('chainChanged', handleChainChanged)`
- Consider supporting providers that implement `removeListener` instead of `off` (optional chaining + fallback).
- Add/adjust a small unit test to ensure cleanup removes the exact handlers that were registered.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+24
to
+86
| const temp1 = await fetch( | ||
| `https://api.litecoin.shapeshift.com/api/v1/account/${address}/utxos`, | ||
| { | ||
| method: 'GET', | ||
| } | ||
| ); | ||
| const temp1Json = await temp1.json(); | ||
|
|
||
| const temp2 = await fetch( | ||
| `https://api.litecoin.shapeshift.com/api/v1/tx/${temp1Json?.[0]?.txid}`, | ||
| { | ||
| method: 'GET', | ||
| } | ||
| ); | ||
| const temp2Json = await temp2.json(); | ||
|
|
||
| const signresult = await walletInvokeSnap({ | ||
| instance: this.provider, | ||
| params: { | ||
| snapId: DEFAULT_SNAP_ID, | ||
| request: { | ||
| method: 'ltc_signTransaction', | ||
| params: { | ||
| transaction: { | ||
| coin: 'Litecoin', | ||
| inputs: [ | ||
| { | ||
| addressNList: temp1Json?.[0]?.path, | ||
| scriptType: 'p2pkh', | ||
| amount: temp1Json?.[0]?.value, | ||
| vout: temp1Json?.[0]?.vout, | ||
| txid: temp1Json?.[0]?.txid, | ||
| hex: temp2Json?.hex, | ||
| }, | ||
| ], | ||
| outputs: [ | ||
| { | ||
| addressType: 'spend', | ||
| amount: tx.amount, | ||
| address: tx.recipientAddress, | ||
| }, | ||
| { | ||
| addressType: 'change', | ||
| amount: ( | ||
| temp1Json?.[0]?.value - parseInt(tx.amount) | ||
| ).toString(), | ||
| // eslint-disable-next-line @typescript-eslint/no-magic-numbers | ||
| addressNList: [2147483692, 2147483650, 2147483648, 1, 0], | ||
| scriptType: 'p2pkh', | ||
| isChange: true, | ||
| }, | ||
| ], | ||
| opReturnData: tx.memo, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| console.log(signresult); | ||
|
|
||
| return { hash: '' }; | ||
| } catch (err) { |
There was a problem hiding this comment.
2. Transfer signer litecoin-only 🐞 Bug ≡ Correctness
ShapeShiftSnap registers a TxType.TRANSFER signer whose implementation hardcodes Litecoin endpoints and the 'ltc_signTransaction' snap method, and it returns an empty hash without broadcasting, so transfers will fail or report success without a txid for non-LTC chains.
Agent Prompt
### Issue description
`ShapeShiftSnapBaseSigner` is registered for `TxType.TRANSFER` but is Litecoin-specific (`ltc_signTransaction`, Litecoin API URLs) and returns an empty hash without broadcasting. This breaks transfers for the wallet's supported non-LTC chains and prevents callers from receiving a real transaction hash.
### Issue Context
The ShapeShift Snap provider advertises support for multiple chains, but `TxType.TRANSFER` currently only implements an LTC signing flow and doesn't publish the transaction.
### Fix Focus Areas
- wallets/provider-shapeshift-snap/src/signer.ts[8-19]
- wallets/provider-shapeshift-snap/src/signer/shapeShiftSnapBaseSigner.ts[19-96]
- wallets/provider-shapeshift-snap/src/helpers.ts[13-22]
### Implementation notes
- Either:
1) Implement transfer signing+broadcast per supported UTXO chain (BTC/BCH/LTC/THOR/etc) and route based on the transaction/network, **or**
2) Restrict registration so `TxType.TRANSFER` is only enabled for chains that are actually implemented, and throw a clear `OPERATION_UNSUPPORTED` error otherwise.
- Ensure `signAndSendTx` broadcasts the signed tx and returns a real txid/hash.
- Remove `console.log` statements (they may leak tx details and clutter logs).
- Add basic guards for missing/empty UTXO set instead of indexing `[0]` blindly.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+42
to
+70
| const broadcastCosmosTransaction = async (instance: any, signedTx: any) => { | ||
| const result = await fetch('https://api.cosmos.shapeshift.com/api/v1/send', { | ||
| method: 'POST', | ||
| body: JSON.stringify({ | ||
| rawTx: signedTx.serialized, | ||
| }), | ||
| }); | ||
|
|
||
| const resultJson = await result.json(); | ||
| return resultJson; | ||
| }; | ||
|
|
||
| class ShapeShiftSnapCosmosSigner implements GenericSigner<CosmosTransaction> { | ||
| private provider: any; | ||
|
|
||
| constructor(provider: any) { | ||
| this.provider = provider; | ||
| } | ||
|
|
||
| public async signMessage(): Promise<string> { | ||
| throw SignerError.UnimplementedError('signMessage'); | ||
| } | ||
|
|
||
| async signAndSendTx(tx: CosmosTransaction): Promise<{ hash: string }> { | ||
| try { | ||
| const signedTx = await signCosmosTransaction(this.provider, tx); | ||
| const result = await broadcastCosmosTransaction(this.provider, signedTx); | ||
|
|
||
| return { hash: result }; |
There was a problem hiding this comment.
3. Cosmos hash type mismatch 🐞 Bug ≡ Correctness
ShapeShiftSnapCosmosSigner returns { hash: result } where result is the parsed JSON response
body from the broadcast endpoint, not a string tx hash, so downstream code expecting a string hash
will break.
Agent Prompt
### Issue description
`ShapeShiftSnapCosmosSigner.signAndSendTx` returns a JSON object as `hash` (the parsed broadcast response), but the function contract expects `hash` to be a string transaction hash.
### Issue Context
The broadcast helper returns `result.json()` and the signer forwards that directly as `{ hash: result }`.
### Fix Focus Areas
- wallets/provider-shapeshift-snap/src/signer/shapeShiftSnapCosmosSigner.ts[42-79]
### Implementation notes
- Parse the broadcast response and extract the actual tx hash field (e.g., `txhash`, `hash`, etc. depending on the API response).
- Return `{ hash: extractedHashString }`.
- Add `Content-Type: application/json` header for the POST.
- Handle non-2xx responses (`if (!result.ok) ...`) and throw a `SignerError(SEND_TX_ERROR, ...)` with useful context.
- Remove `console.log({ err })` or gate it behind a debug flag.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
7 participants
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.
Summary
Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
How did you test this change?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist: