feat: implement avnu for starknet tokens swaps #11552
Conversation
📝 WalkthroughWalkthroughAdds AVNU Starknet swapper support: new AvnuSwapper implementation and API, AVNU SDK dependency, Starknet adapter and fee/nonce changes, token discovery via getKnownTokens, feature flagging/CSP, Coingecko/Starknet mappings, UI icon and fee-call adjustments. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Trade UI
participant AvnuApi as AvnuSwapper / avnuApi
participant AvnuSDK as AVNU SDK
participant StarknetAdapter as Starknet ChainAdapter
participant StarknetRPC as Starknet RPC
User->>UI: Request AVNU quote
UI->>AvnuApi: getTradeQuote(sell,buy,amount)
AvnuApi->>AvnuSDK: fetchQuotes(...)
AvnuSDK-->>AvnuApi: quotes
AvnuApi->>StarknetAdapter: getFeeData({to,value,chainSpecific,sendMax})
StarknetAdapter->>StarknetRPC: estimateFee / get_nonce
StarknetRPC-->>StarknetAdapter: feeEstimate / nonce
StarknetAdapter-->>AvnuApi: feeData
AvnuApi-->>UI: TradeQuote (quoteId, routes, fees)
User->>UI: Confirm
UI->>AvnuApi: getUnsignedStarknetTransaction(quote,slippage)
AvnuApi->>AvnuSDK: quoteToCalls(quote)
AvnuSDK-->>AvnuApi: calldata/calls
AvnuApi->>StarknetRPC: get_nonce(account)
StarknetRPC-->>AvnuApi: nonce
AvnuApi->>StarknetAdapter: getFeeData({from,to,value,tokenContractAddress})
StarknetAdapter->>StarknetRPC: estimateFee(invoke)
StarknetRPC-->>StarknetAdapter: resourceBounds
StarknetAdapter-->>AvnuApi: fee/maxFee
AvnuApi-->>UI: unsigned tx payload (txHash, addressNList, _txDetails)
UI->>StarknetRPC: submit signed tx
StarknetRPC-->>UI: txHash
UI->>AvnuApi: checkTradeStatus(txHash)
AvnuApi->>StarknetRPC: getTransactionReceipt(txHash)
StarknetRPC-->>AvnuApi: receipt {execution_status}
AvnuApi-->>UI: TradeStatus (Confirmed/Failed/Pending)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/swapper/src/swappers/AvnuSwapper/utils/constants.ts (1)
4-10: Consider adding SupportedChainIds type structure.The coding guidelines specify: "Define supported chain IDs for each swapper in utils/constants.ts with both 'sell' and 'buy' properties following the pattern: SupportedChainIds type."
The current implementation uses a simple array, but the standard pattern includes separate sell/buy chain ID arrays:
type SupportedChainIds = { sell: ChainId[] buy: ChainId[] } export const AVNU_SUPPORTED_CHAIN_IDS: SupportedChainIds = { sell: [KnownChainIds.StarknetMainnet], buy: [KnownChainIds.StarknetMainnet], } as constHowever, if AVNU only supports same-chain swaps (Starknet → Starknet), the current simple array may be intentional.
Based on coding guidelines.
.env.development (1)
98-98: Consider alphabetical ordering for feature flags.The static analyzer suggests placing
VITE_FEATURE_AVNU_SWAPbeforeVITE_FEATURE_CETUS_SWAPfor consistency.packages/swapper/src/types.ts (1)
408-411: Consider replacingany[]with a more specific type forroutes.The
routes: any[]violates the coding guideline to avoidanytypes. Other swapper-specific metadata in this file uses explicit types (e.g.,sunioTransactionMetadata.route,chainflipSpecific). If the AVNU SDK provides a type for routes, consider importing and using it here for better type safety and maintainability.packages/swapper/src/utils.ts (1)
435-435: Consider adding type definition for Starknet receipt.The
anytype forreceiptworks but loses type safety. Consider defining a minimal type interface for the receipt properties used (execution_status).🔎 Suggested type definition
+type StarknetTransactionReceipt = { + execution_status?: 'SUCCEEDED' | 'REVERTED' | string +} + export const checkStarknetSwapStatus = async ({ // ... }): Promise<TradeStatus> => { try { const adapter = assertGetStarknetChainAdapter(starknetChainId) const provider = adapter.getStarknetProvider() - const receipt: any = await provider.getTransactionReceipt(txHash) + const receipt = await provider.getTransactionReceipt(txHash) as StarknetTransactionReceiptpackages/swapper/src/swappers/AvnuSwapper/endpoints.ts (1)
38-41: Default slippage inconsistency.The hardcoded fallback of
0.01(1%) differs fromDEFAULT_AVNU_SLIPPAGE_DECIMAL_PERCENTAGE(0.5%) defined in constants. Consider using the centralized constant for consistency.🔎 Suggested fix
+import { getDefaultSlippageDecimalPercentageForSwapper } from '../../../constants' +import { SwapperName } from '../../../types' + // Convert slippage from decimal percentage string to number for AVNU format (e.g., "0.01" = 1%) const slippage: number = slippageTolerancePercentageDecimal ? parseFloat(slippageTolerancePercentageDecimal) - : 0.01 + : parseFloat(getDefaultSlippageDecimalPercentageForSwapper(SwapperName.Avnu))
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/caip/src/adapters/coingecko/index.test.ts (1)
326-336: Consider adding test coverage for Starknet chain mapping.Since this PR adds Starknet token support and includes USDC mappings, consider adding a test case to verify that
chainIdToCoingeckoAssetPlatformcorrectly handles Starknet chain IDs, similar to the existing Ethereum test at line 328.📝 Example test case for Starknet
it('can get CoinGecko asset platform from Starknet ChainId', () => { const chainId = starknetChainId expect(chainIdToCoingeckoAssetPlatform(chainId)).toEqual(CoingeckoAssetPlatform.Starknet) })Note: This assumes
starknetChainIdconstant andCoingeckoAssetPlatform.Starknetare defined in the implementation.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/caip/src/adapters/coingecko/index.test.tspackages/caip/src/adapters/coingecko/utils.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Never assume a library is available - always check imports/package.json first
Prefer composition over inheritance
Write self-documenting code with clear variable and function names
Keep functions small and focused on a single responsibility
Avoid deep nesting - use early returns instead
Prefer procedural and easy to understand code
Never expose, log, or commit secrets, API keys, or credentials
Validate all inputs, especially user inputs
Handle errors gracefully with meaningful messages
Don't silently catch and ignore exceptions
Log errors appropriately for debugging
Provide fallback behavior when possible
Use appropriate data structures for the task
Never add code comments unless explicitly requested
When modifying code, do not add comments that reference previous implementations or explain what changed. Comments should only describe the current logic and functionality.
Use meaningful names for branches, variables, and functions
Always runyarn lint --fixandyarn type-checkafter making changes
Avoidletvariable assignments - preferconstwith inline IIFE switch statements or extract to functions for conditional logic
Files:
packages/caip/src/adapters/coingecko/utils.test.tspackages/caip/src/adapters/coingecko/index.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Avoid useEffect where practical - use it only when necessary and following best practices
Avoid 'any' types - use specific type annotations instead
For default values with user overrides, use computed values (useMemo) instead of useEffect - pattern:userSelected ?? smartDefault ?? fallback
When function parameters are unused due to interface requirements, refactor the interface or implementation to remove them rather than prefixing with underscore
Sanitize data before displaying to prevent XSS
Memoize aggressively - wrap component variables inuseMemoand callbacks inuseCallbackwhere possible
For static JSX icon elements (e.g.,<TbCopy />) that don't depend on state/props, define them as constants outside the component to avoid re-renders instead of using useMemo
Account for light/dark mode usinguseColorModeValuehook
Account for responsive mobile designs in all UI components
When applying styles, use the existing standards and conventions of the codebase
Use Chakra UI components and conventions
All copy/text must use translation keys - never hardcode strings
Use the translation hook:useTranslate()fromreact-polyglot
UseuseFeatureFlag('FlagName')hook to access feature flag values in components
Prefertypeoverinterfacefor type definitions
Use strict typing - avoidany
UseNominaltypes for domain identifiers (e.g.,WalletId,AccountId)
Import types from@shapeshiftoss/caipfor chain/account/asset IDs
UseuseAppSelectorfor Redux state
UseuseAppDispatchfor Redux actions
Memoize expensive computations withuseMemo
Memoize callbacks withuseCallback
**/*.{ts,tsx}: UseResult<T, E>pattern for error handling in swappers and APIs; ALWAYS useOk()andErr()from@sniptt/monads; AVOID throwing within swapper API implementations
ALWAYS use custom error classes from@shapeshiftoss/errorswith meaningful error codes for internationalization and relevant details in error objects
ALWAYS wrap async op...
Files:
packages/caip/src/adapters/coingecko/utils.test.tspackages/caip/src/adapters/coingecko/index.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx,js,jsx}: Write tests for critical business logic
Test edge cases and error conditions
Use descriptive test names that explain behavior
Keep tests isolated and independent
Mock external dependencies appropriately
Files:
packages/caip/src/adapters/coingecko/utils.test.tspackages/caip/src/adapters/coingecko/index.test.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*.{js,jsx,ts,tsx}: Use camelCase for variables, functions, and methods with descriptive names that explain the purpose
Use verb prefixes for functions that perform actions (e.g., fetch, validate, execute, update, calculate)
Use UPPER_SNAKE_CASE for constants and configuration values with descriptive names
Usehandleprefix for event handlers with descriptive names in camelCase
Use descriptive boolean variable names withis,has,can,shouldprefixes
Use named exports for components, functions, and utilities instead of default exports
Use descriptive import names and avoid renaming imports unless necessary
Avoid non-descriptive variable names likedata,item,obj, and single-letter variable names except in loops
Avoid abbreviations in names unless they are widely understood
Avoid generic function names likefn,func, orcallback
Files:
packages/caip/src/adapters/coingecko/utils.test.tspackages/caip/src/adapters/coingecko/index.test.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10231
File: src/components/AssetSearch/components/AssetList.tsx:2-2
Timestamp: 2025-08-08T15:00:49.887Z
Learning: Project shapeshift/web: NeOMakinG prefers avoiding minor a11y/UI nitpicks (e.g., adding aria-hidden to decorative icons in empty states like src/components/AssetSearch/components/AssetList.tsx) within feature PRs; defer such suggestions to a follow-up instead of blocking the PR.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11016
File: packages/swapper/src/swappers/NearIntentsSwapper/swapperApi/getTradeQuote.ts:109-145
Timestamp: 2025-11-12T12:18:00.863Z
Learning: NEAR Intents swapper: The NEAR 1Click API does not provide gas limit estimation logic like other swappers (e.g., magic gasLimit fields). For ERC20 token swaps in getTradeQuote, accurate fee estimation requires token approval and sufficient balance; without these prerequisites, fees may display as 0 or use inaccurate native transfer estimates. This is a known limitation of the NEAR Intents integration.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/pages/RFOX/components/Stake/components/StakeSummary.tsx:112-114
Timestamp: 2025-08-22T13:00:44.879Z
Learning: NeOMakinG prefers to keep PR changes minimal and focused on the core objectives, avoiding cosmetic or defensive code improvements that aren't directly related to the PR scope, even when they would improve robustness.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10128
File: .cursor/rules/error-handling.mdc:266-274
Timestamp: 2025-07-29T10:35:22.059Z
Learning: NeOMakinG prefers less nitpicky suggestions on documentation and best practices files, finding overly detailed suggestions on minor implementation details (like console.error vs logger.error) too granular for cursor rules documentation.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10380
File: src/pages/Dashboard/components/AccountList/AccountTable.tsx:60-0
Timestamp: 2025-09-02T08:34:08.157Z
Learning: NeOMakinG prefers code review comments to focus only on actual PR changes, not pre-existing code issues, unless there are critical security or correctness concerns directly related to the new functionality.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10234
File: src/components/MultiHopTrade/hooks/useGetTradeQuotes/hooks/useTrackTradeQuotes.ts:42-86
Timestamp: 2025-08-08T11:41:22.794Z
Learning: NeOMakinG prefers not to include refactors in move-only PRs; such suggestions should be deferred to follow-up issues instead of being applied within the same PR.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10380
File: src/components/Table/Table.theme.ts:177-180
Timestamp: 2025-09-02T12:38:46.940Z
Learning: NeOMakinG prefers to defer technical debt and CSS correctness issues (like improper hover selectors) to follow-up PRs when the current PR is already large and focused on major feature implementation, even when the issues are valid from a usability/technical perspective.
📚 Learning: 2025-11-24T21:20:04.979Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T21:20:04.979Z
Learning: Applies to **/*.{ts,tsx} : Import types from `shapeshiftoss/caip` for chain/account/asset IDs
Applied to files:
packages/caip/src/adapters/coingecko/index.test.ts
📚 Learning: 2025-11-20T12:00:45.005Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11078
File: src/setupVitest.ts:11-15
Timestamp: 2025-11-20T12:00:45.005Z
Learning: In shapeshift/web, src/setupVitest.ts must redirect 'ethers' to 'ethers5' for shapeshiftoss/hdwallet-trezor (and -trezor-connect), same as ledger and shapeshift-multichain. Removing 'trezor' from the regex causes CI/Vitest failures due to ethers v6 vs v5 API differences.
Applied to files:
packages/caip/src/adapters/coingecko/index.test.ts
📚 Learning: 2025-08-17T21:53:03.806Z
Learnt from: 0xApotheosis
Repo: shapeshift/web PR: 10290
File: scripts/generateAssetData/color-map.json:41-47
Timestamp: 2025-08-17T21:53:03.806Z
Learning: In the ShapeShift web codebase, native assets (using CAIP-19 slip44 namespace like eip155:1/slip44:60, bip122:.../slip44:..., cosmos:.../slip44:...) are manually hardcoded and not generated via the automated asset generation script. Only ERC20/BEP20 tokens go through the asset generation process. The validation scripts should only validate generated assets, not manually added native assets.
Applied to files:
packages/caip/src/adapters/coingecko/index.test.ts
📚 Learning: 2025-12-26T15:45:47.558Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11515
File: scripts/generateAssetData/generateRelatedAssetIndex/generateRelatedAssetIndex.ts:100-122
Timestamp: 2025-12-26T15:45:47.558Z
Learning: In scripts/generateAssetData/generateRelatedAssetIndex/generateRelatedAssetIndex.ts, gomesalexandre is comfortable with the fetchBridgedCategoryMappings function lacking try-catch error handling for CoinGecko API calls. He prefers letting the script crash on API failures rather than adding defensive error handling, consistent with his fail-fast approach for asset generation scripts.
Applied to files:
packages/caip/src/adapters/coingecko/index.test.ts
📚 Learning: 2025-12-03T23:19:39.158Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11275
File: headers/csps/chains/plasma.ts:1-10
Timestamp: 2025-12-03T23:19:39.158Z
Learning: For CSP files in headers/csps/chains/, gomesalexandre prefers using Vite's loadEnv() pattern directly to load environment variables (e.g., VITE_PLASMA_NODE_URL, VITE_MONAD_NODE_URL) for consistency with existing second-class chain CSP files, rather than using getConfig() from src/config.ts, even though other parts of the codebase use validated config values.
Applied to files:
packages/caip/src/adapters/coingecko/index.test.ts
📚 Learning: 2025-11-24T21:20:57.909Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-11-24T21:20:57.909Z
Learning: Applies to packages/swapper/src/swappers/*/utils/constants.ts : Define supported chain IDs for each swapper in utils/constants.ts with both 'sell' and 'buy' properties following the pattern: SupportedChainIds type
Applied to files:
packages/caip/src/adapters/coingecko/index.test.ts
📚 Learning: 2025-12-04T22:57:50.850Z
Learnt from: kaladinlight
Repo: shapeshift/web PR: 11290
File: packages/chain-adapters/src/utxo/zcash/ZcashChainAdapter.ts:48-51
Timestamp: 2025-12-04T22:57:50.850Z
Learning: In packages/chain-adapters/src/**/*ChainAdapter.ts files, the getName() method uses the pattern `const enumIndex = Object.values(ChainAdapterDisplayName).indexOf(ChainAdapterDisplayName.XXX); return Object.keys(ChainAdapterDisplayName)[enumIndex]` to reverse-lookup the enum key from its value. This is the established pattern used consistently across almost all chain adapters (Bitcoin, Ethereum, Litecoin, Dogecoin, Polygon, Arbitrum, Cosmos, etc.) and should be preserved for consistency when adding new chain adapters.
Applied to files:
packages/caip/src/adapters/coingecko/index.test.ts
📚 Learning: 2025-11-24T21:20:57.909Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-11-24T21:20:57.909Z
Learning: Applies to packages/swapper/src/swappers/*/*.ts : Implement filterBuyAssetsBySellAssetId method to filter assets by supported chain IDs in the buy property
Applied to files:
packages/caip/src/adapters/coingecko/index.test.ts
⏰ 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: Call / Static
🔇 Additional comments (2)
packages/caip/src/adapters/coingecko/utils.test.ts (1)
193-195: No issues found. The slip44 value9004is the correct SLIP-0044 registered coin type for StarkNet, andSN_MAINis the standard chain reference for StarkNet mainnet per the CAIP-2 specification. Both values are correctly defined inpackages/caip/src/constants.tsand accurately reflected in the test expectation.packages/caip/src/adapters/coingecko/index.test.ts (1)
3-8: LGTM: Import reorganization improves clarity.The explicit imports from the current module make dependencies clearer and follow TypeScript conventions.
| const usdcOnStarknet = toAssetId({ | ||
| chainNamespace: CHAIN_NAMESPACE.Starknet, | ||
| chainReference: CHAIN_REFERENCE.StarknetMainnet, | ||
| assetNamespace: 'token', | ||
| assetReference: '0x33068f6539f8e6e6b131e6b2b814e6c34a5224bc66947c47dab9dfee93b35fb', | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the specific address and related USDC references on Starknet
# Search for the exact address
rg -n "0x33068f6539f8e6e6b131e6b2b814e6c34a5224bc66947c47dab9dfee93b35fb" --type ts --type json
# Search for USDC references on Starknet
rg -n -i "starknet.*usdc|usdc.*starknet" --type ts --type json -C 2Repository: shapeshift/web
Length of output: 50371
🌐 Web query:
Starknet USDC contract address mainnet official
💡 Result:
Native USDC (Circle) on Starknet mainnet contract:
0x033068f6539f8e6e6b131e6b2b814e6c34a5224bc66947c47dab9dfee93b35fb. [1][2]
Circle and Starknet announced native USDC (launch/migration published Dec 3, 2025). [1][2]
Sources:
- Circle — USDC on Starknet / USDC contract addresses. [1]
- avnu (Starknet ecosystem) migration notes listing the mainnet address. [2]
Fix the Starknet USDC contract address to include the leading zero.
The asset reference should be 0x033068f6539f8e6e6b131e6b2b814e6c34a5224bc66947c47dab9dfee93b35fb (with leading 0x0), not 0x33068f6539f8e6e6b131e6b2b814e6c34a5224bc66947c47dab9dfee93b35fb. Starknet addresses use the full 251-bit felt representation with 63 hex characters following 0x, and the leading zero is significant for proper address resolution on the network.
🤖 Prompt for AI Agents
In packages/caip/src/adapters/coingecko/index.test.ts around lines 197 to 202,
the Starknet USDC assetReference is missing a leading zero; update the
assetReference string to include the leading 0x0 so it becomes
0x033068f6539f8e6e6b131e6b2b814e6c34a5224bc66947c47dab9dfee93b35fb, ensuring the
full 63-hex-character felt representation is used for correct Starknet address
resolution.
| export type TokenInfo = { | ||
| assetId: AssetId | ||
| contractAddress: string | ||
| symbol: string | ||
| name: string | ||
| precision: number | ||
| } | ||
|
|
| // Main provider - NO batching for single RPC calls (getChainId, getTransactionReceipt, etc.) | ||
| // IMPORTANT: Do NOT enable automatic batch mode (batch: 0) as it breaks single RPC calls | ||
| // by making starknet.js expect batched array responses | ||
| this.provider = new RpcProvider({ nodeUrl: args.rpcUrl }) | ||
|
|
||
| // Batched provider - ONLY for parallel balance queries in getAccount() | ||
| // batch: 0 enables automatic batching for concurrent callContract calls | ||
| this.batchedProvider = new RpcProvider({ nodeUrl: args.rpcUrl, batch: 0 }) | ||
|
|
||
| // Default to empty token list if not provided - avoids race condition with asset service initialization | ||
| this.getKnownTokens = args.getKnownTokens ?? (() => []) | ||
| // Rate limit batch requests to avoid overwhelming the RPC endpoint | ||
| this.requestQueue = new PQueue({ | ||
| intervalCap: 1, | ||
| interval: 100, // 1 batch request every 100ms | ||
| concurrency: 1, | ||
| }) |
There was a problem hiding this comment.
Conceptually LGTM, happy to get this in for the sake of progression. A few functional issues to be fixed either in this PR or as direct follow-up:
- Balance update doesn't seem to be consistently happy (see first STRK -> USDC failure, happy on refetch)
- Tx links are sad e.g this guy
- Was able to get into estimation failures on subsequent Txs i.e
Fee estimation failed: {"code":41,"message":"Transaction execution error","data":{"execution_error":"Invalid transaction nonce of contract at address 0x02c7ea72c15d2f118256618c31d390bd6180f9c8716ec290383f59c3d1a38d6f. Account nonce: 0x0000000000000000000000000000000000000000000000000000000000000006; got: 0x0000000000000000000000000000000000000000000000000000000000000005.","transaction_index":0}}- looks like we should ensure fresh nonce is always fetched vs. cached ? (see 5 vs. 6)
| const nonceResponse = await adapter | ||
| .getStarknetProvider() | ||
| .fetch('starknet_getNonce', ['pending', from]) | ||
| const nonceResult: { result?: string; error?: unknown } = await nonceResponse.json() | ||
| const nonce = nonceResult.result || '0x0' |
There was a problem hiding this comment.
preferably-blocking here and in other similar occurrences:
we have adapter.isAccountDeployed() precisely for that use-case, 0x0 should only ever be a valid nonce if not deployed, so wherever we deal with nonces, we should instead make a call to that, and only fallback to 0x0 if not deployed.
Perhaps worth making a reusable adapter getNonce() method like:
getNonce() {
if (!(isAccountDeployed(account)) return '0x0'
const nonceResponse = await adapter
.getStarknetProvider()
.fetch('starknet_getNonce', ['pending', from])
const nonceResult: { result?: string; error?: unknown } = await nonceResponse.json()
if (!nonceResult.result) throw new Error('Failed to fetch nonce')
return nonceResult.result
}WDYT?
| return { | ||
| addressNList: toAddressNList(adapter.getBip44Params({ accountNumber })), | ||
| txHash, | ||
| _txDetails: { | ||
| fromAddress: from, | ||
| calldata: formattedCalldata, | ||
| nonce, | ||
| version, | ||
| resourceBounds, | ||
| chainId: chainIdHex, | ||
| }, | ||
| } |
There was a problem hiding this comment.
Paraphrasing robots:
2. _txDetails missing fields for tx hash computation
File: packages/swapper/src/swappers/AvnuSwapper/endpoints.ts#L192-L203
Problem: Missing nonceDataAvailabilityMode, feeDataAvailabilityMode, tip, paymasterData, accountDeploymentData. Divergence from invokeHashInputs will produce invalid signatures on Starknet v3.
Proposed fix: Make _txDetails a faithful mirror of invokeHashInputs:
const nonceDataAvailabilityMode = 0 as const // L1
const feeDataAvailabilityMode = 0 as const // L1
const tip = '0x0'
const paymasterData: string[] = []
const accountDeploymentData: string[] = []
return {
addressNList: toAddressNList(adapter.getBip44Params({ accountNumber })),
txHash,
_txDetails: {
type: 'INVOKE' as const,
senderAddress: from,
version,
calldata: formattedCalldata,
nonce,
resourceBounds,
chainId: chainIdHex,
nonceDataAvailabilityMode,
feeDataAvailabilityMode,
tip,
paymasterData,
accountDeploymentData,
},
}| ? BigInt(feeEstimate.l1_data_gas_price) | ||
| : BigInt('0x1') | ||
|
|
||
| const resourceBounds = { |
There was a problem hiding this comment.
Paraphrasing Mr Robot:
5. Resource bounds use BigInt directly
Problem: Should convert to hex strings for RPC consistency.
Proposed fix:
const toHex = (x: bigint) => '0x' + x.toString(16)
const resourceBounds = {
l1_gas: {
max_amount: toHex((l1GasConsumed * 500n) / 100n),
max_price_per_unit: toHex((l1GasPrice * 200n) / 100n),
},
// ... same for l2_gas and l1_data_gas
}| slippageTolerancePercentageDecimal, | ||
| assertGetStarknetChainAdapter, | ||
| }) => { | ||
| if (!isExecutableTradeQuote(tradeQuote)) throw new Error('Unable to execute a trade rate quote') |
There was a problem hiding this comment.
worth using SwapErrorRight here? and adding this to skill
Description
1000 birds one stone:
Issue (if applicable)
Nothing to close
Risk
Low, under a flag
Testing
Engineering
Operations
Screenshots (if applicable)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.