aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2022-03-16Merge bitcoin/bitcoin#18815: bench: Add logging benchmarkMarcoFalke
fafe06c379316f165e88b8de7300a716cef25d0a bench: Sort bench_bench_bitcoin_SOURCES (MarcoFalke) fa31dc9b714401b67480232ef552d1479f5e6902 bench: Add logging benchmark (MarcoFalke) Pull request description: Might make finding performance bottlenecks or regressions (https://github.com/bitcoin/bitcoin/pull/17218) easier. For example, fuzzing relies on disabled logging to be as fast as possible. ACKs for top commit: dergoegge: ACK fafe06c379316f165e88b8de7300a716cef25d0a Tree-SHA512: dd858f3234a4dfb00bd7dec4398eb076370a4b9746aa24eecee7da86f6882398a2d086e5ab0b7c9f7321abcb135e7ffc54cc78e60d18b90379c6dba6d613b3f7
2022-03-16Merge bitcoin/bitcoin#23565: doc: rewrite dependencies.mdfanquake
893e18059f1f92a254aa2026312a769c0e711db6 doc: rework dependencies.md (fanquake) Pull request description: This PR rewrites dependencies.md. The current list is hard to parse, includes information that is either incorrect and/or misleading, and duplicates info in other documentation. The list of dependencies is much smaller, because it's now just the actual dependencies of Bitcoin Core, not random Qt things, or the dependencies of other tooling. We don't need _another_ section on configure flag usage, or, to have duplicated lists of dependencies in other build docs, as that somewhat defeats the point of having dependencies.md, and just means more effort keeping things in sync. ACKs for top commit: MarcoFalke: cr ACK 893e18059f1f92a254aa2026312a769c0e711db6 hebasto: ACK 893e18059f1f92a254aa2026312a769c0e711db6, I have reviewed the code and it looks OK, I agree it can be merged. jarolrod: crACK https://github.com/bitcoin/bitcoin/commit/893e18059f1f92a254aa2026312a769c0e711db6 Tree-SHA512: 6750eaf70d5ebc9c364ade1d4b5b689e3094020eeb444a3de93b33d9a57a1577949a461f8209442d3954ccb22ab038c7e8cf6dfff5623e4f2713606b6798c37e
2022-03-16doc: rework dependencies.mdfanquake
2022-03-16Merge bitcoin/bitcoin#24537: rpc: Split mempool RPCs from blockchain.cppMarcoFalke
fad4c8934c7b4342076bd01daa4abf4d83617b17 Add RegisterMempoolRPCCommands helper (MarcoFalke) fafd40b5413aea5f8ec47c24fdb2ccc702e7cdd7 refactor: Avoid int64_t -> size_t -> int64_t conversion (MarcoFalke) fa2a5f301aa678aa7b6be4bb7987f2bcbdaf2be2 rpc: Move mempool RPCs to new file (MarcoFalke) Pull request description: The `blockchain.cpp` file is quite large. This makes it harder to navigate and increases the memory required to compile. Improve on both issues by splitting up the mempool RPCs to a separate file. ACKs for top commit: promag: Code review ACK fad4c8934c7b4342076bd01daa4abf4d83617b17. theStack: Code-review ACK fad4c8934c7b4342076bd01daa4abf4d83617b17 🏞️ Tree-SHA512: 7f13168ea2cbea51eaef05ca1604fddc919480a2128ec7fa6b1f9365ec5e4822c3df93eb408a19f038c627f2309fa282b9f7f7ec45e5e661fc728f6b33157f89
2022-03-16Merge bitcoin/bitcoin#24572: ci: Temporarily use clang-13 to work around ↵MarcoFalke
clang-14 TSan bug fa43933e3b3f8bd2992340bdd744fdab680565f8 ci: Temporarily use clang-13 to work around clang-14 TSan bug (MarcoFalke) Pull request description: There is an increase in intermittent issues in the TSan task. The increase correlates with Ubuntu Jammy's bump of `clang` from `clang-13` to `clang-14`. Temporarily work around that. ACKs for top commit: hebasto: ACK fa43933e3b3f8bd2992340bdd744fdab680565f8, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: adf7d385e1cddaeb2d06c3a3838f87070fb4ffdcc9a37da204b332ab28f40042bd34bd8b6d1d4710511865b4acafa7cf7303c4abf59862c5d29b168e2774da31
2022-03-15Add RegisterMempoolRPCCommands helperMarcoFalke
2022-03-15ci: Temporarily use clang-13 to work around clang-14 TSan bugMarcoFalke
2022-03-15Merge bitcoin/bitcoin#24510: test: check for importprunedfunds RPC errorsMarcoFalke
7573789925f6b5c3d73f43d2ebf8c732945c1824 test: check for importprunedfunds RPC errors (Sebastian Falbesoner) Pull request description: This PR adds missing test coverage for the following errors of the `importprunedfunds` RPC: https://github.com/bitcoin/bitcoin/blob/7003b6ab24f6adfffd71d7b7d4182afde52ff859/src/wallet/rpc/backup.cpp#L320-L322 https://github.com/bitcoin/bitcoin/blob/7003b6ab24f6adfffd71d7b7d4182afde52ff859/src/wallet/rpc/backup.cpp#L332-L334 https://github.com/bitcoin/bitcoin/blob/7003b6ab24f6adfffd71d7b7d4182afde52ff859/src/wallet/rpc/backup.cpp#L338-L340 https://github.com/bitcoin/bitcoin/blob/7003b6ab24f6adfffd71d7b7d4182afde52ff859/src/wallet/rpc/backup.cpp#L343-L345 ACKs for top commit: MarcoFalke: review ACK 7573789925f6b5c3d73f43d2ebf8c732945c1824 Tree-SHA512: b054520d102e5940bdeed2456ca644e91afb187d169b751b1262ce34480e4e9fbe1616ab184a78777c184350dced23508c3d367ed5825cab78bb5ad687fd7dac
2022-03-14Merge bitcoin/bitcoin#24080: policy: Remove unused locktime flagsMarcoFalke
fa8d4d9128c35de0fe715f2e2b99269d23c09cc1 scripted-diff: Clarify CheckFinalTxAtTip name (MarcoFalke) fa4e30b0f36f2e7a09db7d30dca9008ed9dbcb35 policy: Remove unused locktime flags (MarcoFalke) Pull request description: The locktime flags have many issues: * They are passed in by a default argument, which is fragile. It has already lead to bugs like the one fixed in commit e30b6ea194fee3bb95a45e7b732a99566b88f1f5. * They are negative (signed), which doesn't make sense for flags (unsigned in general). According to the review comments when the code was added: "The max on the flags is a fairly weird operation." (https://github.com/bitcoin/bitcoin/pull/6566#issuecomment-150310861) * No call site relies on the default argument and they all pass in a single compile-time constant, rendering most of the code dead and untested. * The dead code calls `GetAdjustedTime` (network adjusted time), which has its own issues. See https://github.com/bitcoin/bitcoin/issues/4521 Fix all issues by removing them ACKs for top commit: ajtowns: ACK fa8d4d9128c35de0fe715f2e2b99269d23c09cc1 theStack: Code-review ACK fa8d4d9128c35de0fe715f2e2b99269d23c09cc1 glozow: ACK fa8d4d9128c35de0fe715f2e2b99269d23c09cc1, agree the default arg `flags` is a massive footgun and just setting max flags is weird. Adding `AtTip` to the names makes sense to me, since they're both testing for *next* block and only ever used for {,re}addition to mempool. Tree-SHA512: 79f4a52f34909eb598d88bbae7afe8abe5f85f45c128483d16aa83dacd0e5579e561b725d01b1e9a931d1821012a51ad2bc6fb2867f8d09ee541f9d234d696f8
2022-03-14Merge bitcoin/bitcoin#24225: wallet: Add sanity checks to DiscourageFeeSnipingAndrew Chow
fa8e76bb9002a126b3645bd9533c282f5dfff111 wallet: Add sanity checks to AntiFeeSnipe (MarcoFalke) Pull request description: I added those sanity checks as part of implementing BIP 326, but I think they make sense on their own. The checks require the transaction to be passed in to `DiscourageFeeSniping`. Also, replace `(int)locktime` cast with the equivalent `int(locktime)` cast. ACKs for top commit: chris-belcher: ACK https://github.com/bitcoin/bitcoin/pull/24225/commits/fa8e76bb9002a126b3645bd9533c282f5dfff111 S3RK: Code review ACK fa8e76bb9002a126b3645bd9533c282f5dfff111 achow101: ACK fa8e76bb9002a126b3645bd9533c282f5dfff111 w0xlt: Code Review ACK https://github.com/bitcoin/bitcoin/pull/24225/commits/fa8e76bb9002a126b3645bd9533c282f5dfff111 Tree-SHA512: 6fe37c85f074851ef09cae8addcb836257089290fecec515c129c3180e9ccb64740aaac76043af10ce7c213e5001568ca6b4b62ae9631f0994ed676b07118074
2022-03-14Merge bitcoin/bitcoin#24505: wallet: Add a deprecation warning for newly ↵MarcoFalke
created legacy wallets 61152183ab18960c8b42cf22ff7168762946678e wallet: Add a deprecation warning for newly created legacy wallets (Andrew Chow) Pull request description: As we slowly deprecate legacy wallets, we need to warn users that are making new legacy wallets that their wallet type is going to be unsupported in the future. ACKs for top commit: jonatack: ACK 61152183ab18960c8b42cf22ff7168762946678e S3RK: reACK 61152183ab18960c8b42cf22ff7168762946678e theStack: ACK 61152183ab18960c8b42cf22ff7168762946678e Tree-SHA512: e89bfb8168869542498958f0c9a2ab302dfd43287f8a49e7d9e09f60438a567bb8b7219a4e569797ee819b30b624f532fcc0b70c6aa0edcb392a301b8ce8b541
2022-03-13Merge bitcoin/bitcoin#24528: rpc: rename getdeploymentinfo status-next to ↵MarcoFalke
status_next 5d7c69b8871aad747969247c7a047b439c5ca59e rpc: rename getdeploymentinfo status-next to status_next (Jon Atack) Pull request description: Rename the `status-next` field to `status_next` in getdeploymentinfo before the RPC is released in v23. Before ``` Result: { (json object) "hash" : "str", (string) requested block hash (or tip) "height" : n, (numeric) requested block height (or tip) "deployments" : { (json object) "xxxx" : { (json object) name of the deployment "type" : "str", (string) one of "buried", "bip9" "height" : n, (numeric, optional) height of the first block which the rules are or will be enforced (only for "buried" type, or "bip9" type with "active" status) "active" : true|false, (boolean) true if the rules are enforced for the mempool and the next block "bip9" : { (json object, optional) status of bip9 softforks (only for "bip9" type) "bit" : n, (numeric, optional) the bit (0-28) in the block version field used to signal this softfork (only for "started" and "locked_in" status) "start_time" : xxx, (numeric) the minimum median time past of a block at which the bit gains its meaning "timeout" : xxx, (numeric) the median time past of a block at which the deployment is considered failed if not yet locked in "min_activation_height" : n, (numeric) minimum height of blocks for which the rules may be enforced "status" : "str", (string) status of deployment at specified block (one of "defined", "started", "locked_in", "active", "failed") "since" : n, (numeric) height of the first block to which the status applies "status-next" : "str", (string) status of deployment at the next block "statistics" : { (json object, optional) numeric statistics about signalling for a softfork (only for "started" and "locked_in" status) "period" : n, (numeric) the length in blocks of the signalling period "threshold" : n, (numeric, optional) the number of blocks with the version bit set required to activate the feature (only for "started" status) "elapsed" : n, (numeric) the number of blocks elapsed since the beginning of the current period "count" : n, (numeric) the number of blocks with the version bit set in the current period "possible" : true|false (boolean, optional) returns false if there are not enough blocks left in this period to pass activation threshold (only for "started" status) }, "signalling" : "str" (string) indicates blocks that signalled with a # and blocks that did not with a - } } } } ``` After ``` Result: { (json object) "hash" : "str", (string) requested block hash (or tip) "height" : n, (numeric) requested block height (or tip) "deployments" : { (json object) "xxxx" : { (json object) name of the deployment "type" : "str", (string) one of "buried", "bip9" "height" : n, (numeric, optional) height of the first block which the rules are or will be enforced (only for "buried" type, or "bip9" type with "active" status) "active" : true|false, (boolean) true if the rules are enforced for the mempool and the next block "bip9" : { (json object, optional) status of bip9 softforks (only for "bip9" type) "bit" : n, (numeric, optional) the bit (0-28) in the block version field used to signal this softfork (only for "started" and "locked_in" status) "start_time" : xxx, (numeric) the minimum median time past of a block at which the bit gains its meaning "timeout" : xxx, (numeric) the median time past of a block at which the deployment is considered failed if not yet locked in "min_activation_height" : n, (numeric) minimum height of blocks for which the rules may be enforced "status" : "str", (string) status of deployment at specified block (one of "defined", "started", "locked_in", "active", "failed") "since" : n, (numeric) height of the first block to which the status applies "status_next" : "str", (string) status of deployment at the next block "statistics" : { (json object, optional) numeric statistics about signalling for a softfork (only for "started" and "locked_in" status) "period" : n, (numeric) the length in blocks of the signalling period "threshold" : n, (numeric, optional) the number of blocks with the version bit set required to activate the feature (only for "started" status) "elapsed" : n, (numeric) the number of blocks elapsed since the beginning of the current period "count" : n, (numeric) the number of blocks with the version bit set in the current period "possible" : true|false (boolean, optional) returns false if there are not enough blocks left in this period to pass activation threshold (only for "started" status) }, "signalling" : "str" (string) indicates blocks that signalled with a # and blocks that did not with a - } } } } ``` Top commit has no ACKs. Tree-SHA512: 4facfd7af3cfb7b6f5495758c4387602802f5e39d9270b162d17350a7f954eab0b74d895f17f0d8dfbc7814d36db7cff56d08c42728432885ea6f4e37aea4aa8
2022-03-13Merge bitcoin/bitcoin#24527: test: set segwit height back to 0 on regtestMarcoFalke
5ce3057c8e8f192921fd5e4bdb95bb15e3f7dbad test: set segwit height back to 0 on regtest (Martin Zumsande) Pull request description: The change of `consensus.SegwitHeight` from 0 to 1 for regtest in #22818 had the effect that if I create a regtest enviroment with current master (or 23.x), and then try to load this chain with an older version (22.x), I get an InitError `Witness data for blocks after height 0 requires validation. Please restart with -reindex` and have to reindex because `BLOCK_OPT_WITNESS` is no longer set for the Genesis block and `NeedsRedownload()` in validation returns `true` with an older version. That might be a bit annoying for tests that use a shared regtest dir with different versions. If people think this is enough of an issue to be worth fixing, I think it should also make it into 23.x ACKs for top commit: theStack: Concept and code-review ACK 5ce3057c8e8f192921fd5e4bdb95bb15e3f7dbad Tree-SHA512: b0e89ff7fc953bc0ae929d2da44cde7149321d987fb4763934f6c9635d00d807129a50b459cc5e69e86bb1819e4b063b969486e8016a1cb8db8f905fa315653d
2022-03-13Merge bitcoin/bitcoin#24533: test: use MiniWallet for feature_maxuploadtarget.pyMarcoFalke
aab552fa3073f27bcedee1d5890e7e8d9e67ffb4 test: use MiniWallet for feature_maxuploadtarget.py (Sebastian Falbesoner) Pull request description: This PR enables one more of the non-wallet functional tests (feature_maxuploadtarget.py) to be run even with the Bitcoin Core wallet disabled by using the MiniWallet instead, as proposed in https://github.com/bitcoin/bitcoin/issues/20078. Note that the adapted helper function `mine_large_block` is currently only is used in this test, i.e. there was no need to change any others. ACKs for top commit: brunoerg: crACK aab552fa3073f27bcedee1d5890e7e8d9e67ffb4 Tree-SHA512: 82fb962ae40e3717419b063af295fc03ac5d5dfe4266dee8ba7f6c86800ede1d32f8fa632c189c30e982badf0c6083fcf6eca4798221f6e284c5e01a8b1a1ed9
2022-03-13Merge bitcoin/bitcoin#24491: contrib: fix implicit function decleration in ↵laanwj
win symbol check e4e9dd3a287f134356044f636e189da704de8ed4 contrib: fix implicit function decleration in win symbol check (fanquake) Pull request description: ```bash test3.c: In function 'main': test3.c:6:21: warning: implicit declaration of function 'CoFreeUnusedLibrariesEx' [-Wimplicit-function-declaration] 6 | CoFreeUnusedLibrariesEx(0,0); ``` ```bash bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum 1907745369f13b0b01583795e395b7e8ecda174a8a3b6309184b14609bfdcb20 guix-build-e4e9dd3a287f/output/dist-archive/bitcoin-e4e9dd3a287f.tar.gz 6973025bd46acdbc327118541f26d36885434305d20a7fa33e0db61f66f8b930 guix-build-e4e9dd3a287f/output/x86_64-w64-mingw32/SHA256SUMS.part 4cdc4efc0d27b3fcfb8f36244dfd956d19ae5df0414dcc23e733c88188f1f93a guix-build-e4e9dd3a287f/output/x86_64-w64-mingw32/bitcoin-e4e9dd3a287f-win-unsigned.tar.gz 022e9743b13f5366cd0f4b52ff8350b42d8c6a506c98363071501a6c4ac735f1 guix-build-e4e9dd3a287f/output/x86_64-w64-mingw32/bitcoin-e4e9dd3a287f-win64-debug.zip 62e65f04fdcacb3d3fbcffbea5204f723f2b27a5f9a62a77abaf0b7ee7de3744 guix-build-e4e9dd3a287f/output/x86_64-w64-mingw32/bitcoin-e4e9dd3a287f-win64-setup-unsigned.exe d773f5ba6afe456b7b5286f0cf98bcb711da8087b96a31f2e38f9c43af44fe96 guix-build-e4e9dd3a287f/output/x86_64-w64-mingw32/bitcoin-e4e9dd3a287f-win64.zip ``` ACKs for top commit: laanwj: Code review ACK e4e9dd3a287f134356044f636e189da704de8ed4 hebasto: ACK e4e9dd3a287f134356044f636e189da704de8ed4, tested on Ubuntu 22.04. Tree-SHA512: e075b052f848a654ed11fb8bc29e2a7b015ab2b44878535d84ac61ecec507410d68e866526c5e0acd1b1b99e65c9d738231208cbb676c8d3f73691317c94c9e0
2022-03-12Merge bitcoin/bitcoin#24526: build: don't install deprecated libevent headersfanquake
339b4a51f6d3558c3489b14efe0c8c195295cf86 build: don't install deprecated libevent headers (fanquake) Pull request description: We don't use the deprecated headers now, and never should do in the future, so there is no need for them to exist in depends. The headers themselves are just full of includes for the newer headers. ACKs for top commit: hebasto: ACK 339b4a51f6d3558c3489b14efe0c8c195295cf86 Tree-SHA512: 736fd9e3b22212da462cc05203dd253806dc59f973090357b705f2742ed4a3b8c3cc44b3173d706527f60ad93e95cf4143ec6b7db4233a489890a98f8e5c8f07
2022-03-12Merge bitcoin/bitcoin#24164: build: Bump minimum required clang/libc++ to 8.0MarcoFalke
fae20e6b50306f91c74037e915aa0ab75a0a6b3b Revert "Avoid the use of P0083R3 std::set::merge" (MarcoFalke) fab53b5fd45cf55a1d4d313e46ffce7396c9590e ci/doc: Set minimum required clang/libc++ version to 8.0 (MarcoFalke) Pull request description: This is not for 23.0, but for 24.0. It comes with the following benefits: * Can use C++17 P0083R3 std::set::merge from libc++ 8.0 * No longer need to provide support for clang-7, which already fails to compile on some architectures (https://github.com/bitcoin/bitcoin/issues/21294#issuecomment-998098483) This should be fine, given that all supported operating systems ship with at least clang-10: * CentOS 8: clang-12 * Stretch: https://packages.debian.org/stretch/clang-11 * Buster: https://packages.debian.org/buster-backports/clang-11 * Bionic: https://packages.ubuntu.com/bionic-updates/clang-10 * Focal: https://packages.ubuntu.com/focal/clang-10 ACKs for top commit: fanquake: ACK fae20e6b50306f91c74037e915aa0ab75a0a6b3b - I think this is fine to do. I would be surprised if in another 6 months time someone was stuck on a system we supported, needing to compile Core, and only had access to Clang 7 or older. As mentioned in the PR description, all systems we currently support, already support multiple newer versions of Clang. hebasto: ACK fae20e6b50306f91c74037e915aa0ab75a0a6b3b, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 3b4c6c130ff40dd7e84934af076863415e5dd661d823c72e3e3832566c65be6e877a7ef9164bbcf394bcea4b897fc29a48db0f231c22ace0e2c9b5638659a628
2022-03-11refactor: Avoid int64_t -> size_t -> int64_t conversionMarcoFalke
2022-03-11rpc: Move mempool RPCs to new fileMarcoFalke
Can be reviewed with: --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2022-03-11Merge bitcoin/bitcoin#24530: wallet: assert BnB's internally calculated ↵Andrew Chow
waste is the same as GetSelectionWaste ec7d73628a6397fca3b5b852d4e97ff918b6d3a6 [wallet] assert BnB internally calculated waste is the same as GetSelectionWaste() (glozow) Pull request description: #22009 introduced a `GetSelectionWaste()` function to determine how much "waste" a coin selection solution has, and is a mirror of the waste calculated inside of `SelectCoinsBnB()`. It would be bad for these two waste metrics to deviate, since it could negatively affect how often we select the BnB solution. Add an assertion to help tests catch a potential accidental change. ACKs for top commit: achow101: ACK ec7d73628a6397fca3b5b852d4e97ff918b6d3a6 Xekyo: ACK ec7d73628a6397fca3b5b852d4e97ff918b6d3a6 Tree-SHA512: 3ab7ad45ae92e7ce3f21824fb975105b6be8382edf47c252df5d13d973a3abdcb675132d223b42fcbb669cca879672c904b8a58d0676e12bf381a9219f4db37c
2022-03-11test: use MiniWallet for feature_maxuploadtarget.pySebastian Falbesoner
This test can now be run even with the Bitcoin Core wallet disabled.
2022-03-11Merge bitcoin/bitcoin#24453: Bugfix: doc: Correct ↵MarcoFalke
change_address/changeAddress in wallet RPC help e8272024ab6ed0817fe5027d8d800acbb87bc9ff doc: Use human-friendly DefaultHint for change_address/changeAddress in wallet RPC help (Luke Dashjr) 9d5e693c9de57ec9ef7e96cdd2f47fdc45431748 Bugfix: doc: Correct type of change_address/changeAddress in wallet RPC help (STR, not STR_HEX) (Luke Dashjr) Pull request description: ACKs for top commit: achow101: ACK e8272024ab6ed0817fe5027d8d800acbb87bc9ff Tree-SHA512: da4db2b241160c93ea66f8c572c69d4688f52a5fd8c32b66b1192925fcb360baf91be9771eaca178f5b08e1920559174260ed57caddcffade48156ec0c83c0bc
2022-03-11Merge bitcoin/bitcoin#24421: miner: always assume we can build witness blocksfanquake
40e871d9b4e55f5b5f7ce2a89157cd3d9f152037 [miner] always assume we can create witness blocks (glozow) Pull request description: Given the low possibility of a reorg reverting the segwit soft fork, there is no longer a need to check whether segwit is active to see if it's okay to add to the block template (see also #23512, #21009, etc). `TestBlockValidity()` is also run on the block template at the end of `CreateNewBlock()`, so any invalid block would be caught there. ACKs for top commit: gruve-p: ACK https://github.com/bitcoin/bitcoin/pull/24421/commits/40e871d9b4e55f5b5f7ce2a89157cd3d9f152037 jnewbery: utACK 40e871d9b4, although I disagree about changing the test for segwit transaction in mempool before activagtion, instead of just removing it: https://github.com/bitcoin/bitcoin/pull/24421#discussion_r822933721. achow101: ACK 40e871d9b4e55f5b5f7ce2a89157cd3d9f152037 theStack: Code-review ACK 40e871d9b4e55f5b5f7ce2a89157cd3d9f152037 Tree-SHA512: bf4860bf2bed8339622d05228d11d60286edb0c32a9a3c434b8d154913c07ea56e50649f4af7009c2a1c6a58a81d2299ab43b41a6f16dee7d08cc89cc1603019
2022-03-11[wallet] assert BnB internally calculated waste is the same as ↵glozow
GetSelectionWaste() These two implementations of waste calculation should never deviate. Still keep the SelectCoinsBnB internal calculation because incremental calculate-as-you-go is much more performant than calling GetSelectionWaste() over and over again.
2022-03-11Merge bitcoin/bitcoin#24524: doc: remove Boost LDFLAGS from netBSD build docsfanquake
8336a06dbdc772b6ee1581fb88d480842fe604db doc: remove Boost LDFLAGS from netBSD build docs (fanquake) Pull request description: We no-longer link against any Boost libs, so we shouldn't need to use any Boost linker flags. ACKs for top commit: hebasto: ACK 8336a06dbdc772b6ee1581fb88d480842fe604db, I have reviewed the code and it looks OK, I agree it can be merged. Also verified that there is no other usage of `BOOST_LDFLAGS` in our codebase or documentation. Tree-SHA512: b7814d10cee789903cb3c613631e184a72f5766cda85261b5f99f9ac207348a2a49c92494c8c1d50163494f6b755c503cf51bf083b31f564dae1b0f493c54c2e
2022-03-11Merge bitcoin/bitcoin#24509: doc: Add `guix` prefix for changes to ↵fanquake
reproducible builds f1f994a122b135160216b6fc56c095b83eeaf812 doc: Add `guix` prefix for changes to reproducible builds (Hennadii Stepanov) Pull request description: Most of contributors already use the `guix:` prefix for changes to `contrib/guix`. Also `guix` is shorter than `build`, and it is more focused/specific. ACKs for top commit: fanquake: ACK f1f994a122b135160216b6fc56c095b83eeaf812 Tree-SHA512: 3f754e80802ec4e871b099ce1f0877e34ecc4816fbe9c49bfd2a7368ef79fed9edf6c65f38eedef2a87367fdc911dc548e0def422d80b66a91ce2e5f35826032
2022-03-11Merge bitcoin/bitcoin#24521: build: Fix Boost.Process detection on macOS arm64fanquake
1d4157a42b77411579eb721b5c6fb533a357959d build: Fix Boost.Process detection on macOS arm64 (Hennadii Stepanov) Pull request description: Could be tested as follows: ``` % brew install boost@1.76 % ./autogen.sh % ./configure --with-boost='/opt/homebrew/opt/boost@1.76' ``` ACKs for top commit: promag: Tested ACK on 1d4157a42b77411579eb721b5c6fb533a357959d with boost 1.76 on macOS arm64. #24523 is required for boost 1.78. Tree-SHA512: 7abd39a78e970ecc051e53b5923dfc31d3f0576cf4ff7fcfb9c8708c857c46a7a595ec36238def83f41158970eeee209980da4b8b70f0ff68f940a38ac9a0471
2022-03-11rpc: rename getdeploymentinfo status-next to status_nextJon Atack
2022-03-10test: set segwit height back to 0 on regtestMartin Zumsande
This was changed in #22818 from 0 to 1. Since it changes BLOCK_OPT_WIT of the genesis block, older versions of bitcoin core would not read regtest directories created with newer versions without a reindex.
2022-03-10build: don't install deprecated libevent headersfanquake
We don't use the deprecated headers now, and never should do in the future, so there is no need for them to exist in depends. The headers themselves are just full of includes for the newer headers.
2022-03-10Merge bitcoin-core/gui#563: qt: Remove network detection based on address in ↵Hennadii Stepanov
BIP21 b7dbc83f23f67048cd6f66f5587381d73fad4894 qt: Remove network detection based on address in BIP21 (laanwj) Pull request description: This is removes some ugly and brittle code that switches the global network to testnet based on a provided address. I think in practice it's very unlikely for testnet BIP21 payment URIs to be used, and if so it's for testing so it's easy enough to manually copy it. Or to specify `-testnet` explicitly. There is already no such case for `-regtest` or `-signet`. After this change it will only accept addresses for the explicitly selected network. Others will result in a "wrong network" popup. There is also a possibility for refactor after this as the initialization order of `PaymentServer::ipcParseCommandLine` isn't important anymore (well, it still has to be before `PaymentServer::ipcSendCommandLine`, maybe even merged with it), but I have not done so here. ACKs for top commit: jonatack: ACK b7dbc83f23f67048cd6f66f5587381d73fad4894 achow101: ACK b7dbc83f23f67048cd6f66f5587381d73fad4894 Tree-SHA512: ebc77c0e5c98f53fe254bcd0f94c9d1c06937b794346e95b14158860d5c607515e71d73b782d2726674dce86d6d4001085d473c6d1bc11c5e6c25ff3fb3cfa22
2022-03-10doc: remove Boost LDFLAGS from netBSD build docsfanquake
We no-longer link against any Boost libs, so we shouldn't need to use any Boost linker flags.
2022-03-10Merge bitcoin/bitcoin#24522: ci: remove compiled-but-unused BDB from MSAN jobMarcoFalke
3566353c5e8c699f5da80cb3f8081e407f1b5813 ci: remove compiled-but-unused BDB from MSAN job (fanquake) Pull request description: Self-compiled BDB was added to this job as opposed to using depends BDB [due to linking issues](https://github.com/bitcoin/bitcoin/pull/18288#discussion_r433189350), however the compiled BDB is not actually used. Remove it for now, given we don't actually lose any coverage (note that BDB is also not currently used in the naitve MSAN fuzz job or for [OSS Fuzz](https://github.com/google/oss-fuzz/blob/master/projects/bitcoin-core/build.sh#L32) builds). In future, we can use depends BDB, however introducing it now will cause false positives, which can be fixed by upgrading the versions of Clang / LLVM we use, however upgrading to those newer versions causes other issues, which appear in standard library code, and require more involved suppressing, which can be solved in a follow up or another PR i.e #23008. Top commit has no ACKs. Tree-SHA512: 9e8fdd95246cafa27cda7bcf0641b428d4573f6748ecdf07cc6205a64351db22ba383ec943e88a69df3694ccb9f125d994b64345a4e44fb6fea4a014504760d1
2022-03-10Merge bitcoin/bitcoin#24404: refactor: Remove confusing P1008R1 violation in ↵fanquake
ATMPArgs faa1aec26b3f354c832e6b995323c9429b178931 Remove confusing P1008R1 violation in ATMPArgs (MarcoFalke) Pull request description: The `= delete` doesn't achieve the stated goal and it is also redundant, since it is not possible to default construct the `ATMPArgs` type. This can be tested with: ```diff diff --git a/src/validation.cpp b/src/validation.cpp index 2813b62462..1c939c0b8a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -519,6 +519,7 @@ public: /** Parameters for child-with-unconfirmed-parents package validation. */ static ATMPArgs PackageChildWithParents(const CChainParams& chainparams, int64_t accept_time, std::vector<COutPoint>& coins_to_uncache) { + ATMPArgs{}; return ATMPArgs{/* m_chainparams */ chainparams, /* m_accept_time */ accept_time, /* m_bypass_limits */ false, ``` Which fails on current master *and* this pull with the following error: ``` validation.cpp:525:22: error: reference member of type 'const CChainParams &' uninitialized ATMPArgs{}; ~^ validation.cpp:470:29: note: uninitialized reference member is here const CChainParams& m_chainparams; ^ 1 error generated. ``` Further reading (optional): * http://open-std.org/JTC1/SC22/WG21/docs/papers/2018/p1008r1.pdf ACKs for top commit: achow101: ACK faa1aec26b3f354c832e6b995323c9429b178931 glozow: code review ACK faa1aec26b3f354c832e6b995323c9429b178931 Tree-SHA512: 16db2c9959a1996eafbfa533dc4d1483761b9d28295aed5a82b86abd7268da37c51c59ddc67c205165ecb415dbe637b12a0e1b3234d50ab0b3b79de66d7bd73e
2022-03-10ci: remove compiled-but-unused BDB from MSAN jobfanquake
Self-compiled BDB was added to this job as opposed to using depends BDB due to linking issues, however the compiled BDB is not actually used. Remove it for now, given we don't actually lose any coverage (note that BDB is also no used the MSAN fuzz job), and in future, we can use depends BDB.
2022-03-10Merge bitcoin/bitcoin#24486: wallet: refactor: dedup sqlite blob bindingfanquake
8ea6167099ddfe9a42464c3c8ae2acd895329d4f wallet: refactor: dedup sqlite blob binding (Sebastian Falbesoner) Pull request description: This refactoring PR deduplicates repeated SQLite blob binding to a statement with a newly introduced helper function `BindBlobToStatement`, abstracting away the calls to `sqlite3_bind_blob(...)`. This should be more readable and less error-prone, e.g. in case that the error handling has to be adapted. As a slight drawback, the function where the binding happens is not printed anymore (`__func__`), i.e. one could argue this is not strictly a refactoring, but IMHO the advantages of deduplication outweigh this; binding errors are purely internal logic errors (wrong use of the sqlite API) rather than something that is dependend on external data like DB content. ACKs for top commit: laanwj: Code review ACK 8ea6167099ddfe9a42464c3c8ae2acd895329d4f achow101: ACK 8ea6167099ddfe9a42464c3c8ae2acd895329d4f klementtan: ACK 8ea6167099ddfe9a42464c3c8ae2acd895329d4f Tree-SHA512: 1de0d214f836bc405a01e98a3a2d71f2deaddc7d23c31cad80219d1614bec92619c06d9a4a091dd563d3e95ffb879649c29745d8f89669b2a5330552c212af3f
2022-03-10wallet: Add a deprecation warning for newly created legacy walletsAndrew Chow
2022-03-10build: Fix Boost.Process detection on macOS arm64Hennadii Stepanov
Could be tested as follows: ``` % brew install boost@1.76 % ./autogen.sh % ./configure --with-boost='/opt/homebrew/opt/boost@1.76' ```
2022-03-10qt: Remove network detection based on address in BIP21laanwj
This is some very ugly and brittle code that switches the global network based on a provided address, remove it. I think in practice it's very unlikely for testnet BIP21 payment URIs to be used, and if so it's for testing so it's easy enough to manually copy it. Or to specify `-testnet` explicitly. There is already no case for `-regtest` or `-signet`.
2022-03-10Merge bitcoin/bitcoin#24469: test: Correctly decode UTF-8 literal string pathsMarcoFalke
2f5fd3cf9225aed439d1de767312bb340972d665 test: Correctly decode UTF-8 literal string paths (Ryan Ofsky) Pull request description: Call `fs::u8path()` to convert some UTF-8 string literals to paths, instead of relying on the implicit conversion. Fake Macro pointed out in https://github.com/bitcoin/bitcoin/pull/24306#discussion_r818566106 that `fs_tests` are incorrectly decoding some literal UTF-8 paths using the current windows codepage, instead of treating them as UTF-8. This could cause test failures depending what environment windows tests are run under. The `fs::path` class exists to avoid problems like this, but because it is lenient with `const char*` conversions, under assumption that they are ["safe as long as the literals are ASCII"](https://github.com/bitcoin/bitcoin/blob/727b0cb59259ac63c627b09b503faada1a89bfb8/src/fs.h#L39), bugs like this are still possible. If we think this is a concern, followup options to try to prevent this bug in the future are: 0. Do nothing 1. Improve the "safe as long as the literals are ASCII" comment. Make it clear that non-ASCII strings are invalid. 2. Drop the implicit `const char*` conversion functions. This would be nice because it would simplifify the `fs::path` class a little, while making it safer. Drawback is that it would require some more verbosity from callers. For example, instead of `GetDataDirNet() / "mempool.dat"` they would have to write `GetDataDirNet() / fs::u8path("mempool.dat")` 3. Keep the implicit `const char*` conversion functions, but make them call `fs::u8path()` internally. Change the "safe as long as the literals are *ASCII*" comment to "safe as long as the literals are *UTF-8*". I'd be happy with 0, 1, or 2. I'd be a little resistant to 3 even though it was would add more safety, because it would slightly increase complexity, and because I think it would encourage representing paths as strings, when I think there are so many footguns associated with paths as strings, that it's best to convert strings to paths at the earliest point possible, and convert paths to strings at the latest point possible. ACKs for top commit: laanwj: Code review ACK 2f5fd3cf9225aed439d1de767312bb340972d665 w0xlt: crACK 2f5fd3c Tree-SHA512: 9c56714744592094d873b79843b526d20f31ed05eff957d698368d66025764eae8bfd5305d5f7b6cc38803f0d85fa5552003e5c6cacf1e076ea6d313bcbc960c
2022-03-10Merge bitcoin/bitcoin#24520: guix: only check for the macOS SDK onceAndrew Chow
e8023100be7ab2e32addfff75a133fb429b8492b guix: only check for the macOS SDK once (fanquake) Pull request description: If we are building for both macOS HOSTS, there's no need to check and print that the SDK exists two times. Currently a Guix build for both HOSTS will print: ```bash ./contrib/guix/guix-build Found macOS SDK at '/SDKs/Xcode-12.2-12B45b-extracted-SDK-with-libcxx-headers', using... Found macOS SDK at '/SDKs/Xcode-12.2-12B45b-extracted-SDK-with-libcxx-headers', using... Checking that we can connect to the guix-daemon... ``` ACKs for top commit: laanwj: Code review ACK e8023100be7ab2e32addfff75a133fb429b8492b achow101: ACK e8023100be7ab2e32addfff75a133fb429b8492b Tree-SHA512: 7e9ee7793c5dc1eb485806ca3d613742397d2cc62525203a168cad1ec96aabfd4e63dc3f2e8d205bdb2f3c2079f731d75c5d162d55ff0d42a7e6f3d01d3a7db1
2022-03-10guix: only check for the macOS SDK oncefanquake
If we are building for both macOS HOSTS, there's no need to check and print that the SDK exists two times.
2022-03-10Merge bitcoin/bitcoin#24516: build, ci: Fix MSVC builds and other improvementsMarcoFalke
c3296b21e40be3a5cb7060ceb8f1b6db1fd79e65 build: Drop `double-conversion` from MSVC dependencies (Hennadii Stepanov) 7ff43e5372c4606ecb75d6892b4bb0ccb4165b80 ci: Invalidate vcpkg binary cache if dependencies changed (Hennadii Stepanov) 20b6c871178f20661b849ad5677bd8ecae55cf19 build: Specify `zeromq` port explicitly for MSVC builds (Hennadii Stepanov) Pull request description: The current MSVC builds are broken due to the bug in the `zeromq` [port](https://github.com/microsoft/vcpkg/pull/22681#issuecomment-1061312320). From [IRC](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2022-03-08#787145): > \<sipsorcery> Looks like it's a problem downloading the zeromq dependency from https://patch-diff.githubusercontent.com/raw/zeromq/libzmq/pull/4311.diff > \<dhruv> sipsorcery: I'm definitely misunderstanding, i actually have no clue which file the CI is failing to download. I'll DM you more details. > \<sipsorcery> It's saying the hash of the patch file has changed. > \<dhruv> so we'd need to verify that the change is not malicious and then commit the new hash? > \<sipsorcery> No that dependency is managed by the vcpkg repo. Seems they might be working on it https://github.com/microsoft/vcpkg/pull/22681#issuecomment-1061312320 > \<dhruv> ok, thanks This PR fixes this issue with specifying the previous port version [explicitly](https://github.com/microsoft/vcpkg/blob/master/docs/users/versioning.md). The current CI task does not fail due to the cached binaries. --- The second commit makes vcpkg binary cache invalid if dependencies changed. The third commit drops `double-conversion` from dependencies as Qt is configured as follows: ``` Configure summary: Build type: win32-msvc (x86_64, CPU features: sse sse2) Compiler: msvc 193131104 Configuration: sse2 aesni sse3 ssse3 sse4_1 sse4_2 avx avx2 avx512f avx512bw avx512cd avx512dq avx512er avx512ifma avx512pf avx512vbmi avx512vl compile_examples f16c largefile msvc_mp precompile_header rdrnd rdseed shani silent x86SimdAlways release c++11 c++14 c++17 c++1z concurrent no-pkg-config static static_runtime stl Build options: ... Qt Core: DoubleConversion ....................... yes Using system DoubleConversion ........ no ... ``` ACKs for top commit: sipsorcery: tACK c3296b21e40be3a5cb7060ceb8f1b6db1fd79e65. Tree-SHA512: 4d694a7d0930889a53eb6ee7a09929f6ffa3f078122b34abe6d75430769bb87c353f7c11146da53c3804e51d4bbfcbb7bc8453f525bcc432928d98eeb66ee35e
2022-03-10Merge bitcoin/bitcoin#24371: util: Fix `ReadBinaryFile` reading beyond maxsizeMarcoFalke
a84650ebd5ac2cbb49f14eb7c98736a3f8215bf1 util: Fix ReadBinaryFile reading beyond maxsize (klementtan) Pull request description: Currently `ReadBinaryFile` will read beyond `maxsize` if `maxsize` is not a multiple of `128` (size of buffer) This is due to `fread` being called with `count = 128` instead of `count = min(128, maxsize - retval.size()` at every iteration The following unit test will fail: ```cpp BOOST_AUTO_TEST_CASE(util_ReadWriteFile) { fs::path tmpfolder = m_args.GetDataDirBase(); fs::path tmpfile = tmpfolder / "read_binary.dat"; std::string expected_text(300,'c'); { std::ofstream file{tmpfile}; file << expected_text; } { // read half the contents in file auto [valid, text] = ReadBinaryFile(tmpfile, expected_text.size() / 2); BOOST_CHECK_EQUAL(text.size(), 150); } } ``` Error: ``` test/util_tests.cpp:2593: error: in "util_tests/util_ReadWriteFile": check text.size() == 150 has failed [256 != 150] ``` ACKs for top commit: laanwj: Code review ACK a84650ebd5ac2cbb49f14eb7c98736a3f8215bf1 theStack: Code-review ACK a84650ebd5ac2cbb49f14eb7c98736a3f8215bf1 Tree-SHA512: 752eebe58bc2102dec199b6775f8c3304d899f0ce36d6a022a58e27b076ba945ccd572858b19137b769effd8c6de73a9277f641be24dfb17657fb7173ea0eda0
2022-03-10build: Drop `double-conversion` from MSVC dependenciesHennadii Stepanov
Qt uses its own `double-conversion`.
2022-03-10ci: Invalidate vcpkg binary cache if dependencies changedHennadii Stepanov
2022-03-10build: Specify `zeromq` port explicitly for MSVC buildsHennadii Stepanov
Current port 4 is broken: - https://github.com/microsoft/vcpkg/pull/22681#issuecomment-1061312320
2022-03-09Merge bitcoin/bitcoin#24506: build, mac: Include arch in codesignature tarballfanquake
0189df1d3171082caf743ef3b0968f43c71303f5 build, mac: Include arch in codesignature tarball (Andrew Chow) 6e9308c6d4ed9fbf909c7234ae31245747183be3 guix: use latest signapple (Andrew Chow) Pull request description: Since we have two architectures for Mac binaries, having the architecture in the code signature tarball generated by `detached-sig-create.sh` allows us to avoid accidentally overwriting an existing code signature tarball during the code signing process. ACKs for top commit: fanquake: ACK 0189df1d3171082caf743ef3b0968f43c71303f5 Tree-SHA512: 7e0d282e4ced1094f36f1d26ff6e18d53449561ab3a1a95ac69eb5ff3d7b33ee4bd8fad004884806064a29541c47f9e5879c2a1fd0f54453413245bdcf53c4c7
2022-03-09Merge bitcoin/bitcoin#24498: qt: Avoid crash on startup if int specified in ↵Andrew Chow
settings.json 5b1aae12ca4a99c6b09349981a4902717a6a5d3e qt: Avoid crash on startup if int specified in settings.json (Ryan Ofsky) 84b0973e35dae63cd1b60199b481e24d54e58c97 test: Add tests for GetArg methods / settings.json type coercion (Ryan Ofsky) Pull request description: Should probably add this change to 23.x as suggested by Luke https://github.com/bitcoin/bitcoin/issues/24457#issuecomment-1059825678. If settings like `prune` are added to `settings.json` in the future, it would be preferable for 23.x releases to respect the setting instead of crash. --- Fix GUI startup crash reported by Rspigler in https://github.com/bitcoin/bitcoin/issues/24457 that happens if `settings.json` contains an integer value for any of the configuration options which GUI settings can currently clash with (-dbcache, -par, -spendzeroconfchange, -signer, -upnp, -natpmp, -listen, -server, -proxy, -proxy, -onion, -onion, -lang, and -prune). The fix is a one-line change in `ArgsManager::GetArg`. The rest of the PR just adds a regression test for the GUI and unit tests for ArgsManager::GetArg methods. ACKs for top commit: laanwj: Code review ACK 5b1aae12ca4a99c6b09349981a4902717a6a5d3e achow101: ACK 5b1aae12ca4a99c6b09349981a4902717a6a5d3e jonatack: Code review ACK 5b1aae12ca4a99c6b09349981a4902717a6a5d3e Tree-SHA512: 958991b4bead9b82a3879fdca0f8d6405e2a212b7c46cf356f078843a4f156e27fd75fc46e2013aa5159582ead06d343c1ed248d678b3e5bbd312f247e37894c
2022-03-09build, mac: Include arch in codesignature tarballAndrew Chow
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-07 16:11:42 +0000