Re-enable disabled wallets whose party matches a signing provider again - #2338
Open
ayushsingh82 wants to merge 1 commit into
Open
Re-enable disabled wallets whose party matches a signing provider again#2338ayushsingh82 wants to merge 1 commit into
ayushsingh82 wants to merge 1 commit into
Conversation
syncWallets() treated a disabled wallet as still "existing" when computing which parties are new, so a party whose wallet got disabled (e.g. no signing provider matched at the time) was never re-checked on subsequent syncs and stayed disabled forever, even after its signing provider became resolvable. Excluding disabled wallets from that check isn't enough on its own: store.addWallet() throws on a duplicate (partyId, networkId), so handlePartiesWithoutWallet() now looks up whether a wallet row already exists and calls updateWallet() to re-enable it instead of addWallet(). Extended UpdateWallet (core-wallet-store, core-wallet-store-sql) with signingProviderId/publicKey so the re-resolved signing provider can actually be persisted on re-enable. Also excluded disabled wallets from the separate rights-update pass, since handlePartiesWithoutWallet already sets fresh rights as part of re-enabling them -- otherwise the same wallet could get reported in both the "updated" and "disabled" result buckets. Fixes canton-network#2206 Signed-off-by: ayushsingh82 <ayushsinghmi711@gmail.com>
Contributor
Author
|
@alexmatson-da whenever you have a chance, would appreciate a look at this one. |
Contributor
|
@pawelstepien-da to review |
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
3 participants
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.
Summary
syncWallets()(wallet-gateway/remote/src/ledger/wallet-sync-service.ts) treated a disabled wallet as still "existing" when computing which parties on the ledger are new. A party whose wallet got disabled (e.g. no signing provider matched at the time) was therefore never re-checked on later syncs and stayed disabled forever, even once its signing provider became resolvable.store.addWallet()throws on a duplicate(partyId, networkId).handlePartiesWithoutWallet()now looks up whether a wallet row already exists for the party and callsupdateWallet()to re-enable it in place instead ofaddWallet().UpdateWallet(core/wallet-store,core/wallet-store-sql) withsigningProviderId/publicKeyso the freshly re-resolved signing provider can actually be persisted when a wallet is re-enabled (previouslyupdateWalletonly supporteddisabled/reason/rights/etc.).handleRightsUpdates), sincehandlePartiesWithoutWalletalready sets fresh rights as part of re-enabling them — otherwise the same wallet could get reported in both theupdatedanddisabledbuckets of the sync result.Fixes #2206
Test plan
updateWallet(not a duplicateaddWalletthrow), and shows up inresult.updated.npx vitest run --project node src/ledger/wallet-sync-service.test.ts— 22/22 passing (one pre-existing, unrelated SQLite-migration failure in a different describe block was confirmed present onmaintoo, before this change).npx vitest runoncore/wallet-store-inmemory— 21/21 passing.eslintandprettier --checkclean on all 4 changed files.tsc --noEmitclean onwallet-gateway/remote,core/wallet-store, andcore/wallet-store-sql.