Add read-only mode to sqlite db and use in `bitcoin-wallet`

willcl-ark commented at 2:08 pm on June 26, 2025: member

Currently our wallet-tool performs 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_only bool to DatabaseOptions. 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

db: add read_only mode to sqlite db options

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.

a81ab0bc17

db: handle read_only mode flag in sqlitedb c2415dc19e

tools: use read-only mode in wallet tool read operations c8734a7aa0

test: add wallet tool indempotence tests

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.

5acceed8e4

DrahtBot commented at 2:08 pm on June 26, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32818.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#31423 (wallet: migration, avoid creating spendable wallet from a watch-only legacy wallet by furszy)
#29641 (scripted-diff: Use LogInfo over LogPrintf [WIP, NOMERGE, DRAFT] by maflcko)

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:

indempotent -> idempotent [“indempotent” is a misspelling of “idempotent”]
indempotence -> idempotence [“indempotence” is the noun form of “idempotent,” spelled with an “i”]

drahtbot_id_4_m

fanquake commented at 3:22 pm on June 26, 2025: member

https://github.com/bitcoin/bitcoin/actions/runs/15904154552/job/44856855999?pr=32818#step:12:437:

 0 test  2025-06-26T14:50:15.271166Z TestFramework (ERROR): Assertion failed 
 1                                   Traceback (most recent call last):
 2                                     File "D:\a\bitcoin\bitcoin\test\functional\test_framework\test_framework.py", line 190, in main
 3                                       self.run_test()
 4                                       ~~~~~~~~~~~~~^^
 5                                     File "D:\a\bitcoin\bitcoin/test/functional/tool_wallet.py", line 450, in run_test
 6                                       self.test_tool_wallet_info()
 7                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~^^
 8                                     File "D:\a\bitcoin\bitcoin/test/functional/tool_wallet.py", line 164, in test_tool_wallet_info
 9                                       assert self.wallet_permissions() in ['400']
10                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
11                                   AssertionError

achow101 commented at 4:00 pm on June 26, 2025: member

How does this compare with #32685?

NowYu commented at 7:47 pm on June 26, 2025: none

Before I get off

willcl-ark commented at 7:55 pm on June 26, 2025: member

How does this compare with #32685?

Ah, I didn’t know (or had perhaps forgotten) about that PR. I came via #15608 and did not check what was open. Shame on me :(

I will mark this as draft and take a look over that PR.

willcl-ark marked this as a draft on Jun 26, 2025

in test/functional/tool_wallet.py:164 in 5acceed8e4

177+
178+        # Test with read-only file permissions
179+        self.log.info('Testing wallet tool info with read-only file permissions')
180+        self.log.debug('Setting wallet file permissions to 400 (read-only)')
181+        os.chmod(self.wallet_path, stat.S_IRUSR)
182+        assert self.wallet_permissions() in ['400']

PeterWrighten commented at 9:04 am on June 27, 2025:

In this code, you should add more seeds number of platform to pass CI.

pinheadmz commented at 10:05 am on June 27, 2025:

🤔

bitcoin deleted a comment on Jun 27, 2025

PeterWrighten commented at 9:07 am on June 27, 2025: none

How does this compare with #32685?

Ah, I didn’t know (or had perhaps forgotten) about that PR. I came via #15608 and did not check what was open. Shame on me :(

I will mark this as draft and take a look over that PR.

Never mind. Your implementation is also great. But seems there is lack of some tests for walletdb and db…

ryanofsky commented at 3:03 pm on June 27, 2025: contributor

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.

DrahtBot added the label Needs rebase on Jul 2, 2025

DrahtBot commented at 8:39 pm on July 2, 2025: contributor

🐙 This pull request conflicts with the target branch and needs rebase.

DrahtBot commented at 1:57 am on September 29, 2025: contributor

⌛ There hasn’t been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it ‘Up for grabs’ with the label, so that it can be picked up in the future.

achow101 commented at 3:19 pm on October 22, 2025: member

Closing this as it has not had any activity in a while. If you are interested in continuing work on this, please leave a comment so that it can be reopened.

achow101 closed this on Oct 22, 2025

Add read-only mode to sqlite db and use in bitcoin-wallet #32818

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)

Add read-only mode to sqlite db and use in `bitcoin-wallet` #32818