(minotari) Restore from keys/rename wallet#44
Conversation
igordanilcenko
commented
Feb 18, 2026
- Implemented wallet restoration from keys (read-only)
- Implemented wallet renaming
- Improved wallet database handling
Summary of ChangesHello @igordanilcenko, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances Minotari wallet management by introducing key-based restoration for read-only access and the ability to rename wallets. It also includes a fundamental improvement to how wallet databases are handled, centralizing them for better performance and maintainability. These changes provide users with more flexible options for wallet recovery and organization, while streamlining the underlying architecture. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
8b6a90e to
52837f5
Compare
e5e32f0 to
868e67d
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces wallet restoration from keys and wallet renaming for Minotari. It also refactors the wallet database handling to use a shared database per wallet type, which is a good improvement for managing multiple wallets. The implementation of the new features seems solid and well-integrated. I've found one potential issue with the atomicity of the wallet renaming process which could lead to an inconsistent state. My specific feedback is in the review comment.
| Future<void> renameWalletFiles(String newWalletName) async { | ||
| // TODO: Implement wallet file renaming | ||
| final currentWalletPath = await pathForWallet(name: walletInfo.name, type: walletInfo.type); | ||
| final currentDirPath = await pathForWalletDir(name: walletInfo.name, type: walletInfo.type); | ||
|
|
||
| final newWalletPath = await pathForWallet(name: newWalletName, type: walletInfo.type); | ||
|
|
||
| printV('renameWalletFiles from \"${walletInfo.name}\" (in ${currentWalletPath} ) to \"$newWalletName\"'); | ||
| final typeDir = await pathForWalletTypeDir(type: walletInfo.type); | ||
| final ffi = MinotariFfi(dataPath: '$typeDir/wallet.db', walletName: walletInfo.name); | ||
| await ffi.rename(newWalletName: newWalletName); | ||
|
|
||
| final currentKeysFile = File('$currentWalletPath.keys'); | ||
| if (currentKeysFile.existsSync()) { | ||
| await currentKeysFile.copy('$newWalletPath.keys'); | ||
| } | ||
|
|
||
| final currentKeysBackupFile = File('$currentWalletPath.keys.backup'); | ||
| if (currentKeysBackupFile.existsSync()) { | ||
| await currentKeysBackupFile.copy('$newWalletPath.keys.backup'); | ||
| } | ||
|
|
||
| await Directory(currentDirPath).delete(recursive: true); | ||
| } |
There was a problem hiding this comment.
The current implementation of renameWalletFiles is not atomic. It renames the wallet in the database before copying the key files. If the file copy operation fails, the wallet could be left in an inconsistent state, potentially making it inaccessible. The database would have the new name, but the key files would still be at the old location.
To make this operation safer, the file operations should be performed before the database update. If the database update fails, the file changes can be reverted. Also, it's better to use the asynchronous exists() instead of the synchronous existsSync() in an async function.
Future<void> renameWalletFiles(String newWalletName) async {
final currentWalletPath = await pathForWallet(name: walletInfo.name, type: walletInfo.type);
final currentDirPath = await pathForWalletDir(name: walletInfo.name, type: walletInfo.type);
final newWalletPath = await pathForWallet(name: newWalletName, type: walletInfo.type);
final newDirPath = await pathForWalletDir(name: newWalletName, type: walletInfo.type);
printV('renameWalletFiles from "${walletInfo.name}" (in ${currentWalletPath} ) to "$newWalletName"');
// Create new directory and copy files first to make the operation more robust.
await Directory(newDirPath).create(recursive: true);
final currentKeysFile = File('$currentWalletPath.keys');
if (await currentKeysFile.exists()) {
await currentKeysFile.copy('$newWalletPath.keys');
}
final currentKeysBackupFile = File('$currentWalletPath.keys.backup');
if (await currentKeysBackupFile.exists()) {
await currentKeysBackupFile.copy('$newWalletPath.keys.backup');
}
try {
final typeDir = await pathForWalletTypeDir(type: walletInfo.type);
final ffi = MinotariFfi(dataPath: '$typeDir/wallet.db', walletName: walletInfo.name);
await ffi.rename(newWalletName: newWalletName);
} catch (e) {
// If DB rename fails, clean up the copied files to revert.
await Directory(newDirPath).delete(recursive: true);
rethrow;
}
// On success, remove the old wallet directory.
await Directory(currentDirPath).delete(recursive: true);
}* feat(minotari): Implement random passphrase generation for wallet creation and restoration * feat(minotari): Add support for restoring wallets from view and spend keys * feat(minotari): Improve database initialization and wallet renaming logic