fa551b3bdd Remove GetAdjustedTime from init.cpp (MarcoFalke)
fa815f8473 Replace addrman.h include with forward decl in net.h (MarcoFalke)
Pull request description:
It seems confusing to call `GetAdjustedTime` there, because no offset could have been retrieved from the network at this point. Even if connman was started, `timedata` needs at least 5 peer connections to calculate an offset.
Fix the confusion by replacing `GetAdjustedTime` with `GetTime`, which does not change behavior.
Also:
* Replace magic number with `MAX_FUTURE_BLOCK_TIME` to clarify the context
* Add test, which passes both on current master and this pull request
* An unrelated refactoring commit, happy to drop
ACKs for top commit:
dongcarl:
Code Review ACK fa551b3bdd, noticed the exact same thing here: e073634c37
mzumsande:
Code Review ACK fa551b3bdd
jnewbery:
Code review ACK fa551b3bdd
shaavan:
ACK fa551b3bdd
theStack:
Code-review ACK fa551b3bdd
Tree-SHA512: 15807a0e943e3e8d8c5250c8f6d7b56afb26002b1e290bf93636a2c747f27e78f01f1de04ce1a83d6339e27284c69c43e077a8467545c4078746f4c1ecb1164d
ffd09281fe rpc: various fixups for dumptxoutset (James O'Beirne)
Pull request description:
This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: #15606)
---
A few fixes to make this RPC actually useful when generating snapshots.
- Generate an assumeutxo hash and display it (sort of a bugfix)
- Add nchaintx to output (necessary for use in chainparams entry)
- Add path of serialized UTXO file to output
ACKs for top commit:
laanwj:
Code review ACK ffd09281fe
Tree-SHA512: b0b5fd5138dea0e21258b1b18ab75bf3fd1628522cc1dbafa81af9cb9fa96562a1c39124fdb31057f256bfc560f462f907e9fe5e209b577b3f57afae2b7be826
fa00447442 scripted-diff: Use clang-tidy syntax for C++ named arguments (MarcoFalke)
fae13c3989 doc: Use clang-tidy comments in crypto_tests (MarcoFalke)
Pull request description:
Incorrect named args are source of bugs, like #22979.
To allow them being checked by `clang-tidy`, use a format it can understand.
ACKs for top commit:
shaavan:
ACK fa00447442
rajarshimaitra:
ACK fa00447442
jonatack:
ACK fa00447442
fanquake:
ACK fa00447442
Tree-SHA512: 4d23a8363da81dfea21a4cd8516ab5e0dc70119e4d503f3f240f38573218b2c2e84083b97e956c62942d78b2f17490f8b3b2e8077d257644fda1d901e2b80507
- Actually generate an assumeutxo hash and display it
- Add nchaintx to output (necessary for use in chainparams entry)
- Add path of serialized UTXO file to output
fa4e09924b refactor: Replace validation.h include with forward-decl in miner.h (MarcoFalke)
fa0739a7d3 style: Sort file list after rename (MarcoFalke)
fa53e3a58c scripted-diff: Move miner to src/node (MarcoFalke)
Pull request description:
It is impossible to run the miner without a node (validation, chainstate, mempool, rpc, ...). Also, the module is in the node library. Thus, it should be moved to `src/node`.
Also, replace the `validation.h` include in the header with a forward-declaration.
ACKs for top commit:
theStack:
Code-review ACK fa4e09924b
Tree-SHA512: 791e6caa5839d8dc83b0f58f3f49bc0a7e3c1710822e8a44dede254c87b6f7531a0586fb95e8a067c181457a3895ad6041718aa2a2fac64cfc136bf04bb851d5
3726a45958 refactor: replace RecursiveMutex m_added_nodes_mutex with Mutex (Sebastian Falbesoner)
7d52ff5c38 refactor: replace RecursiveMutex m_addr_fetches_mutex with Mutex (Sebastian Falbesoner)
d51d2a3bb5 scripted-diff: rename node vector/mutex members in CConnman (Sebastian Falbesoner)
574cc4271a refactor: remove RecursiveMutex cs_totalBytesRecv, use std::atomic instead (Sebastian Falbesoner)
Pull request description:
This PR is related to #19303 and gets rid of the following RecursiveMutex members in class `CConnman`:
* for `cs_totalBytesRecv`, protecting `nTotalBytesRecv`, `std::atomic` is used instead (the member is only increment at one and read at another place, so this is sufficient)
* for `m_addr_fetches_mutex`, protecting `m_addr_fetches`, a regular `Mutex` is used instead (there is no chance that within one critical section, another one is called)
* for `cs_vAddedNodes`, protecting `vAddedNodes`, a regular `Mutex` is used instead (there is no chance that within one critical section, another one is called)
Additionally, the PR takes the chance to rename all node vector members (vNodes, vAddedNodes) and its corresponding mutexes (cs_vNodes, cs_vAddedNodes) to match the coding guidelines via a scripted-diff.
ACKs for top commit:
vasild:
ACK 3726a45958
promag:
Code review ACK 3726a45958.
hebasto:
re-ACK 3726a45958
Tree-SHA512: 4f5ad41ba2eca397795080988c1739c6abb44c1204dddaa75cc38a396fa821fbe1010694ba7bead1b606beaa677661e66da2a5dca233b2937214f63a54848348
fa3e0da06b policy: Treat taproot as always active (MarcoFalke)
Pull request description:
Now that taproot is active, it can be treated as if it was always active for policy for the next major release. This simplifies the code and changes two things:
* Importing `tr` descriptors can be done before the chain is fully synced. This is fine, because the wallet will already generate `tr` descriptors by default (regardless of the taproot status) after commit 47fe7445e7.
* Valid taproot spends won't be rejected from the mempool before taproot is active. This is strictly speaking a bugfix after commit 47fe7445e7, since the wallet may generate taproot spends before the chain is fully synced. For example, a slow node or a purposefully offline node. Currently, the wallet needs the mempool to account for change. See https://github.com/bitcoin/bitcoin/issues/11887.
A similar change was done for segwit v0 in https://github.com/bitcoin/bitcoin/pull/13120 .
This effectively reverts commit c5ec0367d7.
ACKs for top commit:
mjdietzx:
Code Review ACK fa3e0da06b
achow101:
ACK fa3e0da06b
sipa:
utACK fa3e0da06b
gruve-p:
ACK fa3e0da06b
gunar:
Code Review + tACK fa3e0da06
rajarshimaitra:
code review + tACK fa3e0da06b
Tree-SHA512: c6dc7a4e6c345bdec33f256847dc63906ab1696aa683ab9b32a79e715613950884ac3a1a7a44e95f31bb28e58dd64679a616175f7e152b21f5550f3337c8e622
faa3ec2304 span: Add std::byte helpers (MarcoFalke)
fa18038f51 refactor: Use ignore helper when unserializing an invalid pubkey (MarcoFalke)
fabe18d0b3 Use value_type in CDataStream where possible (MarcoFalke)
Pull request description:
This adds (currently unused) span std::byte helpers, so that they can be used in new code.
The refactors are also required for https://github.com/bitcoin/bitcoin/pull/23438, but they are split up because the other pull doesn't compile with msvc right now.
The third commit is not needed for the other pull, but still nice.
ACKs for top commit:
klementtan:
reACK faa3ec2. Verified that all the new `std::byte` helper functions are tested.
laanwj:
Code review ACK faa3ec2304
Tree-SHA512: b1f6af39f03ea4dfebf20d4a8538fa993a6104e7fc92ddf0c4606a7efc3ca9a8c1a4741d98a1418569c11bb9ce9258bf0c0c06d93d85ed7e208902a2db04e407
21b58f430f util: ParseByteUnits - Parse a string with suffix unit [k|K|m|M|g|G|t|T] (Douglas Chimento)
Pull request description:
A convenience utility for parsing human readable strings sizes e.g. `500G` is `500 * 1 << 30`
The argument/setting `maxuploadtarget` now accept human readable byte units `[k|K|m|M|g|G||t|T]`
This change backward compatible, defaults to `M` if no unit specified.
ACKs for top commit:
vasild:
ACK 21b58f430f
ryanofsky:
Code review ACK 21b58f430f. Only changes since last review are dropping optional has_value call, fixing comment punctuation, squashing commits.
Tree-SHA512: c9b85acc0f77c847a0290b27ac5dc586ecc078110cf133063140576a04c11aa9c553159b9b4993488edcf6e60db6837de7c83b2964639bc21e8ffa4d455a5eb7
88cc481092 Modify copyright header on Bech32 code (Samuel Dobson)
5599813b80 Add lots of comments to Bech32 (Samuel Dobson)
2eb5792ec7 Add release notes for validateaddress Bech32 error detection (MeshCollider)
42d6a029e5 Refactor and add more tests for validateaddress (Samuel Dobson)
c4979f77c1 Add boost tests for bech32 error detection (MeshCollider)
02a7bdee42 Add error_locations to validateaddress RPC (Samuel Dobson)
b62b67e06c Add Bech32 error location function (Samuel Dobson)
0b06e720c0 More detailed error checking for base58 addresses (Samuel Dobson)
Pull request description:
Addresses (partially) #16779 - no GUI change in this PR
Adds a LocateError function the bech32 library, which is then called by `validateaddress` RPC, (and then eventually from a GUI tool too, future work). I think modifying validateaddress is nicer than adding a separate RPC for this.
Includes tests.
Based on https://github.com/sipa/bech32/blob/master/ecc/javascript/bech32_ecc.js
Credit to sipa for that code
ACKs for top commit:
laanwj:
Code review and manually tested ACK 88cc481092
ryanofsky:
Code review ACK 88cc481092 with caveat that I only checked the new `LocateErrors` code to try to verify it didn't have unsafe or unexpected operations or loop forever or crash. Did not try to verify behavior corresponds to the spec. In the worst case bugs here should just affect error messages not actual decoding of addresses so this seemed ok.
w0xlt:
tACK 88cc481
Tree-SHA512: 9c7fe9745bc7527f80a30bd4c1e3034e16b96a02cc7f6c268f91bfad08a6965a8064fe44230aa3f87e4fa3c938f662ff4446bc682c83cb48c1a3f95cf4186688
faba1abe46 Sort file list after rename (MarcoFalke)
fa8f60e311 scripted-diff: Move minisketchwrapper to src/node (MarcoFalke)
Pull request description:
The newly added wrapper is currently in the node library, but not placed in the node directory. While it is possible to use the wrapper outside of a node context (for example in a utility), it seems unlikely. Either way, I think the wrapper should either be moved to the util lib+dir or the node lib+dir, not something in-between.
Also, fix incorrect comment `BITCOIN_DBWRAPPER_H`.
ACKs for top commit:
fanquake:
ACK faba1abe46. I saw the comment in #21515, however given there hasn't been any new activity there, I'm going to merge this now.
Tree-SHA512: fccc0cfd1fee661152a1378587b96795ffb7a7eceb6d2c27ea5401993fd8b9c0a92579fdba61203917ae6565269cb28d0973464fb6201dabf72a5143495d3e77
f1c33ee4ac tests: implement BIP341 test vectors (Pieter Wuille)
ac3037df11 tests: BIP341 test vector generation (Pieter Wuille)
ca83ffc2ea tests: add deterministic signing mode to ECDSA (Pieter Wuille)
c98c53f20c tests: abstract out precomputed BIP341 signature hash elements (Pieter Wuille)
a5bde018b4 tests: give feature_taproot access to sighash preimages (Pieter Wuille)
5140825096 tests: add more fields to TaprootInfo (Pieter Wuille)
2478c6730a Make signing follow BIP340 exactly w.r.t. aux randomness (Pieter Wuille)
Pull request description:
This PR adds code to `test/functional/feature_taproot.py` which runs through a (deterministic) scenario covering several aspects of the wallet side of BIP341 (scriptPubKey computation from keys/scripts, control block computation, key path spending), with the ability to output test vectors in mediawiki format based on this scenario. The generated tests are then also included directly in `src/test/script_tests.cpp` and `src/test/script_standard_tests.cpp`.
I intend to add these test vectors to BIP341 itself: https://github.com/bitcoin/bips/pull/1225
ACKs for top commit:
laanwj:
Code review ACK f1c33ee4ac
Tree-SHA512: fcf7109539cb214d3190516b205cd32d2b1b452f14aa66f4107acfaa8bfc7d368f626857f1935665a4342eabc0b9ee8aba608a7c0a2494bec0b498e723439c9d
fa4baf0756 fuzz: Rework ConsumeScript (MarcoFalke)
Pull request description:
This should make it easier for the fuzz engine to explore multisig code
paths. See discussion in https://github.com/bitcoin/bitcoin/issues/23105
The downside is that all fuzz inputs that use ConsumeScript are now
invalidated and need to be re-generated.
Another downside may be that most multisig scripts from ConsumeScript are
using likely not fully valid pubkeys.
ACKs for top commit:
jamesob:
ACK fa4baf0756
Tree-SHA512: 15814afdee76b05ff7a71c0f07bbd1b3cff30d709d5c1e68fd230c5f5d16e673e42709a4fab84d4a896bc27f972f917fe7c1d1b32c2bf4209658b18da97e478b
214d9055ac fuzz: replace every fuzzer-controlled loop with a LIMITED_WHILE loop (Andrew Poelstra)
Pull request description:
Limits the number of iterations to 1000 rather than letting the fuzzer do millions or billions of iterations on a single core.
ACKs for top commit:
MarcoFalke:
cr ACK 214d9055ac
Tree-SHA512: 9741c32ccd126ea656e5c93371b7136eaa2f92dc9a490dd4d39642503b1a41174f3368245153e508c3b608fe37ab89800b67ada97b740e3b5a3728bb506429d3
Blindly chose a cap of 10000 iterations for every loop, except for
the two in script_ops.cpp and scriptnum_ops.cpp which appeared to
(sometimes) be deserializing individual bytes; capped those to one
million to ensure that sometimes we try working with massive scripts.
There was also one fuzzer-controlled loop in timedata.cpp which was
already capped, so I left that alone.
git grep 'while (fuzz' should now run clean except for timedata.cpp
libsecp256k1's secp256k1_schnorrsig_sign only follows BIP340 exactly
if an aux_rand32 argument is passed. When no randomness is used
(as is the case in the current codebase here), there is no impact
on security between not providing aux_rand32 at all, or providing
an empty one. Yet, for repeatability/testability it is simpler
to always use an all-zero one.
29173d6c6c ubsan: add minisketch exceptions (Cory Fields)
54b5e1aeab Add thin Minisketch wrapper to pick best implementation (Pieter Wuille)
ee9dc71c1b Add basic minisketch tests (Pieter Wuille)
0659f12b13 Add minisketch dependency (Gleb Naumenko)
0eb7928ab8 Add MSVC build configuration for libminisketch (Pieter Wuille)
8bc166d5b1 build: add minisketch build file and include it (Cory Fields)
b2904ceb85 build: add configure checks for minisketch (Cory Fields)
b6487dc4ef Squashed 'src/minisketch/' content from commit 89629eb2c7 (fanquake)
Pull request description:
This takes over #21859, which has [recently switched](https://github.com/bitcoin/bitcoin/pull/21859#issuecomment-921899200) to my integration branch. A few more build issues came up (and have been fixed) since, and after discussing with sipa it was decided I would open a PR to shepherd any final changes through.
> This adds a `src/minisketch` subtree, taken from the master branch of https://github.com/sipa/minisketch, to prepare for Erlay implementation (see #21515). It gets configured for just supporting 32-bit fields (the only ones we're interested in in the context of Erlay), and some code on top is added:
> * A very basic unit test (just to make sure compilation & running works; actual correctness checking is done through minisketch's own tests).
> * A wrapper in `minisketchwrapper.{cpp,h}` that runs a benchmark to determine which field implementation to use.
Only changes since my last update to the branch in the previous PR have been rebasing on master and fixing an issue with a header in an introduced file.
ACKs for top commit:
naumenkogs:
ACK 29173d6c6c
Tree-SHA512: 1217d3228db1dd0de12c2919314e1c3626c18a416cf6291fec99d37e34fb6eec8e28d9e9fb935f8590273b8836cbadac313a15f05b4fd9f9d3024c8ce2c80d02
