Skip to content

feat: move isDevnet check to wallet-sdk - #2200

Merged
fayi-da merged 2 commits into
mainfrom
fayi/1795-sdk-migrate-portfolio-scanproxy
Jul 27, 2026
Merged

feat: move isDevnet check to wallet-sdk#2200
fayi-da merged 2 commits into
mainfrom
fayi/1795-sdk-migrate-portfolio-scanproxy

Conversation

@fayi-da

@fayi-da fayi-da commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Part of #2149

  • move isDevNet check to wallet-sdk
  • make Tap amulet only operation
  • delete unused services and dependencies

@fayi-da
fayi-da requested review from a team, Copilot, mateuszpiatkowski-da and rukmini-basu-da and removed request for Copilot July 24, 2026 16:13
Comment thread sdk/wallet-sdk/src/wallet/init/initializedSDK.ts

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR continues the #1795 “wallet-sdk replacement” work by shifting DevNet detection and DevNet tap behavior to use @canton-network/wallet-sdk, while removing the now-unneeded Portfolio service abstraction and related dependencies in the example app.

Changes:

  • Add AmuletNamespace.isDevNet() to the wallet SDK and cover it with a unit test.
  • Make DevNet tap in the Portfolio example an “Amulet-only” operation (no instrument selection), wiring taps through wallet-sdk + submitViaProvider.
  • Remove unused Portfolio service/context modules and drop @canton-network/core-splice-client from the Portfolio example’s dependencies.

Reviewed changes

Copilot reviewed 18 out of 19 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
yarn.lock Removes an unused workspace dependency entry related to the Portfolio example’s dependency cleanup.
sdk/wallet-sdk/src/wallet/namespace/amulet/namespace.ts Adds isDevNet() to the Amulet namespace, delegating to amuletService.
sdk/wallet-sdk/src/wallet/namespace/amulet/amulet.test.ts Adds a unit test for the new isDevNet() wrapper.
sdk/wallet-sdk/src/wallet/init/initializedSDK.ts Documents/retains the validator-proxy behavior when validatorUrl is configured.
examples/portfolio/tests/utils.ts Updates tap test helper to assert fixed “Amulet” instrument rather than selecting an instrument.
examples/portfolio/tests/settings-next.spec.ts Updates tap amount expectation to include fractional precision.
examples/portfolio/tests/next-utils.ts Updates Next UI tap helper to assert fixed “Amulet” instrument rather than selecting an instrument.
examples/portfolio/src/services/resolve.ts Removes Amulet service resolver plumbing; keeps only token-standard service resolution (but leaves some dead code as noted).
examples/portfolio/src/services/portfolio-service.ts Deletes the PortfolioService interface (no longer used).
examples/portfolio/src/services/portfolio-service-implementation.ts Deletes the Portfolio service implementation (tap/isDevNet now handled via wallet-sdk).
examples/portfolio/src/main.tsx Removes PortfolioProvider from the app tree.
examples/portfolio/src/hooks/useIsDevNet.ts Simplifies hook to use wallet-sdk-based query options.
examples/portfolio/src/hooks/query-options.ts Moves isDevNet query to wallet-sdk (but currently keys off the wrong validatorUrl as noted).
examples/portfolio/src/hooks/query-keys.ts Replaces the global isDevNet key with a validator-specific key.
examples/portfolio/src/contexts/PortfolioProvider.tsx Deletes the provider (no longer needed).
examples/portfolio/src/contexts/PortfolioContext.tsx Deletes the context and usePortfolio hook (no longer needed).
examples/portfolio/src/components/tap-settings.tsx Makes Settings-page tap an Amulet-only operation executed via wallet-sdk.
examples/portfolio/src/components/settings/devnet-tap-dialog.tsx Makes dialog tap Amulet-only and executes via wallet-sdk; updates the UI accordingly.
examples/portfolio/package.json Drops @canton-network/core-splice-client from the Portfolio example’s dependencies.
Comments suppressed due to low confidence (1)

examples/portfolio/src/services/resolve.ts:51

  • ledgerClient singleton is now dead code: it is never assigned/returned anywhere and is only cleared. Keeping this around makes the resolver harder to understand and can trip unused-code checks.
// Global, but so is the dApp SDK.
const ledgerClient: { singleton: LedgerClient | undefined } = {
    singleton: undefined,
}
const tokenStandardService: { singleton: TokenStandardService | undefined } = {
    singleton: undefined,
}
// Can be called to reset clients on disconnects.
export const clear = () => {
    ledgerClient.singleton = undefined
    tokenStandardService.singleton = undefined
}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread examples/portfolio/src/hooks/query-options.ts
Comment thread examples/portfolio/src/services/resolve.ts
Comment thread examples/portfolio/src/components/settings/devnet-tap-dialog.tsx Outdated
Copilot AI review requested due to automatic review settings July 24, 2026 16:56
@fayi-da
fayi-da force-pushed the fayi/1795-sdk-migrate-portfolio-scanproxy branch from 6aa6a2a to b5175d3 Compare July 24, 2026 16:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 1 comment.

Comment thread examples/portfolio/package.json
fayi-da added 2 commits July 24, 2026 19:53
Part of #1795

- move isDevNet check to wallet-sdk
- make Tap amulet only operation
- delete unused services and dependencies

Signed-off-by: Fayi Femi-Balogun <fayimora.femibalogun@digitalasset.com>
Signed-off-by: Fayi Femi-Balogun <fayimora.femibalogun@digitalasset.com>
Copilot AI review requested due to automatic review settings July 24, 2026 18:54
@fayi-da
fayi-da force-pushed the fayi/1795-sdk-migrate-portfolio-scanproxy branch from b5175d3 to d866128 Compare July 24, 2026 18:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

examples/portfolio/src/hooks/query-options.ts:167

  • The non-TLS validatorUrl warning is now emitted only inside the isDevNet query. Other flows (e.g. tapping via sdk.amulet.tap(...)) can still use an http: validator endpoint without ever triggering this warning if useIsDevNet isn't queried. Consider moving this check to a central place that always runs when the SDK is configured (e.g. useWalletSdk during walletSdk.SDK.create, or config validation) so it consistently covers all validator interactions.
            const url = new URL(validatorUrl)
            if (url.protocol === 'http:') {
                logger.warn(
                    { validatorUrl: url.toString() },
                    'Using a non-TLS validator endpoint. This is acceptable only in trusted environments. Set validatorUrl in portfolio config to an HTTPS endpoint if the validator API is reachable over an untrusted network.'

@rukmini-basu-da rukmini-basu-da left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@fayi-da
fayi-da merged commit 0d652c1 into main Jul 27, 2026
28 checks passed
@fayi-da
fayi-da deleted the fayi/1795-sdk-migrate-portfolio-scanproxy branch July 27, 2026 17:54
mateuszpiatkowski-da pushed a commit that referenced this pull request Aug 10, 2026
Signed-off-by: Fayi Femi-Balogun <fayimora.femibalogun@digitalasset.com>
mateuszpiatkowski-da pushed a commit that referenced this pull request Aug 13, 2026
Signed-off-by: Fayi Femi-Balogun <fayimora.femibalogun@digitalasset.com>
Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants