These methods haven't been used since the chainstate db cache has been
switched from per-tx to per-txout model years ago (PR #10195, commit
d342424301).
In Qt 6, registration of `QDataStream` streaming operators is done
automatically. Consequently, `qRegisterMetaTypeStreamOperators()` does
no longer exist. Calls to this method have to be removed when porting
to Qt 6.
See https://doc.qt.io/qt-6/qtcore-changes-qt6.html#the-qmetatype-class
58a9601dff build: globally define NOMINMAX (fanquake)
Pull request description:
Define (and document) `NOMINMAX` once, rather than across multiple
source files.
Defining this prevents the definition of min/max macros when using
mingw-w64, which may conflict with unprefixed std::min/max usage. While
that might not be the case for us, we'd always prefer to use the standard
library in any case.
For example:
73cadc06c6/mingw-w64-headers/include/ntdef.h (L289-L300)
Note that we already define NOMINMAX globally when building with MSVC.
Guix Build (arm64):
```bash
d3a3b7045dc1677f6a0a2a73a484f156c81ae764058003d9e870b346912b744a guix-build-58a9601dffa6/output/arm-linux-gnueabihf/SHA256SUMS.part
3e66540a3f8c8a10864ab2fed69581241fa41af86bbb028e5f7c3dd4ba859c64 guix-build-58a9601dffa6/output/arm-linux-gnueabihf/bitcoin-58a9601dffa6-arm-linux-gnueabihf-debug.tar.gz
78756e20d45e327cfd7f9e65858bf6d3814bcbe08f9f825fd6dfc9dff999ea6d guix-build-58a9601dffa6/output/arm-linux-gnueabihf/bitcoin-58a9601dffa6-arm-linux-gnueabihf.tar.gz
11073e88d4fd0411c5119a3dca3a90788693fa9aa5134339c84be98ae893cd77 guix-build-58a9601dffa6/output/arm64-apple-darwin/SHA256SUMS.part
deffd5f8c6286be34bc35e71ec70300bacb37e1b1a83e67c0833cb57d7a45529 guix-build-58a9601dffa6/output/arm64-apple-darwin/bitcoin-58a9601dffa6-arm64-apple-darwin-unsigned.dmg
acee7e98c5ec41f67e86c78dc5b45fa8bc82de86a04b8c43dbf9c59e7aff36a9 guix-build-58a9601dffa6/output/arm64-apple-darwin/bitcoin-58a9601dffa6-arm64-apple-darwin-unsigned.tar.gz
83f7cbaf6680fe8981db9260b97ca87d609a76c0857a744c7d406645d2484e1b guix-build-58a9601dffa6/output/arm64-apple-darwin/bitcoin-58a9601dffa6-arm64-apple-darwin.tar.gz
b8c73b40a5e307e9e7e482ce92164990d442f3f105a5240ec6eb96a775cb35d5 guix-build-58a9601dffa6/output/dist-archive/bitcoin-58a9601dffa6.tar.gz
cc435cd925771af7e261d0121047339ea8fddb0d1548b699c12108a62988cd32 guix-build-58a9601dffa6/output/powerpc64-linux-gnu/SHA256SUMS.part
7a68bd3181a054056b0a5eb6e830b90ac4ba8435114127d5f1720643011aa78f guix-build-58a9601dffa6/output/powerpc64-linux-gnu/bitcoin-58a9601dffa6-powerpc64-linux-gnu-debug.tar.gz
bc55b95e263c455a964d9463a3ee60dabee1d10cefc6641ed29a3b1b317d61e0 guix-build-58a9601dffa6/output/powerpc64-linux-gnu/bitcoin-58a9601dffa6-powerpc64-linux-gnu.tar.gz
49df78009d80af02262806c6c395e2c884a979b1ea13d01aa27d8188403e29d1 guix-build-58a9601dffa6/output/powerpc64le-linux-gnu/SHA256SUMS.part
29dc7a0e10707b3511fa2afb6977df7ebbb67f796d8be5a042abc14eba764aef guix-build-58a9601dffa6/output/powerpc64le-linux-gnu/bitcoin-58a9601dffa6-powerpc64le-linux-gnu-debug.tar.gz
51b7f8e1bccff1e2ce1860bbc382eefe648b90cc3374cdfa3a95a7454386e77d guix-build-58a9601dffa6/output/powerpc64le-linux-gnu/bitcoin-58a9601dffa6-powerpc64le-linux-gnu.tar.gz
e62e46d8cebbbfc0f587e930acb648fcae99cfe8b2f63aeba98e46e3338fe1e3 guix-build-58a9601dffa6/output/riscv64-linux-gnu/SHA256SUMS.part
fa5d0a074ca586583bf08dbf748909b3ff5e0a54a2e5aaa88abec666e17b4e72 guix-build-58a9601dffa6/output/riscv64-linux-gnu/bitcoin-58a9601dffa6-riscv64-linux-gnu-debug.tar.gz
684b2917fd27a41f884bb6870f7fac847d52b6f8b40df5779d1c674409f7cd14 guix-build-58a9601dffa6/output/riscv64-linux-gnu/bitcoin-58a9601dffa6-riscv64-linux-gnu.tar.gz
7d7cfd0212b49eec48c7f8dc0d97add53096685dfd646feac466c27a45d20c97 guix-build-58a9601dffa6/output/x86_64-apple-darwin/SHA256SUMS.part
d70ae6d060b7832f8741dc5d1958cc0d32702605c863254303107246deec0aa6 guix-build-58a9601dffa6/output/x86_64-apple-darwin/bitcoin-58a9601dffa6-x86_64-apple-darwin-unsigned.dmg
930f3ec43896404208ebdb582c9175e3a5a2470d778722e0001addde84dad99a guix-build-58a9601dffa6/output/x86_64-apple-darwin/bitcoin-58a9601dffa6-x86_64-apple-darwin-unsigned.tar.gz
2d8a9d12aadcf60634db953fcb8bd496a002608e9a64eb7d60bb7ffe1f94489f guix-build-58a9601dffa6/output/x86_64-apple-darwin/bitcoin-58a9601dffa6-x86_64-apple-darwin.tar.gz
10363729ece6e1c2cbdf435483006191bf17d1def2d318ff8357197d91c06ded guix-build-58a9601dffa6/output/x86_64-linux-gnu/SHA256SUMS.part
d50ec8e4f72e8b064b196eb0ece212f7b0b126f4b8b644c4451084cbf0416072 guix-build-58a9601dffa6/output/x86_64-linux-gnu/bitcoin-58a9601dffa6-x86_64-linux-gnu-debug.tar.gz
471e12b8715ecff4d99121c4bb3288ef4b005ca468810a714c67ea3e7c6669e9 guix-build-58a9601dffa6/output/x86_64-linux-gnu/bitcoin-58a9601dffa6-x86_64-linux-gnu.tar.gz
d63946401952d131fdf5df9442c52151d86e53f019234b5ad16fdef0d2976356 guix-build-58a9601dffa6/output/x86_64-w64-mingw32/SHA256SUMS.part
5359782e1eb6f449338f18e053ad82f25382d968690208ae5739d9338eb7bdc7 guix-build-58a9601dffa6/output/x86_64-w64-mingw32/bitcoin-58a9601dffa6-win64-debug.zip
0d387d5a4cb1d712556a3fe5b4bd1e928bb5fbbe57a85ee06c746f132a6b1ec5 guix-build-58a9601dffa6/output/x86_64-w64-mingw32/bitcoin-58a9601dffa6-win64-setup-unsigned.exe
dbfd7419d1d764e853a9dc041e276669b488aea4a80e21e4a175b6c3e512e70c guix-build-58a9601dffa6/output/x86_64-w64-mingw32/bitcoin-58a9601dffa6-win64-unsigned.tar.gz
0ba07504d9d5a12af9144e8b386b2640b48dba067d47c694a44ecffe56b0c0fc guix-build-58a9601dffa6/output/x86_64-w64-mingw32/bitcoin-58a9601dffa6-win64.zip
```
ACKs for top commit:
laanwj:
Code review ACK 58a9601dff
Tree-SHA512: d1c22b3d0d21ef8f9f605ef6ca06353e3f48536d84f3531f93d613a6ccbbe62f12fae0ed09e8b9a8940b0ef33f9d41d9991eb56fbe7c4ab48f0ce7fcf44e08b1
7ab72b9d2a qt: Fix `BitcoinAmountField`'s base widget (Hennadii Stepanov)
3262542104 qt, refactor: Fix `sendcoinsentry.ui` indentation (Hennadii Stepanov)
f3c7603329 qt, refactor: Convert `SendCoinsEntry` to a sub-`QWidget` (Hennadii Stepanov)
6420fb2005 qt, refactor: Drop unused `QFrame`s in `SendCoinsEntry` (Hennadii Stepanov)
Pull request description:
The `SendCoins_UnauthenticatedPaymentRequest` and `SendCoins_AuthenticatedPaymentRequest` sub-`QFrame`'s of the `SendCoinsEntry` widget have been unused since bitcoin/bitcoin#17165.
Removed all dead code. The resulted `SendCoinsEntry` widget has been simplified.
ACKs for top commit:
w0xlt:
Tested ACK 7ab72b9d2a
shaavan:
reACK 7ab72b9d2a
Tree-SHA512: a46db90d60fae584b52cc7edae910c295351cb3627e04d225708c50c04f7fdd81d2755e055115612a12a3c841e78c31bdcd57bed9feb1d3909f7a2f6e76bd356
be8d0dba15 test: refactor: save MiniWallet mode explicitly (Sebastian Falbesoner)
Pull request description:
Rather than abusing the member variables `self._priv_key` and `self._address` to determine the MiniWallet mode, save it explicitly (`self._mode`) in the constructor and use that instead to increase the readability and maintainability of the code.
ACKs for top commit:
MarcoFalke:
review ACK be8d0dba15🔳
Tree-SHA512: 882c635e39c89911d995917a0603395158ee86dc46b26a49815756db67c61a7aa35059eddf1dc6f2933a77169941426b98bd463e60e39515a596b1b69edf89df
Rather than abusing the member variables self._priv_key and
self._address to determine the MiniWallet mode, save it explicitly
instead in the constructor to increase the readability and
maintainability of the code.
40566e21c0 If -prune=0 is set, Uncheck Prune on Intro page (Jadi)
Pull request description:
If the bitcoin-qt is started with -prune=0 arg, On the Intro page,
the Prune Checkbox will be unchecked too, to prevent confusions.
refs: https://github.com/bitcoin/bitcoin/issues/25052
ACKs for top commit:
hebasto:
re-ACK 40566e21c0
Tree-SHA512: d5e0b76a7d20ae806e61a416fd907650f15a744a5823d0f8b57a634cb099bb135199e69a787bd54ecde2cf84e95633f40ff407a722350f337b27de395a6e0f78
d338712886 scripted-diff: rename fAllowOtherInputs -> m_allow_other_inputs (furszy)
8dea74a8ff refactor: use GetWalletTx in SelectCoins instead of access mapWallet (furszy)
b4e2d4d4ee wallet: move "use-only coinControl inputs" below the selected inputs lookup (furszy)
25749f1df7 wallet: unify “allow/block other inputs“ concept (furszy)
Pull request description:
Seeking to make the `CoinControl` options less confusing/redundant.
It should have no functional changes.
The too long to read technical description; remove `m_add_inputs`, we can use the already existent `fAllowOtherInputs` flag.
In #16377 the `CoinControl` flag ‘m_add_inputs’ was added to tell the coin filtering and selection process two things:
- Coin Filtering: Only use the provided inputs. Skip the Rest.
- Coin Selection: Search the wtxs-outputs and append all the `CoinControl` internal and external selected outpoints to the selection result (skipping all the available output checks). Nothing else.
Meanwhile, in `CoinControl` we already have a flag ‘fAllowOtherInputs’ which is already saying:
- Coin Filtering: Only use the provided inputs. Skip the Rest.
- Coin Selection: If false, no selection process -> append all the `CoinControl` selected outpoints to the selection result (while they passed all the `AvailableCoins` checks and are available in the 'vCoins' vector).
### Changes
As can notice, the first point in the coin filtering process is duplicated in the two option flags. And the second one, is slightly different merely because it takes into account whether the coin is on the `AvailableCoins` vector or not.
So it makes sense to merge ‘m_add_inputs’ and ‘fAllowOtherInputs’ into a single field for the coin filtering process while introduce other changes to add the missing/skipped internal and external coins into 'vCoins' vector if they were manually selected by the user.
——————————————————————————————————
Just as an extra note:
On top of this, I’m working on unifying/untangling further the coin filtering and selection processes so we have less duplicate functionality in both processes.
ACKs for top commit:
laanwj:
Code review ACK d338712886
Tree-SHA512: 98920b80dd787cfe737dacd4c59575dfa8393c799b55f2aaef9aed2b15c61470715a88663557b49c7400938220f99af7690be01980a8684f4f71947407f21750
0d8e68d705 refactor: move DEFAULT_*_LIMIT assertions from validation to policy (fanquake)
9c94f3b3a7 refactor: move EXTRA_DESCENDANT_TX_SIZE_LIMIT to policy/policy.h (fanquake)
39c6036253 refactor: use braced initialization in policy/policy.h (fanquake)
01ccfbe3aa scripted-diff: use static constexpr in policy/policy.h (fanquake)
62d56bb714 refactor: Move DEFAULT_DESCENDANT_SIZE_LIMIT to policy/policy.h (fanquake)
a34aa4c187 refactor: Move DEFAULT_DESCENDANT_LIMIT to policy/policy.h (fanquake)
05fc5fdc13 refactor: Move DEFAULT_ANCESTOR_SIZE_LIMIT to policy/policy.h (fanquake)
da8d304960 refactor: Move DEFAULT_ANCESTOR_LIMIT to policy/policy.h (CAnon)
Pull request description:
Picks up #25295. Which was a follow up to [a comment in #25254](https://github.com/bitcoin/bitcoin/pull/25254#discussion_r890595318).
Moves policy constants from validation.h to policy.h.
ACKs for top commit:
laanwj:
Code review ACK 0d8e68d705
w0xlt:
reACK 0d8e68d705
darosior:
ACK 0d8e68d705
Tree-SHA512: 79900b09dc3a8020b5053ec734f462cb6e8184ed2b76e9d8afae7fe5331bbc906daaa42c0f622782797d971aaf5698aa0155511ec1d15582cc7675c271664a8d
Do the closing in `Sock::Reset()` and remove the standalone
`CloseSocket()`.
This reduces the exposure of low-level sockets (i.e. integer file
descriptors) outside of the `Sock` class.
Define (and document) `NOMINMAX` once, rather than across multiple
source files.
Defining this prevents the definition of min/max macros when using
mingw-w64, which may conflict with unprefixed std::min/max usage. While
that might not be the case for us, we'd always prefer to use the standard
library in any case.
For example:
73cadc06c6/mingw-w64-headers/include/ntdef.h (L289-L300)
Otherwise, RPC commands such as `walletcreatefundedpsbt` will not support the manual selection of locked, spent and externally added coins.
Full explanation is inside #25118 comments but brief summary is:
`vCoins` at `SelectCoins` time could not be containing the manually selected input because, even when they were selected by the user, the current `AvailableCoins` flow skips locked and spent coins.
Extra note: this is an intermediate step to unify the `fAllowOtherInputs`/`m_add_inputs` concepts. It will not be a problem anymore in the future when we finally decouple the wtx-outputs lookup process from `SelectCoins` and don't skip the user's manually selected coins in `AvailableCoins`.
Seeking to make the `CoinControl` option less confusing/redundant.
In #16377 the `CoinControl` flag ‘m_add_inputs’ was added to tell the coin filtering and selection process two things:
- Coin Filtering: Only use the provided inputs. Skip the Rest.
- Coin Selection: Search the wtxs-outputs and append all the `CoinControl` selected outpoints to the selection result (skipping all the available output checks). Nothing else.
Meanwhile, in `CoinControl` we already have a flag ‘fAllowOtherInputs’ which is already saying:
- Coin Filtering: Only use the provided inputs. Skip the Rest.
- Coin Selection: If false, no selection process -> append all the `CoinControl` selected outpoints to the selection result (while they passed all the `AvailableCoins` checks and are available in the 'vCoins' vector).
As can notice, the first point in the coin filtering process is duplicated in the two option flags. And the second one, is slightly different merely because it takes into account whether the coin is on the `AvailableCoins` vector or not.
So it makes sense to merge ‘m_add_inputs’ and ‘fAllowOtherInputs’ into a single field for the coin filtering process while introduce other changes to add the missing/skipped coins into 'vCoins' vector if they were manually selected by the user (follow-up commits).
7ca8726f63 wallet: fix warning: "argument name 'feerate' in comment does not match parameter name" (furszy)
Pull request description:
Should solve the tiny https://github.com/bitcoin/bitcoin/pull/25005#issuecomment-1159403854.
Which merely happens for the extra "=" character after the comma.
ACKs for top commit:
Empact:
Code Review ACK 7ca8726f63
Tree-SHA512: e5368c1114f715bd93cb653c607fd0942ab0b79f709ed7aa627b3fc7e7efd096c92c5c86908c7f26c363b21e391a8faa812727eb32c285e54da3ce0429290361
fd5c996d16 wallet: GetAvailableBalance, remove double walk-through every available coin (furszy)
162d4ad10f wallet: add 'only_spendable' filter to AvailableCoins (furszy)
cdf185ccfb wallet: remove unused IsSpentKey(hash, index) method (furszy)
4b83bf8dbc wallet: avoid extra IsSpentKey -> GetWalletTx lookups (furszy)
3d8a282257 wallet: decouple IsSpentKey(scriptPubKey) from IsSpentKey(hash, n) (furszy)
a06fa94ff8 wallet: IsSpent, 'COutPoint' arg instead of (hash, index) (furszy)
91902b7720 wallet: IsLockedCoin, 'COutPoint' arg instead of (hash, index) (furszy)
9472ca0a65 wallet: AvailableCoins, don't call 'wtx.tx->vout[i]' multiple times (furszy)
4ce235ef8f wallet: return 'CoinsResult' struct in `AvailableCoins` (furszy)
Pull request description:
This started in #24845 but grew out of scope of it.
So, points tackled:
1) Avoid extra `GetWalletTx` lookups inside `AvailableCoins -> IsSpentKey`.
`IsSpentKey` was receiving the tx hash and index to internally lookup the tx inside the wallet's map. As all the `IsSpentKey` function callers already have the wtx available, them can provide the `scriptPubKey` directly.
2) Most of the time, we call `Wallet::AvailableCoins`, and later on the process, skip the non-spendable coins from the result in subsequent for-loops. So to speedup the process: introduced the ability to filter by "only_spendable" coins inside `Wallet::AvailableCoins` directly.
(the non-spendable coins skip examples are inside `AttemptSelection->GroupOutputs` and `GetAvailableBalance`).
4) Refactored `AvailableCoins` in several ways:
a) Now it will return a new struct `CoinsResult` instead of receiving the vCoins vector reference (which was being cleared at the beginning of the method anyway). --> this is coming from #24845 but cherry-picked it here too to make the following commits look nicer.
b) Unified all the 'wtx.tx->vout[I]' calls into a single call (coming from this comment https://github.com/bitcoin/bitcoin/pull/24699#discussion_r854163032).
5) The wallet `IsLockedCoin` and `IsSpent` methods now accept an `OutPoint` instead of a hash:index. Which let me cleanup a bunch of extra code.
6) Speeded up the wallet 'GetAvailableBalance': filtering `AvailableCoins` by spendable outputs only and using the 'AvailableCoins' retrieved `total_amount` instead of looping over all the retrieved coins once more.
-------------------------------------------------------
Side topic, all this process will look even nicer with #25218
ACKs for top commit:
achow101:
ACK fd5c996d16
brunoerg:
crACK fd5c996d16
w0xlt:
Code Review ACK fd5c996d16
Tree-SHA512: 376a85476f907f4f7d1fc3de74b3dbe159b8cc24687374d8739711ad202ea07a33e86f4e66dece836da3ae6985147119fe584f6e672f11d0450ba6bd165b3220
dcf36fe8e3 test: implement 'bech32m' mode for `getnewdestination()` helper (Sebastian Falbesoner)
1999dcfa40 test: add helpers for creating P2TR scripts/addresses from output key (Sebastian Falbesoner)
Pull request description:
This PR adds the missing 'bech32m' mode for the `getnewdestination()` helper and sets it as default, i.e. the function returns a tuple (output x-only-pubkey, scriptPubKey, taproot address) now if not specified otherwise. In a preparation commit, the helpers `output_key_to_p2tr{_script}` are introduced. Note that in contrast to all other common script output types, there are usually _two_ keys involved in creating a taproot output (internal key and output key), hence the prefix `output_` is used to clarify that the output key is expected and the helpers don't do any key tweaking.
Thanks to michaelfolkson (for pointing out this TODO that I forgot about) and sipa (for patiently explaining basic things about BIP341).
ACKs for top commit:
michaelfolkson:
ACK dcf36fe8e3
w0xlt:
reACK dcf36fe8e3
Tree-SHA512: 5bb8d5fd96c63092ede10c3f022ffb2e13c14e333c4aa73348d95deb70cbf0a74745218dc4a7c419eb846793dd69e8217a7b4332a13ae2b2758e100b51fb1a9f
ce1c8104aa build: Remove unused `LIBBITCOIN_KERNEL` variable (Hennadii Stepanov)
Pull request description:
Noticed that while working on moving the build system to CMake. But I [am not the first](https://github.com/bitcoin/bitcoin/pull/24322/files#r860472867) one :)
ACKs for top commit:
laanwj:
ACK ce1c8104aa
Tree-SHA512: 877b9f0d64c4c72f403335d7a8462e551f6f8cd5648a211f980d6da5ed7683521d6549f6acf15ac8e55f67915c556201a1980228c975a22135507746e2f392ce
241c4d047e doc: Correct comment describing value of MAX_FILE_SIZE_PSBT as in MiB (Ben Woosley)
64f81a38b9 doc: Correct nPruneTarget misidentifying units of variable (darosior)
Pull request description:
In https://github.com/bitcoin/bitcoin/pull/15848, darosior fixed up a comment which mis-identified the units of a constant.
Another comment misidentified a value as in MiB rather than MB.
ACKs for top commit:
laanwj:
Code review ACK 241c4d047e
darosior:
ACK 241c4d047e, with or without https://github.com/bitcoin/bitcoin/pull/25299#discussion_r892705277
Tree-SHA512: 96c03a35140e5c53759f387bd292a8f8f621ba74c3cf6621939fad40f48892d23141c747ad3ab4fd71108e3b737670175abc2eb3990a1bd1660366c55d61ddf8
f7595f1354 build: add and use CXX_STANDARD in depends (fanquake)
7e7b3e42fa build: add and use C_STANDARD in depends (fanquake)
Pull request description:
By explicitly setting a C standard version we avoid any potential for issues/differences in libraries that may come about due to C STD version, as well as avoid potentially being opted into newer code / features in libraries when compiler defaults change (i.e as of 11.0.0, Clang now defaults to gnu17 over gnu11).
This should be a no-op for our release builds, because it's just explicitly setting the default that is [already being used](https://github.com/fanquake/core-review/blob/master/compiler-defaults.md). However this is relevant for anyone building depends with a newer compiler.
I found [one broken `__STDC_VERSION__` check in the](https://github.com/miniupnp/miniupnp/pull/552) miniupnpc header.
At the same time, add `CXX_STANDARD` for setting our C++ standard, and use that over setting `-std=c++17` for cxx packages.
Guix builds:
```bash
```
ACKs for top commit:
dongcarl:
Code Review ACK f7595f1354
laanwj:
Code review ACK f7595f1354
Tree-SHA512: 9255190d91ba3de20762b1d6af35c59d64f3d77a52bbe9a3f1dfb6bcf16daef66054ebef96b58e7285cd01bf613e69a78bd5e8681c21293e254f23d1fa7b0f71
