feat(chain-adapters): add Aptos chain integration with NEAR Intents support#12349
Open
swdiscordia wants to merge 13 commits into
Open
feat(chain-adapters): add Aptos chain integration with NEAR Intents support#12349swdiscordia wants to merge 13 commits into
swdiscordia wants to merge 13 commits into
Conversation
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds feature-flagged Aptos chain support: CAIP identifiers and assets, HDWallet Ed25519 support, Aptos chain adapter (sign/broadcast/parse/fees) and tests, swapper/NEAR Intents wiring, asset generation/CoinGecko mapping, UI/state/plugin integration, CSP and env configuration. ChangesAptos Chain Integration
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
added 7 commits
May 17, 2026 01:26
- Add hdwallet-core/src/aptos.ts with AptosWallet interfaces - Add SLIP-44 637 for Aptos in utils.ts - Add supportsAptos/infoAptos in wallet.ts - Add hdwallet-native AptosAdapter (Ed25519 + SHA3-256) - Add MixinNativeAptosWallet/MixinNativeAptosWalletInfo - Wire into native.ts (initialize, wipe, describePath, mixins) Local only - no PR
swdiscordia
force-pushed
the
feat/aptos-support
branch
from
d37ebe6 to
f00e21c
Compare
Wires the Aptos chain adapter, HDWallet, and types (added in prior commits) into the full app: asset generation via Panora token list, CoinGecko adapter, plugin indexer URL, NEAR Intents Aptos support (web-xeq.3), RFOX bridge, send modal, trade execution dispatch, state migrations bump (v334), e2e fixtures, and ButterSwap test mock updates for the AptosSwapperDeps ripple. Also fixes audit findings: - getRpcUrl() on AptosChainAdapter to replace unsafe protected cast - VITE_APTOS_INDEXER_URL validation + type declaration - Record<AssetId, ...> typing on test fixture marketDataByAssetIdUsd Refs: web-xeq.1 (chain adapter), web-xeq.3 (NEAR Intents Aptos), web-xeq.4 (qabot fixtures). PanoraSwapper (web-xeq.2) intentionally deferred to a separate PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Aptos node, when called with estimate_max_gas_amount=true, overwrites max_gas_amount in the response with the sender's AFFORDABILITY ceiling (balance / gas_unit_price), NOT a tight recommendation. Reading that field as the gas estimate caused fees to be inflated by 100-300x. For a wallet holding 1.08 APT at gas_unit_price=100, the node returned max_gas_amount=1,084,352. Multiplied by the prioritized gas estimate (150 octas), the displayed network fee came out to 1.62 APT (~\$1.54) for what should be a ~0.01 APT transfer. Fix: prefer sim.gas_used (real simulated consumption) with the Aptos CLI's 1.5x safety factor (gas_used * 3 / 2). Cap at the affordability ceiling defensively. Bump MIN_MAX_GAS_AMOUNT from 12,000 to 20,000 to safely cover real transfer_coins consumption (historical on-chain: 5,500-10,500 units) when simulation fails (e.g., dummy pubkey rejected by INVALID_AUTH_KEY). Refs: - https://aptos.dev/build/guides/system-integrators-guide ("estimate uses min(max_gas_amount, gas_used * safety_factor)", CLI safety_factor = 1.5) - https://aptos.dev/network/blockchain/gas-txn-fee - Direct fullnode simulation confirmed max_gas_amount echoed as account_balance / gas_unit_price when the flag is set. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Aptos Ledger HDWallet integration was deferred to a follow-up PR (static analysis surfaced 5 show-stopper bugs that need hardware testing to validate). Drops the @ledgerhq/hw-app-aptos dependency and its orphan lockfile entries. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
swdiscordia
force-pushed
the
feat/aptos-support
branch
from
f00e21c to
d577262
Compare
![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: 9
🤖 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/chain-adapters/src/aptos/AptosChainAdapter.ts`:
- Around line 197-215: Replace the manual regex-based validation in
validateAddress with the Aptos SDK's AccountAddress.fromString() call: call
AccountAddress.fromString(address) inside validateAddress (in try/catch) and
return valid=true when it succeeds, otherwise catch the thrown error and return
valid=false; ensure you still return the appropriate
ValidAddressResultType.Valid or ValidAddressResultType.Invalid values and keep
the method's Promise-based signature. Use the validateAddress function name and
AccountAddress.fromString symbol to locate where to change the logic.
- Around line 504-505: The current status assignment in AptosChainAdapter.ts
uses tx.success falsiness and treats undefined as Confirmed; change the logic in
the method where `tx` is processed (the line assigning `const status =
tx.success === false ? TxStatus.Failed : TxStatus.Confirmed`) to explicitly
handle three cases: if `tx.success === true` set TxStatus.Confirmed; if
`tx.success === false` set TxStatus.Failed; otherwise (undefined) set
TxStatus.Pending, and ensure any fields derived from confirmation (confirmedAt,
confirmations count) are only populated when status is Confirmed.
- Around line 521-523: Replace direct string comparisons of Aptos addresses with
AccountAddress.equals(): import AccountAddress from '`@aptos-labs/ts-sdk`',
construct AccountAddress instances for the values created in AptosChainAdapter
(e.g., for recipient and sender variables defined in the block with const
recipient = String(...), const sender = tx.sender ?? ''), and compare using
AccountAddress.fromHex(recipient).equals(AccountAddress.fromHex(sender)) (or the
appropriate AccountAddress.fromX factory used in the project) wherever the code
currently does string equality (notably the comparisons referenced around the
recipient/amount/sender handling at the later checks), so address format/casing
differences are handled reliably.
In `@packages/hdwallet-native/src/aptos.ts`:
- Around line 19-21: Replace the thrown Error in aptosNextAccountPath with a
delegation to the core helper: call and return core.aptosNextAccountPath(_msg)
(or the appropriate core helper that computes the next Aptos account path) so
the mixin uses the shared logic instead of throwing.
In `@packages/swapper/src/swappers/NearIntentsSwapper/endpoints.ts`:
- Around line 399-405: The current call to adapter.buildSendApiTransaction(...)
passes an empty chainSpecific causing Aptos transfers to default to APT; detect
when the route/asset is a non-native Aptos token and populate chainSpecific with
the coin type string (the token's coin type identifier) before calling
adapter.buildSendApiTransaction (similar to how tokenId is passed for Solana).
Update the call site that invokes buildSendApiTransaction to supply
chainSpecific: { coinType: <asset.coinType or route.token.coinType> } for
non-native Aptos assets while keeping it empty for native APT.
In `@packages/swapper/src/utils.ts`:
- Around line 535-537: The receive-transfer detection uses
t.to.includes(address) which can produce substring matches; change
receiveTransfer lookup in the tx.transfers scan (function/variable
receiveTransfer, TransferType.Receive) to perform an exact Aptos address
equality by normalizing both sides (ensure consistent 0x prefix and lowercase)
and comparing with === (or array membership on a normalized array if t.to is an
array), so you only match the exact recipient address.
In `@packages/utils/src/treasury.ts`:
- Around line 22-23: treasuryChainIds currently includes
KnownChainIds.AptosMainnet while the corresponding Aptos entry in the treasury
address mapping uses a placeholder/dead address; either remove
KnownChainIds.AptosMainnet from the treasuryChainIds array until you have the
real treasury address, or update the Aptos address constant in the
TREASURY_ADDRESSES (or equivalent Aptos address constant) to the correct live
treasury address; locate the treasuryChainIds array and the Aptos address
constant in this module and make the change so an enabled chain never points at
a placeholder address.
In `@src/context/WalletProvider/Ledger/constants.ts`:
- Line 45: availableLedgerAppAssetIds currently includes aptosAssetId when
VITE_FEATURE_APTOS is enabled but LedgerHDWallet does not implement Aptos
support (LedgerHDWallet, _supportsAptos, supportsAptos), causing false
positives; remove the conditional inclusion of aptosAssetId from the
availableLedgerAppAssetIds array (i.e., delete the
...(getConfig().VITE_FEATURE_APTOS ? [aptosAssetId] : []), entry) so Ledger is
no longer advertised as supporting Aptos until LedgerHDWallet implements Aptos
support and sets _supportsAptos = true and/or implements the AptosWallet
interface.
In `@src/lib/utils/aptos.ts`:
- Around line 9-12: The isAptosChainAdapter type guard calls getChainId without
ensuring that method exists, which can throw for plain objects; update
isAptosChainAdapter to first check that chainAdapter is non-null object and that
(chainAdapter as any).getChainId is a function (e.g., typeof (chainAdapter as
any).getChainId === 'function') before invoking it, and return false if not
present; you can also wrap the call in a try/catch to return false on unexpected
errors so assertGetAptosChainAdapter behavior remains predictable.
🪄 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: 973ad940-e479-4457-b4db-52b939ead6a4
⛔ Files ignored due to path filters (3)
packages/caip/src/adapters/coingecko/generated/aptos_861fb8e6/adapter.jsonis excluded by!**/generated/**packages/caip/src/adapters/coingecko/generated/index.tsis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (79)
.env.env.development.env.productione2e/fixtures/aptos-chain-integration.yamlheaders/csps/chains/aptos.tsheaders/csps/index.tspackages/caip/src/adapters/coingecko/index.tspackages/caip/src/adapters/coingecko/utils.tspackages/caip/src/constants.tspackages/chain-adapters/package.jsonpackages/chain-adapters/src/aptos/AptosChainAdapter.test.tspackages/chain-adapters/src/aptos/AptosChainAdapter.tspackages/chain-adapters/src/aptos/index.tspackages/chain-adapters/src/aptos/types.tspackages/chain-adapters/src/index.tspackages/chain-adapters/src/types.tspackages/hdwallet-core/src/aptos.tspackages/hdwallet-core/src/index.tspackages/hdwallet-core/src/utils.tspackages/hdwallet-core/src/wallet.tspackages/hdwallet-native/src/aptos.tspackages/hdwallet-native/src/crypto/isolation/adapters/aptos.tspackages/hdwallet-native/src/native.tspackages/public-api/src/swapperDeps.tspackages/swapper/package.jsonpackages/swapper/src/swappers/ButterSwap/swapperApi/getTradeQuote.test.tspackages/swapper/src/swappers/ButterSwap/swapperApi/getTradeRate.test.tspackages/swapper/src/swappers/NearIntentsSwapper/NearIntentsSwapper.tspackages/swapper/src/swappers/NearIntentsSwapper/endpoints.tspackages/swapper/src/swappers/NearIntentsSwapper/swapperApi/getTradeQuote.tspackages/swapper/src/swappers/NearIntentsSwapper/swapperApi/getTradeRate.tspackages/swapper/src/swappers/NearIntentsSwapper/types.tspackages/swapper/src/swappers/utils/helpers/helpers.tspackages/swapper/src/swappers/utils/test-data/cryptoMarketDataById.tspackages/swapper/src/thorchain-utils/getL1RateOrQuote.tspackages/swapper/src/types.tspackages/swapper/src/utils.tspackages/types/src/base.tspackages/utils/src/assetData/baseAssets.tspackages/utils/src/assetData/getBaseAsset.tspackages/utils/src/chainIdToFeeAssetId.tspackages/utils/src/getAssetNamespaceFromChainId.tspackages/utils/src/getChainShortName.tspackages/utils/src/getNativeFeeAssetReference.tspackages/utils/src/treasury.tsscripts/generateAssetData/aptos/index.tsscripts/generateAssetData/coingecko.tsscripts/generateAssetData/generateAssetData.tsscripts/generateAssetData/generateRelatedAssetIndex/generateChainRelatedAssetIndex.tsscripts/generateAssetData/generateRelatedAssetIndex/generateRelatedAssetIndex.tsscripts/generateAssetData/generateTrustWalletUrl/generateTrustWalletUrl.tssrc/components/Modals/Send/utils.tssrc/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeNetworkFeeCryptoBaseUnit.tsxsrc/components/MultiHopTrade/hooks/useGetTradeQuotes/getTradeQuoteOrRateInput.tssrc/components/TradeAssetSearch/hooks/useGetPopularAssetsQuery.tsxsrc/config.tssrc/constants/chains.tssrc/context/PluginProvider/PluginProvider.tsxsrc/context/WalletProvider/Ledger/constants.tssrc/hooks/useActionCenterSubscribers/useSendActionSubscriber.tsxsrc/hooks/useWalletSupportsChain/useWalletSupportsChain.tssrc/lib/account/account.tssrc/lib/account/aptos.tssrc/lib/asset-service/service/AssetService.tssrc/lib/coingecko/constants.tssrc/lib/coingecko/utils.tssrc/lib/tradeExecution.tssrc/lib/utils/aptos.tssrc/pages/Markets/components/MarketsRow.tsxsrc/pages/RFOX/components/Stake/Bridge/hooks/useRfoxBridge.tssrc/plugins/activePlugins.tssrc/plugins/aptos/index.tsxsrc/state/apis/swapper/helpers/swapperApiHelpers.tssrc/state/slices/opportunitiesSlice/mappings.tssrc/state/slices/portfolioSlice/utils/index.tssrc/state/slices/preferencesSlice/preferencesSlice.tssrc/test/mocks/store.tssrc/vite-env.d.ts
Resolves all 9 actionable comments from the CodeRabbit review on the Aptos chain integration PR. Chain adapter (packages/chain-adapters/src/aptos/AptosChainAdapter.ts): - validateAddress now uses AccountAddress.fromString() from the SDK, which correctly accepts both short-form (0x1) and full-form Aptos addresses, instead of a rigid regex that rejected 0x1. - parseTx status logic now handles all three cases explicitly: success === true → Confirmed, false → Failed, undefined (pending tx) → Pending. Previously undefined was incorrectly treated as Confirmed. - Sender/recipient equality now goes through AccountAddress.equals via a normalized eq() helper so casing/length variants of the same Aptos address compare correctly. - buildSendApiTransaction now honors chainSpecific.coinType so non-APT CoinStore coins on Aptos can be transferred via NEAR Intents (or any future swapper). Aptos types (packages/chain-adapters/src/aptos/types.ts): - BuildTxInput gains coinType?: string (Aptos CoinStore type, defaults to APT). NEAR Intents Aptos route (packages/swapper/src/swappers/NearIntentsSwapper/endpoints.ts): - getUnsignedAptosTransaction detects non-native Aptos coins via assetNamespace (slip44 = native APT, otherwise assetReference is the Move CoinStore type) and forwards coinType to the chain adapter. Aptos swap status check (packages/swapper/src/utils.ts): - checkAptosSwapStatus normalizes recipient addresses via AccountAddress before comparing, replacing the substring-style t.to.includes(address) check. Aptos util (src/lib/utils/aptos.ts): - isAptosChainAdapter type guard now checks that getChainId is a function and wraps the call in try/catch so plain objects can't throw. Native HDWallet (packages/hdwallet-native/src/aptos.ts): - aptosNextAccountPath now delegates to core.aptosNextAccountPath instead of throwing. Treasury (packages/utils/src/treasury.ts): - Drop KnownChainIds.AptosMainnet from treasuryChainIds and remove the placeholder DAO_TREASURY_APTOS constant. The Aptos treasury entry will be added when the real DAO multisig address is available. Treasury helpers (packages/swapper/src/swappers/utils/helpers/helpers.ts): - Drop the AptosMainnet entry from DAO_TREASURY_BY_CHAIN_ID. Ledger constants (src/context/WalletProvider/Ledger/constants.ts): - Remove conditional inclusion of aptosAssetId from availableLedgerAppAssetIds. LedgerHDWallet does not implement AptosWallet; Aptos Ledger support will land in a focused follow-up PR with physical-device testing. Tests (packages/chain-adapters/src/aptos/AptosChainAdapter.test.ts): - vi.mock now keeps the real exports of @aptos-labs/ts-sdk (AccountAddress, Ed25519PublicKey, etc.) and only mocks the network-using Aptos client, AptosConfig, and Network. Without this, the new AccountAddress.equals comparisons would throw silently. - validateAddress test cases updated to reflect the SDK's behavior: short-form (0x1) and unprefixed full-form are now accepted; only truly malformed inputs are rejected. Refs: CodeRabbit review on PR shapeshift#12349 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…first verifyLedgerAppOpen() previously called getCoin() and getLedgerAppName() unconditionally before checking the wallet type. Both switches throw "Unsupported chainId" for any chain not enumerated (e.g. Aptos in this PR, since the Aptos Ledger commit was intentionally deferred). Result: AptosChainAdapter.getAddress() threw for Native wallets too because the chain adapter calls verifyLedgerAppOpen() in its address path. Move the isLedger(wallet) early-return to the top of the function so the chain lookups only happen for actual Ledger wallets. Native and other non-Ledger wallets bypass the gate entirely. Side benefit: future chain additions to the app don't need a simultaneous ledgerAppGate.ts update unless they actually ship Ledger support. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/chain-adapters/src/aptos/AptosChainAdapter.ts`:
- Around line 249-255: The send/build path currently calls estimateMaxGasAmount
and hard-fails if simulation errors; update the build flow in
buildSendApiTransaction (the code that calls this.estimateMaxGasAmount and
this.client.transaction.build.simple) to catch errors from estimateMaxGasAmount
and fall back to using MIN_MAX_GAS_AMOUNT (the same constant used by getFeeData)
before calling this.client.transaction.build.simple so that transient simulation
failures do not abort transaction building.
In `@packages/swapper/src/swappers/NearIntentsSwapper/endpoints.ts`:
- Around line 382-424: Replace direct throws in getUnsignedAptosTransaction and
getAptosTransactionFees with swapper Result-style errors: instead of throwing
when !isExecutableTradeQuote(tradeQuote) or when nearIntentsSpecific is missing
or when step.feeData.networkFeeCryptoBaseUnit is missing, return
makeSwapErrorRight(...) populated with the appropriate TradeQuoteError enum
value (e.g., TradeQuoteError.UnableToExecuteQuote or
TradeQuoteError.MissingNetworkFee) and include contextual info (stepIndex,
sellAsset.assetId, tradeQuote id/metadata). Use the Result helpers (Ok/Err or
Promise.resolve(Err(...)) as the surrounding code expects) and reference the
existing helpers isExecutableTradeQuote, getExecutableTradeStep,
makeSwapErrorRight, and TradeQuoteError to locate where to change the error
paths.
🪄 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: f4fb07f3-ccd7-4def-875b-4cf1b7f8ae5b
📒 Files selected for processing (8)
packages/chain-adapters/src/aptos/AptosChainAdapter.test.tspackages/chain-adapters/src/aptos/AptosChainAdapter.tspackages/chain-adapters/src/aptos/types.tspackages/chain-adapters/src/utils/ledgerAppGate.tspackages/hdwallet-native/src/aptos.tspackages/swapper/src/swappers/NearIntentsSwapper/endpoints.tspackages/swapper/src/utils.tssrc/lib/utils/aptos.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/chain-adapters/src/aptos/types.ts
- packages/swapper/src/utils.ts
- src/lib/utils/aptos.ts
- packages/chain-adapters/src/aptos/AptosChainAdapter.test.ts
Comment on lines
+249
to
+255
| const maxGasAmount = Number(await this.estimateMaxGasAmount(from, data)) | ||
|
|
||
| const transaction = await this.client.transaction.build.simple({ | ||
| sender: from, | ||
| data, | ||
| options: { maxGasAmount }, | ||
| }) |
Contributor
There was a problem hiding this comment.
Add a fallback max gas in send build path to avoid hard-failing on simulation errors.
Line 249 currently makes buildSendApiTransaction fail if gas simulation fails transiently. You already use MIN_MAX_GAS_AMOUNT fallback in getFeeData; the send path should mirror that resilience.
Suggested fix
- const maxGasAmount = Number(await this.estimateMaxGasAmount(from, data))
+ let maxGasAmount = Number(MIN_MAX_GAS_AMOUNT)
+ try {
+ maxGasAmount = Number(await this.estimateMaxGasAmount(from, data))
+ } catch {
+ maxGasAmount = Number(MIN_MAX_GAS_AMOUNT)
+ }🤖 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/chain-adapters/src/aptos/AptosChainAdapter.ts` around lines 249 -
255, The send/build path currently calls estimateMaxGasAmount and hard-fails if
simulation errors; update the build flow in buildSendApiTransaction (the code
that calls this.estimateMaxGasAmount and this.client.transaction.build.simple)
to catch errors from estimateMaxGasAmount and fall back to using
MIN_MAX_GAS_AMOUNT (the same constant used by getFeeData) before calling
this.client.transaction.build.simple so that transient simulation failures do
not abort transaction building.
Comment on lines
+382
to
+424
| getUnsignedAptosTransaction: ({ | ||
| stepIndex, | ||
| tradeQuote, | ||
| from, | ||
| assertGetAptosChainAdapter, | ||
| }: GetUnsignedAptosTransactionArgs) => { | ||
| if (!isExecutableTradeQuote(tradeQuote)) throw new Error('Unable to execute a trade rate quote') | ||
|
|
||
| const step = getExecutableTradeStep(tradeQuote, stepIndex) | ||
|
|
||
| const { accountNumber, sellAsset, nearIntentsSpecific } = step | ||
| if (!nearIntentsSpecific) throw new Error('nearIntentsSpecific is required') | ||
|
|
||
| const adapter = assertGetAptosChainAdapter(sellAsset.chainId) | ||
|
|
||
| const to = nearIntentsSpecific.depositAddress | ||
| const value = step.sellAmountIncludingProtocolFeesCryptoBaseUnit | ||
|
|
||
| // Native APT lives at the slip44 namespace; non-native Aptos coins encode | ||
| // their Move CoinStore type as the asset reference and must be passed | ||
| // through to the adapter so the typeArguments target the right coin. | ||
| const { assetNamespace, assetReference } = fromAssetId(sellAsset.assetId) | ||
| const chainSpecific = | ||
| assetNamespace === ASSET_NAMESPACE.slip44 ? {} : { coinType: assetReference } | ||
|
|
||
| return adapter.buildSendApiTransaction({ | ||
| to, | ||
| from, | ||
| value, | ||
| accountNumber, | ||
| chainSpecific, | ||
| }) | ||
| }, | ||
|
|
||
| getAptosTransactionFees: ({ tradeQuote, stepIndex }: GetUnsignedAptosTransactionArgs) => { | ||
| 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.
Use swapper Result-based errors instead of throwing raw Error.
Line 388, Line 393, Line 417, and Line 421 throw directly in newly added swapper API handlers. In this layer, these should return swapper-typed errors (makeSwapErrorRight + TradeQuoteError) with context, not uncaught exceptions.
As per coding guidelines, **/*.{ts,tsx}: “Use Result<T, E> pattern for error handling in swappers and APIs; ALWAYS use Ok() and Err() from @sniptt/monads; AVOID throwing within swapper API implementations” and **/swapper{s,}/**/*.{ts,tsx}: “ALWAYS use makeSwapErrorRight for swapper errors with TradeQuoteError enum for error codes and provide detailed error information`.
🤖 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/NearIntentsSwapper/endpoints.ts` around lines
382 - 424, Replace direct throws in getUnsignedAptosTransaction and
getAptosTransactionFees with swapper Result-style errors: instead of throwing
when !isExecutableTradeQuote(tradeQuote) or when nearIntentsSpecific is missing
or when step.feeData.networkFeeCryptoBaseUnit is missing, return
makeSwapErrorRight(...) populated with the appropriate TradeQuoteError enum
value (e.g., TradeQuoteError.UnableToExecuteQuote or
TradeQuoteError.MissingNetworkFee) and include contextual info (stepIndex,
sellAsset.assetId, tradeQuote id/metadata). Use the Result helpers (Ok/Err or
Promise.resolve(Err(...)) as the surrounding code expects) and reference the
existing helpers isExecutableTradeQuote, getExecutableTradeStep,
makeSwapErrorRight, and TradeQuoteError to locate where to change the error
paths.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@scripts/generateAssetData/aptos/index.ts`:
- Around line 24-26: The code calls uniqBy on tokensOnly before prepending the
native APT (unfreeze(aptos)), so a duplicate assetId from CoinGecko can still
appear; change the flow to build the combined array first and then deduplicate
by assetId (i.e., produce [unfreeze(aptos), ...tokensOnly] and run uniqBy(...,
'assetId') on that combined array), replacing the current use of allAssets and
returning the deduped result.
- Around line 13-21: Replace the simple console.error(result.reason) with a
structured log that includes context (e.g., the batch item/index, the operation
name) and the full error object/stack (capture result.reason and any surrounding
metadata) so you log both message and error details for debugging; also rename
the regex constant nativeAptCoinPattern to UPPER_SNAKE_CASE
NATIVE_APT_COIN_PATTERN (preserve the same regex value
/^aptos:[^/]+\/aptosCoin:(0x0*a|0x1::aptos_coin::AptosCoin)$/i) to follow
constant naming conventions and update any references accordingly.
🪄 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: 9ce3f83c-150c-410c-9489-3afbe8757357
📒 Files selected for processing (2)
scripts/generateAssetData/aptos/index.tsscripts/generateAssetData/coingecko.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/generateAssetData/coingecko.ts
Comment on lines
+13
to
+21
| console.error(result.reason) | ||
| return [] | ||
| }) | ||
|
|
||
| // Filter out the native APT token from CoinGecko to avoid duplicates | ||
| // CoinGecko includes native APT both as the slip44 base asset (added manually) and | ||
| // as the coin-standard token (0x1::aptos_coin::AptosCoin) at the same metadata | ||
| // address 0xa under the aptosCoin namespace. | ||
| const nativeAptCoinPattern = /^aptos:[^/]+\/aptosCoin:(0x0*a|0x1::aptos_coin::AptosCoin)$/i |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use structured error context and constant naming conventions.
At Line 13, logging only result.reason makes troubleshooting harder in batch scripts. At Line 21, the regex constant should follow UPPER_SNAKE_CASE.
♻️ Proposed fix
- console.error(result.reason)
+ console.error('Failed to fetch Aptos assets from CoinGecko', {
+ chainId: aptosChainId,
+ reason: result.reason,
+ })
return []
})
@@
- const nativeAptCoinPattern = /^aptos:[^/]+\/aptosCoin:(0x0*a|0x1::aptos_coin::AptosCoin)$/i
- const tokensOnly = assets.filter(asset => !nativeAptCoinPattern.test(asset.assetId))
+ const NATIVE_APT_COIN_PATTERN = /^aptos:[^/]+\/aptosCoin:(0x0*a|0x1::aptos_coin::AptosCoin)$/i
+ const tokensOnly = assets.filter(asset => !NATIVE_APT_COIN_PATTERN.test(asset.assetId))As per coding guidelines, "ALWAYS log errors for debugging using structured logging with relevant context and error metadata" and "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 `@scripts/generateAssetData/aptos/index.ts` around lines 13 - 21, Replace the
simple console.error(result.reason) with a structured log that includes context
(e.g., the batch item/index, the operation name) and the full error object/stack
(capture result.reason and any surrounding metadata) so you log both message and
error details for debugging; also rename the regex constant nativeAptCoinPattern
to UPPER_SNAKE_CASE NATIVE_APT_COIN_PATTERN (preserve the same regex value
/^aptos:[^/]+\/aptosCoin:(0x0*a|0x1::aptos_coin::AptosCoin)$/i) to follow
constant naming conventions and update any references accordingly.
Comment on lines
+24
to
+26
| const allAssets = uniqBy(tokensOnly, 'assetId') | ||
|
|
||
| return [unfreeze(aptos), ...allAssets] |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Guarantee final assetId uniqueness after prepending native APT.
uniqBy() runs before adding unfreeze(aptos), so duplicates can still leak if CoinGecko returns the same native assetId. Deduplicate the final merged list instead.
✅ Proposed fix
- const allAssets = uniqBy(tokensOnly, 'assetId')
-
- return [unfreeze(aptos), ...allAssets]
+ return uniqBy([unfreeze(aptos), ...tokensOnly], 'assetId')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const allAssets = uniqBy(tokensOnly, 'assetId') | |
| return [unfreeze(aptos), ...allAssets] | |
| return uniqBy([unfreeze(aptos), ...tokensOnly], 'assetId') |
🤖 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 `@scripts/generateAssetData/aptos/index.ts` around lines 24 - 26, The code
calls uniqBy on tokensOnly before prepending the native APT (unfreeze(aptos)),
so a duplicate assetId from CoinGecko can still appear; change the flow to build
the combined array first and then deduplicate by assetId (i.e., produce
[unfreeze(aptos), ...tokensOnly] and run uniqBy(..., 'assetId') on that combined
array), replacing the current use of allAssets and returning the deduped result.
Replaces the bespoke Panora-based fetcher with the same CoinGecko
tokenlist pattern used by every other non-EVM chain (Solana, Sui,
Near, Ton, Tron). Removes the VITE_PANORA_API_KEY requirement from
the asset generation pipeline.
CoinGecko exposes two different platform slugs for Aptos: their
/api/v3 endpoints (and our CoinGecko adapter) use 'aptos-network',
but the static tokenlist at tokens.coingecko.com only responds to
'aptos' ('aptos-network' returns 403 there). Hardcoded the right
slug in scripts/generateAssetData/coingecko.ts to keep the existing
'aptos-network' adapter value untouched for API consumers.
Verified `pnpm run generate:chain aptos` returns 70 assets from
CoinGecko without any API key.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
swdiscordia
force-pushed
the
feat/aptos-support
branch
from
f37f911 to
aa50930
Compare
Contributor
Author
Code reviewFound 2 issues:
web/packages/hdwallet-core/src/aptos.ts Lines 84 to 96 in aa50930
web/src/components/Modals/Send/utils.ts Lines 555 to 570 in aa50930 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
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.
Description
Adds first-class Aptos blockchain support to ShapeShift Web, plus NEAR Intents routing for Aptos so users can swap into/out of APT and Aptos-native assets via solver-paid cross-chain intents.
Scope of this PR (10 commits):
hdwallet-coreAptos interfaces, SLIP-44 637,supportsAptos/infoAptos;hdwallet-nativeEd25519 + SHA3-256AptosAdapterwith mixins wired intonative.ts(initialize, wipe, describePath).AptosChainAdapter(account discovery, balances, BCS-encodedSimpleTransaction.submit.simple, fee estimation, broadcast). 19/19 unit tests pass.VITE_FEATURE_APTOSflag, CSP headers, asset generation from the Panora token list, CoinGecko market data adapter, treasury / base assets, indexer URL.Swapper/SwapperApi.e2e/fixtures/aptos-chain-integration.yamlfor future automated regression.sim.gas_used * 1.5(Aptos CLI safety factor) instead ofmax_gas_amount, which the node returns as the sender's affordability ceiling and inflated displayed fees by 100–300×.pnpm-lock.yaml.Beads references: web-xeq.1 (chain adapter), web-xeq.3 (NEAR Intents Aptos route), web-xeq.4 (qabot fixtures).
Issue (if applicable)
closes #12348
Risk
This is a HIGH-RISK PR and should carry the
High-Risklabel + 2 reviewer approvals per the template warning. It introduces a brand-new on-chain transaction type and signing path:SimpleTransaction.submit.simple(signed payloads, sequence numbers, gas params).hdwallet-nativewith SLIP-44 637 wired throughcore,native, mixins, and the wallet info interface (supportsAptos,infoAptos).AptosChainAdapter: account discovery, fee estimation, signing, broadcast, balance reads — fresh code, not reusing an existing adapter.tradeExecution.ts; Aptos swapper types threaded throughSwapper/SwapperApi.Risk is contained by feature flag (see Operations) —
VITE_FEATURE_APTOS=falsein.env.production, so prod users do not see Aptos until the flag is flipped.SimpleTransactionBCS-signed transfers. Existing tx types unchanged.hdwallet-core(new Aptos interfaces) andhdwallet-native(new adapter + mixins +native.tsinit/wipe/describePath wiring). Wallets without Aptos support gracefully hide the chain viasupportsAptos(wallet) === false.0x1::aptos_account::transfer_coinsfor native sends; cross-chain swaps go through the NEAR Intents solver (no direct Aptos contract write from the swapper).Testing
Engineering
Local setup:
feat/aptos-supportandpnpm install.VITE_FEATURE_APTOS=trueis already set in.env.development— no extra config needed.pnpm dev.APT→ verify the entry shows Aptos branding (network icon + name).Automated checks (all green on this branch):
pnpm run type-check— 0 errors.pnpm run lint— 0 errors (9 pre-existing warnings unrelated to this PR).pnpm exec vitest run packages/chain-adapters/src/aptos— 19 / 19 pass.e2e/fixtures/aptos-chain-integration.yamlis included for futureqabotruns (web-xeq.4).Manual end-to-end verification performed by the author on a real Aptos-funded wallet (Native):
0.04119354 APT).Operations
Ship state of feature flags:
.env.production:VITE_FEATURE_APTOS=false.env.development:VITE_FEATURE_APTOS=truePost-flag-flip operations checklist (when ops is ready to enable Aptos in prod):
fix(aptos)commit).AptosSwapperDepsplumbing or the v334 migrations.Screenshots (if applicable)
Accounts — Aptos in Native wallet
Test wallet showing the Aptos account discovered with balance.
Trade — USDC Arbitrum → APT Aptos via NEAR Intents
Quote loaded, route shows NEAR Intents.
Confirm Details
Swap via NEAR Intents in ~40s.
Sign & Swap
Estimated completion 40s, transaction fee + receive address visible.
Processing
"Your swap of 1 USDC to 1.043159 APT is being processed."
Success — full cross-chain swap proof (both legs on-chain)
NEAR Intents is an off-chain intent-matching protocol, so the "trade" itself does not appear as a single on-chain event — it materialises as (1) a USDC send on Arbitrum to a NEAR Intents deposit address and (2) a corresponding APT receive on Aptos delivered by a solver. Both legs are visible and linked by amount + timing:
Leg 1 — Source: USDC send on Arbitrum (signed by the test wallet)
0xd3c5f4698a85a6541eebe7246e97fac51e6a7901b5b5f147ed0c53e1b6e6265aUSDC.transfer(0x2bb42737cabf30d78476a46b896b0a6ef021660d, 1_000_000)— exactly 1 USDC to a NEAR Intents intent-deposit address0xBb3F…1A82Leg 2 — Destination: APT receive on Aptos (fulfilled by a solver ~15s later)
0x41e1c62345fbfc0397998b0191c4501ede1475f3446f89e2abc59f9beb0571b50x1::aptos_account::transfer_coins(AptosCoin, 0x304ba231…, 104_315_877)— exactly 1.04315877 APT to the test wallet's Aptos account0xd1a1c180…1af6446(5,320 prior txs, alltransfer_coins/transfer_fungible_assets)https://fullnode.mainnet.aptoslabs.com/v1/transactions/by_version/5300181646The output amount 1.04315877 APT matches exactly the 1.043159 APT quoted in the Sign & Swap screen above. Combined with the 15-second source→destination gap, this proves the NEAR Intents solver path round-tripped successfully end-to-end.
Known unrelated issues
The pre-trade warning "Make sure you have ETH in your Arbitrum One account, you'll need it for transactions" misfires for NEAR Intents trades that originate via deposit-to-address (the solver pays destination gas, and the user only needs the source asset on the source chain — not gas on the destination). Origin:
src/components/MultiHopTrade/components/TradeInput/components/ConfirmSummary.tsx:270-284. This is not Aptos-specific — it also affects THORChain / Chainflip / NEAR Intents flows generally. Will be addressed in a separate follow-up PR; out of scope here.Summary by CodeRabbit
New Features
Bug Fixes