Skip to content

add support for LevelDB#10401

Draft
ecdsa wants to merge 6 commits into
masterfrom
levelDB
Draft

add support for LevelDB#10401
ecdsa wants to merge 6 commits into
masterfrom
levelDB

Conversation

@ecdsa

@ecdsa ecdsa commented Jan 8, 2026

Copy link
Copy Markdown
Member

This is still a draft, but it passes all the tests :-). See #4823 for context.

Note: I am NOT suggesting to use LevelDB for end-user wallets. The goal of this PR is to have scalable storage for lightning forwarding nodes.

Also, this PR is not really about using LevelDB. It is about making our code able to use multiple types of storage (99% of the work here is about making our code database agnostic). Now that the logic is in place, we could use any other kind of database. I used levelDB because it is simple enough :-).

Main changes:

  • The StoredDict logic has been separated from json_db and moved to its own module.
  • The wallet code interacts with a StoredDict object, that is database agnostic.

As a consequence:

  • WalletDB no longer inherits from JsonDB
  • StoredDict has a database field, that inherits from BaseDB
  • JsonDB and LevelDB are child classes of BaseDB
  • WalletDB upgrades are database agnostic.

Todo:

  • The levelDB storage needs a version number, in order to describes changes in encoding
  • We could use cbor for encoding, but that's another dependency (currently using json)
  • LevelDB should have unit tests
  • unit tests for stored_dict and json_db are currently mixed, should be separated.

Note that JsonDB requires to have the whole database in memory. In contrast, LevelDB does not, and uses iterators. That's really the main advantage of LevelDB. We could, in theory, rewrite JsonDB so that it does not load the whole database in memory either, with some sort of low level parsing (counting braces). Not sure if we could get acceptable performance from that approach.

PS: I hate sqlite. please do not suggest adding that.

@ecdsa ecdsa force-pushed the levelDB branch 20 times, most recently from c3a526b to eb06528 Compare January 15, 2026 10:33
@brain-zhang

brain-zhang commented Jan 15, 2026

Copy link
Copy Markdown

I tested the new LevelDB backend support in Electrum on macOS 15.6.1, and it works very well. Thank you for this improvement 👍
I have two small suggestions that could further improve usability and completeness:

Dependency declaration
Currently, the plyvel dependency is only added to the testing requirements.
Since LevelDB is now a supported backend, it would be better to add plyvel to all relevant requirements files (e.g. default/runtime requirements), so users do not need to install it manually when using LevelDB in production.

GUI wallet creation option
In the wallet creation GUI, it would be helpful to add an option to choose the wallet storage backend (e.g. JSONDB vs LevelDB).
This could be implemented as a dropdown or selection box, with a sensible default value to preserve current behavior and avoid confusion for new users.

I believe these two small changes would make the LevelDB support more complete and user-friendly.

@ecdsa ecdsa force-pushed the levelDB branch 5 times, most recently from a06f4b4 to 3c5327e Compare January 16, 2026 11:17
@brain-zhang

Copy link
Copy Markdown

Bug Report: LevelDB lock not released after restore or create command with --level_db flag

Summary

When using the restore or create command with the --level_db flag to create a wallet, the LevelDB database lock is not released. This causes subsequent load_wallet calls to fail with an "already held by process" error until the electrum daemon is restarted.

Steps to Reproduce

  1. Start electrum in daemon mode:

    ./run_electrum daemon -d
  2. Create/restore a wallet with LevelDB storage:

    ./run_electrum restore "your seed phrase here" --level_db
  3. Try to load the wallet:

    ./run_electrum load_wallet

Expected Behavior

The load_wallet command should successfully load the wallet.

Actual Behavior

The command fails with the following error:

plyvel._plyvel.IOError: b'IO error: lock /path/to/wallet/LOCK: already held by process'

The wallet cannot be loaded until the daemon is stopped and restarted.

Root Cause Analysis

In electrum/commands.py, the restore() and create() commands call restore_wallet_from_text() or create_new_wallet() respectively, which create a wallet with an open LevelDB connection. However, when these commands return, they only extract the path and message from the result dict, discarding the wallet object without closing its storage database.

Since LevelDB only allows a single process to hold the lock on a database at a time, the orphaned connection keeps the lock, preventing any subsequent attempt to open the same database.

Problematic code in commands.py:

@command('')
async def restore(self, text, passphrase=None, password=None, encrypt_file=True, wallet_path=None, use_levelDB=False):
    # ...
    d = restore_wallet_from_text(...)
    return {
        'path': d['wallet'].storage.path,  # wallet object is accessed but never closed
        'msg': d['msg'],
    }

Proposed Solution

Close the storage database before returning from the restore() and create() commands:

@command('')
async def restore(self, text, passphrase=None, password=None, encrypt_file=True, wallet_path=None, use_levelDB=False):
    # ...
    d = restore_wallet_from_text(...)
    wallet = d['wallet']
    result = {
        'path': wallet.storage.path,
        'msg': d['msg'],
    }
    # Close the storage DB to release the LevelDB lock (if any),
    # so that subsequent load_wallet calls can open the database.
    if wallet.storage and wallet.storage._db:
        wallet.storage._db.close()
    return result

The same fix should be applied to the create() command.

Environment

  • OS: macOS (MacOS15.6.2)
  • Electrum version: 49848a2
  • Python version: [CPython 3.12.0]

@ecdsa

ecdsa commented Jan 20, 2026

Copy link
Copy Markdown
Member Author

Bug Report: LevelDB lock not released after restore or create command with --level_db flag

Thanks for the report. Your suggested fix makes sense, I will add it.

Please note that if you find an issue in a pull request, you can add your report/suggestion as a comment to the code. This allows the comment to be later "resolved" once it is addressed, instead of being visible permanenty in this thread.

@ecdsa ecdsa force-pushed the levelDB branch 2 times, most recently from f077c2b to 6167449 Compare April 27, 2026 15:41
@ecdsa ecdsa force-pushed the levelDB branch 23 times, most recently from f7b1486 to ab2ba99 Compare May 19, 2026 10:28
run: |
sudo apt-get update
sudo apt-get -y install libgl1 libegl1 libxkbcommon0 libdbus-1-3
sudo apt-get -y install libgl1 libegl1 libxkbcommon0 libdbus-1-3 libleveldb-dev

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a note re the CI failures:
https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#pull_request_target

pull_request_target
This event runs in the context of the default branch of the base repository, rather than in the context of the merge commit, as the pull_request event does

i.e. it will not install libleveldb-dev as it uses the yml file from master, not from the PR, when running the CI

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks. how do you suggest to fix this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One workaround that you are not going to like is that as the yml also has a workflow_dispatch trigger, you can run it manually using that.

On this page:
https://github.com/spesmilo/electrum/actions/workflows/security-review.yml
Select the levelDB branch (to specify that you want the .yml file from that branch), and then enter the PR number.

I started one run now: https://github.com/spesmilo/electrum/actions/runs/26505326899/job/78056093888

workflow_dispatch2

ecdsa added 6 commits June 24, 2026 18:56
  - WalletDB no longer inherits from JsonDB, it uses a StoredDict
  - JsonDB inherits from BaseDB
  - FileStorage is only seen by JsonDB
  - calling JSonDB constructor creates the storage

note: this commit temporarily breaks DB upgrades
this restores DB upgrades.
(DB upgrades require not converting stored classes)
Add unit test of atomicity.
Use a write batch for DB upgrades.
this is generic, could work with another key-value backend
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.

3 participants