feat: sui support excluding tokens #11238
Conversation
|
Warning Rate limit exceeded@gomesalexandre has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 56 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (14)
📒 Files selected for processing (63)
📝 WalkthroughWalkthroughAdds Sui mainnet support across the app: env/config flags and CSP, a Sui chain adapter and plugin, CAIP/constants and asset plumbing, CoinGecko and asset-generation integration, swapper / NearIntents additions, wallet/account derivation, UI send/trade/fee flows, state and feature-flag wiring, and tests/hooks updates. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI (Send / Trade)
participant Swapper as Swapper API
participant Adapter as Sui Chain Adapter
participant Wallet as Wallet (HDWallet)
participant SuiNet as Sui Network
UI->>Swapper: getUnsignedSuiTransaction(args)
Swapper->>Adapter: buildSendApiTransaction(args)
Adapter-->>Swapper: SuiSignTx (unsigned)
UI->>Wallet: sign SuiSignTx
Wallet->>Adapter: signTransaction (via adapter callbacks)
Adapter-->>Wallet: signed payload
Wallet-->>UI: signed tx
UI->>Adapter: broadcastTransaction(signedTx)
Adapter->>SuiNet: submitTransactionBlock(signedTx)
SuiNet-->>Adapter: txHash
loop Poll tx status
UI->>Adapter: getSuiTransactionStatus(txHash)
Adapter->>SuiNet: getTransactionBlock(showEffects=true)
SuiNet-->>Adapter: effects/status
Adapter-->>UI: TxStatus (Confirmed/Failed/Unknown)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: 4
🧹 Nitpick comments (4)
packages/swapper/src/swappers/NearIntentsSwapper/utils/helpers/helpers.ts (1)
43-57: Consider simplifying the SUI lookup logic.The current implementation checks for SUI in two ways:
- Line 55:
TOKEN_LOOKUP_CHAINS.includes(nearNetwork as any)- Line 57:
asset.chainId === suiChainIdSince
nearNetworkis derived fromchainIdToNearIntentsChain[asset.chainId], these checks are redundant when the mapping includes SUI. This creates an inconsistency with Solana's pattern (which uses only the chainId check).Consider one of these approaches for consistency and clarity:
Option 1: Match Solana's pattern (remove the array check)
-const TOKEN_LOOKUP_CHAINS = ['sui'] as const - export const assetToNearIntentsAsset = async (asset: Asset): Promise<string | null> => { // ... - // NEP-245 chains (BSC, Polygon, Avalanche, Optimism), Solana, and SUI require token lookup + // NEP-245 chains (BSC, Polygon, Avalanche, Optimism, Tron), Solana, and SUI require token lookup // Asset IDs use hashed format that can't be generated from contract addresses const requiresLookup = NEP245_CHAINS.includes(nearNetwork as any) || - TOKEN_LOOKUP_CHAINS.includes(nearNetwork as any) || asset.chainId === solanaChainId || asset.chainId === suiChainIdOption 2: Remove the redundant chainId check
- // NEP-245 chains (BSC, Polygon, Avalanche, Optimism), Solana, and SUI require token lookup + // NEP-245 chains (BSC, Polygon, Avalanche, Optimism, Tron), Solana, and SUI require token lookup // Asset IDs use hashed format that can't be generated from contract addresses const requiresLookup = NEP245_CHAINS.includes(nearNetwork as any) || TOKEN_LOOKUP_CHAINS.includes(nearNetwork as any) || - asset.chainId === solanaChainId || - asset.chainId === suiChainId + asset.chainId === solanaChainId.env.development (1)
80-80: SUI feature flag wiring looks consistent; consider keeping keys ordered
VITE_FEATURE_SUI=truematches the existing feature-flag naming pattern and should integrate cleanly with the preferences/config wiring described in the learnings. dotenv-linter suggests placing this beforeVITE_FEATURE_SWAPPER_FIAT_RAMPSto keep keys ordered, but this is non-functional and can be done opportunistically.src/components/MultiHopTrade/hooks/useGetTradeQuotes/getTradeQuoteOrRateInput.ts (1)
19-20: Sui trade input wiring looks good; consider a dedicated Sui chain-id type laterThe new
CHAIN_NAMESPACE.Suibranch correctly usesassertGetSuiChainAdapterand mirrors the Solana/Tron pattern for derivingsendAddressand constructing the quote/rate input, so behavior-wise this is fine.Type-wise, casting the Sui chainId to
CosmosSdkChainIdis a bit of a stretch (same as the existing Solana branch). When/if aSuiChainId(or similar) alias is introduced in@shapeshiftoss/types, this case would be a good candidate to tighten the typing to reflect Sui explicitly.Also applies to: 210-227
src/lib/utils/sui.ts (1)
5-5: Inconsistent import path.This import uses a relative path from
packages/instead of the@shapeshiftoss/unchained-clientpackage alias. This could cause build issues or module resolution problems.-import { TxStatus } from 'packages/unchained-client/src/types' +import { TxStatus } from '@shapeshiftoss/unchained-client'
gomesalexandre
left a comment
gomesalexandre
left a comment
There was a problem hiding this comment.
few blocking bits, tackling as we speak
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeNetworkFeeCryptoBaseUnit.tsx
Outdated
Show resolved
Hide resolved
packages/swapper/src/swappers/NearIntentsSwapper/utils/helpers/helpers.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/chain-adapters/src/sui/SuiChainAdapter.ts (2)
174-180: Use BigInt for SUI gas parameters.This was flagged in a previous review. The SUI SDK's
setGasBudget()andsetGasPrice()acceptnumber | bigint, and usingBigInt()provides better type alignment with SUI's u64 gas model.
339-339: Remove console.error before production.This debug statement should be removed as the error is already handled by
ErrorHandler. A previous review addressed this, but theconsole.errorremains.} catch (err) { - console.error('[SUI broadcastTransaction] error:', err) return ErrorHandler(err, {
🧹 Nitpick comments (2)
packages/swapper/src/swappers/NearIntentsSwapper/utils/helpers/helpers.ts (1)
43-43: SUI token-lookup wiring matches existing pattern; consider tightening the commentThe new
TOKEN_LOOKUP_CHAINSconstant and the extendedrequiresLookupcondition correctly route Sui (both native and tokens) through the token-lookup path alongside NEP‑245 chains and Solana, which is what we want for hashed asset IDs.The only nit is that the comment now reads as if SUI were itself a NEP‑245 chain. For clarity, you could reword it like this:
- // NEP-245 chains (BSC, Polygon, Avalanche, Optimism, TRON, SUI) and Solana require token lookup + // NEP-245 chains (BSC, Polygon, Avalanche, Optimism, TRON), plus SUI and Solana, require token lookupThis keeps the behavior as-is but makes the intent clearer for future readers.
Also applies to: 51-57
src/state/slices/preferencesSlice/preferencesSlice.ts (1)
21-31: Sui feature flag wiring is consistent; consider persisted-state migrationAdding
Sui: booleantoFeatureFlagsand initializingSui: getConfig().VITE_FEATURE_SUIininitialState.featureFlagsis aligned with the existing feature‑flag pattern and matches the newVITE_FEATURE_SUIenv var.One thing to double‑check: this slice is persisted, so older persisted
preferences.featureFlagsobjects won’t have aSuiproperty. At runtime that just reads asundefined(falsy), so gating still behaves as “off”, but it does mean the persisted shape diverges frominitialState.If you want to strictly follow the migrations guideline for persisted slices, you may want a small migration that adds
Sui: getConfig().VITE_FEATURE_SUIto existingpreferences.featureFlags.Also applies to: 155-166
gomesalexandre
left a comment
gomesalexandre
left a comment
There was a problem hiding this comment.
Retested Monad/SUI after merge fix, CI fix, re-re-regen , looking gucci!
gomesalexandre
force-pushed
the
implement-sui
branch
from
278867d to
3eec9f1
Compare
gomesalexandre
left a comment
gomesalexandre
left a comment
There was a problem hiding this comment.
Re-stamp after squash and confirming gome's happy
Description
Same as TRON:
What this pr doesnt do:
Notes:
Issue (if applicable)
Nothing baby
Risk
Medium, new chain under a flag
Testing
Engineering
Operations
Screenshots (if applicable)
https://jam.dev/c/402c5a3a-57b4-46fb-8666-e1f2b2ccf836
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.