feat(swapper): integrate Garden Finance for BTC and wrapped BTC swaps#12355
Open
swdiscordia wants to merge 24 commits into
Open
feat(swapper): integrate Garden Finance for BTC and wrapped BTC swaps#12355swdiscordia wants to merge 24 commits into
swdiscordia wants to merge 24 commits into
Conversation
Integrates Garden Finance as a new swapper to unlock the BTC ↔ strkBTC
route on Starknet, which strkBTC's federation explicitly restricts to
this single pair (all other strkBTC routes are blacklisted in Garden's
/policy as of strkBTC launch — May 2026).
Architecture:
- Deposit-to-address pattern (mirrors NearIntentsSwapper / Chainflip)
- For BTC source: UTXO send to Garden HTLC P2TR script address
- For strkBTC source: Starknet INVOKE multi-call combining approve +
initiate, pattern mirrors AvnuSwapper.getUnsignedStarknetTransaction
- Status polling via GET /v2/orders/{id} with completion signal
destination_swap.redeem_tx_hash populated (per Garden docs)
Verified live via the Garden v2 API during integration spike:
- Quote, order creation, and status flows for both directions
- Affiliate fee mechanism (bps to base:cbbtc → DAO_TREASURY_BASE)
- Affiliate fee asset matrix (3 of 6 OpenAPI-listed pairs work today)
Verification:
- pnpm run type-check: clean
- pnpm exec vitest run helpers.test.ts: 20 tests pass
- pnpm exec eslint: clean across all touched files
- garden-icon.png (128x128) sourced from garden.finance/apple-icon.png
INTEGRATION.md covers the complete API spec, the Merry Docker localnet
setup for end-to-end testing, response shapes, the affiliate fee asset
matrix, and the JDK requirement for the build chain.
Follow-ups before production:
- Stand up weekly affiliate fee claim workflow (DAO ops, off-code)
- Manual mainnet test ~$20 in each direction
- Promote VITE_FEATURE_GARDEN_SWAP=true to .env.production only after
governance sign-off given Garden's Oct 2025 solver-layer exploit
history (documented in INTEGRATION.md).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Strip code comments from Garden swapper files (global-rules.md forbids non-explicit code comments; INTEGRATION.md remains the canonical docs surface). - Default VITE_FEATURE_GARDEN_SWAP=false in .env so prod builds ship with Garden gated; flip to true in .env.development for local work. - Empty VITE_GARDEN_API_KEY default in .env so the integration key is not committed; engineers must source their own key locally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Patches the upstream-regenerated generatedAssetData.json with the strkBTC entry, recomputes the manifest hash (01c69736 -> b9f4280e), and rebuilds the .br/.gz siblings. Also strips remaining comments from scripts/generateAssetData/starknet/index.ts to match the no-code-comments rule. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Filter Solana/Tron as source chains in isSupportedGardenPair. Until
execute methods are wired they cannot initiate; previously the registry
accepted them and tradeExecution.ts crashed with "missing implementation
for executeSolanaTransaction".
- Drop invented `deadline` field from GardenOrder. Not documented by
Garden; expiry is handled via source_swap.refund_tx_hash already.
- Drop dead gardenSpecific.estimatedTimeMs and evmInitiate.gasLimit.
Replaced by the step-level estimatedExecutionTimeMs; gas is re-estimated
at execute time.
- Drop dead gardenSupportedChainIds export — never imported anywhere.
- Switch isGardenBitcoinInitiate to a positive shape check (presence of
`amount`) instead of inferring by absence of `initiate_transaction`.
- Surface Garden's affiliate cut as protocolFees on the buy asset
following the Avnu pattern (requiresBalance: false, deducted from
output). Previously protocolFees was hardcoded empty so the UI showed
no fee at all.
Shapes verified against Garden's published docs:
/v2/quote → docs.garden.finance/api-reference/quickstart
/v2/orders → POST shape per source chain
GET /v2/orders/{id} → source_swap/destination_swap field list
affiliate_fees → docs.garden.finance/developers/affiliate-fees
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move the multi-call INVOKE building flow (toHexString, RPC fee estimate, resourceBounds calculation, invoke-hash construction) into a shared packages/swapper/src/starknet-utils/ module. AvnuSwapper and GardenSwapper now both call buildStarknetInvokeTx with their pre-formatted calldata, keeping each swapper responsible only for assembling its own Call array. No behaviour change for Avnu — the extracted code is byte-equivalent to its previous inline implementation. Garden's endpoints.ts shrinks from 333 to ~140 lines and the duplication that was flagged in code review is eliminated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
strkBTC was incorrectly pointed at btcAssetId, which caused getAffiliateBps to treat BTC↔strkBTC as a related-asset swap (returning 0 bps) and skip the standard ShapeShift fee. Every other BTC-pegged asset in the registry (WBTC, cbBTC, …) uses a self-reference or sibling cross-chain pointer per the convention; this aligns strkBTC with that pattern. Bumped clearAssetsMigrations to invalidate stale localStorage so the corrected relatedAssetKey is picked up on next load. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Revert public/generated/* and src/state/migrations/index.ts back to upstream/develop. The .github/workflows/generate.yml job regenerates these files every day at 9 AM UTC and opens a feat: regenerate asset data PR. Including them in a feature PR is unnecessary noise that chronically conflicts with the daily bot PR. Only the source change in scripts/generateAssetData/starknet/index.ts (which adds the strkBTC hardcoded entry) is needed; the next bot run after merge will pick it up and regenerate generatedAssetData.json plus the compressed siblings, and bump clearAssetsMigrations via the codemod. To test the integration locally before that bot run, run: pnpm run generate:asset-data or manually inject the strkBTC entry into public/generated/ generatedAssetData.json and rebuild .br/.gz with Node zlib. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous doc showed a `fee: 30` example without explicit units, which led to ambiguity. Now documents: - `fee` (integer) is in BIPS per Garden's OpenAPI spec (100 bips = 1%) - `fixed_fee` (string) is in USD per Garden's OpenAPI spec - Values are solver-set and vary per route (snapshot table for BTC↔WBTC, BTC↔cbBTC, BTC↔strkBTC, WBTC↔cbBTC verified live) - Garden's fee is already baked into the displayed destination amount; the ShapeShift protocolFees field surfaces only the affiliate cut on top, matching the AvnuSwapper convention Sources verified: - docs.garden.finance/api-reference/openapi.json (schema) - docs.garden.finance/api-reference/endpoint/get-quote.md - Live API at api.garden.finance/v2/quote across 4 routes Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Audit against NEAR Intents (the closest analog cross-chain solver swapper) showed Garden's checkTradeStatus did not return actualBuyAmountCryptoBaseUnit. NEAR Intents extracts amountOut from its status response; ShapeShift's post-trade UI uses this field to show the user the real received amount rather than the quoted estimate. Garden's order response exposes destination_swap.filled_amount, which matches the destination_swap.amount for fully-filled HTLC swaps. Wire that through mapGardenOrderToTxStatus and the checkTradeStatus return. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…covery Previously self-referenced (commit ba36446), which fixed the isRelatedAssetSwap false-positive against btcAssetId but lost the market-data fallback chain. ShapeShift's market-service-manager (market-service-manager.ts:103-126) walks relatedAssetIds to find a price for assets not directly indexed by CoinGecko/CoinCap. strkBTC is too new for CoinGecko (search returns empty), so self-reference meant no USD price displayed on the sell side. Pointing relatedAssetKey to the canonical wBTC.eth (0x2260...c599) puts strkBTC in the wrapped-BTC family for price inheritance via the wBTC.eth CoinGecko price (~1:1 pegged to BTC), matching the cross-chain wrapped-asset convention (cf. cbBTC.base.relatedAssetKey = cbBTC.arbitrum, USDC.base.relatedAssetKey = USDC.eth, etc.). Fee implications: - BTC ↔ strkBTC : 60 bps SS fee (different families) ✓ - wBTC.eth ↔ strkBTC : 0 bps (same wrapper family, treated as bridging within wrapped-BTC ecosystem like USDC.eth ↔ USDC.base) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…xes)
Comprehensive review-feedback preemption. Aligns Garden with the
canonical patterns used by Chainflip / Avnu / Across / NearIntents
in the swapper package.
Correctness:
- Add canonical `affiliateFee: buildAffiliateFee({ strategy: 'buy_asset', ... })`
step field on both getTradeQuote and getTradeRate. Without this the
affiliate-fee UI was broken for Garden trades (Avnu + NearIntents both
set this — every other swapper that takes affiliate uses this helper).
- Distinguish insufficient liquidity from no route: map
`"insufficient liquidity"` to `TradeQuoteError.SellAmountBelowTradeFee`
instead of `NoRouteFound`. Lets the UI prompt the user to retry with a
smaller amount (matches Across `AcrossErrorCode.InsufficientLiquidity`
precedent).
- Replace `DAO_TREASURY_*` fee-simulation placeholders with dead/burn
addresses (`0x000...dead` for EVM). Across and Debridge both use
burn addresses; pointing fee simulations at the DAO treasury would
pollute on-chain analytics.
Convention alignment:
- Singleton `gardenService` exported at module level (Chainflip pattern,
`chainflipService.ts:16`). API key passed via per-call axios config
headers instead of recreating the service on every call.
- Source chain gate is now an explicit `GardenSupportedSourceChainIds`
ChainId array (Chainflip/Across/Debridge pattern) instead of a
CHAIN_NAMESPACE Set.
- Drop the hardcoded `GARDEN_BLACKLIST_PAIRS` list — Garden's /quote
endpoint already returns `"No order pair found"` for unsupported
pairs, which the `isNoRouteFoundError` path maps correctly. The list
was both redundant and a maintenance burden.
- `checkTradeStatus` uses `fromAssetId(...).chainNamespace ===
CHAIN_NAMESPACE.Starknet` instead of `chainId.startsWith('starknet:')`.
- Inline `hexToDecimalString` at its single call site
(`fromHex(value, 'bigint').toString()` mirrors Bebop pattern).
Dead-code removal:
- Helpers: drop `slippageDecimalToBps` (and its 4 tests) and
`getGardenSourceChainId` — zero callers.
- Constants: drop `STRKBTC_TOKEN_CONTRACT_ADDRESS` re-export (now
module-private), `GARDEN_BITCOIN_ASSET`, `GARDEN_STRKBTC_ASSET`,
`gardenBitcoinAssetId`/`btcChainId` re-exports,
`GARDEN_QUOTE_DEADLINE_MS`, `GARDEN_DEFAULT_SLIPPAGE_DECIMAL_PERCENTAGE`.
- Types: drop `GardenOrder.integrator`/`.version`,
`GardenOrderRequest.slippage`/`.nonce`, `GardenAccount.delegate`,
`GardenEvmTransactionData.gas_limit` — never read.
- `GardenAffiliateFeeAsset` narrowed to the only value actually used.
- `gardenAssetRegistry` entries dropped unused `decimals` and
`contractAddress` columns (~50 LOC).
Docs:
- INTEGRATION.md: reconcile status mapping with the actual code
(no `deadline` field at order level; remove that bullet) and
clarify the HTLC-timeout / refund_tx_hash semantics.
Tests: 18/18 pass (was 22, dropped 4 dead `slippageDecimalToBps` cases).
Type-check: Garden-clean. Lint: clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous audit-driven change (commit ab867e4) mapped "insufficient liquidity" to SellAmountBelowTradeFee to "distinguish" it from NoRouteFound, matching Across's precedent. On re-examination this is semantically wrong: - SellAmountBelowTradeFee → translation: "Sell amount lower than fee" (i.e., amount is TOO SMALL relative to fees) - NoRouteFound → translation: "No routes found for this pair and amount" (i.e., couldn't route this specific amount) Garden's "insufficient liquidity" error means the trade amount is TOO BIG for the solver inventory available right now. Showing the user "Sell amount lower than fee" would be actively misleading — the opposite direction. The TradeQuoteError enum comment for NoRouteFound reads exactly: "the swapper does support the pair but couldn't find a route for the amount specified" — perfect match for insufficient liquidity. Across has the same problem in their code (constant.ts:88), but copying a wrong precedent doesn't make it right. Tests: 18/18 still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds client-side validation against Garden's documented per-asset hard
caps (min_amount, max_amount from /v2/assets) before the /v2/quote
call. Three UX improvements:
1. Sub-minimum amounts surface as SellAmountBelowMinimum with proper
metadata { minAmountCryptoBaseUnit, assetId } so the UI displays
"Minimum: 0.0001 strkBTC" (the existing validateTradeQuote.ts code
path already consumes this metadata; no UI change).
2. Above-maximum amounts surface as NoRouteFound with maxAmountCryptoBaseUnit
metadata so future UI work can display the hard cap.
3. Saves one /v2/quote round-trip in both cases (Garden returns the
identical "expected amount to be within the range of X to Y" error
for both directions and we'd have to parse it to differentiate).
Implementation follows Chainflip's fetchChainFlipAssets pattern. The
/v2/assets response is fairly static (changes when Garden adds/removes
an asset) so it's cached at module level with a 1h TTL — independent
from the gardenService's per-URL cache (which is tuned for /quote
freshness).
This does not change any swapper interface, any UI component, or any
core logic — it's purely additive validation in the swapperApi.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four agents audited Garden against (1) Garden docs verbatim,
(2) ShapeShift swapper conventions across NearIntents/Avnu/Chainflip/
Across, (3) Garden code SLOP detection, (4) cross-cutting modification
verification. This commit applies the items that survive scrutiny.
Constants:
- Revert Starknet placeholder from `0x000...0000` (invalid felt =
undeployed account, may be rejected by Starknet adapter
`getFeeData`) to `DAO_TREASURY_STARKNET` (valid deployed account;
fee simulation does not actually send funds so no analytics
pollution concern).
- Drop unused `strkbtcAssetId` export (was only consumed by the
helpers test). Inline the literal in the test file instead.
- Drop unused `STRKBTC_TOKEN_CONTRACT_ADDRESS` (the address is
inlined where actually needed in `gardenAssetRegistry`).
Types:
- Drop `typed_data?: unknown` from `GardenStarknetInitiateResult` and
`GardenEvmInitiateResult`. INTEGRATION.md explicitly says we don't
use this path; keeping the field as `unknown` gives a false
impression of gasless support.
Caching:
- Add promise-deduplication to `fetchGardenAssets` so concurrent first
callers don't both trigger a network fetch. Module-level cache
pattern is kept (vs the `createCache`/cachedUrls service-level
approach) because the `/v2/assets` TTL (1h) differs from the
service-level `/v2/quote` TTL (5s) and merging them would either
over-cache quotes or under-cache assets.
Docs:
- Document the asymmetric source-vs-destination support in
INTEGRATION.md: the registry enumerates Garden's full asset list
for destination resolution, but `GardenSupportedSourceChainIds`
narrows source-side initiation to chains we have execution wired
for. Solana/Tron are buy-only until `typed_data` flow + Solana/Tron
execute paths are implemented (web-c68.6).
Items considered and rejected:
- Solana/Tron "registry vs source list mismatch" flagged by code
reviewer is intentional architecture, not a bug. The registry must
contain destination-side entries to map AssetId↔Garden ID for
routes like `BTC → solana:cbbtc` to work.
- `affiliate_fees[].fee` unit was flagged as potentially 100x off;
verified live API + Garden docs ("Fees are expressed in basis
points") confirm BPS int is correct.
- `catch {}` returning `undefined` on fee estimation failure differs
from NearIntents (`'0'`) but matches Across (`undefined`).
`undefined` more honestly signals "unknown" vs "free".
Tests: 18/18 pass. Type-check clean. Lint clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the made-up Bitcoin "dead address" bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq — which I had unconsciously copied from packages/public-api/src/routes/quote/types.ts:108 where gomes uses it as an OpenAPI example, not a documented burn address. Two issues with the previous choice: 1. It's not a verifiable burn address. Could belong to someone. 2. It's a P2WPKH output (~31 bytes), but Garden's BTC HTLC is P2TR (~43 bytes), making the fee estimate ~5-6% too low. DAO_TREASURY_BITCOIN (bc1q9xrjfet2a05r3jvsxx66rru7pysevk5dvqasdw9eeea3rfqlk33qr4hghh) is: - A ShapeShift-controlled and documented address - P2WSH, which has the same ~43 byte output size as Garden's P2TR HTLC → more accurate fee simulation Bitcoin fee simulation via chain adapter does not actually send funds (it just sizes the would-be tx), so there is no analytics pollution concern raised by the previous audit. Rename: GARDEN_DEAD_ADDRESS_BY_NAMESPACE → GARDEN_FEE_PLACEHOLDER_BY_NAMESPACE to reflect that these are simulation placeholders, not burn addresses. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Garden strkBTC AssetId was injected into generatedAssetData.json but relatedAssetIndex.json (the pre-computed index that drives the family-grouped UI) was not regenerated, so strkBTC was orphaned from the wBTC family at runtime. Added strkBTC to manualRelatedAssetIndex in the pipeline source so future REGEN_ALL runs preserve the mapping (strkBTC isn't on CoinGecko/Zerion, so it would not be inferred automatically), regenerated the index + pre-compressed siblings, and bumped the content hash + clearAssetsMigrations to invalidate stale client state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
selectGroupedAssetsWithBalances previously sorted family variants by fiatAmount descending only. When a user's only held variant has not yet had its market-data price resolved (typical for brand-new assets priced via relatedAssetKey fallback), its fiatAmount is 0 and it ties with empty variants — sort then preserves insertion order and the held variant ends up buried at the tail. Added a primary sort key on cryptoAmount > 0 so held variants surface first regardless of fiat resolution, with the existing fiatAmount sort as tiebreaker. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Garden's /orders endpoint can return a populated destination_swap. redeem_tx_hash with filled_amount still at "0" before the destination amount is finalized. mapGardenOrderToTxStatus propagated the literal "0" string into actualBuyAmountCryptoBaseUnit, and the post-swap toast in SwapNotification falls back to expectedBuyAmountCryptoPrecision only on undefined (not on the truthy "0" string), so the toast read "to 0 BTC is complete" instead of the expected destination amount. Omit actualBuyAmountCryptoBaseUnit when filled_amount is "0" or empty so the fallback kicks in, with unit-test coverage for both edges. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR integrates Garden Finance as a new swapper to enable BTC ↔ wrapped-BTC swaps across UTXO, EVM, and Starknet chains. The implementation adds configuration, types, asset registry, API integration, trade endpoints, transaction building, swapper registration, UI wiring, state management, and comprehensive documentation. ChangesGarden Swapper Feature Integration
🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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: 7
🧹 Nitpick comments (3)
packages/swapper/src/types.ts (1)
461-471: ⚡ Quick winDeduplicate
gardenSpecifictype to avoid contract drift.The same nested shape is defined twice. Please extract a shared type alias and reference it in both places so future Garden metadata changes stay consistent.
♻️ Proposed refactor
+type GardenSpecificStepMetadata = { + orderId: string + bitcoinDepositAddress?: string + starknetCalls?: { to: string; selector: string; calldata: string[] }[] + evmInitiate?: { + to: string + data: string + value: string + allowanceContract: string + } +} ... - gardenSpecific?: { - orderId: string - bitcoinDepositAddress?: string - starknetCalls?: { to: string; selector: string; calldata: string[] }[] - evmInitiate?: { - to: string - data: string - value: string - allowanceContract: string - } - } + gardenSpecific?: GardenSpecificStepMetadata ... - gardenSpecific?: { - orderId: string - bitcoinDepositAddress?: string - starknetCalls?: { to: string; selector: string; calldata: string[] }[] - evmInitiate?: { - to: string - data: string - value: string - allowanceContract: string - } - } + gardenSpecific?: GardenSpecificStepMetadataAlso applies to: 592-602
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/types.ts` around lines 461 - 471, The nested shape used for gardenSpecific is duplicated; extract it into a single type alias (e.g., GardenSpecific or GardenMetadata) that captures { orderId: string; bitcoinDepositAddress?: string; starknetCalls?: { to: string; selector: string; calldata: string[] }[]; evmInitiate?: { to: string; data: string; value: string; allowanceContract: string } } and replace both inline occurrences (the gardenSpecific property at the current location and the other occurrence around lines 592-602) with that new alias so both references share the same definition.packages/swapper/src/swappers/GardenSwapper/utils/helpers/helpers.ts (1)
20-33: ⚡ Quick winRename exported constant to UPPER_SNAKE_CASE for guideline compliance.
Please rename
GardenSupportedSourceChainIds(and dependent usages) to an UPPER_SNAKE_CASE constant name to match project conventions.♻️ Suggested diff
-export const GardenSupportedSourceChainIds: readonly ChainId[] = [ +export const GARDEN_SUPPORTED_SOURCE_CHAIN_IDS: readonly ChainId[] = [ btcChainId, ltcChainId, ethChainId, baseChainId, bscChainId, arbitrumChainId, monadChainId, hyperEvmChainId, megaethChainId, starknetChainId, ] -const gardenSupportedSourceChainIdSet = new Set(GardenSupportedSourceChainIds) +const gardenSupportedSourceChainIdSet = new Set(GARDEN_SUPPORTED_SOURCE_CHAIN_IDS)As per coding guidelines, "Use UPPER_SNAKE_CASE for constants and configuration values with descriptive names".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/swappers/GardenSwapper/utils/helpers/helpers.ts` around lines 20 - 33, Rename the exported constant GardenSupportedSourceChainIds to UPPER_SNAKE_CASE (e.g., GARDEN_SUPPORTED_SOURCE_CHAIN_IDS) and update all dependent usages; also update the derived set declaration (gardenSupportedSourceChainIdSet) to reference the new name. Ensure exported symbol name is changed where declared (export const GardenSupportedSourceChainIds) and any imports/usages across the package are updated to the new GARDEN_SUPPORTED_SOURCE_CHAIN_IDS identifier to keep consistency with project conventions.packages/swapper/src/swappers/GardenSwapper/swapperApi/getTradeRate.ts (1)
155-157: ⚡ Quick winDon’t swallow fee-estimation errors silently.
The empty catch hides why network fee calculation fails. Please emit a structured log with chain namespace and asset context before returning
undefined.As per coding guidelines, "ALWAYS log errors for debugging using structured logging with relevant context and error metadata".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/swappers/GardenSwapper/swapperApi/getTradeRate.ts` around lines 155 - 157, In getTradeRate's catch block do not swallow errors: capture the thrown error (e.g., catch (err)) and emit a structured error log before returning undefined; log should include the chain namespace and asset context (e.g., chainNamespace, sellAsset/buyAsset identifiers) and the error object/metadata using the module's logger instance (e.g., logger or this.logger) so callers still get undefined but the failure is recorded for debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/swapper/src/starknet-utils/buildStarknetInvokeTx.ts`:
- Around line 41-76: The fee-estimation path (calls to
adapter.getStarknetProvider().fetch and subsequent handling of
estimateResult/feeEstimate in buildStarknetInvokeTx) must be wrapped in a
try/catch and converted to an AsyncResultOf promise pattern; catch RPC/json
failures and replace raw Error throws with typed errors from
`@shapeshiftoss/errors` (use a meaningful error code and include context like
method='starknet_estimateFee', request payload=estimateTx, and any response
error/result), and return/propagate the Result type instead of throwing so
callers can distinguish and internationalize errors.
- Around line 5-15: The toHexString function currently coerces ambiguous inputs
(e.g. "1234" -> 0x4d2) and returns malformed values for invalid inputs (e.g.
undefined -> 0xundefined); change toHexString so it only accepts (a) strings
that already start with "0x" (return as-is) or (b) unambiguous decimal strings
matching /^\d+$/ which you convert deterministically to hex (use BigInt or a
safe integer-to-hex helper instead of blind String coercion/num.toHex), and
otherwise throw a descriptive error; update the implementation points around
toHexString and any calls to num.toHex to stop silently falling back and to fail
fast on invalid/ambiguous inputs.
In `@packages/swapper/src/swappers/GardenSwapper/endpoints.ts`:
- Around line 42-43: The code currently sets satoshiPerByte =
(feeData.chainSpecific as UtxoFeeData | undefined)?.satsPerByte ?? '0', which
silently defaults to '0' and can create stuck UTXO transactions; update the
logic to fail fast instead: check that feeData and (feeData.chainSpecific as
UtxoFeeData)?.satsPerByte are present and valid (non-empty/positive) and if not
throw or return an explicit error (e.g., via an Error or a failed response) from
the function that uses satoshiPerByte; reference the variables feeData,
UtxoFeeData, satsPerByte, and satoshiPerByte when implementing the validation so
callers never proceed with a zero fallback.
In `@packages/swapper/src/swappers/GardenSwapper/types.ts`:
- Around line 88-116: Harden the three type guards by fully validating all
required fields and their primitive types instead of the current partial checks:
in isGardenBitcoinInitiate ensure result has 'to' as a string and 'amount' as a
number (and any other required primitives for Bitcoin orders); in
isGardenStarknetInitiate ensure 'initiate_transaction' is a non-null object,
'selector' is a string, 'calldata' is an array and each calldata element is the
expected primitive (e.g., string or number) and any other required Starknet
fields are present; in isGardenEvmInitiate ensure 'initiate_transaction' is a
non-null object and that required fields like 'chain_id' are number, 'to' is
string, 'data' is string (and 'value' is number/string if applicable) so
malformed/missing primitives are rejected before downstream use (update checks
inside isGardenBitcoinInitiate, isGardenStarknetInitiate, and
isGardenEvmInitiate).
In `@packages/swapper/src/swappers/GardenSwapper/utils/fetchFromGarden.ts`:
- Around line 39-75: The fetchGardenQuote function (and the sibling helpers in
the same file) must not allow thrown exceptions to escape; wrap the entire async
body of fetchGardenQuote (the call to gardenService.get using
GARDEN_API_BASE_URL and authHeaders, the params construction, and response
handling) in a try/catch, log structured context (apiKey, from, to, fromAmount,
affiliateBps) on catch, and convert any unexpected failure into an Err using
makeSwapErrorRight (with a clear message, code via errorMessageToTradeQuoteError
where appropriate, and details including the caught error). Ensure the catch
returns Err(...) instead of throwing so the function preserves the Result
contract; apply the same pattern to the other helpers referenced in the file.
In `@scripts/generateAssetData/starknet/index.ts`:
- Around line 89-92: The output can duplicate the strkBTC entry because
tokensOnly only filters nativeStrkTokenPattern; update the filtering of
modifiedAssets (used to build tokensOnly) to also exclude the strkbtc asset by
comparing asset.assetId against strkbtcAsset.assetId (or the unique id field
used on strkbtcAsset) so tokensOnly = modifiedAssets.filter(asset =>
!nativeStrkTokenPattern.test(asset.assetId) && asset.assetId !==
strkbtcAsset.assetId), then return [starknetBaseAsset, strkbtcAsset,
...tokensOnly]; this ensures strkbtcAsset (from variable strkbtcAsset) is not
duplicated if it appears in modifiedAssets.
---
Nitpick comments:
In `@packages/swapper/src/swappers/GardenSwapper/swapperApi/getTradeRate.ts`:
- Around line 155-157: In getTradeRate's catch block do not swallow errors:
capture the thrown error (e.g., catch (err)) and emit a structured error log
before returning undefined; log should include the chain namespace and asset
context (e.g., chainNamespace, sellAsset/buyAsset identifiers) and the error
object/metadata using the module's logger instance (e.g., logger or this.logger)
so callers still get undefined but the failure is recorded for debugging.
In `@packages/swapper/src/swappers/GardenSwapper/utils/helpers/helpers.ts`:
- Around line 20-33: Rename the exported constant GardenSupportedSourceChainIds
to UPPER_SNAKE_CASE (e.g., GARDEN_SUPPORTED_SOURCE_CHAIN_IDS) and update all
dependent usages; also update the derived set declaration
(gardenSupportedSourceChainIdSet) to reference the new name. Ensure exported
symbol name is changed where declared (export const
GardenSupportedSourceChainIds) and any imports/usages across the package are
updated to the new GARDEN_SUPPORTED_SOURCE_CHAIN_IDS identifier to keep
consistency with project conventions.
In `@packages/swapper/src/types.ts`:
- Around line 461-471: The nested shape used for gardenSpecific is duplicated;
extract it into a single type alias (e.g., GardenSpecific or GardenMetadata)
that captures { orderId: string; bitcoinDepositAddress?: string; starknetCalls?:
{ to: string; selector: string; calldata: string[] }[]; evmInitiate?: { to:
string; data: string; value: string; allowanceContract: string } } and replace
both inline occurrences (the gardenSpecific property at the current location and
the other occurrence around lines 592-602) with that new alias so both
references share the same definition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 16e2f312-c3ec-405e-95da-32d1a718df1e
⛔ Files ignored due to path filters (10)
public/generated/asset-manifest.jsonis excluded by!**/generated/**public/generated/asset-manifest.json.bris excluded by!**/generated/**public/generated/asset-manifest.json.gzis excluded by!**/*.gz,!**/generated/**public/generated/generatedAssetData.jsonis excluded by!**/generated/**public/generated/generatedAssetData.json.bris excluded by!**/generated/**public/generated/generatedAssetData.json.gzis excluded by!**/*.gz,!**/generated/**public/generated/relatedAssetIndex.jsonis excluded by!**/generated/**public/generated/relatedAssetIndex.json.bris excluded by!**/generated/**public/generated/relatedAssetIndex.json.gzis excluded by!**/*.gz,!**/generated/**src/components/MultiHopTrade/components/TradeInput/components/SwapperIcon/garden-icon.pngis excluded by!**/*.png
📒 Files selected for processing (36)
.env.env.developmentheaders/csps/defi/swappers/Garden.tsheaders/csps/index.tspackages/public-api/src/config.tspackages/public-api/src/env.tspackages/swapper/src/constants.tspackages/swapper/src/index.tspackages/swapper/src/starknet-utils/buildStarknetInvokeTx.tspackages/swapper/src/starknet-utils/index.tspackages/swapper/src/swappers/AvnuSwapper/endpoints.tspackages/swapper/src/swappers/GardenSwapper/GardenSwapper.tspackages/swapper/src/swappers/GardenSwapper/INTEGRATION.mdpackages/swapper/src/swappers/GardenSwapper/constants.tspackages/swapper/src/swappers/GardenSwapper/endpoints.tspackages/swapper/src/swappers/GardenSwapper/gardenAssetRegistry.tspackages/swapper/src/swappers/GardenSwapper/index.tspackages/swapper/src/swappers/GardenSwapper/swapperApi/getTradeQuote.tspackages/swapper/src/swappers/GardenSwapper/swapperApi/getTradeRate.tspackages/swapper/src/swappers/GardenSwapper/types.tspackages/swapper/src/swappers/GardenSwapper/utils/fetchFromGarden.tspackages/swapper/src/swappers/GardenSwapper/utils/gardenService.tspackages/swapper/src/swappers/GardenSwapper/utils/helpers/helpers.test.tspackages/swapper/src/swappers/GardenSwapper/utils/helpers/helpers.tspackages/swapper/src/types.tsscripts/generateAssetData/generateRelatedAssetIndex/generateRelatedAssetIndex.tsscripts/generateAssetData/starknet/index.tssrc/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeButtonProps.tsxsrc/components/MultiHopTrade/components/TradeInput/components/SwapperIcon/SwapperIcon.tsxsrc/config.tssrc/lib/tradeExecution.tssrc/state/helpers.tssrc/state/migrations/index.tssrc/state/slices/portfolioSlice/selectors.tssrc/state/slices/preferencesSlice/preferencesSlice.tssrc/test/mocks/store.ts
Comment on lines
+41
to
+76
| const chainIdHex = await adapter.getStarknetProvider().getChainId() | ||
| const nonce = await adapter.getNonce(normalizedFrom) | ||
|
|
||
| const version = '0x3' as const | ||
| const estimateTx = { | ||
| type: 'INVOKE', | ||
| version, | ||
| sender_address: normalizedFrom, | ||
| calldata: formattedCalldata, | ||
| signature: [], | ||
| nonce, | ||
| resource_bounds: { | ||
| l1_gas: { max_amount: '0x186a0', max_price_per_unit: '0x5f5e100' }, | ||
| l2_gas: { max_amount: '0x0', max_price_per_unit: '0x0' }, | ||
| l1_data_gas: { max_amount: '0x186a0', max_price_per_unit: '0x1' }, | ||
| }, | ||
| tip: '0x0', | ||
| paymaster_data: [], | ||
| account_deployment_data: [], | ||
| nonce_data_availability_mode: 'L1', | ||
| fee_data_availability_mode: 'L1', | ||
| } | ||
|
|
||
| const estimateResponse = await adapter | ||
| .getStarknetProvider() | ||
| .fetch('starknet_estimateFee', [[estimateTx], ['SKIP_VALIDATE'], 'latest']) | ||
| const estimateResult: StarknetEstimateResult = await estimateResponse.json() | ||
|
|
||
| if (estimateResult.error) { | ||
| throw new Error(`Fee estimation failed: ${JSON.stringify(estimateResult.error)}`) | ||
| } | ||
|
|
||
| const feeEstimate = estimateResult.result?.[0] | ||
| if (!feeEstimate) { | ||
| throw new Error('Fee estimation failed: no estimate returned') | ||
| } |
Contributor
There was a problem hiding this comment.
Preserve typed swapper errors in the shared RPC path.
This helper now performs the provider calls for both Avnu and Garden, but RPC/json failures and the fee-estimation branches still escape as raw Errors. That makes execution failures harder to classify, localize, and internationalize once they bubble through the swapper layer. Wrap this path in typed error handling with structured context instead of throwing generic exceptions.
As per coding guidelines, "ALWAYS wrap async operations in try-catch blocks and use AsyncResultOf utility for converting promises to Results" and "ALWAYS use custom error classes from @shapeshiftoss/errors with meaningful error codes for internationalization and relevant details in error objects".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/swapper/src/starknet-utils/buildStarknetInvokeTx.ts` around lines 41
- 76, The fee-estimation path (calls to adapter.getStarknetProvider().fetch and
subsequent handling of estimateResult/feeEstimate in buildStarknetInvokeTx) must
be wrapped in a try/catch and converted to an AsyncResultOf promise pattern;
catch RPC/json failures and replace raw Error throws with typed errors from
`@shapeshiftoss/errors` (use a meaningful error code and include context like
method='starknet_estimateFee', request payload=estimateTx, and any response
error/result), and return/propagate the Result type instead of throwing so
callers can distinguish and internationalize errors.
Tightens Garden swapper and the shared Starknet utility against the seven issues CodeRabbit flagged on PR shapeshift#12355 (review on commit 2c2551b). All changes are surgical hardening — no behaviour change on the happy path. - starknet-utils/buildStarknetInvokeTx - toHexString now rejects non-string inputs and ambiguous strings (previously `'1234'` was re-encoded as `0x4d2` and `undefined` fell through to `0xundefined`). Accepts `0x`-prefixed strings, case-only hex with at least one a-f letter, and pure decimal integers; throws otherwise. - starknet_estimateFee RPC failures and decode errors are wrapped with descriptive context (sender address, RPC method) instead of escaping as opaque generic Errors. - GardenSwapper/endpoints.ts: fail fast when `satsPerByte` is missing on the UTXO fee data instead of defaulting to '0' (which would produce stuck or invalid UTXO transactions). - GardenSwapper/types.ts: harden the three initiate type guards to validate every required field (`order_id`, `to`, `amount`, etc.) and its primitive type so malformed Garden payloads are rejected upstream of the chain-specific execution paths. - GardenSwapper/swapperApi/getTradeQuote.ts: wrap chain-specific fee estimation in a try/catch that degrades to undefined network fee. Previously a fee-estimation throw after `createGardenOrder` had already created a remote order would surface as a quote failure and invite duplicate order creation on retry. - GardenSwapper/utils/fetchFromGarden.ts: wrap each of fetchGardenQuote, createGardenOrder, and fetchGardenOrder in a try/catch so unexpected throws (axios, JSON parse, etc.) convert to Err(makeSwapErrorRight) instead of escaping the Result contract. - scripts/generateAssetData/starknet/index.ts: filter strkbtcAsset out of `tokensOnly` before concatenation so the output cannot contain duplicate strkBTC entries if CoinGecko ever indexes the token. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four subagents re-reviewed PR shapeshift#12355 in parallel (architecture conformance, AI slop, conventions compliance, adversarial security). This commit addresses the actionable findings. - GardenSwapper/swapperApi/getTradeQuote.ts: extract parseGardenEvmValue helper that accepts both 0x-prefixed and pure-decimal value formats (Garden's OpenAPI spec is ambiguous; mainnet responses are 0x-prefixed but we don't want a viem fromHex throw to escape the Result return type if Garden ever returns decimal). Wrap the buildGardenSpecific call site in a try/catch that converts any payload-shape exception into Err(InvalidResponse), preserving the Promise<Result<...>> contract. - scripts/generateAssetData/generateRelatedAssetIndex: remove the debug console.log + double zerionFungiblesSchema.parse() artefact (line 201-207), and replace the bespoke chunkArray helper with lodash/chunk (already imported as a sibling lodash function). - .env.development: declare VITE_GARDEN_API_KEY (empty default) per swapper-integration contract item 8 ("Environment variables — .env, .env.development, .env.production — VITE_[SWAPPER]_API_KEY"). The actual key continues to live in .env.development.local. Verified non-actionable / false positives during this review: - The Garden EVM 'approval_transaction' payload is intentionally ignored: ShapeShift's standard allowance flow consumes the step's allowanceContract field (useAllowanceApproval, useAllowanceReset, useIsApprovalInitiallyNeeded) and builds its own ERC20 approve() call. Garden's pre-built calldata is redundant — same pattern as every other EVM-source swapper in the repo. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/swapper/src/swappers/GardenSwapper/endpoints.ts`:
- Around line 23-181: The methods getUnsignedUtxoTransaction,
getUtxoTransactionFees, getUnsignedEvmTransaction, getEvmTransactionFees,
getUnsignedStarknetTransaction, and getStarknetTransactionFees currently throw
raw Error instances and allow async exceptions (e.g., evm.getFees) to bubble;
change these to return the swapper Result error path using makeSwapErrorRight
with the appropriate TradeQuoteError variant instead of throwing. Replace each
throw new Error(...) with returning
makeSwapErrorRight(TradeQuoteError.<appropriate_variant>, { message: '...',
details: {...} }) and wrap any awaited calls (like evm.getFees) in try/catch to
convert thrown errors into
makeSwapErrorRight(TradeQuoteError.QuoteFailedOrNetworkFeeFailure, { cause }) so
structured metadata is preserved. Ensure all earlier guards
(isExecutableTradeQuote, missing gardenSpecific fields, missing
satsPerByte/networkFeeCryptoBaseUnit, missing xpub) return a makeSwapErrorRight
with suitable TradeQuoteError values rather than throwing, and update return
types to the Result pattern expected by the swapper API.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ba0d41cd-906c-4489-a809-a47d8b0c7aaf
📒 Files selected for processing (8)
.env.developmentpackages/swapper/src/starknet-utils/buildStarknetInvokeTx.tspackages/swapper/src/swappers/GardenSwapper/endpoints.tspackages/swapper/src/swappers/GardenSwapper/swapperApi/getTradeQuote.tspackages/swapper/src/swappers/GardenSwapper/types.tspackages/swapper/src/swappers/GardenSwapper/utils/fetchFromGarden.tsscripts/generateAssetData/generateRelatedAssetIndex/generateRelatedAssetIndex.tsscripts/generateAssetData/starknet/index.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/swapper/src/swappers/GardenSwapper/swapperApi/getTradeQuote.ts
- packages/swapper/src/swappers/GardenSwapper/types.ts
- packages/swapper/src/swappers/GardenSwapper/utils/fetchFromGarden.ts
- packages/swapper/src/starknet-utils/buildStarknetInvokeTx.ts
Comment on lines
+23
to
+181
| getUnsignedUtxoTransaction: ({ | ||
| stepIndex, | ||
| tradeQuote, | ||
| assertGetUtxoChainAdapter, | ||
| xpub, | ||
| accountType, | ||
| }) => { | ||
| if (!isExecutableTradeQuote(tradeQuote)) throw new Error('Unable to execute a trade rate quote') | ||
|
|
||
| const step = getExecutableTradeStep(tradeQuote, stepIndex) | ||
|
|
||
| const { sellAsset, accountNumber, gardenSpecific, feeData } = step | ||
| if (!gardenSpecific?.bitcoinDepositAddress) { | ||
| throw new Error('gardenSpecific.bitcoinDepositAddress is required for UTXO source') | ||
| } | ||
| if (!xpub) throw new Error('xpub is required for UTXO transactions') | ||
|
|
||
| const adapter = assertGetUtxoChainAdapter(sellAsset.chainId) | ||
|
|
||
| const satoshiPerByte = (feeData.chainSpecific as UtxoFeeData | undefined)?.satsPerByte | ||
| if (!satoshiPerByte) { | ||
| throw new Error('Missing satsPerByte in quote fee data') | ||
| } | ||
|
|
||
| return adapter.buildSendApiTransaction({ | ||
| accountNumber, | ||
| to: gardenSpecific.bitcoinDepositAddress, | ||
| value: step.sellAmountIncludingProtocolFeesCryptoBaseUnit, | ||
| sendMax: false, | ||
| chainSpecific: { | ||
| satoshiPerByte, | ||
| accountType, | ||
| }, | ||
| xpub, | ||
| }) | ||
| }, | ||
|
|
||
| getUtxoTransactionFees: ({ tradeQuote, stepIndex }) => { | ||
| if (!isExecutableTradeQuote(tradeQuote)) throw new Error('Unable to execute a trade rate quote') | ||
| const step = getExecutableTradeStep(tradeQuote, stepIndex) | ||
| if (!step.feeData.networkFeeCryptoBaseUnit) { | ||
| throw new Error('Missing network fee in quote') | ||
| } | ||
| return Promise.resolve(step.feeData.networkFeeCryptoBaseUnit) | ||
| }, | ||
|
|
||
| getUnsignedEvmTransaction: async ({ | ||
| from, | ||
| stepIndex, | ||
| tradeQuote, | ||
| supportsEIP1559, | ||
| assertGetEvmChainAdapter, | ||
| }) => { | ||
| if (!isExecutableTradeQuote(tradeQuote)) throw new Error('Unable to execute a trade rate quote') | ||
|
|
||
| const step = getExecutableTradeStep(tradeQuote, stepIndex) | ||
| const { accountNumber, sellAsset, gardenSpecific } = step | ||
| if (!gardenSpecific?.evmInitiate) { | ||
| throw new Error('gardenSpecific.evmInitiate is required for EVM source') | ||
| } | ||
|
|
||
| const adapter = assertGetEvmChainAdapter(sellAsset.chainId as EvmChainId) | ||
| const { to, data, value } = gardenSpecific.evmInitiate | ||
|
|
||
| const feeData = await evm.getFees({ | ||
| adapter, | ||
| data, | ||
| to, | ||
| value, | ||
| from, | ||
| supportsEIP1559, | ||
| }) | ||
|
|
||
| return adapter.buildSendApiTransaction({ | ||
| accountNumber, | ||
| from, | ||
| to, | ||
| value, | ||
| chainSpecific: { contractAddress: undefined, data, ...feeData }, | ||
| }) | ||
| }, | ||
|
|
||
| getEvmTransactionFees: async ({ | ||
| from, | ||
| stepIndex, | ||
| tradeQuote, | ||
| supportsEIP1559, | ||
| assertGetEvmChainAdapter, | ||
| }) => { | ||
| if (!isExecutableTradeQuote(tradeQuote)) throw new Error('Unable to execute a trade rate quote') | ||
|
|
||
| const step = getExecutableTradeStep(tradeQuote, stepIndex) | ||
| const { sellAsset, gardenSpecific } = step | ||
| if (!gardenSpecific?.evmInitiate) { | ||
| throw new Error('gardenSpecific.evmInitiate is required for EVM source') | ||
| } | ||
|
|
||
| const adapter = assertGetEvmChainAdapter(sellAsset.chainId as EvmChainId) | ||
| const { to, data, value } = gardenSpecific.evmInitiate | ||
|
|
||
| const feeData = await evm.getFees({ | ||
| adapter, | ||
| data, | ||
| to, | ||
| value, | ||
| from, | ||
| supportsEIP1559, | ||
| }) | ||
|
|
||
| return feeData.networkFeeCryptoBaseUnit | ||
| }, | ||
|
|
||
| getUnsignedStarknetTransaction: ({ | ||
| stepIndex, | ||
| tradeQuote, | ||
| from, | ||
| assertGetStarknetChainAdapter, | ||
| }) => { | ||
| if (!isExecutableTradeQuote(tradeQuote)) throw new Error('Unable to execute a trade rate quote') | ||
|
|
||
| const step = getExecutableTradeStep(tradeQuote, stepIndex) | ||
|
|
||
| const { accountNumber, sellAsset, gardenSpecific } = step | ||
| if (!gardenSpecific?.starknetCalls || gardenSpecific.starknetCalls.length === 0) { | ||
| throw new Error('gardenSpecific.starknetCalls is required for Starknet source') | ||
| } | ||
|
|
||
| const adapter = assertGetStarknetChainAdapter(sellAsset.chainId) | ||
| const normalizedFrom = validateAndParseAddress(from) | ||
|
|
||
| const fullCalldata: string[] = [gardenSpecific.starknetCalls.length.toString()] | ||
| for (const call of gardenSpecific.starknetCalls) { | ||
| const normalizedContractAddress = validateAndParseAddress(call.to) | ||
| fullCalldata.push( | ||
| normalizedContractAddress, | ||
| call.selector, | ||
| call.calldata.length.toString(), | ||
| ...call.calldata.map(String), | ||
| ) | ||
| } | ||
|
|
||
| const formattedCalldata = fullCalldata.map(toHexString) | ||
|
|
||
| return buildStarknetInvokeTx({ | ||
| formattedCalldata, | ||
| normalizedFrom, | ||
| accountNumber, | ||
| adapter, | ||
| }) | ||
| }, | ||
|
|
||
| getStarknetTransactionFees: ({ tradeQuote, stepIndex }) => { | ||
| if (!isExecutableTradeQuote(tradeQuote)) throw new Error('Unable to execute a trade rate quote') | ||
| const step = getExecutableTradeStep(tradeQuote, stepIndex) | ||
| if (!step.feeData.networkFeeCryptoBaseUnit) { | ||
| throw new Error('Missing network fee in quote') | ||
| } | ||
| return Promise.resolve(step.feeData.networkFeeCryptoBaseUnit) | ||
| }, |
Contributor
There was a problem hiding this comment.
Preserve swapper error contracts instead of throwing raw Errors.
This implementation throws directly in multiple swapper API methods and allows async exceptions to bubble (e.g., fee calls), which bypasses the required Result/makeSwapErrorRight(TradeQuoteError, …) path and drops structured error metadata.
As per coding guidelines: "**/*.{ts,tsx}: Use Result<T, E> pattern … AVOID throwing within swapper API implementations" and "**/swapper{s,}/**/*.{ts,tsx}: ALWAYS use makeSwapErrorRight for swapper errors with TradeQuoteError enum."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/swapper/src/swappers/GardenSwapper/endpoints.ts` around lines 23 -
181, The methods getUnsignedUtxoTransaction, getUtxoTransactionFees,
getUnsignedEvmTransaction, getEvmTransactionFees,
getUnsignedStarknetTransaction, and getStarknetTransactionFees currently throw
raw Error instances and allow async exceptions (e.g., evm.getFees) to bubble;
change these to return the swapper Result error path using makeSwapErrorRight
with the appropriate TradeQuoteError variant instead of throwing. Replace each
throw new Error(...) with returning
makeSwapErrorRight(TradeQuoteError.<appropriate_variant>, { message: '...',
details: {...} }) and wrap any awaited calls (like evm.getFees) in try/catch to
convert thrown errors into
makeSwapErrorRight(TradeQuoteError.QuoteFailedOrNetworkFeeFailure, { cause }) so
structured metadata is preserved. Ensure all earlier guards
(isExecutableTradeQuote, missing gardenSpecific fields, missing
satsPerByte/networkFeeCryptoBaseUnit, missing xpub) return a makeSwapErrorRight
with suitable TradeQuoteError values rather than throwing, and update return
types to the Result pattern expected by the swapper API.
…eError
Garden returns HTTP 400 with a structured body (e.g.
{status: "Error", error: "Insufficient liquidity"}) for documented
business errors. makeSwapperAxiosServiceMonadic was treating any
non-2xx as a generic axios error and emitting QueryFailed before our
callers could inspect data.error, so Garden's "Insufficient liquidity"
and "expected amount to be within the range..." strings never reached
errorMessageToTradeQuoteError. The end-user-visible result was the
generic "An error occurred getting a quote" toast instead of the
informative "No routes found for this pair and amount" or
"Sell amount below Garden minimum" message.
Set validateStatus: () => true in gardenService's axios config so
non-2xx responses are returned to the swapperApi layer with their
body intact. The existing data.status !== 'Ok' branches in
fetchGardenQuote / createGardenOrder / fetchGardenOrder already
handle the body and call errorMessageToTradeQuoteError, which now
correctly maps:
- "Insufficient liquidity" -> TradeQuoteError.NoRouteFound
- "No order pair found" -> TradeQuoteError.NoRouteFound
- "expected amount to be within ..." -> TradeQuoteError.SellAmountBelowMinimum
Reproduced live against api.garden.finance/v2/orders for BTC -> LTC
at 0.000103 BTC, which is above Garden's min but exceeds the
solver's current LTC inventory.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Garden's POST /orders returns approval_transaction: null on Starknet source when the user already has sufficient strkBTC allowance to the HTLC contract from a prior swap (i.e., the second strkBTC -> BTC swap no longer needs a fresh approval). buildGardenSpecific previously wrapped both calls verbatim: starknetCalls: [order.approval_transaction, order.initiate_transaction] With approval_transaction === null this produced [null, initiate]. getUnsignedStarknetTransaction's calldata-build loop then crashed with "Cannot read properties of null (reading 'to')" because the loop body dereferences call.to without a null guard. - types.ts: GardenStarknetInitiateResult.approval_transaction is now typed as GardenStarknetCall | null to reflect the actual API shape. - getTradeQuote.ts: filter null out of starknetCalls before assigning to gardenSpecific, so the multi-call only contains real calls. Reproduced live: second strkBTC -> BTC swap on the same Starknet account after a successful first swap surfaces the null approval. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When the local fee simulation reverts (most commonly when the user is exploring a rate with insufficient balance — the simulated ERC20 transferFrom / Starknet transfer / UTXO sendmany fails on-chain), Garden previously bubbled the catch up to `undefined`, which the UI surfaces as the literal string "(unknown)" on the quote tile. NearIntents handles the same case by returning '0' on per-branch sim failure (cf. NearIntentsSwapper/swapperApi/getTradeRate.ts Monad branch), so the quote tile reads "$0.00" — same end state for the user (they cannot sign anyway, the CTA reads "Insufficient Funds"), but visually consistent with sibling swappers in the available-quotes list. Replace the single outer try/catch returning undefined with per-branch catches returning '0', preserving the `return undefined` early-exits for wallet-not-connected conditions (where '0' would be misleading). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…per_clean # Conflicts: # public/generated/asset-manifest.json # public/generated/asset-manifest.json.br # public/generated/asset-manifest.json.gz # public/generated/generatedAssetData.json.br # public/generated/generatedAssetData.json.gz # public/generated/relatedAssetIndex.json.br # public/generated/relatedAssetIndex.json.gz
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/swapper/src/swappers/GardenSwapper/swapperApi/getTradeRate.ts (1)
115-124: ⚡ Quick winLog fee-estimation fallbacks before returning
'0'.These
catchblocks currently erase the cause of the fallback. A transient RPC outage and an insufficient-funds simulation both become an unexplained zero-fee rate, which makes production debugging much harder.As per coding guidelines,
**/*.{ts,tsx}: ALWAYS log errors for debugging using structured logging with relevant context and error metadata.Also applies to: 132-141, 150-161
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swapper/src/swappers/GardenSwapper/swapperApi/getTradeRate.ts` around lines 115 - 124, The catch blocks that swallow errors when calling adapter.getFeeData (the block returning '0' after awaiting adapter.getFeeData for { to: placeholderTo, value: sellAmount, chainSpecific: { pubkey } }) must capture the thrown error (catch (err)) and emit a structured log entry with relevant context (include placeholderTo, sellAmount, pubkey and that this was fee estimation for utxoFee.fast.txFee) before returning '0'; apply the same pattern to the other similar try/catch sites around the adapter.getFeeData usages referenced (lines ~132-141 and ~150-161) so all fee-estimation fallbacks log error metadata instead of silently returning '0'.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/swapper/src/swappers/GardenSwapper/swapperApi/getTradeRate.ts`:
- Around line 111-158: The adapter assertions (deps.assertGetUtxoChainAdapter,
deps.assertGetStarknetChainAdapter, deps.assertGetEvmChainAdapter) must be moved
inside each guarded try block so adapter-resolution failures are handled as
Result errors instead of throwing; for each branch (CHAIN_NAMESPACE.Utxo,
.Starknet, .Evm) call the assertGet*ChainAdapter inside the try, catch any error
and return Err(...) (using Err from `@sniptt/monads`) with the appropriate
SwapErrorRight payload, and keep the existing getFeeData/getFees calls inside
the same try so all failures follow the Result<TradeRate[], SwapErrorRight>
pattern rather than letting adapter assertions reject the endpoint.
---
Nitpick comments:
In `@packages/swapper/src/swappers/GardenSwapper/swapperApi/getTradeRate.ts`:
- Around line 115-124: The catch blocks that swallow errors when calling
adapter.getFeeData (the block returning '0' after awaiting adapter.getFeeData
for { to: placeholderTo, value: sellAmount, chainSpecific: { pubkey } }) must
capture the thrown error (catch (err)) and emit a structured log entry with
relevant context (include placeholderTo, sellAmount, pubkey and that this was
fee estimation for utxoFee.fast.txFee) before returning '0'; apply the same
pattern to the other similar try/catch sites around the adapter.getFeeData
usages referenced (lines ~132-141 and ~150-161) so all fee-estimation fallbacks
log error metadata instead of silently returning '0'.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bd9f2478-8548-4e5e-a298-97ff904617b7
⛔ Files ignored due to path filters (9)
public/generated/asset-manifest.jsonis excluded by!**/generated/**public/generated/asset-manifest.json.bris excluded by!**/generated/**public/generated/asset-manifest.json.gzis excluded by!**/*.gz,!**/generated/**public/generated/generatedAssetData.jsonis excluded by!**/generated/**public/generated/generatedAssetData.json.bris excluded by!**/generated/**public/generated/generatedAssetData.json.gzis excluded by!**/*.gz,!**/generated/**public/generated/relatedAssetIndex.jsonis excluded by!**/generated/**public/generated/relatedAssetIndex.json.bris excluded by!**/generated/**public/generated/relatedAssetIndex.json.gzis excluded by!**/*.gz,!**/generated/**
📒 Files selected for processing (1)
packages/swapper/src/swappers/GardenSwapper/swapperApi/getTradeRate.ts
Comment on lines
+111
to
+158
| if (chainNamespace === CHAIN_NAMESPACE.Utxo) { | ||
| const adapter = deps.assertGetUtxoChainAdapter(sellAsset.chainId) | ||
| const pubkey = (input as GetUtxoTradeRateInput).xpub | ||
| if (!pubkey || !placeholderTo) return undefined | ||
| try { | ||
| const utxoFee = await adapter.getFeeData({ | ||
| to: placeholderTo, | ||
| value: sellAmount, | ||
| chainSpecific: { pubkey }, | ||
| sendMax: false, | ||
| }) | ||
| return utxoFee.fast.txFee | ||
| } catch { | ||
| return '0' | ||
| } | ||
| } | ||
|
|
||
| if (chainNamespace === CHAIN_NAMESPACE.Starknet) { | ||
| if (!sendAddress || !placeholderTo) return undefined | ||
| const adapter = deps.assertGetStarknetChainAdapter(sellAsset.chainId) | ||
| const tokenContractAddress = contractAddressOrUndefined(sellAsset.assetId) | ||
| try { | ||
| const starknetFee = await adapter.getFeeData({ | ||
| to: placeholderTo, | ||
| value: sellAmount, | ||
| chainSpecific: { from: sendAddress, tokenContractAddress }, | ||
| sendMax: false, | ||
| }) | ||
| return starknetFee.fast.txFee | ||
| } catch { | ||
| return '0' | ||
| } | ||
| } | ||
|
|
||
| if (chainNamespace === CHAIN_NAMESPACE.Evm) { | ||
| if (!sendAddress || !placeholderTo) return undefined | ||
| const adapter = deps.assertGetEvmChainAdapter(sellAsset.chainId as EvmChainId) | ||
| const contractAddress = contractAddressOrUndefined(sellAsset.assetId) | ||
| const data = evm.getErc20Data(placeholderTo, sellAmount, contractAddress) | ||
| try { | ||
| const feeData = await evm.getFees({ | ||
| adapter, | ||
| data: data || '0x', | ||
| to: contractAddress ?? placeholderTo, | ||
| value: contractAddress ? '0' : sellAmount, | ||
| from: sendAddress, | ||
| supportsEIP1559: (input as GetEvmTradeRateInput).supportsEIP1559, | ||
| }) |
Contributor
There was a problem hiding this comment.
Move the adapter assertions inside the guarded path.
Line 112, Line 130, and Line 147 call assertGet*ChainAdapter() before the try. If adapter resolution throws, this endpoint rejects instead of returning Err(...), which breaks the Result<TradeRate[], SwapErrorRight> contract.
Proposed fix
if (chainNamespace === CHAIN_NAMESPACE.Utxo) {
- const adapter = deps.assertGetUtxoChainAdapter(sellAsset.chainId)
const pubkey = (input as GetUtxoTradeRateInput).xpub
if (!pubkey || !placeholderTo) return undefined
try {
+ const adapter = deps.assertGetUtxoChainAdapter(sellAsset.chainId)
const utxoFee = await adapter.getFeeData({
to: placeholderTo,
value: sellAmount,
@@
if (chainNamespace === CHAIN_NAMESPACE.Starknet) {
if (!sendAddress || !placeholderTo) return undefined
- const adapter = deps.assertGetStarknetChainAdapter(sellAsset.chainId)
const tokenContractAddress = contractAddressOrUndefined(sellAsset.assetId)
try {
+ const adapter = deps.assertGetStarknetChainAdapter(sellAsset.chainId)
const starknetFee = await adapter.getFeeData({
to: placeholderTo,
value: sellAmount,
@@
if (chainNamespace === CHAIN_NAMESPACE.Evm) {
if (!sendAddress || !placeholderTo) return undefined
- const adapter = deps.assertGetEvmChainAdapter(sellAsset.chainId as EvmChainId)
const contractAddress = contractAddressOrUndefined(sellAsset.assetId)
const data = evm.getErc20Data(placeholderTo, sellAmount, contractAddress)
try {
+ const adapter = deps.assertGetEvmChainAdapter(sellAsset.chainId as EvmChainId)
const feeData = await evm.getFees({
adapter,
data: data || '0x',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/swapper/src/swappers/GardenSwapper/swapperApi/getTradeRate.ts`
around lines 111 - 158, The adapter assertions (deps.assertGetUtxoChainAdapter,
deps.assertGetStarknetChainAdapter, deps.assertGetEvmChainAdapter) must be moved
inside each guarded try block so adapter-resolution failures are handled as
Result errors instead of throwing; for each branch (CHAIN_NAMESPACE.Utxo,
.Starknet, .Evm) call the assertGet*ChainAdapter inside the try, catch any error
and return Err(...) (using Err from `@sniptt/monads`) with the appropriate
SwapErrorRight payload, and keep the existing getFeeData/getFees calls inside
the same try so all failures follow the Result<TradeRate[], SwapErrorRight>
pattern rather than letting adapter assertions reject the endpoint.
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.
Garden Swapper Integration — BTC ↔ wrapped BTC
Description
Adds Garden Finance as a new swapper, providing native BTC ↔ wrapped BTC routes across 11 chains via Garden's HTLC + intent-solver model. Mirrors the deposit-to-address pattern already used by
NearIntentsSwapper, with per-chain initiate flavors for UTXO (Bitcoin / Litecoin), Starknet, and EVM sources.The integration includes the strkBTC Starknet-native BTC peg (Garden's flagship Starknet asset, launched May 2026), which surfaced three additional display / status bugs that are fixed in the final commits on this branch.
Issue
closes #12354
Supported assets
Per
packages/swapper/src/swappers/GardenSwapper/gardenAssetRegistry.ts(24 entries across 11 chains):arbitrum:wbtcarbitrum:ibtcbase:cbbtcbase:cbltcbitcoin:btcbnbchain:btcbethereum:usdtethereum:wbtcethereum:cbbtcethereum:usdchyperevm:ubtclitecoin:ltcmegaeth:btc.bmonad:monmonad:usdcsolana:solsolana:cashsolana:usdcsolana:cbbtcstarknet:wbtcstarknet:strkbtctron:usdtSource-chain support is asymmetric (see
INTEGRATION.md § Source-chain support is asymmetric):GardenSupportedSourceChainIds= Bitcoin, Litecoin, Ethereum, Base, BNB Chain, Arbitrum, Monad, HyperEVM, MegaETH, Starknet. Solana and Tron are destination-only until their gaslesstyped_dataflows are wired (tracked separately).Garden also supports
botanix:btcandcitrea:cbtc(BTC-pegged chains). They are not in our registry yet — future scope.Operations setup⚠️
Garden requires a partner account for API access. Before enabling in production:
garden-app-idAPI keyVITE_GARDEN_API_KEY=<key>in.env.productionVITE_FEATURE_GARDEN_SWAP=trueonly after governance sign-off given Garden's security history (see INTEGRATION.md § Trust & security context)The same key is used for
GET /quote,POST /orders,GET /orders/{id}, andGET /apps/earnings(affiliate fee claim, treasury-side weekly batch).Affiliate fees are paid to
DAO_TREASURY_BASEinbase:cbbtc, denominated in BTC to avoid Ethereum mainnet withdrawal friction (see INTEGRATION.md § Affiliate fee asset constraint).Risk
Medium risk. Behind a feature flag (
VITE_FEATURE_GARDEN_SWAP=falseby default in.env). New on-chain transactions:Garden had a ~$10M solver-layer exploit on 2025-10-30 — per Garden's post-mortem (with EY) the HTLC contracts were not compromised and no user funds were affected. Trail of Bits audited the contracts (Nov 2025). Pre-hack, ZachXBT raised concerns about volume composition — these are surfaced for governance sign-off.
Testing
Engineering
VITE_GARDEN_API_KEYin.env.development.localVITE_FEATURE_GARDEN_SWAP=truein.env.developmentpnpm devFor local-only validation without mainnet funds, use Garden's Docker localnet
merry(see INTEGRATION.md § Local end-to-end testing with Merry). Note: Merry does not include Starknet, so strkBTC ↔ BTC specifically can only be exercised on mainnet.Operations
Once
VITE_FEATURE_GARDEN_SWAP=trueis enabled in production after governance sign-off, ops validation should cover:Confirmed(redeem tx hash present)refund_tx_hash)GET /apps/earningsE2E test — strkBTC → BTC (mainnet) ✅
Performed with a test wallet on a Portless dev environment. Public artifacts only below.
starknet:strkbtc(Garden token0x0787150e306e6eae…)bitcoin:btc0.00031648 strkBTC(~$24.22)0.00031363 BTC(~$24.06)1 strkBTC = 0.99099469 BTC0.33751324 STRK(~$0.0137)SUCCEEDEDUI flow validated
Swap via Garden in ~20sestimated completion.Awaiting swap via Gardenstate.Your swap of 0.000316 strkBTC to 0.000314 BTC is being processed.filled_amount="0"guard added in the final commit, the post-swap toast now correctly falls back toexpectedBuyAmountCryptoPrecisionwhen Garden returnsredeem_tx_hashbeforefilled_amountis finalized.My Crypto / WBTC family expansion
relatedAssetKey).Final-PR bug fixes summary
Three issues surfaced and fixed during the final E2E pass (last commits on this branch):
relatedAssetIndex.jsonwas not regenerated after strkBTC was injected intogeneratedAssetData.json→ strkBTC was orphaned from the wBTC family at runtime. Fix: added strkBTC tomanualRelatedAssetIndexin the pipeline source (scripts/generateAssetData/generateRelatedAssetIndex/generateRelatedAssetIndex.ts), regenerated the JSON +.br+.gzsiblings, bumped the content hash inasset-manifest.json, and bumped twoclearAssetsMigrationsentries to evict stale localStorage.Single-variant family rows displayed the canonical primary's identity instead of the variant the user actually held. Fix: in
selectGroupedAssetsWithBalances(src/state/slices/portfolioSlice/selectors.ts), added a primary sort key oncryptoAmount > 0so held variants surface first regardless of whether their market-data price has resolved viarelatedAssetKeyfallback.mapGardenOrderToTxStatuspropagatedfilled_amount: "0"asactualBuyAmountCryptoBaseUnitwhen Garden returnsredeem_tx_hashbefore the destination amount is finalized → the post-swap toast displayedto 0 BTC is complete(the??fallback toexpectedBuyAmountCryptoPrecisiononly triggers onundefined, not on the string"0"). Fix: omitactualBuyAmountCryptoBaseUnitwhenfilled_amountis"0"or empty, with two unit-test cases covering the edge.Quality gates
pnpm exec eslint: clean on all touched filespnpm run type-check: 0 errorsfilled_amount="0"edge)feature-dev:code-reviewersubagent) — all HIGH and MEDIUM findings addressed before commitScreenshots
End-to-end swap strkBTC → BTC via Garden (mainnet, test wallet, ~$24):
1. Quote view — strkBTC selected, Garden as protocol
2. Confirm Details — Swap via Garden in ~20s
3. Trade view — Transaction Fee + Awaiting swap via Garden
4. Sign & Swap — processing toast with expected amounts
5. Trade success — order complete, BTC received
Source tx on Voyager (Starknet): https://voyager.online/tx/0x21a20c8f3128c6f08e572c03a2dee7b01a71abfbb16f2340a92eb88ac12d894
Summary by CodeRabbit
Release Notes
New Features
VITE_FEATURE_GARDEN_SWAPflag.Improvements