Add read-only mode to sqlite db and use in bitcoin-wallet
#32818
Conversation
Adds a read_only bool to DatabaseOptions. This can be used by the sqlite dbwrapper to open the database in readonly mode. Add helper function to check if we are opened in read_only mode.
Adds various tests checking that the wallet tool does not modify the wallet db file when performing read-only operations, and that it can operate on read-only db files.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32818. ReviewsSee the guideline for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. LLM Linter (✨ experimental)Possible typos and grammar issues:
drahtbot_id_4_m |
|
https://github.com/bitcoin/bitcoin/actions/runs/15904154552/job/44856855999?pr=32818#step:12:437: test 2025-06-26T14:50:15.271166Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "D:\a\bitcoin\bitcoin\test\functional\test_framework\test_framework.py", line 190, in main
self.run_test()
~~~~~~~~~~~~~^^
File "D:\a\bitcoin\bitcoin/test/functional/tool_wallet.py", line 450, in run_test
self.test_tool_wallet_info()
~~~~~~~~~~~~~~~~~~~~~~~~~~^^
File "D:\a\bitcoin\bitcoin/test/functional/tool_wallet.py", line 164, in test_tool_wallet_info
assert self.wallet_permissions() in ['400']
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError |
|
How does this compare with #32685? |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| self.log.info('Testing wallet tool info with read-only file permissions') | ||
| self.log.debug('Setting wallet file permissions to 400 (read-only)') | ||
| os.chmod(self.wallet_path, stat.S_IRUSR) | ||
| assert self.wallet_permissions() in ['400'] |
There was a problem hiding this comment.
In this code, you should add more seeds number of platform to pass CI.
Never mind. Your implementation is also great. But seems there is lack of some tests for walletdb and db... |
|
Note IIRC, the SQLITE_OPEN_READONLY flag only guarantees that sqlite won't try to write to the main database file, not any wal or jrn files associated with the database. So if the goal is being able to access wallets on read-only filesystems this may not always work and the immutable mode might need to be used instead. |
|
🐙 This pull request conflicts with the target branch and needs rebase. |
Currently our
wallet-toolperforms a few "read-only" operations (info,dump) on sqlite databases. However, currently this involves opening the wallet in RW mode, with the side-effects of modifying the wallet file, and failing to open a "read-only" wallet file.See #15608 for more context.
Since we have dropped the BDB wallet, this change got a lot simpler; the BDB parser is now custom and only operates in read-only mode anyway.
This change adds a
read_onlybool toDatabaseOptions. This can be used by the sqlite dbwrapper to open the database in readonly mode.Includes tests to verify indempotence of wallet tool "read only" operations.
Fixes: #15608