Create an in-memory database that can easily replace leveldb and assist in tests

FWIW, unittests already run leveldb in-memory: https://github.com/bitcoin/bitcoin/blob/master/src/test/util/setup_common.cpp#L207-L208

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #25202 (log: Use severity-based logging for leveldb/libevent messages, reverse LogPrintLevel order by laanwj)
• #25172 (refactor: use std:: prefix for std lib funcs by fanquake)
• #24232 (assumeutxo: add init and completion logic by jamesob)
• #22663 (dbwrapper: properly destroy objects in case CDBWrapper ctor throws by Crypt-iQ)

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.

The functional test wallet_send.py seems to be flaky, unrelated to this PR.

Not sure if this is desirable. LevelDB is de facto part of the consensus layer, and may not be safe to just substitute willy-nilly.

LevelDB is de facto part of the consensus layer

In fact, this PR is the first step to solve that problem. LevelDB should be only a key/value store. While I totally understand your concern, this PR will make it possible to quantify our reliance on LevelDB specifics, if any. In no way is this PR meant to advocate for substituting LevelDB in a negligent fashion. We advocate only for a correct design to better understand the bitcoin protocol.

It's NOT solvable. Adding another layer in the middle can only introduce problems, not solve it.

It's NOT solvable. Adding another layer in the middle can only introduce problems, not solve it.

Even though I understand your concerns, the claim that this is not solvable requires evidence, which is lacking here. I'm not even fully onboard on the idea that LevelDB is part of the "consensus layer", but I understand that LevelDB is part of the consensus library; after all it's a key/value store and it should be just that. The solution to this fear, however, is not keeping it untouched (because it was touched before, quite often over the last decade). But the solution is to quantify how it works and make it part of the protocol and run proper tests, like it has always been done in this repo.

And I'm happy to do more to make this PR more appealing and address any concerns, whether it's tests or something else.

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

_{Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".}

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.

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.

I am interested. But no one seems to have said anything for me to react.

Reopening so this can be discussed. The code change seems very simple, but more reviewers need to weigh in on whether having this abstraction is good or bad.

@TheQuantumPhysicist, since the first listed benefit of the change is "Easier testing for the bitcoin core application layer" maybe you could motivate this change a little more by adding new tests that take advantage of the abstraction.

Reopening so this can be discussed. The code change seems very simple, but more reviewers need to weigh in on whether having this abstraction is good or bad.

@TheQuantumPhysicist, since the first listed benefit of the change is "Easier testing for the bitcoin core application layer" maybe you could motivate this change a little more by adding new tests that take advantage of the abstraction.

I'm more than happy to add tests once there's agreement on the PR. Quite frankly I'm a little disappointed that no one cared, which is OK, but let's not get ahead of ourselves adding more code and work before people show the least interest.

And to get on the practicality train, the reviewer can see now there's abstract interfaces now for storage and iteration over storage, which can be used to define behavior or even mock the storage to simulate errors with something like gmock. It's not hard to see how interface-behavior can be well-defined and tested.

Cheers!

I am interested. But no one seems to have said anything for me to react.

This PR has needed rebase for a while. Often reviewers will not look at PRs that need rebasing as such review will be invalidated following rebase, and may also no longer be applicable.

I am interested. But no one seems to have said anything for me to react.

This PR has needed rebase for a while. Often reviewers will not look at PRs that need rebasing as such review will be invalidated following rebase, and may also no longer be applicable.

I'll take a look and see if It's not needed anymore.