feat: allow to alter signing provider for wallets by mateuszpiatkowski-da · Pull Request #2333 · canton-network/wallet

mateuszpiatkowski-da · 2026-08-20T13:22:38Z

.

…dd `changeSigningProvider` endpoint Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

…form Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

…it wallet form Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

…allow-to-alter-signing-provider-for-wallets Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

mjuchli-da

You nailed it conceptually! Left a few tweaks

mjuchli-da · 2026-08-24T20:37:57Z

        {
-            "name": "listSigningProviderVaults",
-            "description": "Lists available vaults of a signing provider",
+            "name": "listSigningProviderKeys",


why does that replace listSigningProviderVaults instead of it being an addition?

AFAIK this method returns a set of keys assigned to a signing provider. "Vaults" is a reserved keyword for Fireblocks, but it doesn't make sense to keep this naming convention when extending the functionality to other SP, hence the change in naming :)

mjuchli-da · 2026-08-24T20:39:13Z

+            }
+        },
+        {
+            "name": "changeSigningProvider",


should we use a more generalized updateWallet for this?

I'm a little reluctant about this - updateWallet would suggest the ability to edit any/all properties of a specified wallet, whereas here we're only interested in updating the portion of the data associated with the SP. If you want I could create a generic method called like that and enable the user to update any property and use it to update the SP data where required

mjuchli-da · 2026-08-24T20:47:01Z

    '/api-keys/add',
    '/api-keys',
    '/login',
+    '/parties/edit',


Can be a follow-up but I suggest changing routes that involve /parties with /wallets

Wasn't this change Wallets -> Parties in what is visible in UI a product decision?

mjuchli-da · 2026-08-24T20:50:26Z

+        protected signingDriver: SigningDriverInterface
    ) {}

+    private handleSigningError<T extends object>(result: SigningError | T): T {


The name is misleading. I initially thought it's related to signing, but it's related to a signing driver api call.

Also the method is repetitive; perhaps something to extract to a central location

mjuchli-da · 2026-08-24T20:53:26Z

        return this.store.updateWallet(walletUpdate)
    }
-
-    async getVaults(userId: UserId): Promise<{ vaults: string[] }> {


Ah I start to understand that vaults and keys are the same here.

Thus vaults listing was redundant; correct me if i'm wrong

Exactly, Fireblocks vaults are equal to keys. Fireblocks had it earlier, because unlike other drivers it doesn't allow creating keys, you have to select pre-existing vault when creating a party.

mjuchli-da · 2026-08-24T20:54:43Z

+        protected signingDriver: SigningDriverInterface
    ) {}

+    private handleSigningError<T extends object>(result: SigningError | T): T {


mjuchli-da · 2026-08-24T20:54:57Z

+        protected signingDriver: SigningDriverInterface
    ) {}

+    private handleSigningError<T extends object>(result: SigningError | T): T {


mjuchli-da · 2026-08-24T20:56:41Z

+        protected signingDriver: SigningDriverInterface
    ) {}

+    private handleSigningError<T extends object>(result: SigningError | T): T {


pawelstepien-da · 2026-08-25T11:13:09Z

                    signingProviderId:
                        resolvedSigningProvider.signingProviderId,
                    disabled: !isMatched,
+                    userId: '',


Why empty userId? You can get current user ID by this.authContext.userId, but store.addWallet doesn't need it anyway, because it also extracts userId from authContext.

pawelstepien-da · 2026-08-25T11:24:50Z

        })
    }

