feat: add Ledger app gate to Sui chain adapter

[gomesalexandre]

Description

Add Ledger app verification to Sui chain adapter to ensure the Ledger Sui app is open before wallet operations. This prompts users to open the Sui app on their Ledger device if needed during address derivation and transaction signing.

Adds verifyLedgerAppOpen calls to:

• getAddress - verifies app is open before deriving addresses
• signTransaction - verifies app is open before signing
• signAndBroadcastTransaction - verifies app is open before signing and broadcasting

The Sui app configuration was already present in ledgerAppGate.ts, so only the adapter integration was needed.

Issue

N/A

Testing

• Try and do a SUI Tx without app opened
• Ensure you're prompted to do so

Screenshots

https://jam.dev/c/21b43dd8-720a-4b26-bc0a-19c16f8e4074

Summary by CodeRabbit

• Bug Fixes
  • Added validation to ensure Ledger app is open before performing wallet operations.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The Sui chain adapter now imports verifyLedgerAppOpen and calls it as a pre-flight check after wallet presence is confirmed in getAddress, signTransaction, and signAndBroadcastTransaction to ensure the Ledger app is open before proceeding.

Changes

Cohort / File(s)	Change Summary
Ledger App Verification packages/chain-adapters/src/sui/SuiChainAdapter.ts	Added import of verifyLedgerAppOpen and invoked it in getAddress, signTransaction, and signAndBroadcastTransaction after wallet presence checks to verify the Ledger app is open before executing wallet operations.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

• Single-file change with the same guard added in three methods; review focus: correct import path and placement of calls.

Possibly related PRs

• feat: wire up hdwallet-ledger with SUI support bits  #11250 — Related Sui Ledger changes (BIP44/address derivation and ledger wiring).
• feat: add Ledger app gate for Solana #10590 — Adds verifyLedgerAppOpen pre-checks to the Solana chain adapter.
• feat: more wallet support for second-class chains #11396 — Updates shared ledger gate flow and mappings referenced by Ledger verification checks.

Suggested reviewers

• NeOMakinG
• kaladinlight

Poem

🐰 I hopped to the ledger, gave it a peep,

"Is your Sui app awake? No secrets to keep."
A nibble, a twitch, then the gate opened wide,
Now transactions can hop safely, with me by your side. ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat: add Ledger app gate to Sui chain adapter' directly and accurately summarizes the main change—adding Ledger app verification to the Sui chain adapter.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch capy/cap-7-424c7dc7

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between cbd704e and 7fb1635.

📒 Files selected for processing (1)

