c8992e89594a54edf283e4916f794475070b5114 test: Fix fuzzer compilation on macOS fixes#19557 (freenancial)
Pull request description:
fixes#19557
Before the fix:
```
➜ bitcoin git:(fix-fuzzer-macos) make
Making all in src
CXX test/fuzz/addition_overflow-addition_overflow.o
In file included from test/fuzz/addition_overflow.cpp:7:
./test/fuzz/util.h:335:13: error: no matching function for call to 'AdditionOverflow'
if (AdditionOverflow((uint64_t)fuzzed_file->m_offset, random_bytes.size())) {
^~~~~~~~~~~~~~~~
./test/fuzz/util.h:201:16: note: candidate template ignored: deduced conflicting types for parameter 'T' ('unsigned long long' vs. 'unsigned long')
NODISCARD bool AdditionOverflow(const T i, const T j) noexcept
^
./test/fuzz/util.h:346:13: error: no matching function for call to 'AdditionOverflow'
if (AdditionOverflow(fuzzed_file->m_offset, n)) {
^~~~~~~~~~~~~~~~
./test/fuzz/util.h:201:16: note: candidate template ignored: deduced conflicting types for parameter 'T' ('long long' vs. 'long')
NODISCARD bool AdditionOverflow(const T i, const T j) noexcept
^
```
After the fix:
```
➜ bitcoin git:(fix-fuzzer-macos) ./configure --enable-fuzz --with-sanitizers=fuzzer,address,undefined CC=/usr/local/opt/llvm/bin/clang CXX=/usr/local/opt/llvm/bin/clang++ --disable-asm && make clean && make -j5
...
...
CXXLD test/fuzz/uint256_deserialize
Making all in doc/man
make[1]: Nothing to be done for `all'.
make[1]: Nothing to be done for `all-am'.
```
ACKs for top commit:
fanquake:
ACK c8992e89594a54edf283e4916f794475070b5114 - tested that compiling works on macOS.
MarcoFalke:
review ACK c8992e89594a54edf283e4916f794475070b5114
Tree-SHA512: 965cdc61b30db0e2209c91b29f0d42de927a9a5b85e1e70f22d1452e0955f876726c7a8c1d1a5f448f12bf24eec3000802071cd4ae28d8605343fd43d174ca84
1554b54d47d7e24ce2491f57d24e56d38ceb7649 Static asserts for consistency of fee defaults. (Daniel Kraft)
Pull request description:
This adds `static_assert`'s that ensure that the default values given for fee levels in the wallet (minimum fee and incremental feerate increase) are at least as high as the corresponding levels configured in the core node policy. Since the core policy values are enforced by the network, it makes sense for the wallet to be conservative and above (or at least not below) this.
ACKs for top commit:
laanwj:
code review ACK 1554b54d47d7e24ce2491f57d24e56d38ceb7649, these assumptions seem straightforward
Tree-SHA512: 50e5adf082f467062334377f82a3ee75bcfd436afc65bd0eb33c8d0549d6d90fd1f48c31f60cabe523eb59be9efa8ae0879e9e09cd51ca9c1bd466631ce03cf4
completion of 698a717e from dash#5163 by including:
- 4fe338ab3ed73b3ffb20eedf95500c56ec2920e1
- e8b215a086d91a8774210bb6ce8d1560aaaf0789
- 16d9bfc4172b4f6ce24a3cd1a1cfa3933cd26751
398045ba8b3694931069f88ec95553b3207dd1a6 cli -netinfo: print oversized/extreme ping times as "-" (Jon Atack)
773f4c99c00c0b1d8c1b53cb99ba571337100953 cli -netinfo: handle longer tor v3 local addresses (Jon Atack)
33e987452f869c279f2491499939e51e0af8364c cli -netinfo: make age column variable-width (Jon Atack)
f8a1c4d9469cb496fdafaf6f4d94977687df9190 cli -netinfo: various quick updates and fixes (Jon Atack)
Pull request description:
Quick fixups and updates for v0.21.0:
- [x] handle larger BIP155 `addrv2` addresses
- [x] add Signet chain
- [x] add an additional space between the `net` and `mping` columns; add missing `tinyformat` and `algorithm` headers
- [x] s/uptime/age/ per 0xB10C suggestion, and make the column auto-adjusting variable width
- [x] display `-` for oversized mping/ping times like `1.17348e+06`, as reported by practicalswift
Edit: removed the release note commit, as this PR was not merged before the notes were moved to the wiki. It's here:
```
- A new `bitcoin-cli -netinfo` command returns a network peer connections
dashboard that displays data from the `getpeerinfo` and `getnetworkinfo` RPCs
in a human-readable format. An optional integer argument from `0` to `4` may
be passed to see various levels of detail. (#19643)
```
ACKs for top commit:
michaelfolkson:
ACK 398045ba8b3694931069f88ec95553b3207dd1a6
Emzy:
Tested ACK 398045ba8b3694931069f88ec95553b3207dd1a6
Tree-SHA512: 0625ee840141bafbfcaf8f1fce53f8f850ae91721b2bdad4279372da87c18a1fe3a214d90bfdbbabdf6da38d58290d7dd0f1109b4e2ca5d20cacf417d6ced0f9
c82336c493b112160d781974d4066fcb956b85f6 Remove references to CreateWalletFromFile (fanquake)
Pull request description:
`CWallet::CreateWalletFromFile()` was removed in 8b5e7297c02f3100a9cb27bfe206e3fc617ec173 but these references remain.
ACKs for top commit:
hebasto:
ACK c82336c493b112160d781974d4066fcb956b85f6
Tree-SHA512: 3dd50fe0cd5a60bbc96d265107d4739f3e08f943435f3772038963ac4be9e4a87a863412ac0d571226ea66d71550b17b52f01b9d46a6282d49feae1508fd682e
## Issue being fixed or feature implemented
Trivial refactor to enable this to be const
## What was done?
Used a lambda and const
## How Has This Been Tested?
Compiling
## Breaking Changes
None
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that
apply._
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
fa4bc897fc9332a5666ca2f3e78492cd67ee6128 fuzz: Fail if message type is not fuzzed (MarcoFalke)
faefed8cd5d26a485f5f6df824d7c90967c826f3 fuzz: Count message type fuzzers before main() (MarcoFalke)
Pull request description:
`process_message_*` is a nice way to quickly fuzz a single message type. However, the offered message types are outdated and all BIPs implemented in the last years are missing.
Fix that by adding them and failing when the number of message types don't add up.
ACKs for top commit:
practicalswift:
cr ACK fa4bc897fc9332a5666ca2f3e78492cd67ee6128: patch looks correct and touches only `src/test/fuzz/`
Tree-SHA512: 8c98374b50fb4ab2ff2550daeab4c6e9f486bfe847466d217d4bc97d119adc99a82b87b56f47535b1cf8f844232bc7fa1230712a9147cda514ae78851556f988
fac726b1b8331b267973138bbd2bff5304774315 doc: Fixup docs in fuzz/script_assets_test_minimizer.cpp (MarcoFalke)
fafca47adc2476f19f7926de4d55b64b0286e41c fuzz: Hide script_assets_test_minimizer (MarcoFalke)
Pull request description:
This is not an actual fuzz target. It is a hack to exploit the built-in capability of fuzz engines to measure coverage.
ACKs for top commit:
practicalswift:
cr ACK fac726b1b8331b267973138bbd2bff5304774315: patch looks correct and touches only `src/test/fuzz/`
Tree-SHA512: 0652dd8d9e95746b0906be4044467435d8204a34a30366ae9bdb75b9cb0788d429db7cedf2760fd543565d9d4f7ee206873ed10a29dd715a792a26337f65b53c
eecb7ab105a4a59d09cd55b124c5ad563846fe11 [doc] clarify -peertimeout and -timeout descriptions (gzhao408)
Pull request description:
The debug-only option `-peertimeout` is used to delay `InactivityCheck()`, whereas the `-timeout` option specifies socket timeouts (`nConnectTimeout`). The current descriptions are a bit misleading and hard to tell apart. I think it would save dev/review time to update them 🤷
ACKs for top commit:
MarcoFalke:
ACK eecb7ab105a4a59d09cd55b124c5ad563846fe11 nice doc fixup
jnewbery:
ACK eecb7ab105a4a59d09cd55b124c5ad563846fe11
Tree-SHA512: 71d2e6c31664b9f7f0b053ecf3be21c6c55472553fa7478d8526ba3be8d54979bceafca63d87b8b2488c11f409c332ac795da613ff8101546b18d9cd8bcceb50
8f0b64fb513e8c6cdd1f115856100a4ef5afe23e Better error messages for invalid addresses (Bezdrighin)
Pull request description:
This PR addresses #20809.
We add more detailed error messages in case an invalid address is provided inside the 'validateaddress' and 'getaddressinfo' RPC calls. This also covers the case when a user provides an address from a wrong network.
We also add a functional test to test the new error messages.
ACKs for top commit:
kristapsk:
ACK 8f0b64fb513e8c6cdd1f115856100a4ef5afe23e
meshcollider:
Code review ACK 8f0b64fb513e8c6cdd1f115856100a4ef5afe23e
Tree-SHA512: ca0f806ab573e96b79e98d9f8c810b81fa99c638d9b5e4d99dc18c8bd2568e6a802ec305fdfb2983574a97a19a46fd53b77645f8078fb77e9deb24ad2a22cf93
7487bc9900d28e1b5361cba882fd8783aafc7092 Fix BlockToJsonVerbose benchmark (Martin Ankerl)
Pull request description:
Currently it was not possible to run just the BlockToJsonVerbose benchmark because it did not set up everything it needed, running `bench_bitcoin -filter=BlockToJsonVerbose` caused this assert to fail:
```
bench_bitcoin: chainparams.cpp:506: const CChainParams& Params(): Assertion `globalChainParams' failed.
```
Initializing TestingSetup fixes this.
ACKs for top commit:
theStack:
Tested ACK 7487bc9900d28e1b5361cba882fd8783aafc7092 🐎
Tree-SHA512: 27b9702cb4bacc0475710f7b31f41844e83b8a0787685380749505d179aba724728604d4e4e2e3b3cb38cde88ab12f170881b5d3eb615872ee84632e85312166
… best height
## Issue being fixed or feature implemented
Platform wants to know the height of the bestchainlock when they call
submitchainlock; sooo we change the API of submitchainlock to also
return the height
## What was done?
Adjust API and tests
## How Has This Been Tested?
New tests added for this behavior
## Breaking Changes
Not really any; I **guess** that return value could be considered
breaking change; but going from nothing -> something feels unlikely to
break anything although it in theory could.
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that
apply._
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
---------
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: Konstantin Akimov <knstqq@gmail.com>
BACKPORT NOTICE:
define/endif moved in original backport but some changes are not backported yet
---------
fa3967efdb07f1d22372f4ee2e602ea1fad04a57 test: Replace ARRAYLEN with C++11 ranged for loop (MarcoFalke)
fafc5290538fde76c3780976f4b2c11dc9f24d19 test: Run AssetTest even if built --with-libs=no (MarcoFalke)
faf58ab139949ca35b33217d010b350c9a59c61d ci: Add --with-libs=no to one ci config (MarcoFalke)
Pull request description:
`script_assets_test` doesn't call libbitcoinconsensus, so it seems confusing to require it
ACKs for top commit:
fanquake:
ACK fa3967efdb07f1d22372f4ee2e602ea1fad04a57 - looks ok to me.
Tree-SHA512: 8744fef64c5d7dc19a0ef4ae9b3df5b3f356253bf000f12723064794c5e50df071824d436059985f1112d94c1b530b65cbeb6b8435114a91b195480620eafc59
31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0 [util] add RunCommandParseJSON (Sjors Provoost)
c17f54ee535faaedf9033717403e1f775b5f1530 [ci] use boost::process (Sjors Provoost)
32128ba682033560d6eb2e4848a9f77a842016d2 [doc] include Doxygen comments for HAVE_BOOST_PROCESS (Sjors Provoost)
3c84d85f7d218fa27e9343c5cd1a55e519218980 [build] msvc: add boost::process (Sjors Provoost)
c47e4bbf0b44f2de1278f9538124ec98ee0815bb [build] make boost-process opt-in (Sjors Provoost)
929cda5470f98d1ef85c05b1cad4e2fb9227e3b0 configure: add ax_boost_process (Sjors Provoost)
8314c23d7b39fc36dde8b40b03b6efbe96f85698 [depends] boost: patch unused variable in boost_process (Sjors Provoost)
Pull request description:
Prerequisite for external signer support in #16546. Big picture overview in [this gist](https://gist.github.com/Sjors/29d06728c685e6182828c1ce9b74483d).
This adds a new dependency [boost process](https://github.com/boostorg/process/tree/boost-1.64.0). This is part of Boost since 1.64 which is part of `depends`. Because the minimum Boost version is 1.47, this functionality is skipped for older versions of Boost.
Use `./configure --with-boost-process` to opt in, which checks for the presence of Boost::Process.
We add `UniValue runCommandParseJSON(const std::string& strCommand)` to `system.{h,cpp}` which calls an arbitrary command and processes the JSON returned by it. This is currently only called by the test suite.
~For testing purposes this adds a new regtest-only RPC method `runcommand`, as well as `test/mocks/command.py` used by functional tests.~ (this is no longer the case)
TODO:
- [ ] review boost process in #15440
ACKs for top commit:
achow101:
ACK 31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0
hebasto:
re-ACK 31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0, only rebased (verified with `git range-diff`) and removed an unintentional tab character since the [previous](https://github.com/bitcoin/bitcoin/pull/15382#pullrequestreview-458371035) review.
meshcollider:
Very light utACK 31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0, although I am not very confident with build stuff.
promag:
Code review ACK 31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0, don't mind the nit.
ryanofsky:
Code review ACK 31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0. I left some comments below that could be ignored or followed up later. The current change is clean and comprehensive.
Tree-SHA512: c506e747014b263606e1f538ed4624a8ad7bcf4e025cb700c12cc5739964e254dc04a2bbb848996b170e2ccec3fbfa4fe9e2b3976b191222cfb82fc3e6ab182d
## Issue being fixed or feature implemented
Dashmate wanted a way to know if it is safe to restart the masternode.
This new RPC indicates the number of active DKG sessions, and the number
of blocks until next potential DKG.
## What was done?
Examples of responses:
`{'active_dkgs': 0, 'next_dkg': 22}`
## How Has This Been Tested?
`feature_llmq_rotation.py` was updated
## Breaking Changes
no
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
---------
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
## Issue being fixed or feature implemented
RPC `getassetunlockstatuses` is now accepting an extra optional
parameter `height`.
When a valid `height` is passed, then the RPC returns the status of
AssetUnlock indexes up to this specific block. (Requested by Platform
team)
## What was done?
Note that in order to avoid cases that can lead to deterministic result,
when `height` is passed, then the only `chainlocked` and `unknown`
outcomes are possible.
## How Has This Been Tested?
`feature_asset_locks.py` was updated.
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
---------
Co-authored-by: Konstantin Akimov <knstqq@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
## Issue being fixed or feature implemented
Trivial bug: `keypoolrefill` internally updates the status of the
progress bar each second.
```
m_storage.UpdateProgress(strMsg, static_cast<int>(dProgress));
```
However it can happen that one second is not enough time to make
significant progress and
`static_cast<int>(dProgress) = 0`.
Calling the function with `0` as progress opens a new progress bar and
this led to problems like #5730
## What was done?
trivially make sure to update the progress only if the parameter is >0
## How Has This Been Tested?
rpc does not spam anymore
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that
apply._
- [X] I have performed a self-review of my own code
- [X] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
519cae8fd6e44aef3470415d7c5e12acb0acd9f4 gui: Delay interfaces::Node initialization (Russell Yanofsky)
102abff9eb6c267af64f2a3560712147d1896e13 gui: Replace interface::Node references with pointers (Russell Yanofsky)
91aced7c7e6e75c1f5896b7e3843015177f32748 gui: Remove unused interfaces::Node references (Russell Yanofsky)
e1336316250ab5cb0ed654b1e593378a6e0769ce gui: Partially revert #10244 gArgs and Params changes (Russell Yanofsky)
Pull request description:
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).
---
This is a partial revert of https://github.com/bitcoin/bitcoin/pull/10244. It changes gui code to go back to using gArgs and Params() functions directly instead of using interfaces::Node to handle arguments.
These changes were originally pushed as part of https://github.com/bitcoin/bitcoin/pull/19461. Motivation is to support a new GUI process connecting to an already running node process. Details are explained in commit messages, but in addition to spawning a new bitcoin-node process, we want bitcoin-gui to connect to an existing bitcoin-node process. So for that reason it should be able to parse its own parameters, rather than rely on the node.
ACKs for top commit:
MarcoFalke:
re-ACK 519cae8fd6, only change is rebase and addressed nits of my previous review last week 🌄
Tree-SHA512: 9c339dd82ba78bcc7b887b84d872f35ccc7dfa3d271691e6eafe8a2048cbbe3bdde1e810ce33d0714d75d048c9de3470e9e9b6f8306a6047d1cb3548f6858dc8
241434200ec2067673d8522fee4f1228abfd8247 refactor: qt: Use vQueueNotifications.clear() (João Barbosa)
989e579d07bb5031639060b717f7a0be15d10e29 qt: Make transaction notification queue wallet specific (João Barbosa)
7b3b2303f44031c3545651858f697a495c3ea37a move-only: Define TransactionNotification before TransactionTablePriv (João Barbosa)
Pull request description:
Currently `vQueueNotifications` holds transactions of any wallet, but the queue is dispatched on a given wallet and it assumes notifications are of that wallet.
This means that some transactions can be missed if multiple wallets are loaded.
Fix this by having a queue for each wallet.
ACKs for top commit:
jonasschnelli:
utACK 241434200ec2067673d8522fee4f1228abfd8247
hebasto:
ACK 241434200ec2067673d8522fee4f1228abfd8247, I have reviewed the code and it looks OK, I agree it can be merged.
ryanofsky:
Code review ACK 241434200ec2067673d8522fee4f1228abfd8247. Only change is dropping one commit
Tree-SHA512: 61beac5a16ed659e3a25ad145dbceafcef963aaf8f9838355298949ec2324e2bd760f59353cd251d30cf0334d8dc1642a1f3821d8a9eec092533b581f6ce86db
