Previously, the `utxocache:flush` tracepoint was in the wrong scope and
reached every time `CChainState::FlushStateToDisk` was called, even when
there was no flushing of the cache. The tracepoint is now properly scoped
and will be reached during a full flush.
Inside the scope, the `fDoFullFlush` value will always be `true`, so it
doesn't need to be logged separately. Hence, it's dropped from the
tracepoint arguments.
The logic of these functions is already covered by existing unit tests
using publicly exposed functions of the interface.
Therefore, removing them does not decrease test coverage.
Switches to AddrMan for tests that use no features of AddrManTest.
Also removes unusued AddrManTest variables
Co-Authored-By: Amiti Uttarwar <amiti@uttarwar.org>
fa50d8b66e doc: Remove TODO comment in tx_verify (MarcoFalke)
Pull request description:
The comment has no clear motivation, so it seems better to remove it and fix it when there is a reason.
An alternative (if a fix isn't possible when there is a clear motivation) would be to create an issue thread for easier discussion.
ACKs for top commit:
fanquake:
ACK fa50d8b66e
Tree-SHA512: e9c25bab46a73b7c2db288c62ed9838a5e794b3b72db494173f4502da60b58dec4383064964c0842932cd30e4251fc01ad0c28681e2ef6cb442482eea2bad595
fa993d0e7e doc: Fix -changetype help text (MarcoFalke)
Pull request description:
This was forgotten in commit 3ac38058ce
ACKs for top commit:
shaavan:
ACK fa993d0e7e
w0xlt:
ACK fa993d0
josibake:
ACK fa993d0e7e
Tree-SHA512: 9f84b1168e3b3ab06e5c1f4915a1874598b273099eb5878ed28c3a66f1484e34c836fd3c68c4131bee541f3428052f6b66e02b192170752d1082de689d44cd4d
fa562fdd5e doc: Remove fixed TODO from wallet/feebumper (MarcoFalke)
Pull request description:
Fixed in commit 9522b53a91
ACKs for top commit:
shaavan:
ACK fa562fdd5e
Tree-SHA512: 968fda0994020c369b7acfc01db109d0f50d4c137fadf533ae55d0e14a353ebbde4320e798cf89e5489f1020c459712631b3967976c1f73d99db8a2d1cbad982
7b481f015a Fix Racy ParseOpCode function initialization (Jeremy Rubin)
Pull request description:
If multiple callers call ParseOpCode concurrently it will cause a race condition. We can either move the static to it's own area and require init be called explicitly, or just allow concurrent first callers to race to fill in an atomic variable that never changes thereafter. The second approach is taken here.
Static initialization *is* threadsafe, but only insofar as definining the variable being guaranteed to be called once. This is used incorrectly here.
practicalswift --> are there tools we can deploy to catch usage like this?
ACKs for top commit:
MarcoFalke:
re-ACK 7b481f015a 🗣
Tree-SHA512: cbf9dc3af26d7335305026f32ce8472a018309b89b3d81a67357e59fbeed72c37b5b8a6e30325ea68145c3b2403867be82de01f22decefb6e6717cf0c0045633
ff5f6dea53 scripted-diff: Rename interfaces::WalletClient to interfaces::WalletLoader (Russell Yanofsky)
Pull request description:
Name has been confusing since it was introduced, and it was pointed in recent review club https://bitcoincore.reviews/10102 that it was particularly unclear how `interfaces::WalletClient` was different from `interfaces::Wallet`.
ACKs for top commit:
w0xlt:
ACK ff5f6de
Tree-SHA512: 26fa10baa457e76da1933adab187e9be61b8d76cff1cf2c73ad4320461c7e31fb9db07b7c2486998294826beb4a1aca255c14903920b443db6213e653c5f7e0a
826e12b010 test: call VerifyLoadedChainstate during ChainTestingSetup (James O'Beirne)
Pull request description:
for additional coverage and similarity to actual init process.
Followup to #23280.
ACKs for top commit:
dongcarl:
Code Review ACK 826e12b010
ryanofsky:
Code review ACK 826e12b010
Tree-SHA512: a4e7fd25e5d7a08b1e154ae6daf67c3048260a2684b0e569b544dd826693b7b969db9923b191e499cb8d8d0a2a73eb9330ff45909313145a9abb6052eb8c3ad9
Name has been confusing since it was introduced, and it was pointed in
recent review club as https://bitcoincore.reviews/10102 that it was
particularly unclear how interfaces::WalletClient was different from
interfaces::Wallet.
-BEGIN VERIFY SCRIPT-
ren() { git grep -l "$1" src | xargs sed -i "s/$1/$2/g"; }
ren WalletClient WalletLoader
ren walletClient walletLoader
ren wallet_client wallet_loader
ren "wallet clients release the wallet" "wallet pointer owners release the wallet"
ren "wallet client" "wallet loader"
ren "Wallet client" "Wallet loader"
-END VERIFY SCRIPT-
a2a92317ad rpc: Add warning to user about newkeypool command (Samuel Dobson)
Pull request description:
This PR prevents `newkeypool` from being run on non-HD wallets, because this would require a new backup every time, so it isn't very safe.
David Harding also suggested [here](https://github.com/bitcoin/bitcoin/pull/23093#issuecomment-945350003) that the RPC help text should include a warning to the users about the interaction between newkeypool.
ACKs for top commit:
achow101:
ACK a2a92317ad
Tree-SHA512: 0aa497900f1d179764bce13ffce0bb081ba2ca354492bf2e04b21d0212e960b3ed13a386fbf65602b6b50461f4056a0285752ef707d312da28e82449cd8ea048
fada6c65d2 wallet: Strictly match tx change type to improve privacy (MarcoFalke)
Pull request description:
Currently the change type will only match a destination by accident, making it easier to determine the change.
Fix that by strictly matching one of the destinations.
ACKs for top commit:
S3RK:
Concept & Approach ACK fada6c6. Also did light code review .
achow101:
ACK fada6c65d2
prayank23:
tACK fada6c65d2
w0xlt:
tACK fada6c6
Tree-SHA512: 2b072c3c32debac7b0bef07a6df9a8f1a631e0f7d556b859973f18894ca490225582dc13e4588b29fa205ffbcd30fb632d5313b304d10ad17a26adc3f7684471
92082ea0bb Change time variable type to std::chrono::seconds in src/net_processing.cpp (Shashwat)
Pull request description:
- This is a follow-up to PR #23758
- This changes the remaining time variable in `net_processing.cpp` from **int64_t** to **std::chrono::seconds**
ACKs for top commit:
naumenkogs:
ACK 92082ea0bb
hebasto:
re-ACK 92082ea0bb
Tree-SHA512: 559e351d9046d4ba2b842ae38da13b4befc7feee71f0762f97907812471e2840b0d43c90c92222d15619fe40cc21f11d40900500ca07b470f7ac8b0046cc1d68
bf4f817135 refactor: addrman_select test (josibake)
5a64dc018c refactor: addrman_evictionworks test (josibake)
e281fccd8a refactor: addrman_noevict test (josibake)
8bdd9240d4 refactor: addrman_selecttriedcollisions test (josibake)
Pull request description:
As a follow-up to #23713 , this PR refactors the remaining tests in `src/tests/addrman_tests.cpp` to use the output from `AddrMan::Good()` where appropriate.
ACKs for top commit:
naumenkogs:
ACK bf4f817135
mzumsande:
Code Review ACK bf4f817135
Tree-SHA512: 93cc127aecff42c1c174daa04911af7e3460a5c40ddf96952fe4a6ab86fa1ff22d66724326abb709008d7f9f79c26c55c6d62753c40059c9ac60f869507ec913
