| Age | Commit message (Collapse) | Author |
|---|
|
To prepare for migrating wallets that are not loaded, when migration
occurs in the GUI, it should not rely on a WalletModel existing.
Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
|
|
In SetupLegacyScriptPubKeyMan, a base LegacyDataSPKM will be created if
the database has the format "bdb_ro" (i.e. the wallet was opened only
for migration purposes).
All of the loading functions are now called with a LegacyDataSPKM object
instead of LegacyScriptPubKeyMan.
|
|
In order to load the necessary data for migrating a legacy wallet
without the full LegacyScriptPubKeyMan, move the data storage and
loading components to LegacyDataSPKM. LegacyScriptPubKeyMan now
subclasses that.
|
|
d51fbab4b32d56765e8faab6ad01245fb259b0ca wallet, test: Be able to always swap BDB endianness (Ava Chow)
0b753156ce60c29efb2386954ba7555ad8f642f5 test: Test bdb_ro dump of wallet without reset LSNs (Ava Chow)
c1984f128284589423b7e0cc06c9a3b23a242d95 test: Test dumping dbs with overflow pages (Ava Chow)
fd7b16e391ed320e35255157a28be14c947ef30a test: Test dumps of other endian BDB files (Ava Chow)
6ace3e953f0864bd7818f040c59a1bc70aa47512 bdb: Be able to make byteswapped databases (Ava Chow)
d9878903fb34939dee8e1462f079acc68110253d Error if LSNs are not reset (Ava Chow)
4d7a3ae78e55f25868979f1bd920857a4aecb825 Berkeley RO Database fuzz test (TheCharlatan)
3568dce9e93295674cdf5458c5bdf93ff01fd0a2 tests: Add BerkeleyRO to db prefix tests (Ava Chow)
70cfbfdadf16d3b115309c6938f07ef5b96c7cc1 wallettool: Optionally use BERKELEY_RO as format when dumping BDB wallets (Ava Chow)
dd57713f6ede3d46e97ee7df87c10001b0bf4c3d Add MakeBerkeleyRODatabase (Ava Chow)
6e50bee67d1d58aecd8a0ce8b7c3f5a7979365f5 Implement handling of other endianness in BerkeleyRODatabase (Ava Chow)
cdd61c9cc108df8e13f4e3891ff2c96355b3ee38 wallet: implement independent BDB deserializer in BerkeleyRODatabase (Ava Chow)
ecba23097955dad7208baa687fc405c846aee794 wallet: implement BerkeleyRODatabase::Backup (Ava Chow)
0c8e72847603540bb29b8b8aeb80fa3f2e3a2c9a wallet: implement BerkeleyROBatch (Ava Chow)
756ff9b478484b17c4a6e65c171c2e4fecb21ad4 wallet: add dummy BerkeleyRODatabase and BerkeleyROBatch classes (Ava Chow)
ca18aea5c4975ace4e307be96c74641d203fa389 Add AutoFile::seek and tell (Ava Chow)
Pull request description:
Split from #26596
This PR adds `BerkeleyRODatabase` which is an independent implementation of a BDB file parser. It provides read only access to a BDB file, and can therefore be used as a read only database backend for wallets. This will be used for dumping legacy wallet records and migrating legacy wallets without the need for BDB itself.
Wallettool's `dump` command is changed to use `BerkeleyRODatabase` instead of `BerkeleyDatabase` (and `CWallet` itself) to demonstrate that this parser works and to test it against the existing wallettool functional tests.
ACKs for top commit:
josibake:
reACK https://github.com/bitcoin/bitcoin/commit/d51fbab4b32d56765e8faab6ad01245fb259b0ca
TheCharlatan:
Re-ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca
furszy:
reACK d51fbab4b32d56765e8faab6ad01245fb259b0ca
laanwj:
re-ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca
theStack:
ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca
Tree-SHA512: 1e7b97edf223b2974eed2e9eac1179fc82bb6359e0a66b7d2a0c8b9fa515eae9ea036f1edf7c76cdab2e75ad994962b134b41056ccfbc33b8d54f0859e86657b
|
|
Implements MakeBerkeleyRODatabase and adds DatabaseFormat::BERKELEY_RO
so that MakeDatabase can use BerkeleyRO as the backend database.
|
|
-BEGIN VERIFY SCRIPT-
perl -0777 -pi -e 's/#if defined\(HAVE_CONFIG_H\)\n#include <config\/bitcoin-config.h>.*\n#endif.*\n/#include <config\/bitcoin-config.h> \/\/ IWYU pragma: keep\n/g' $( git grep -l '#include <config/bitcoin-config.h>' )
-END VERIFY SCRIPT-
|
|
9d1dbbd4ceb8c04340927f5127195dc306adf3fc scripted-diff: Fix bitcoin_config_h includes (TheCharlatan)
Pull request description:
As mentioned in https://github.com/bitcoin/bitcoin/pull/26924#issuecomment-1403449932 and https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1922334399, it is currently not safe to remove `bitcoin-config.h` includes from headers because some unrelated file might be depending on it.
See also #26972 for discussion.
Solve this by including the file directly everywhere it's required, regardless of whether or not it's already included by another header.
There should be no functional change here, but it will allow us to safely remove includes from headers in the future.
~I'm afraid it's a bit tedious to reproduce these commits, but it's reasonably straightforward:~
Edit: See note below
```bash
# All commands executed from the src/ subdir.
# Collect all tokens from bitcoin-config.h.in
# Isolate the tokens and remove blank lines
# Replace newlines with | and remove the last trailing one
# Collect all files which use these tokens
# Filter out subprojects (proper forwarding can be verified from Makefiles)
# Filter out .rc files
# Save to a text file
git grep -E -l `grep undef config/bitcoin-config.h.in | cut -d" " -f2 | grep -v '^$' | tr '\n' '|' | sed 's/|$//'` | grep -v -e "^leveldb/" -e "^secp256k1/" -e "^crc32c/" -e "^minisketch/" -e "^Makefile" -e "\.rc$" > files-with-config-include.txt
# Find all files from the above list which don't include bitcoin-config.h
git grep -L -E "config/bitcoin-config.h" -- `cat files-with-config-include.txt`
# Include them manually with the exception of some files in crypto:
# crypto/sha256_arm_shani.cpp crypto/sha256_avx2.cpp crypto/sha256_sse41.cpp crypto/sha256_x86_shani.cpp
# These are exceptions which don't use bitcoin-config.h, rather the Makefile.am adds these cppflags manually.
# Commit changes. This should match the first commit of this PR.
# Use the same search as above to find all files which DON'T use any config tokens
git grep -E -L `grep undef config/bitcoin-config.h.in | cut -d" " -f2 | grep -v '^$' | tr '\n' '|' | sed 's/|$//'` | grep -v -e "^leveldb/" -e "^secp256k1/" -e "^crc32c/" -e "^minisketch/" -e "^Makefile" -e "\.rc$" > files-without-config-include.txt
# Manually remove the includes and commit changes. This should match the second commit of this PR.
```
Edit: I'll keep this old description for posterity, but the manual approach has been replaced with a scripted diff from TheCharlatan
ACKs for top commit:
maflcko:
ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3f 🚪
TheCharlatan:
ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3fc
hebasto:
ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3fc, I have reviewed the code and it looks OK.
fanquake:
ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3fc
Tree-SHA512: f11ddc4ae6a887f96b954a6b77f310558ddb271088a3fda3edc833669c4251b7f392515224bbb8e5f67eb2c799b4ffed3b07d96454e82ec635c686d0df545872
|
|
performance of descriptor wallets
e041ed9b755468d205ed48b29f6c4b9e9df8bc9f wallet: Retrieve ID from loaded DescSPKM directly (Ava Chow)
39640dd34e980e69d13664ddbc2a7612a1888ab4 wallet: Use scriptPubKeyCache in GetSolvingProvider (Ava Chow)
b410f68791143800968f4c628beda1c9f898b4f6 wallet: Use scriptPubKey cache in GetScriptPubKeyMans (Ava Chow)
edf4e73a163739a64eb9a54cd95413583a0e5c1f wallet: Use scriptPubKey cache in IsMine (Ava Chow)
37232332bd7253422ea92a8c9eeb36b12fc84b56 wallet: Cache scriptPubKeys for all DescriptorSPKMs (Ava Chow)
99a0cddbc04e8bfea3748a6cb4c0107392fab94f wallet: Introduce a callback called after TopUp completes (Ava Chow)
b27682593266c99507c720855cb34f5f7d363dd2 bench: Add a benchmark for ismine (Ava Chow)
Pull request description:
Wallets that have a ton of non-ranged descriptors (such as a migrated non-HD wallet) perform fairly poorly due to looping through all of the wallet's `ScriptPubKeyMan`s. This is done in various places, such as `IsMine`, and helper functions for fetching a `ScriptPubKeyMan` and a `SolvingProvider`. This also has a bit of a performance impact on standard descriptor wallets, although less noticeable due to the small number of SPKMs.
As these functions are based on doing `IsMine` for each `ScriptPubKeyMan`, we can improve this performance by caching `IsMine` scriptPubKeys for all descriptors and use that to determine which `ScriptPubKeyMan` to actually use for those things. This cache is used exclusively and we no longer iterate the SPKMs.
Also added a benchmark for `IsMine`.
ACKs for top commit:
ryanofsky:
Code review ACK e041ed9b755468d205ed48b29f6c4b9e9df8bc9f. Just suggested changes since last review
josibake:
ACK https://github.com/bitcoin/bitcoin/pull/26008/commits/e041ed9b755468d205ed48b29f6c4b9e9df8bc9f
furszy:
Code review ACK e041ed9b
Tree-SHA512: 8e7081991a025e682e9dea838b4543b0d179832d1c47397fb9fe7a97fa01eb699c15a5d5a785634926844fc83a46e6ac07ef753119f39d84423220ef8a548894
|
|
Instead of iterating m_spk_managers a DescriptorSPKM has been loaded in
order to get it's ID to compare, have LoadDescriptorSPKM return a
reference to the loaded DescriptorSPKM so it can be queried directly.
|
|
-BEGIN VERIFY SCRIPT-
regex_string='^(?!//).*(AC_APPLE_UNIVERSAL_BUILD|BOOST_PROCESS_USE_STD_FS|CHAR_EQUALS_INT8|CLIENT_VERSION_BUILD|CLIENT_VERSION_IS_RELEASE|CLIENT_VERSION_MAJOR|CLIENT_VERSION_MINOR|COPYRIGHT_HOLDERS|COPYRIGHT_HOLDERS_FINAL|COPYRIGHT_HOLDERS_SUBSTITUTION|COPYRIGHT_YEAR|ENABLE_ARM_SHANI|ENABLE_AVX2|ENABLE_EXTERNAL_SIGNER|ENABLE_SSE41|ENABLE_TRACING|ENABLE_WALLET|ENABLE_X86_SHANI|ENABLE_ZMQ|HAVE_BOOST|HAVE_BUILTIN_CLZL|HAVE_BUILTIN_CLZLL|HAVE_BYTESWAP_H|HAVE_CLMUL|HAVE_CONSENSUS_LIB|HAVE_CXX20|HAVE_DECL_BE16TOH|HAVE_DECL_BE32TOH|HAVE_DECL_BE64TOH|HAVE_DECL_BSWAP_16|HAVE_DECL_BSWAP_32|HAVE_DECL_BSWAP_64|HAVE_DECL_FORK|HAVE_DECL_FREEIFADDRS|HAVE_DECL_GETIFADDRS|HAVE_DECL_HTOBE16|HAVE_DECL_HTOBE32|HAVE_DECL_HTOBE64|HAVE_DECL_HTOLE16|HAVE_DECL_HTOLE32|HAVE_DECL_HTOLE64|HAVE_DECL_LE16TOH|HAVE_DECL_LE32TOH|HAVE_DECL_LE64TOH|HAVE_DECL_PIPE2|HAVE_DECL_SETSID|HAVE_DECL_STRERROR_R|HAVE_DEFAULT_VISIBILITY_ATTRIBUTE|HAVE_DLFCN_H|HAVE_DLLEXPORT_ATTRIBUTE|HAVE_ENDIAN_H|HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR|HAVE_FDATASYNC|HAVE_GETENTROPY_RAND|HAVE_GETRANDOM|HAVE_GMTIME_R|HAVE_INTTYPES_H|HAVE_LIBADVAPI32|HAVE_LIBCOMCTL32|HAVE_LIBCOMDLG32|HAVE_LIBGDI32|HAVE_LIBIPHLPAPI|HAVE_LIBKERNEL32|HAVE_LIBOLE32|HAVE_LIBOLEAUT32|HAVE_LIBSHELL32|HAVE_LIBSHLWAPI|HAVE_LIBUSER32|HAVE_LIBUUID|HAVE_LIBWINMM|HAVE_LIBWS2_32|HAVE_MALLOC_INFO|HAVE_MALLOPT_ARENA_MAX|HAVE_MINIUPNPC_MINIUPNPC_H|HAVE_MINIUPNPC_UPNPCOMMANDS_H|HAVE_MINIUPNPC_UPNPERRORS_H|HAVE_NATPMP_H|HAVE_O_CLOEXEC|HAVE_POSIX_FALLOCATE|HAVE_PTHREAD|HAVE_PTHREAD_PRIO_INHERIT|HAVE_STDINT_H|HAVE_STDIO_H|HAVE_STDLIB_H|HAVE_STRERROR_R|HAVE_STRINGS_H|HAVE_STRING_H|HAVE_STRONG_GETAUXVAL|HAVE_SYSCTL|HAVE_SYSCTL_ARND|HAVE_SYSTEM|HAVE_SYS_ENDIAN_H|HAVE_SYS_PRCTL_H|HAVE_SYS_RESOURCES_H|HAVE_SYS_SELECT_H|HAVE_SYS_STAT_H|HAVE_SYS_SYSCTL_H|HAVE_SYS_TYPES_H|HAVE_SYS_VMMETER_H|HAVE_THREAD_LOCAL|HAVE_TIMINGSAFE_BCMP|HAVE_UNISTD_H|HAVE_VM_VM_PARAM_H|LT_OBJDIR|PACKAGE_BUGREPORT|PACKAGE_NAME|PACKAGE_STRING|PACKAGE_TARNAME|PACKAGE_URL|PACKAGE_VERSION|PTHREAD_CREATE_JOINABLE|QT_QPA_PLATFORM_ANDROID|QT_QPA_PLATFORM_COCOA|QT_QPA_PLATFORM_MINIMAL|QT_QPA_PLATFORM_WINDOWS|QT_QPA_PLATFORM_XCB|QT_STATICPLUGIN|STDC_HEADERS|STRERROR_R_CHAR_P|USE_ASM|USE_BDB|USE_DBUS|USE_NATPMP|USE_QRCODE|USE_SQLITE|USE_UPNP|_FILE_OFFSET_BITS|_LARGE_FILES)'
exclusion_files=":(exclude)src/minisketch :(exclude)src/crc32c :(exclude)src/secp256k1 :(exclude)src/crypto/sha256_arm_shani.cpp :(exclude)src/crypto/sha256_avx2.cpp :(exclude)src/crypto/sha256_sse41.cpp :(exclude)src/crypto/sha256_x86_shani.cpp"
git grep --perl-regexp --files-with-matches "$regex_string" -- '*.cpp' $exclusion_files | xargs git grep -L "bitcoin-config.h" | while read -r file; do line_number=$(awk -v my_file="$file" '/\/\/ file COPYING or https?:\/\/www.opensource.org\/licenses\/mit-license.php\./ {line = NR} /^\/\// && NR == line + 1 {while(getline && /^\/\//) line = NR} END {print line+1}' "$file"); sed -i "${line_number}i\\\\n\#if defined(HAVE_CONFIG_H)\\n#include <config/bitcoin-config.h>\\n\#endif" "$file"; done;
git grep --perl-regexp --files-with-matches "$regex_string" -- '*.h' $exclusion_files | xargs git grep -L "bitcoin-config.h" | while read -r file; do sed -i "/#define.*_H/a \\\\n\#if defined(HAVE_CONFIG_H)\\n#include <config/bitcoin-config.h>\\n\#endif" "$file"; done;
for file in $(git grep --files-with-matches 'bitcoin-config.h' -- '*.cpp' '*.h' $exclusion_files); do if ! grep -q --perl-regexp "$regex_string" $file; then sed -i '/HAVE_CONFIG_H/{N;N;N;d;}' $file; fi; done;
-END VERIFY SCRIPT-
The first command creates a regular expression for matching all bitcoin-config.h symbols in the following form: ^(?!//).*(AC_APPLE_UNIVERSAL_BUILD|BOOST_PROCESS_USE_STD_FS|...|_LARGE_FILES). It was generated with:
./autogen.sh && printf '^(?!//).*(%s)' $(awk '/^#undef/ {print $2}' src/config/bitcoin-config.h.in | paste -sd "|" -)
The second command holds a list of files and directories that should not be processed. These include subtree directories as well as some crypto files that already get their symbols through the makefile.
The third command checks for missing bitcoin-config headers in .cpp files and adds the header if it is missing.
The fourth command checks for missing bitcoin-config headers in .h files and adds the header if it is missing.
The fifth command checks for unneeded bitcoin-config headers in sources files and removes the header if it is unneeded.
|
|
|
|
'RunWithinTxn()' provides a way to execute db operations within a
transactional context. It avoids writing repetitive boilerplate code for
starting and committing the database transaction.
|
|
9a3c5c8697659e34d0476103af942a4615818f4e scripted-diff: rename ZapSelectTx to RemoveTxs (furszy)
83b762845f5804f23b63526d403b2f327fe99632 wallet: batch and simplify ZapSelectTx process (furszy)
595d50a1032ad7ffa9945464c86aa57f16665e93 wallet: migration, remove extra NotifyTransactionChanged call (furszy)
a2b071f9920c2f4893afcc43a152f593c03966bf wallet: ZapSelectTx, remove db rewrite code (furszy)
Pull request description:
Work decoupled from #28574. Brother of #28894.
Includes two different, yet interconnected, performance and code improvements to the zap wallet transactions process.
1) As the goal of the `ZapSelectTx` function is to erase tx records that match any of the inputted hashes. There is no need to traverse the whole database record by record. We could just check if the tx exist, and remove it directly by calling `EraseTx()`.
2) Instead of performing single write operations per removed tx record, this PR batches them all within a single atomic db txn.
Moreover, these changes will enable us to consolidate all individual write operations that take place during the wallet migration process into a single db txn in the future.
ACKs for top commit:
achow101:
ACK 9a3c5c8697659e34d0476103af942a4615818f4e
josibake:
ACK https://github.com/bitcoin/bitcoin/pull/28987/commits/9a3c5c8697659e34d0476103af942a4615818f4e
Tree-SHA512: fb2ecc48224c400ab3b1fbb32e174b5b13bf03794717727f80f01f55fb183883b067a68c0a127b2de8885564da15425d021a96541953bf38a72becc2e9929ccf
|
|
The goal of the function is to erase the wallet transactions that
match the inputted hashes. There is no need to traverse the database,
reading record by record, to then perform single entry removals for
each of them.
To ensure consistency and improve performance, this change-set removes
all tx records within a single atomic db batch operation, as well as
it cleans up code, improves error handling and simplifies the
transactions removal process entirely.
This optimizes the removal of watch-only transactions during the wallet
migration process and the 'removeprunedfunds' RPC command.
|
|
When unreachable code isn't compiled, compile failures are not detected.
Fix this by leaving it unreachable, but compiling it.
Fixes https://github.com/bitcoin/bitcoin/pull/28999#discussion_r1465010916
Can be reviewed with --ignore-all-space
|
|
|
|
|
|
The remaining places are handled easier outside a scripted-diff.
-BEGIN VERIFY SCRIPT-
sed --regexp-extended -i 's/CDataStream ([0-9a-zA-Z_]+)\(SER_[A-Z]+, [A-Z_]+_VERSION\);/DataStream \1{};/g' $( git grep -l CDataStream)
sed -i 's/, CDataStream/, DataStream/g' src/wallet/walletdb.cpp
-END VERIFY SCRIPT-
|
|
|
|
CScriptID should be next to CScript just as CKeyID is next to CPubKey
|
|
The `UNKNOWN_DESCRIPTOR` error comes from the
`WalletDescriptor::DeserializeDescriptor` std::ios_base
exception, which contains further information about the
parsing error.
|
|
descriptor ID
5df988b534df842ddb658ad2a7ff0f12996c8478 test: add coverage for descriptor ID (furszy)
6a9510d2dabde1c9ee6c4226e3d16ca32eb48ac5 wallet: bugfix, always use apostrophe for spkm descriptor ID (furszy)
97a965d98f1582ea1b1377bd258c9088380e1b8b refactor: extract descriptor ID calculation from spkm GetID() (furszy)
1d207e3931cf076f69d4a8335cdd6c8ebb2a963f wallet: do not allow loading descriptor with an invalid ID (furszy)
Pull request description:
Aiming to fix #27915.
As we re-write the descriptor's db record every time that
the wallet is loaded (at `TopUp` time), if the spkm ID differs
from the one in db, the wallet will enter in an unrecoverable
corruption state (due to the storage of a descriptor with an ID
that is not linked to any other descriptor record in DB), and
no soft version will be able to open it anymore.
Because we cannot change the past, to stay compatible between
releases, we need to always use the apostrophe version for the
spkm IDs.
ACKs for top commit:
achow101:
ACK 5df988b534df842ddb658ad2a7ff0f12996c8478
Sjors:
tACK 5df988b534df842ddb658ad2a7ff0f12996c8478
Tree-SHA512: f63fc4aac7d21a4e515657471758d28857575e751865bfa359298f8b89b2568970029ca487a873c1786a5716325f453f06cd417ed193f3366417f6e8c2987332
|
|
fa38d862358b87219b12bf31236c52f28d9fc5d6 Use only Span{} constructor for byte-like types where possible (MarcoFalke)
fa257bc8312b91c2d281f48ca2500d9cba353cc5 util: Allow std::byte and char Span serialization (MarcoFalke)
Pull request description:
Seems odd to require developers to cast all byte-like spans passed to serialization to `unsigned char`-spans. Fix that by passing and accepting byte-like spans as-is. Finally, add tests and update the code to use just `Span` where possible.
ACKs for top commit:
sipa:
utACK fa38d862358b87219b12bf31236c52f28d9fc5d6
achow101:
ACK fa38d862358b87219b12bf31236c52f28d9fc5d6
ryanofsky:
Code review ACK fa38d862358b87219b12bf31236c52f28d9fc5d6. This looks great. The second commit really removes a lot of boilerplate and shows why the first commit is useful.
Tree-SHA512: 788592d9ff515c3ebe73d48f9ecbb8d239f5b985af86f09974e508cafb0ca6d73a959350295246b4dfb496149bc56330a0b5d659fc434ba6723dbaba0b7a49e5
|
|
If the computed descriptor's ID doesn't match the wallet's
DB spkm ID, return early from the loading process to prevent
DB data from being modified in any post-loading procedure
(e.g 'TopUp' updates the descriptor's data).
|
|
Instead of iterating the database to load the wallet, we now load
particular kinds of records in an order that we want them to be loaded.
So it is no longer necessary to iterate the entire database to load the
wallet.
|
|
Instead of loading decryption keys as we iterate the database, load them
explicitly.
|
|
Instead of dealing with these records when iterating the entire
database, find and handle them explicitly.
Loading of OLD_KEY records is bumped up to a LOAD_FAIL error as we will
not be able to use these types of keys which can lead to users missing
funds.
|
|
Instead of loading active spkm records as we come across them when
iterating the database, load them explicitly.
Due to exception handling changes, deserialization errors are now
treated as critical.
|
|
Instead of loading tx records as we come across them when iterating the
database, load them explicitly.
|
|
Instead of loading address book records as we come across them when
iterating the database, load them explicitly
Due to exception handling changes, deserialization errors are now
treated as critical.
The error message for noncritical errors has also been updated to
reflect that there's more data that could be missing than just address
book entries and tx data.
|
|
Instead of loading descriptor wallet records as we come across them when
iterating the database, loading them explicitly.
Exception handling for these records changes to a per-record type basis,
rather than globally. This results in some records now failing with a
critical error rather than a non-critical one.
|
|
Instead of loading legacy wallet records as we come across them when
iterating the database, load them explicitly.
Exception handling for these records changes to a per-record type basis,
rather than globally. This results in some records now failing with a
critical error rather than a non-critical one.
|
|
This removes bloat that is not needed.
|
|
To prepare to remove ReadKeyValue, change salvage to not use it
|
|
|
|
|
|
|
|
|
|
Move wallet flags loading to its own function in WalletBatch
The return value is changed to be TOO_NEW rather than CORRUPT when
unknown flags are found.
|
|
Move minversion loading to its own function in WalletBatch
|
|
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.
|
|
69d43905b7f1d4849dfaea1b5744ee473ccc8744 test: add coverage for wallet read write db deadlock (furszy)
12daf6fcdcbf5c7f03038338d843df3877714b24 walletdb: scope bdb::EraseRecords under a single db txn (furszy)
043fcb0b053eee6021cc75e3d3f41097f52698c0 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 69d43905b7f1d4849dfaea1b5744ee473ccc8744
hebasto:
re-ACK 69d43905b7f1d4849dfaea1b5744ee473ccc8744
Tree-SHA512: b3773be78925f674e962f4a5c54b398a9d0cfe697148c01c3ec0d68281cc5c1444b38165960d219ef3cf1a57c8ce6427f44a876275958d49bbc0808486e19d7d
|
|
This has been superseded by the MockableDatabase. Remove to avoid
confusion as to which type of mock database to use for testing.
|
|
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`.
|
|
It's only used by salvage, so make it local to that only.
|
|
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.
|
|
|
|
object with proper return codes
4aebd832a405090c2608e4b60bb4f34501bcea61 db: Change DatabaseCursor::Next to return status enum (Andrew Chow)
d79e8dcf2981ef1964a2fde8c472b5de1ca1c963 wallet: Have cursor users use DatabaseCursor directly (Andrew Chow)
7a198bba0a1d0a0f0fd4ca947955cb52b84bdd4b wallet: Introduce DatabaseCursor RAII class for managing cursor (Andrew Chow)
69efbc011bb74fcd8dd9ed2a8a5d31bc9e323c10 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 4aebd832a405090c2608e4b60bb4f34501bcea61
Tree-SHA512: 5d0be56a18de5b08c777dd5a73ba5a6ef1e696fdb07d1dca952a88ded07887b7c5c04342f9a76feb2f6fe24a45dc31f094f1f5d9500e6bdf4a44f4edb66dcaa1
|