+    async getWallet(constraint: WalletUniqueConstraint) {


I'm not sure about manually passing userId to store. Other methods take userId from authContext (assertConnected). User Alice shouldn't be able to get Wallet that belongs to Bob and it's good to have this constraint on store level. Unless you need to be able to get Wallet of any user from some service, I would adhere to convention that store gets userId from authContext, not from a param.

pawelstepien-da · 2026-08-25T11:27:34Z

+                        "additionalProperties": false,
+                        "properties": {
+                            "userId": {
+                                "title": "userId",


This allows a user to log in as user Bob to Network1 and get Wallet of user Alice for Network2. Even if UI constrains it, one could execute a request manually.

pawelstepien-da · 2026-08-25T11:47:28Z

+        changeSigningProvider: async (
+            params: ChangeSigningProviderParams
+        ): Promise<null> => {
+            const { signingProviderId, partyId, publicKey } = params


I'm not sure if we should trust that request payload contains singingProviderId that exists and a publicKey that belongs to that signing provider. Perhaps we should try to do listSigningProviderKeys (or extract a subset of that method this one) and compare request params against result before changing wallet record?

pawelstepien-da · 2026-08-25T11:48:36Z

+            const keys = await walletAllocationService.getKeys(
+                assertConnected(authContext),
+                params.signingProviderId as SigningProvider
+            )
+            if (
+                !drivers[params.signingProviderId as SigningProvider] ||
+                !keys
+            ) {


I would switch order - first check that driver is registered, act on it (getKeys) afterwards.

pawelstepien-da · 2026-08-25T11:56:07Z

+                .publicKeys=${this.publicKeys}
+                .selectedSigningProvider=${this.selectedSigningProvider}
+                .partyId=${this.walletConstraint.partyId}
+                .selectedPublicKeyId=${this.selectedPublicKey}


Does that work? Inside edit-form we compare wallet.publicKey against key.id. I assumed those are not (or not always) equal.

pawelstepien-da · 2026-08-25T12:16:33Z

    ): Promise<void> {
-        // For dApp-triggered approvals the popup is opened with
-        // `closeafteraction`, so success is signalled by the popup closing.
+        // dApp-triggered approvals often close the popup, but newer flows can


I preferred previous comment.
It's not like it happens "often" or "rarely", nor there are "newer" and "older" flows. It is deterministic based on whether approve screen is triggereed via dapp-sdk, or by going to WG transaction list, you will get query param closeafteraction or not.

pawelstepien-da · 2026-08-25T12:18:13Z

-                .getByRole('button', { name: 'Create' })
-                .click()
+            const submitButton = (await this.page()).getByRole('button', {
+                name: /^(Add|Create)$/,


There is more than one button text?

pawelstepien-da · 2026-08-25T12:21:35Z


+    private _onWalletEdit(e: WalletCardEditEvent) {
+        setLocationHref(
+            `/parties/edit?partyId=${e.wallet.partyId}&networkId=${e.wallet.networkId}&userId=${e.wallet.userId}`


Could you use toRelPath also here? It could work on with dev but fail with build without that.

pawelstepien-da · 2026-08-25T12:24:43Z

+        try {
+            await this.userClient.request({
+                method: 'changeSigningProvider',
+                params: event,


I wouldn't pass DOM Event directly to API, better extract specific properties from it.

pawelstepien-da · 2026-08-25T12:44:32Z

+            if (network === undefined) {
+                throw new Error('No network session found')
+            }
+            if (!network.adminAuth) {


This method doesn't really need adminAuth, but I understand that we need it for PartyAllocationService constructor, even tho we only need it to create a fingerprint which doesn't require adminAuth.
I created a separate tech debt issue to address those unnecessary adminAuth dependencies #2360

pawelstepien-da · 2026-08-26T14:01:07Z

+                return this.securosysAllocator
+            case SigningProvider.WALLET_KERNEL:
+                return this.kernelAllocator
+            case SigningProvider.PARTICIPANT:


Now that we have BitGo driver merged, you will need to sync your branch with main and add case for it here and make sure getKeys is available in it's allocator.

…provider-for-wallets

Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>

Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

mateuszpiatkowski-da added 5 commits August 18, 2026 10:08

feat(core-wallet-store,wallet-gateway,core-wallet-user-rpc-client): a…

9c96f0f

…dd `changeSigningProvider` endpoint Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

refactor(wallet-gateway): update changeSigningProvider endpoint

9b810ff

Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

feat(core-wallet-store-sql): enable modifying signing provider in sql db

7997754

Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

refactor(core-wallet-ui-components): extract common code from wallet …

678cfbb

…form Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

feat: add getWallet endpoint, refactor code by reusing it, prepare ed…

af2f26b

…it wallet form Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

mateuszpiatkowski-da self-assigned this Aug 20, 2026

mateuszpiatkowski-da linked an issue Aug 20, 2026 that may be closed by this pull request

Allow to alter Signing Provider for Wallets #2277

Open

mateuszpiatkowski-da added 4 commits August 20, 2026 18:04

feat(wallet-gateway,core-wallet-ui-components): enable editing party

ada8a13

Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

feat: finishing touches for wallet-gateway

9108b85

Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

fix: fix unit test failing

69dda4e

Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

refactor: revert to imterface system

58635cd

Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

mateuszpiatkowski-da marked this pull request as ready for review August 21, 2026 14:10

mateuszpiatkowski-da requested a review from a team as a code owner August 21, 2026 14:10

mateuszpiatkowski-da added 4 commits August 21, 2026 16:11

Merge remote-tracking branch 'origin/main' into mateuszpiatkowski-da/…

ba648c6

…allow-to-alter-signing-provider-for-wallets Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

fix: fix build

065faa2

Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

feat: fix e2e tests

58adab1

Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

fix: fix build

de63412

Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

mjuchli-da reviewed Aug 24, 2026

View reviewed changes

pawelstepien-da reviewed Aug 25, 2026

View reviewed changes

Comment thread wallet-gateway/remote/src/web/frontend/parties/index.ts

pawelstepien-da reviewed Aug 25, 2026

View reviewed changes

pawelstepien-da reviewed Aug 26, 2026

View reviewed changes

pawelstepien-da and others added 3 commits August 26, 2026 16:14

Merge branch 'main' into mateuszpiatkowski-da/allow-to-alter-signing-…

8c1912d

…provider-for-wallets

feat: allow to alter signing provider for wallets (#2370)

0755452

Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>

chore: auto-update types

b1274e2

Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>

Uh oh!

Conversation

mateuszpiatkowski-da commented Aug 20, 2026 • edited by pawelstepien-da Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mjuchli-da left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pawelstepien-da Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pawelstepien-da Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mateuszpiatkowski-da commented Aug 20, 2026 •

edited by pawelstepien-da

Loading

pawelstepien-da Aug 25, 2026 •

edited

Loading

pawelstepien-da Aug 25, 2026 •

edited

Loading