fa92e60f38cb109fe5a3c7acfe1017ffebc388cc refactor: Make httpserver work queue a unique_ptr (MarcoFalke)
Pull request description:
This simplifies the code a bit because `if (p) { delete p; p = nullptr; }` can be replaced by a call to the `reset()` member.
ACKs for top commit:
promag:
Core review ACK fa92e60f38cb109fe5a3c7acfe1017ffebc388cc.
jonatack:
ACK fa92e60f38cb109fe5a3c7acfe1017ffebc388cc code review, debug build clean, ran test/functional/interface*.py tests locally as a sanity check
hebasto:
ACK fa92e60f38cb109fe5a3c7acfe1017ffebc388cc, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 6b122162317dd4ad6889341745c7ac1903a3ee510f6548f46dc356308442a6eff13eb8dc604c38ba18783e7a66d2b836d641a8594ff980a010c12c97f3856684
8888cf45f5e45b38cb830f9c94cafbf622e1fe5f Remove unused wallet pointer from NotifyAddressBookChanged (MarcoFalke)
faf36403038afb3df3ddd963bd6c352d3eff4da8 Remove unused wallet pointer from NotifyTransactionChanged signal (MarcoFalke)
Pull request description:
The signals are members of the wallet, so passing the pointer would be redundant even if it was used.
Also, fix `with` -> `without`, which was forgotten in commit ca4cf5cff6f.
ACKs for top commit:
jonatack:
Code review ACK 8888cf45f5e45b38cb830f9c94cafbf622e1fe5f also verified with/without lock cs_wallet status for each of the two functions and debian clang 11 debug build clean
promag:
Code review ACK 8888cf45f5e45b38cb830f9c94cafbf622e1fe5f.
theStack:
Code review ACK 8888cf45f5e45b38cb830f9c94cafbf622e1fe5f
Tree-SHA512: e3b80931ce9bcb05213619f5435ac7c21d3c7848643950a70db610902bd1803c92bb75e501d46b0e519bc576901f160e088e8882c4f1adce892a80df565f897b
fa0d9211ef87a682573aaae932c0c440acbcb8a8 refactor: Remove chainparams arg from CChainState member functions (MarcoFalke)
fa389471251f043ec25e7b01e59b37d3b921ce54 refactor: Remove ::Params() global from inside CChainState member functions (MarcoFalke)
Pull request description:
The `::Params()` global is verbose and confusing. Also it makes tests a bit harder to write because they'd have to mock a global.
Fix all issues by simply using a member variable that points to the right params.
(Can be reviewed with `--word-diff-regex=.`)
ACKs for top commit:
jnewbery:
ACK fa0d9211ef87a682573aaae932c0c440acbcb8a8
kiminuo:
utACK fa0d9211
theStack:
ACK fa0d9211ef87a682573aaae932c0c440acbcb8a8 🍉
Tree-SHA512: 44676b19c9ed471ccb536331d3029bad192d7d50f394fd7b8527ec431452aeec8c4494164b9cf8e16e0123c4463b16be864366c6b599370032c17262625a0356
fa9ebedec3f982bb5bb459ea33d74c94d9b5cec4 Reject invalid coin height and output index when loading assumeutxo (MarcoFalke)
Pull request description:
It should be impossible to have a coin at a height higher than the height of the snapshot block, so reject those early to avoid integer wraparounds and hash collisions later on.
Same for the outpoint index.
Both issues were found by fuzzing:
* The height issue by OSS-Fuzz: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34793
* The outpoint issue by my fuzz server: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34793#c2
ACKs for top commit:
practicalswift:
cr ACK fa9ebedec3f982bb5bb459ea33d74c94d9b5cec4: patch looks correct
jamesob:
crACK fa9ebedec3
theStack:
Code review ACK fa9ebedec3f982bb5bb459ea33d74c94d9b5cec4
benthecarman:
crACK fa9ebedec3f982bb5bb459ea33d74c94d9b5cec4
Tree-SHA512: dae7caee4b3862b23ebdf2acb7edec4baf75b0dbf1409b370b1a73aa6b632b317ebfac596dcbaf4edfb1301b513f45465ea75328962460f35e2af0d7e547c9ac
During each CI run, for macos native environment, python packages lief
and zmq are rebuilt everytime which wastes a lot of resources and time.
The latest version of pip directly fetches pre-built binaries. Through
this commit pip version is upgraded in macos environment before
installation of these packages.
dd49f1b52e5602bf3f5ffd54733cba99f0dfa22a doc: doc/README.md IRC updates (Jon Atack)
Pull request description:
Follow-up to https://github.com/bitcoin/bitcoin/pull/22109:
- update remaining link and reference from Freenode to Libera
- remove defunct #bitcoin-dev channel
- other minor fixups
ACKs for top commit:
fanquake:
ACK dd49f1b52e5602bf3f5ffd54733cba99f0dfa22a
Tree-SHA512: de20a8f3889028def7139be1cd0fa15daa3351ca987a16090d0caa6b4cd397e370400e2b3033ec776e270aa3a1ab085ada20fbdcc063ac88e2ad3b8351a596e7
18c2027c76b54f1a68a0daf98bb06d988b19447d doc: Document minimum required libc++ version (Hennadii Stepanov)
Pull request description:
The minimum required libc++ version to compile the current master is 7.0.
See #22324.
ACKs for top commit:
fanquake:
ACK 18c2027c76b54f1a68a0daf98bb06d988b19447d
Tree-SHA512: a2811236a600caea5b9fd156993a0195fc3f1c231b48d0e369250ebb759f7153fd974390de388b91b367d9dc5e115922a2cab9d3b8ad872a2724c07b63b9fcf4
6cf4ea71878c0a83f2e49831e4dfa119c53761b7 Avoid the use of P0083R3 std::set::merge (Pieter Wuille)
Pull request description:
This use was introduced in #21365, but as pointed out in #22339, this causes compatibility problems.
Just avoid its use for now.
ACKs for top commit:
jonatack:
re-ACK 6cf4ea71878c0a83f2e49831e4dfa119c53761b7
benthecarman:
ACK 6cf4ea71878c0a83f2e49831e4dfa119c53761b7
hebasto:
ACK 6cf4ea71878c0a83f2e49831e4dfa119c53761b7, successfully compiled on the following systems:
Tree-SHA512: 2b3fdcadb7de98963ebb0b192bd956aa68526457fe5b374c74a69ea10d5b68902763148f11abbcc471010bcdc799e0804faef5f8e8ff8a509b3a053c0cb0ba39
.PHONY does not take patterns (such as print-%) as prerequisites.
Have print-% depend on FORCE and mark FORCE as phony.
$ # on master
$ make print-host
host=x86_64-pc-linux-gnu
$ touch print-host
$ make print-host
make: 'print-host' is up to date.
$
$ git co mark_print_as_phony
Switched to branch 'mark_print_as_phony'
$ make print-host
host=x86_64-pc-linux-gnu
$ touch FORCE
$ make print-host
host=x86_64-pc-linux-gnu
Instead of having a large blob of cache merging code in TopUp, refactor
this into DescriptorCache so that it can merge and provide a diff
(another DescriptorCache containing just the items that were added).
Then TopUp can just write everything that was in the diff.
This reverts commit 09e25071f40c564af08a1386c39c4f2d8eb484b6.
The changes made in this commit have turned out to be unnecessary and
confusing, so it is being reverted.
fa34cb80248cc39a73fc393f65c3cfc62e849556 cli: Avoid truncating -rpcwaittimeout (MarcoFalke)
Pull request description:
`seconds` is not enough precision to "exactly" store a timestamp n seconds into the future. Improve the precision by using `microseconds`. Fixes#22325
Also, use chrono literals.
ACKs for top commit:
jonatack:
ACK fa34cb80248cc39a73fc393f65c3cfc62e849556 review, debug-built, tested
theStack:
Tested ACK fa34cb80248cc39a73fc393f65c3cfc62e849556
Tree-SHA512: 7158da8545f9998a82bcc8636e04564efdb1e1be43b4288298c151b4df29ad47a2760259eefadd4a01db92ea18a1e017f3febc1cd8c69a4b28c86180229d8c90
754f134a50cc56cdf0baf996d909c992770fcc97 wallet: Add error message to GetReservedDestination (Andrew Chow)
87a0e7a3b7c0ffd545e537bd497cdc3e67d045f6 Disallow bech32m addresses for legacy wallet things (Andrew Chow)
6dbe4d10728f882986ed0d9ed77bc736f051c662 Use BECH32M for tr() desc, WitV1Taproot, and WitUnknown CTxDests (Andrew Chow)
699dfcd8ad9487a4e04c1ffc68211e84e126b3d2 Opportunistically use bech32m change addresses if available (Andrew Chow)
0262536c34567743e527dad46912c9ba493252cd Add OutputType::BECH32M (Andrew Chow)
177c15d2f7cd5406ddbce8217fc023057539b828 Limit LegacyScriptPubKeyMan address types (Andrew Chow)
Pull request description:
Currently bech32m addresses are classfied as bech32. Because bech32m is incompatible with bech32, we need to define a new `OutputType` for it so that it can be handled correctly. This PR adds `OutputType::BECH32M`, updates all of the relevant `OutputType` classifications, and handle requests for bech32m addresses. There is now a `bech32m` address type string that can be used.
* `tr()` descriptors now report their output type as `OutputType::BECH32M`. `WtinessV1Taproot` and `WitnessUnknown` are also classified as `OutputType::BECH32M`.
* Bech32m addresses are completely disabled for legacy wallets. They cannot be imported (explicitly disallowed in `importaddress` and `importmulti`), will not be created when getting all destinations for a pubkey, and will not be added with `addmultisigaddress`. Additional protections have been added to `LegacyScriptPubKeyMan` to disallow attempting to retrieve bech32m addresses.
* Since Taproot multisigs are not implemented yet, `createmultisig` will also disallow the bech32m address type.
* As Taproot is not yet active, `DescriptorScriptPubKeyMan` cannot and will not create a `tr()` descriptor. Protections have been added to make sure this cannot occur.
* The change address type detection algorithm has been updated to return `bech32m` when there is a segwit v1+ output script and the wallet has a bech32m `ScriptPubKeyMan`, falling back to bech32 if one is not available.
ACKs for top commit:
laanwj:
re-review ACK 754f134a50cc56cdf0baf996d909c992770fcc97
Sjors:
re-utACK 754f134: only change is switching to `bech32m` in two `wallet_taproot.py` test cases.
fjahr:
re-ACK 754f134a50cc56cdf0baf996d909c992770fcc97
jonatack:
ACK 754f134a50cc56cdf0baf996d909c992770fcc97
Tree-SHA512: 6ea90867d3631d0d438e2b08ce6ed930f37d01323224661e8e38f183ea5ee2ab65b5891394a3612c7382a1aff907b457616c6725665a10c320174017b998ca9f
fadddd13eef4428f5fa7237583d4be41a9335cd9 test: Add missing syncwithvalidationinterfacequeue (MarcoFalke)
faa211fc6e3d4984b8edff1d762dd4cba205d982 test: Misc cleanup (MarcoFalke)
fa1668bf5084a190b26b022b9e625a7be3defa6e test: Run pep-8 (MarcoFalke)
facd97ae0f0d816107aa3bc9de321244200636a0 scripted-diff: Renames (MarcoFalke)
Pull request description:
The index on the block filters is running in the background on the validation interface. To avoid intermittent test failures, it needs to be synced.
Also other cleanups.
ACKs for top commit:
lsilva01:
Tested ACK fadddd13ee on Ubuntu 20.04
Tree-SHA512: d858405db426a2f9d5620059dd88bcead4e3fba3ccc6bd8023f624b768fbcfa2203246fb0b2db620490321730d990f0e78063b21a26988c969cb126d4bd697bd
bdb8b9a347e68f80a2e8d44ce5590a2e8214b6bb test: doc: improve doc for `from_hex` helper (mention `to_hex` alternative) (Sebastian Falbesoner)
191405420815d49ab50184513717a303fc2744d6 scripted-diff: test: rename `FromHex` to `from_hex` (Sebastian Falbesoner)
a79396fe5f8f81c78cf84117a87074c6ff6c9d95 test: remove `ToHex` helper, use .serialize().hex() instead (Sebastian Falbesoner)
2ce7b47958c4a10ba20dc86c011d71cda4b070a5 test: introduce `tx_from_hex` helper for tx deserialization (Sebastian Falbesoner)
Pull request description:
There are still many functional tests that perform conversions from a hex-string to a message object (deserialization) manually. This PR identifies all those instances and replaces them with a newly introduced helper `tx_from_hex`.
Instances were found via
* `git grep "deserialize.*BytesIO"`
and some of them manually, when it were not one-liners.
Further, the helper `ToHex` was removed and simply replaced by `.serialize().hex()`, since now both variants are in use (sometimes even within the same test) and using the helper doesn't really have an advantage in readability. (see discussion https://github.com/bitcoin/bitcoin/pull/22257#discussion_r652404782)
ACKs for top commit:
MarcoFalke:
review re-ACK bdb8b9a347e68f80a2e8d44ce5590a2e8214b6bb 😁
Tree-SHA512: e25d7dc85918de1d6755a5cea65471b07a743204c20ad1c2f71ff07ef48cc1b9ad3fe5f515c1efaba2b2e3d89384e7980380c5d81895f9826e2046808cd3266e
957f3584277d33b44b9f1618142f743e4083eb5d build: remove check for Boost Process header (fanquake)
df2c93321765c7f77935fd92d31722d7b65a53db build: remove workaround for Boost and std::atomic (fanquake)
2bf211696b8cd355856995ba821c74e67385b77d build: set minimum required Boost to 1.64.0 (fanquake)
Pull request description:
Setting a newer minimum required Boost means we can remove the awkward header / compile check for Boost Process.
If we don't do this, we should at-least make Boost Process being missing no longer a failure, otherwise anyone building using Boost < 1.64.0 would have to pass `--disable-external-signer` as well.
The only system I can see that is affected here, (doesn't have new enough system packages) is Debian Oldstable. However, anyone compiling there, can use depends. They can also no-longer use the system GCC (6.0), and I'd assume would be using Clang 7, which would be the newest compiler available to them. It's extended, LTS support also end in 1 year from now, so anyone still using it should be considering upgrading.
Debian Buster (Stable) has 1.67+, Ubuntu Bionic has 1.65+, any of the BSDs, recent Fedora, macOS etc all also have well and truly new enough Boost versions available.
I think this is something we should just do for 22.0. If not, definitely for 23.0.
Fixes#22319. Compiling Bitcoin Core should work, as `windows.h` will be included.
Alternative to #22294.
Would also close#22269.
#19128 could be re-opened.
ACKs for top commit:
laanwj:
Tested ACK 957f3584277d33b44b9f1618142f743e4083eb5d that this fixes#22269.
MarcoFalke:
review ACK 957f3584277d33b44b9f1618142f743e4083eb5d
Tree-SHA512: a8ffa7933dce8bf994892ef16664103d7b4e1008e52628e9becb918a7727232dfb51b23100a82dc2b60cd9af5877abc32dc2d3754a7e1b3ac5410a92fdf393f3
