Since the kernel library no longer depends on the system file, move it
to the common library instead in accordance to the diagram in
doc/design/libraries.md.
69d43905b7 test: add coverage for wallet read write db deadlock (furszy)
12daf6fcdc walletdb: scope bdb::EraseRecords under a single db txn (furszy)
043fcb0b05 wallet: bugfix, GetNewCursor() misses to provide batch ptr to BerkeleyCursor (furszy)
Pull request description:
Decoupled from #26644 so it can closed in favor of #26715.
Basically, with bdb, we can't make a write operation while we are traversing the db with the same db handler. These two operations are performed in different txn contexts and cause a deadlock.
Added coverage by using `EraseRecords()` which is the simplest function that executes this process.
To replicate it, need bdb support and drop the first commit. The test will run forever.
ACKs for top commit:
achow101:
ACK 69d43905b7
hebasto:
re-ACK 69d43905b7
Tree-SHA512: b3773be78925f674e962f4a5c54b398a9d0cfe697148c01c3ec0d68281cc5c1444b38165960d219ef3cf1a57c8ce6427f44a876275958d49bbc0808486e19d7d
so we erase all the records atomically or abort the entire
procedure.
and, at the same time, we can share the same db txn context
for the db cursor and the erase functionality.
extra note from the Db.cursor doc:
"If transaction protection is enabled, cursors must be
opened and closed within the context of a transaction"
thus why added a `CloseCursor` call before calling to
`TxnAbort/TxnCommit`.
This is cleanup that doesn't change external behavior.
- Removes awkward `StringMap` intermediate representation
- Simplifies CWallet code, deals with used address and received request
serialization in walletdb.cpp
- Adds test coverage and documentation
- Reduces memory usage
This PR doesn't change externally observable behavior. Internally, only change
in behavior is that EraseDestData deletes directly from database because the
`StringMap` is gone. This is more direct and efficient because it uses a single
btree lookup and scan instead of multiple lookups
Motivation for this cleanup is making changes like #18550, #18192, #13756
easier to reason about and less likely to result in unintended behavior and
bugs
Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
Instead of storing and passing around fixed strings for the purpose of
an address, use an enum.
This also rationalizes the CAddressBookData struct, documenting all fields and
making them public, and simplifying the representation to avoid bugs like
https://github.com/bitcoin/bitcoin/pull/26761#discussion_r1134615114 and make
it not possible to invalid address data like change addresses with labels.
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
The fs.* files are already part of the libbitcoin_util library. With the
introduction of the fs_helpers.* it makes sense to move fs.* into the
util/ directory as well.
4aebd832a4 db: Change DatabaseCursor::Next to return status enum (Andrew Chow)
d79e8dcf29 wallet: Have cursor users use DatabaseCursor directly (Andrew Chow)
7a198bba0a wallet: Introduce DatabaseCursor RAII class for managing cursor (Andrew Chow)
69efbc011b Move SafeDbt out of BerkeleyBatch (Andrew Chow)
Pull request description:
Instead of having database cursors be tied to a particular `DatabaseBatch` object and requiring its setup and teardown be separate functions in that batch, we can have cursors be separate RAII classes. This makes it easier to create and destroy cursors as well as having cursors that have slightly different behaviors.
Additionally, since reading data from a cursor is a tri-state, this PR changes the return value of the `Next` function (formerly `ReadAtCursor`) to return an Enum rather than the current system of 2 booleans. This greatly simplifies and unifies the code that deals with cursors as now there is no confusion as to what the function returns when there are no records left to be read.
Extracted from #24914
ACKs for top commit:
furszy:
diff ACK 4aebd83
theStack:
Code-review ACK 4aebd832a4
Tree-SHA512: 5d0be56a18de5b08c777dd5a73ba5a6ef1e696fdb07d1dca952a88ded07887b7c5c04342f9a76feb2f6fe24a45dc31f094f1f5d9500e6bdf4a44f4edb66dcaa1
f496528556 walletdb: refactor: drop unused `FindWalletTx` parameter and rename (Sebastian Falbesoner)
Pull request description:
Since commit 3340dbadd3 ("Remove -zapwallettxes"), the `FindWalletTx` helper is only needed to read tx hashes, so drop the other parameter and rename the method accordingly.
ACKs for top commit:
S3RK:
code review ACK f496528556
achow101:
ACK f496528556
vincenzopalazzo:
ACK https://github.com/bitcoin/bitcoin/pull/26702/commits/f496528556a67107d3d75d9c2ae345f7f4565d77
Tree-SHA512: ead85bc724462f9e920f9d7fe89679931361187579ffd6e63427c8bf5305cd5f71da24ed84f3b1bd22a12be46b5abec13f11822e71a3e1a63bf6cf49de950ab5
Next()'s result is a tri-state - failed, more to go, complete. Replace
the way that this is returned with an enum with values FAIL, MORE, and
DONE rather than with two booleans.
Since commit 3340dbadd3 ("Remove
-zapwallettxes"), the `FindWalletTx` helper is only needed to read tx
hashes, so drop the other parameter and rename the method accordingly.
