e359ba6b35edebf02b968fe60cae48473ed88826 doc: Drop a note about Intel-based Macs (Hennadii Stepanov)
Pull request description:
The work on building stuff during the recent months made the removed note obsolete.
ACKs for top commit:
fanquake:
ACK e359ba6b35edebf02b968fe60cae48473ed88826
Tree-SHA512: 8cf851c8602ef004c9ca009a97345b828bacbb6ecf1eee803d3ce64870a9766c196849b8843237e7bc1be5697de928b759a6dfa0407022c144d23d0293322200
5347c9732ff24bfcdcd62d0291972c86c80558dc doc: update multisig-tutorial.md to default wallet type (Jon Atack)
Pull request description:
Follow-up to #24281 and https://github.com/bitcoin/bitcoin/pull/24281#issuecomment-1033996386. The default wallet type was changed to descriptor wallets in #23002.
ACKs for top commit:
laanwj:
ACK 5347c9732ff24bfcdcd62d0291972c86c80558dc
michaelfolkson:
ACK 5347c9732ff24bfcdcd62d0291972c86c80558dc
achow101:
ACK 5347c9732ff24bfcdcd62d0291972c86c80558dc
theStack:
Concept and code-review ACK 5347c9732ff24bfcdcd62d0291972c86c80558dc
Tree-SHA512: 8074a33ad253ecb7d3f78645a00c808c7c224996cc1748067928aa59ef31a58f24fcfc75169494b26a19c7fbbf23bbd78516ab4102bc52fa92f08f1f49b18b63
9a5b4d78921ffd16c78d3661447faa065426da67 doc: Delete old line of code that was commented out (Michael Folkson)
Pull request description:
In #23288 MarcoFalke [highlighted](https://github.com/bitcoin/bitcoin/pull/23288/files#r739817055) an old BOOST_CHECK that was commented out and replaced by a different BOOST_CHECK. I think this can be deleted and wasn't deliberately left in by achow101.
ACKs for top commit:
achow101:
ACK 9a5b4d78921ffd16c78d3661447faa065426da67
jonatack:
ACK 9a5b4d78921ffd16c78d3661447faa065426da67
Tree-SHA512: 69aa71d362bb190c75d617766f6af945a43211ddb315ccfef5ebab2beefe020032d99f0d0d4b312aa349e605ba712a8bc91d7f0a22427681e08c95f8a6ea7e64
bce9aaf31e2b0428e686e151324f8561ad71f11f Unit tests for IsWitnessProgram and IsP2WSH. (Daniel Kraft)
Pull request description:
This adds basic unit tests for `CScript::IsPayToWitnessScriptHash` and `CScript::IsWitnessProgram`, similar to the existing tests for `CScript::IsPayToScriptHash`. These tests are probably not super important given the other existing tests for segwit related code, but may be useful in catching some errors early.
This implements #14737.
ACKs for top commit:
aureleoules:
tACK bce9aaf31e2b0428e686e151324f8561ad71f11f (`make check)`.
Tree-SHA512: 3cff5efc4ac53079289c72bfba8b1937bc103baadd32bb1fba41e78017f65f9cca17678c3202ad0711eae42b351d4132d9ed9b4e2dc07d138298691a09c4e822
3c74f775ac956de4da4fc076b2360b687531cd63 Update signapple for platform identifier fix (Andrew Chow)
Pull request description:
Apparently #23134 is caused by the platform identifier field being set to the incorrect value in our code signatures. The problem has been resolved in signapple, and so guix should point to the latest commit containing the fix.
I suppose guix does not strictly need to have this; only the macOS code signer will need to have the fix.
Fixes#23134
ACKs for top commit:
gruve-p:
ACK 3c74f775ac
hebasto:
re-ACK 3c74f775ac956de4da4fc076b2360b687531cd63
fanquake:
ACK 3c74f775ac956de4da4fc076b2360b687531cd63
Tree-SHA512: 7df844793fa77be4ddc4ef02f26980d6368b50421b7bd9a15f7d6a0c3b5c5f4f0cc0889e065689956583a2173875d33406dbe3a52a72c75a7f23a33c733c2378
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
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 893e18059f
Tree-SHA512: 6750eaf70d5ebc9c364ade1d4b5b689e3094020eeb444a3de93b33d9a57a1577949a461f8209442d3954ccb22ab038c7e8cf6dfff5623e4f2713606b6798c37e
Before this change the send confirmation dialog would keep the Send option disabled. The Create Unsigned choice would actually send. This is potentially confusing.
With this change the Create Unsigned button will not attempt to sign and always produce a PSBT. The Send button will attempt to sign, and only return a PSBT if more signatures are needed.
When using an external signer, the Create Unsigned option only appears when PSBT controls are enabled in the wallet settings.
This commit maintains the pre-existing behavior of filling the PSBT (without signing) even when not using an external signer.
Closes#551
Co-authored-by: Jon Atack <jon@atack.com>
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
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
This commit effectively splits the "load block index itself" logic from
"derive Chainstate variables from loaded block index" logic.
This means that BlockManager::LoadBlockIndex{,DB} will only load what's
relevant to the BlockManager.
I strongly recommend reviewing with the following git-diff flags:
--color-moved=dimmed_zebra --color-moved-ws=allow-indentation-change
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
This is a performance optimization - rather than track all visited values
in a bool vector, track the selected index in a vector. This results in a
complexity reduction of O(utxo_size) to O(selection_size).
PSBT signing was changed to use SIGHASH_DEFAULT by default in #22514.
The signet miner script sets the sighash type of the created PSBT to
SIGHASH_ALL, hence this leads to a sighash mismatch when the
`walletprocesspsbt` RPC is called. Fix this by explicitly passing the
correct sighash type.
Note that the same change was needed in one of our functional tests,
see commit d3992669df826899a3de78a77a366dab46028026.
Reported by gruve-p.
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 fa8e76bb90
S3RK:
Code review ACK fa8e76bb9002a126b3645bd9533c282f5dfff111
achow101:
ACK fa8e76bb9002a126b3645bd9533c282f5dfff111
w0xlt:
Code Review ACK fa8e76bb90
Tree-SHA512: 6fe37c85f074851ef09cae8addcb836257089290fecec515c129c3180e9ccb64740aaac76043af10ce7c213e5001568ca6b4b62ae9631f0994ed676b07118074
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
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
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
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
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
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