• packages/chain-adapters/src/sui/SuiChainAdapter.ts (4 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/chain-adapters/src/sui/SuiChainAdapter.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Install and Cache

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[NeOMakinG]

https://jam.dev/c/2257af7e-8fec-4801-8154-77870e40b73d

Send has been prompting the app opening

Don't we have a way to naturally open that as an automated system instead of having to add this manually everywhere in every adapters?

[gomesalexandre]

Don't we have a way to naturally open that as an automated system instead of having to add this manually everywhere in every adapters?

Probably could abstract that even one more level up yes, though I'm not sure exactly how that would be holistically, since we need to decide which places do need app open if that makes sense.

For second-class EVM chain-adapters like this one though, there should be no-need to duplicate code.
@coderabbitai pls create an issue to leverage base adapter (EvmBaseAdapter) for all the ledger app-gating methods and give it a quick hour or so to leverage base impl (with gating there) and extend with own logic, it's OOP so may as well make good use of it right?

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Create a GitHub issue for refactoring Ledger app gating to base adapter

gh issue create \
  --repo shapeshift/web \
  --title "Refactor: Leverage EvmBaseAdapter for Ledger app gating across all EVM chain adapters" \
  --body "## Context

In PR #11450, Ledger app verification was added to the Sui chain adapter by manually calling \`verifyLedgerAppOpen\` in multiple methods (\`getAddress\`, \`signTransaction\`, \`signAndBroadcastTransaction\`).

For chain adapters that extend \`EvmBaseAdapter\`, this creates code duplication. We should leverage OOP principles to move the Ledger app gating logic to the base adapter implementation.

## Objective

Move Ledger app-gating logic to \`EvmBaseAdapter\` so that:
- Child adapters automatically inherit the gating behavior
- Child adapters can extend with their own logic as needed
- Code duplication is eliminated across second-class EVM chain adapters

## Proposed Changes

1. Add \`verifyLedgerAppOpen\` calls to the base implementation in \`EvmBaseAdapter\` for:
   - \`getAddress\`
   - \`signTransaction\`
   - \`signAndBroadcastTransaction\`
   
2. Remove duplicate \`verifyLedgerAppOpen\` calls from child adapter implementations that extend \`EvmBaseAdapter\`

3. Ensure child adapters can still extend with their own logic when needed

## Effort Estimate

Approximately 1 hour

## References

- PR #11450: https://github.com/shapeshift/web/pull/11450
- Comment: https://github.com/shapeshift/web/pull/11450#discussion_r1887234567

Requested by: @gomesalexandre" \
  --assignee gomesalexandre

Length of output: 46

---

@gomesalexandre: I've created issue #11478 to track the refactoring of Ledger app gating logic into EvmBaseAdapter.

The issue covers:

• Moving verifyLedgerAppOpen calls to the base adapter for getAddress, signTransaction, and signAndBroadcastTransaction
• Removing duplicate calls from child adapters that extend EvmBaseAdapter
• Preserving the ability for child adapters to extend with their own logic

The issue has been assigned to you and includes an estimated effort of ~1 hour. This should eliminate the code duplication across second-class EVM chain adapters while leveraging OOP inheritance properly.

---

🧠 Learnings used

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/hooks/useSimulateEvmTransaction.ts:0-0
Timestamp: 2025-09-12T13:44:17.019Z
Learning: gomesalexandre prefers letting chain adapter errors throw naturally in useSimulateEvmTransaction rather than adding explicit error handling for missing adapters, consistent with his fail-fast approach and dismissal of defensive validation as "stale" in WalletConnect transaction simulation flows.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/utils/tenderly/index.ts:0-0
Timestamp: 2025-09-12T13:26:26.277Z
Learning: gomesalexandre uses "we do that errwhere ser" (everywhere, sir) to indicate that client-side API key exposure is an accepted codebase-wide pattern, consistent with his comfort level for development/preparatory phases across the WalletConnect/Tenderly integration.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/SendTransactionConfirmation.tsx:42-50
Timestamp: 2025-09-12T12:00:33.924Z
Learning: gomesalexandre prefers maintaining consistency with existing code patterns in WalletConnect modals, including side-effects-during-render for error handling (showErrorToast + handleReject), rather than introducing isolated refactors that would make the codebase inconsistent.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx:0-0
Timestamp: 2025-09-12T10:21:26.693Z
Learning: gomesalexandre explained that in WalletConnect V2, the request context chainId comes from params?.chainId following CAIP2 standards, making both the request params chainId and EIP-712 domain chainId equally reliable sources. He considers both approaches trustworthy ("both gucci") for WalletConnect dApps integration.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx:133-137
Timestamp: 2025-09-12T10:15:10.389Z
Learning: gomesalexandre has identified that EIP-712 domain chainId should be preferred over request context chainId for accuracy in WalletConnect dApps structured signing flows. The domain chainId from the parsed message is more specific and accurate than the general request context, especially for asset resolution and network-specific operations.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/SendTransactionConfirmation.tsx:42-50
Timestamp: 2025-09-12T12:00:33.924Z
Learning: gomesalexandre prefers maintaining consistency with existing code patterns across WalletConnect modal components, including side-effects-during-render for error handling (showErrorToast + handleReject calls before return null), rather than introducing isolated refactors that would create inconsistency in the codebase.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10566
File: src/hooks/useLedgerAccountGuard/useLedgerAccountGuard.tsx:4-4
Timestamp: 2025-09-23T10:36:13.916Z
Learning: gomesalexandre dismisses suggestions to expand PR scope beyond the current migration when the PR is already systematically implementing the desired pattern (like KeyManager.Ledger migration) within focused boundaries, preferring to keep scope contained rather than doing comprehensive codebase-wide cleanups.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10566
File: src/hooks/useLedgerConnectionState.ts:1-1
Timestamp: 2025-09-24T20:44:29.333Z
Learning: gomesalexandre prefers not to touch complex connection state logic in useLedgerConnectionState when the current implementation is working, even when potential stale closure issues are identified, demonstrating his cautious approach to modifying intricate state management code within PR scope.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10206
File: src/config.ts:127-128
Timestamp: 2025-08-07T11:20:44.614Z
Learning: gomesalexandre prefers required environment variables without default values in the config file (src/config.ts). They want explicit configuration and fail-fast behavior when environment variables are missing, rather than having fallback defaults.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/ContractInteractionBreakdown.tsx:0-0
Timestamp: 2025-09-13T16:45:18.813Z
Learning: gomesalexandre prefers aggressively deleting unused/obsolete code files ("ramboing") rather than fixing technical issues in code that won't be used, demonstrating his preference for keeping codebases clean and PR scope focused.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10458
File: src/plugins/walletConnectToDapps/types.ts:7-7
Timestamp: 2025-09-10T15:34:29.604Z
Learning: gomesalexandre is comfortable relying on transitive dependencies (like abitype through ethers/viem) rather than explicitly declaring them in package.json, preferring to avoid package.json bloat when the transitive dependency approach works reliably in practice.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10503
File: .env:56-56
Timestamp: 2025-09-16T13:17:02.938Z
Learning: gomesalexandre prefers to enable feature flags globally in the base .env file when the intent is to activate features everywhere, even when there are known issues like crashes, demonstrating his preference for intentional global feature rollouts over cautious per-environment enablement.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10249
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:447-503
Timestamp: 2025-08-13T17:07:10.763Z
Learning: gomesalexandre prefers relying on TypeScript's type system for validation rather than adding defensive runtime null checks when types are properly defined. They favor a TypeScript-first approach over defensive programming with runtime validations.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/hooks/useActionCenterSubscribers/useThorchainLpDepositActionSubscriber.tsx:61-66
Timestamp: 2025-08-14T17:51:47.556Z
Learning: gomesalexandre is not concerned about structured logging and prefers to keep console.error usage as-is rather than implementing structured logging patterns, even when project guidelines suggest otherwise.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10413
File: src/components/Modals/FiatRamps/fiatRampProviders/onramper/utils.ts:29-55
Timestamp: 2025-09-02T14:26:19.028Z
Learning: gomesalexandre prefers to keep preparatory/reference code simple until it's actively consumed, rather than implementing comprehensive error handling, validation, and robustness improvements upfront. They prefer to add these improvements when the code is actually being used in production.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:396-402
Timestamp: 2025-08-14T17:55:57.490Z
Learning: gomesalexandre is comfortable with functions/variables that return undefined or true (tri-state) when only the truthy case matters, preferring to rely on JavaScript's truthy/falsy behavior rather than explicitly returning boolean values.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10783
File: src/context/ModalStackProvider/useModalRegistration.ts:30-41
Timestamp: 2025-10-16T11:14:40.657Z
Learning: gomesalexandre prefers to add lint rules (like typescript-eslint/strict-boolean-expressions for truthiness checks on numbers) to catch common issues project-wide rather than relying on code review to catch them.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10206
File: src/lib/moralis.ts:47-85
Timestamp: 2025-08-07T11:22:16.983Z
Learning: gomesalexandre prefers console.error over structured logging for Moralis API integration debugging, as they find it more conventional and prefer to examine XHR requests directly rather than rely on structured logs for troubleshooting.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10418
File: src/plugins/walletConnectToDapps/components/header/WalletConnectToDappsHeaderButton.tsx:0-0
Timestamp: 2025-09-08T22:00:48.005Z
Learning: gomesalexandre dismissed an aria-label accessibility suggestion with "meh" in PR #10418 for WalletConnectToDappsHeaderButton.tsx, consistent with the team's pattern of deferring minor a11y improvements to follow-up PRs rather than expanding feature PR scope.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx:21-24
Timestamp: 2025-09-12T13:16:27.004Z
Learning: gomesalexandre declined to add error boundaries to WalletConnect modals in PR #10461, stating "no error boundaries in this pr ser", consistent with his preference to keep PR scope focused and defer tangential improvements to separate efforts.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11170
File: patches/@shapeshiftoss+bitcoinjs-lib+7.0.0-shapeshift.0.patch:9-19
Timestamp: 2025-11-25T21:43:10.838Z
Learning: In shapeshift/web, gomesalexandre will not expand PR scope to fix latent bugs in unused API surface (like bitcoinjs-lib patch validation methods) when comprehensive testing proves the actual used code paths work correctly, preferring to avoid costly hdwallet/web verdaccio publish cycles and full regression testing for conceptual issues with zero runtime impact.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/utils/tenderly/index.ts:0-0
Timestamp: 2025-09-12T11:56:19.437Z
Learning: gomesalexandre rejected verbose try/catch error handling for address validation in Tenderly integration (PR #10461), calling the approach "ugly" but still implemented safety measures in commit ad7e424b89, preferring cleaner safety implementations over defensive programming patterns.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11012
File: src/context/WalletProvider/Vultisig/components/Connect.tsx:24-59
Timestamp: 2025-11-19T16:59:50.569Z
Learning: In src/context/WalletProvider/*/components/Connect.tsx files across the ShapeShift web codebase, the established pattern for handling null/undefined adapter from getAdapter() is to simply check `if (adapter) { ... }` without an else clause. All wallet Connect components (Coinbase, Keplr, Phantom, Ledger, MetaMask, WalletConnectV2, KeepKey, Vultisig) follow this pattern—they reset loading state after the if block but do not show error messages when adapter is null. This is an intentional design decision and should be maintained for consistency.

---

Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10879
File: src/context/WalletProvider/WalletConnectV2/components/WalletConnectDirectRow.tsx:64-81
Timestamp: 2025-10-22T22:11:22.918Z
Learning: In early WalletConnect POC/features behind a flag, gomesalexandre prioritizes connection correctness/stability over UX polish; minimal safety guards (like preventing concurrent connects) are preferred, while visuals will be wired later by reallybeard.