Skip to content

chore: prevent DB race on Keymaster#1020

Merged
Bushstar merged 6 commits into
mainfrom
bush/keymaster-db-race
Sep 9, 2025
Merged

chore: prevent DB race on Keymaster#1020
Bushstar merged 6 commits into
mainfrom
bush/keymaster-db-race

Conversation

@Bushstar

@Bushstar Bushstar commented Sep 9, 2025

Copy link
Copy Markdown
Contributor

@Bushstar Bushstar requested review from Copilot and macterra September 9, 2025 10:32

Copilot AI 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.

Pull Request Overview

This PR prevents database race conditions in Keymaster by implementing a centralized wallet mutation mechanism with locking. The changes address potential concurrency issues where multiple operations could modify the wallet simultaneously, ensuring thread-safe operations through an abstract base class with built-in locking.

  • Introduces AbstractBase class with a promise-based locking mechanism for wallet operations
  • Refactors all wallet storage classes to extend AbstractBase instead of implementing WalletBase directly
  • Replaces direct load/modify/save patterns with atomic mutateWallet operations throughout Keymaster

Reviewed Changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/keymaster/src/db/abstract-base.ts New abstract base class providing thread-safe updateWallet method with promise-based locking
packages/keymaster/src/types.ts Adds updateWallet method to WalletBase interface
packages/keymaster/src/keymaster.ts Refactors wallet operations to use new mutateWallet helper for atomic updates
packages/keymaster/src/db/*.ts Updates all wallet storage implementations to extend AbstractBase
tests/keymaster/wallet.test.ts Adds test coverage for new functionality
packages/keymaster/rollup.cjs.config.js Adds build configuration for new abstract base module

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread packages/keymaster/src/keymaster.ts Outdated
Comment thread packages/keymaster/src/keymaster.ts Outdated
@macterra

macterra commented Sep 9, 2025

Copy link
Copy Markdown
Contributor

Just for the record, no material change to keymaster service performance (using json db). Running ./kc perf-test 1000 took ~22s before, now 25s.

@Bushstar Bushstar merged commit 7bb91e5 into main Sep 9, 2025
14 checks passed
@Bushstar Bushstar deleted the bush/keymaster-db-race branch September 9, 2025 14:56
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.

Prevent keymaster race condition on wallet

3 participants