multichain/exchange api work, tx system refactor and token2022 improvements#24
Draft
multichain/exchange api work, tx system refactor and token2022 improvements#24
Conversation
…rt, tx refactor, improve error handling, better init
Contributor
WalkthroughThis change introduces multi-network Solana chain registration and substantially refactors wallet and token service logic. The chain registration now supports mainnet, devnet, testnet, and localnet. The SolanaService gains swap orchestration, advanced token metadata parsing (including Token-2022 TLV support), transaction construction utilities, and expanded wallet data retrieval. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SolanaWalletService
participant SolanaService
participant Exchange/Jupiter
participant Blockchain
User->>SolanaWalletService: getPortfolio()
activate SolanaWalletService
SolanaWalletService->>SolanaService: lazy-load (if not initialized)
activate SolanaService
SolanaWalletService->>SolanaService: getPortfolio()
SolanaService->>Blockchain: fetch token accounts
SolanaService->>Blockchain: fetch balances
SolanaService-->>SolanaWalletService: portfolio data
deactivate SolanaService
SolanaWalletService-->>User: portfolio (cached or fresh)
deactivate SolanaWalletService
User->>SolanaWalletService: initiate swap
activate SolanaWalletService
SolanaWalletService->>SolanaService: selectExchange()
SolanaService->>Exchange/Jupiter: register/select
SolanaWalletService->>SolanaService: doSwapOnExchange(exchHndl, walletSet)
activate SolanaService
SolanaService->>SolanaService: getSwapAmounts(txDetails)
SolanaService->>SolanaService: buildAndSendTransaction(instructions, signers)
SolanaService->>Blockchain: sign & send transaction
Blockchain-->>SolanaService: transaction confirmation
SolanaService->>SolanaService: parse fees & amounts from receipt
SolanaService-->>SolanaWalletService: swap result
deactivate SolanaService
SolanaWalletService-->>User: swap confirmation
deactivate SolanaWalletService
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| * @returns A promise that resolves to the wallet's portfolio. | ||
| */ | ||
| public async getPortfolio(owner?: string): Promise<siWalletPortfolio> { | ||
| if (!this.solanaService) throw new Error('Solana Service is required for Solana Wallet Service: getPortfolio') |
There was a problem hiding this comment.
Bug: Bug
The SolanaWalletService has a race condition: its constructor asynchronously loads this.solanaService, but public methods like getPortfolio and getBalance perform synchronous checks. This causes methods to fail if called before the service is fully initialized, as the previous lazy-loading pattern was replaced without updating these checks.
| const quoteResponse = await exService.getQuote({ | ||
| inputMint: src.assetRef, | ||
| outputMint: trg.assetRef, | ||
| amount: w.inAmount, |
There was a problem hiding this comment.
Bug: Bug
In doSwapOnExchange, w.inAmount is rounded for validation, but the original unrounded value is used in the getQuote API call. This inconsistency means the API may receive an unvalidated fractional amount, potentially leading to precision issues or unexpected swap behavior.
| //const decimals = t.account.data.parsed.info.tokenAmount.decimals; | ||
| //const balance = Number(amountRaw) / (10 ** decimals); | ||
| //const ca = new PublicKey(t.account.data.parsed.info.mint); | ||
| if (t === null) continue |
There was a problem hiding this comment.
Bug: Bug
The getBalance method includes an unnecessary null check (if (t === null) continue) when iterating heldTokens. Since heldTokens is typed as KeyedParsedTokenAccount[], its elements are guaranteed to be non-null, making this check always false.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
src/index.ts(1 hunks)src/service.ts(46 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
Comment on lines
+2347
to
2364
| const walletPubkeys = walletAddresses.map(a => new PublicKey(a)); | ||
| const atAs = walletPubkeys.map(w => getAssociatedTokenAddressSync(mint, w)); | ||
| const balances: Record<string, number> = {}; | ||
|
|
||
| // Convert to base58 strings for secure storage | ||
| const publicKey = newKeypair.publicKey.toBase58(); | ||
| const privateKey = bs58.encode(newKeypair.secretKey); | ||
| // fetch mint decimals once | ||
| const decimals = await this.getDecimal(mint); | ||
|
|
||
| // Clear the keypair from memory | ||
| newKeypair.secretKey.fill(0); | ||
| // fetch ATAs in batches | ||
| const infos = await this.batchGetMultipleAccountsInfo(atAs, 'getTokenBalanceForWallets'); | ||
|
|
||
| return { | ||
| publicKey, | ||
| privateKey, | ||
| }; | ||
| } catch (error) { | ||
| logger.error(`Error creating wallet: ${error}`); | ||
| throw new Error('Failed to create new wallet'); | ||
| } | ||
| return { | ||
| publicKey, | ||
| privateKey, | ||
| }; | ||
| } catch (error) { | ||
| logger.error(`Error creating wallet: ${error}`); | ||
| throw new Error('Failed to create new wallet'); | ||
| } |
Contributor
There was a problem hiding this comment.
Fix getTokenBalanceForWallets return logic
The new getTokenBalanceForWallets body now returns { publicKey, privateKey }, but those identifiers are undefined here and the method never emits the balance map it promises. This is a compile-time/runtime blocker. We need to iterate the fetched accounts, decode their balances, and return the populated balances object.
Apply this diff to restore the intended behaviour:
// fetch ATAs in batches
const infos = await this.batchGetMultipleAccountsInfo(atAs, 'getTokenBalanceForWallets');
-
- return {
- publicKey,
- privateKey,
- };
- } catch (error) {
- logger.error(`Error creating wallet: ${error}`);
- throw new Error('Failed to create new wallet');
- }
- }
+ for (let i = 0; i < infos.length; i += 1) {
+ const info = infos[i];
+ const owner = walletAddresses[i];
+ if (!owner) {
+ continue;
+ }
+
+ if (!info) {
+ balances[owner] = 0;
+ continue;
+ }
+
+ try {
+ const programId = info.owner.equals(TOKEN_2022_PROGRAM_ID) ? TOKEN_2022_PROGRAM_ID : TOKEN_PROGRAM_ID;
+ const account = unpackAccount(atAs[i], info, programId);
+ const rawAmount = Number(account.amount);
+ balances[owner] = rawAmount / 10 ** decimals;
+ } catch (err) {
+ this.runtime.logger.warn({ mint: mint.toBase58(), owner }, 'getTokenBalanceForWallets: failed to decode account');
+ balances[owner] = 0;
+ }
+ }
+
+ return balances;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const walletPubkeys = walletAddresses.map(a => new PublicKey(a)); | |
| const atAs = walletPubkeys.map(w => getAssociatedTokenAddressSync(mint, w)); | |
| const balances: Record<string, number> = {}; | |
| // Convert to base58 strings for secure storage | |
| const publicKey = newKeypair.publicKey.toBase58(); | |
| const privateKey = bs58.encode(newKeypair.secretKey); | |
| // fetch mint decimals once | |
| const decimals = await this.getDecimal(mint); | |
| // Clear the keypair from memory | |
| newKeypair.secretKey.fill(0); | |
| // fetch ATAs in batches | |
| const infos = await this.batchGetMultipleAccountsInfo(atAs, 'getTokenBalanceForWallets'); | |
| return { | |
| publicKey, | |
| privateKey, | |
| }; | |
| } catch (error) { | |
| logger.error(`Error creating wallet: ${error}`); | |
| throw new Error('Failed to create new wallet'); | |
| } | |
| return { | |
| publicKey, | |
| privateKey, | |
| }; | |
| } catch (error) { | |
| logger.error(`Error creating wallet: ${error}`); | |
| throw new Error('Failed to create new wallet'); | |
| } | |
| const walletPubkeys = walletAddresses.map(a => new PublicKey(a)); | |
| const atAs = walletPubkeys.map(w => getAssociatedTokenAddressSync(mint, w)); | |
| const balances: Record<string, number> = {}; | |
| // fetch mint decimals once | |
| const decimals = await this.getDecimal(mint); | |
| // fetch ATAs in batches | |
| const infos = await this.batchGetMultipleAccountsInfo(atAs, 'getTokenBalanceForWallets'); | |
| for (let i = 0; i < infos.length; i += 1) { | |
| const info = infos[i]; | |
| const owner = walletAddresses[i]; | |
| if (!owner) { | |
| continue; | |
| } | |
| if (!info) { | |
| balances[owner] = 0; | |
| continue; | |
| } | |
| try { | |
| const programId = info.owner.equals(TOKEN_2022_PROGRAM_ID) ? TOKEN_2022_PROGRAM_ID : TOKEN_PROGRAM_ID; | |
| const account = unpackAccount(atAs[i], info, programId); | |
| const rawAmount = Number(account.amount); | |
| balances[owner] = rawAmount / 10 ** decimals; | |
| } catch (err) { | |
| this.runtime.logger.warn({ mint: mint.toBase58(), owner }, 'getTokenBalanceForWallets: failed to decode account'); | |
| balances[owner] = 0; | |
| } | |
| } | |
| return balances; | |
| } |
🤖 Prompt for AI Agents
In src/service.ts around lines 2347 to 2364, the method currently returns
undefined { publicKey, privateKey } instead of the promised balances map;
replace that stray return with logic that iterates the fetched account infos
(infos) in the same order as atAs/walletAddresses, for each info decode its
token balance using the fetched decimals (use tokenAmount.uiAmount or compute
amount / 10**decimals) or default to 0 for missing/null accounts, populate
balances[walletAddress] = decodedBalance, and finally return the populated
balances object; ensure any try/catch around this block does not return
unrelated values and throws a clear error on failure.
Comment on lines
+3119
to
+3154
| public async transferSplToken(from: Keypair, to: PublicKey, mint: PublicKey, amount: number): Promise<string> { | ||
| const serviceKeypair = await this.getWalletKeypair(); | ||
|
|
||
| // Get token decimals | ||
| const decimals = await this.getDecimal(mint); | ||
| const adjustedAmount = BigInt(amount * 10 ** decimals); | ||
|
|
||
| // Get associated token addresses | ||
| const senderATA = getAssociatedTokenAddressSync(mint, from.publicKey); | ||
| const recipientATA = getAssociatedTokenAddressSync(mint, to); | ||
|
|
||
| const instructions: TransactionInstruction[] = []; | ||
|
|
||
| // Check if recipient ATA exists, create if not | ||
| const recipientATAInfo = await this.connection.getAccountInfo(recipientATA); | ||
| if (!recipientATAInfo) { | ||
| instructions.push( | ||
| createAssociatedTokenAccountInstruction( | ||
| from.publicKey, | ||
| recipientATA, | ||
| to, | ||
| mint | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| // Add transfer instruction | ||
| instructions.push( | ||
| createTransferInstruction( | ||
| senderATA, | ||
| recipientATA, | ||
| from.publicKey, | ||
| adjustedAmount | ||
| ) | ||
| ); | ||
|
|
Contributor
There was a problem hiding this comment.
Token-2022 transfers fail without program-aware ATAs
When transferring SPL tokens you always derive ATAs and build instructions under the legacy TOKEN_PROGRAM_ID. For Token-2022 mints the associated accounts are seeded with TOKEN_2022_PROGRAM_ID, so the addresses and instructions produced here are wrong and the transaction will be rejected. Pull the mint’s owner, choose the correct program id, and pass it through to ATA derivation and instruction builders.
Apply this diff so Token-2022 mints work correctly:
- // Get associated token addresses
- const senderATA = getAssociatedTokenAddressSync(mint, from.publicKey);
- const recipientATA = getAssociatedTokenAddressSync(mint, to);
+ const mintInfo = await this.connection.getAccountInfo(mint);
+ if (!mintInfo) {
+ throw new Error(`Mint ${mint.toBase58()} not found`);
+ }
+ const tokenProgramId = mintInfo.owner.equals(TOKEN_2022_PROGRAM_ID) ? TOKEN_2022_PROGRAM_ID : TOKEN_PROGRAM_ID;
+
+ // Get associated token addresses
+ const senderATA = getAssociatedTokenAddressSync(mint, from.publicKey, false, tokenProgramId);
+ const recipientATA = getAssociatedTokenAddressSync(mint, to, false, tokenProgramId);
@@
- createAssociatedTokenAccountInstruction(
- from.publicKey,
- recipientATA,
- to,
- mint
- )
+ createAssociatedTokenAccountInstruction(
+ from.publicKey,
+ recipientATA,
+ to,
+ mint,
+ tokenProgramId
+ )
);
@@
- createTransferInstruction(
- senderATA,
- recipientATA,
- from.publicKey,
- adjustedAmount
- )
+ createTransferInstruction(
+ senderATA,
+ recipientATA,
+ from.publicKey,
+ adjustedAmount,
+ [],
+ tokenProgramId
+ )
Member
Author
|
ok service.ts got messed up in this PR. Looking at resolving |
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
1 participant
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.
Note
Adds exchange-driven swap execution, robust Token‑2022 metadata parsing, and a refactored transaction/portfolio flow; also registers Solana nets with INTEL_CHAIN.
doSwapOnExchange, plusgetSwapAmounts; basicselectExchange.buildAndSendTransactionand unify send viasendTx; addtransferSplTokenandexecuteCustomTransaction; updatetransferSolto new flow.parseTLVExtensions,parseToken2022MetadataExtension,parseToken2022SymbolFromMintOrPtr); enhancegetTokensSymbolsandparseTokenAccounts(supply/decimals caching, isMutable, program detection).ensurePublicKey; addgetCachedData; streamlineupdateWalletDatafallback; improvegetBalancesByAddrshandling and addgetPubkeyFromSecret.transferSol; make balance lookups tolerate nulls;getTokenAccountsByKeypairsreturns nullable arrays.mainnet,devnet,testnet,localnet) withINTEL_CHAINincluding RPC URLs.Written by Cursor Bugbot for commit c438b70. This will update automatically on new commits. Configure here.
Summary by CodeRabbit