5b914fe081 refactor: add const to llmq_ctx, isman, clhandler in rpc code (Konstantin Akimov)
Pull request description:
## What was done?
Add missing const for all usages of llmq_ctx, llmq_ctx->isman and llmq_ctx->clhandler in rpc code.
## Issue being fixed or feature implemented
Added `const` helps to read rpc code to see that none of mentioned objects are actually changed.
## How Has This Been Tested?
Run unit and functional tests
## 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)_
ACKs for top commit:
UdjinM6:
utACK 5b914fe081
PastaPastaPasta:
utACK 5b914fe081
Tree-SHA512: dfaa99887250638e90f0c87271cf3c70833d4199797050be6a8b84939f78d857419a3e9e3da73035aca10e27782f28f43f156c69380d1c8df892c97c468c76d7
a275bda266 refactor: removed duplicated code with errors messages for txindex, timestampindex, spentindex (Konstantin Akimov)
dd1b36636c feat: show human friendly error if missing spentindex, txindex or addressindex (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
Currently user receive error `Unable to get spent info` or even worse `No information available for address` which doesn't say anything about required extra indexes.
Also, every call of RPC `getrawtransaction` causes this error logs on high level if spent index is disabled, but actually it just means that no couple extra fields are hidden in output which is expected if no index.
node0 2024-12-03T18:54:33.349605Z [ http] [httpserver.cpp:248] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:40052
node0 2024-12-03T18:54:33.349634Z [httpworker.3] [rpc/request.cpp:180] [parse] [rpc] ThreadRPCServer method=getrawtransaction user=__cookie__
node0 2024-12-03T18:54:33.349729Z [httpworker.3] [util/system.h:57] [error] ERROR: Spent index not enabled
node0 2024-12-03T18:54:33.349735Z [httpworker.3] [util/system.h:57] [error] ERROR: Spent index not enabled
node0 2024-12-03T18:54:33.349738Z [httpworker.3] [util/system.h:57] [error] ERROR: Spent index not enabled
node0 2024-12-03T18:54:33.349808Z [httpworker.3] [httprpc.cpp:93] [~RpcHttpRequest] [bench] HTTP RPC request handled: user=__cookie__ command=getrawtransaction external=false status=200 elapsed_time_ms=0
node0 2024-12-03T18:54:33.349998Z [ http] [httpserver.cpp:248] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:40052
node0 2024-12-03T18:54:33.350027Z [httpworker.0] [rpc/request.cpp:180] [parse] [rpc] ThreadRPCServer method=getrawtransaction user=__cookie__
node0 2024-12-03T18:54:33.350128Z [httpworker.0] [util/system.h:57] [error] ERROR: Spent index not enabled
node0 2024-12-03T18:54:33.350133Z [httpworker.0] [util/system.h:57] [error] ERROR: Spent index not enabled
node0 2024-12-03T18:54:33.350137Z [httpworker.0] [util/system.h:57] [error] ERROR: Spent index not enabled
## What was done?
Improved all usages of extra indexes `spentindex`, `txindex` and `addressindex` in RPC implementation.
Affected RPCc:
- getaddressmempool
- getaddressutxos
- getaddressdeltas
- getaddressbalance
- getaddresstxids
- getspentinfo
- getblockhashes
## How Has This Been Tested?
Run unit&functional tests.
```
$ dash-cli getaddressutxos '["yW4kiSd2pytXC2erbjm6crt1PGBvbwS4YX"]'
Address index is disabled. You should run Dash Core with -addressindex (requires reindex) (code -32600)
```
Check logs after calling getrawtransaction:
```
node0 2024-12-03T19:10:00.378770Z [httpworker.3] [rpc/request.cpp:180] [parse] [rpc] ThreadRPCServer method=getrawtransaction user=__cookie__
node0 2024-12-03T19:10:00.378861Z [httpworker.3] [httprpc.cpp:93] [~RpcHttpRequest] [bench] HTTP RPC request handled: user=__cookie__ command=getrawtransaction external=false status=500 elapsed_time_ms=0
node0 2024-12-03T19:10:00.379017Z [ http] [httpserver.cpp:248] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:44984
node0 2024-12-03T19:10:00.379036Z [httpworker.0] [rpc/request.cpp:180] [parse] [rpc] ThreadRPCServer method=getrawtransaction user=__cookie__
node0 2024-12-03T19:10:00.379090Z [httpworker.0] [httprpc.cpp:93] [~RpcHttpRequest] [bench] HTTP RPC request handled: user=__cookie__ command=getrawtransaction external=false status=500 elapsed_time_ms=0
node0 2024-12-03T19:10:00.379240Z [ http] [httpserver.cpp:248] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:44984
```
## Breaking Changes
Error type and message changed. It's no more `RPC_INVALID_ADDRESS_OR_KEY` but `RPC_INVALID_REQUEST`.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] 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)_
ACKs for top commit:
kwvg:
utACK a275bda266
UdjinM6:
utACK a275bda266
Tree-SHA512: d53373aba794035173375811e333e940efb1081bed46e18846b2b54d60036ee52487c65f9b84ac687b2be2a30f3a68fb75afbb2c89e52b0774740892863a04df
e905ae0f4b merge bitcoin#25499: Use steady clock for all millis bench logging (Kittywhiskers Van Gogh)
492654db49 merge bitcoin#24662: Use system time instead of adjusted network time (Kittywhiskers Van Gogh)
bc3ec30144 merge bitcoin#24697: refactor address relay time (Kittywhiskers Van Gogh)
730cdf241a test: remove leftovers missed in bitcoin#25514 (dash#6097) (Kittywhiskers Van Gogh)
c7cb26ba05 merge bitcoin#25456: Use steady_clock for getrpcinfo durations (Kittywhiskers Van Gogh)
ea3c727e02 merge bitcoin#25245: Remove no-op TIME_INIT on deser (Kittywhiskers Van Gogh)
2d33cfba41 merge bitcoin#25101: Add mockable clock type (Kittywhiskers Van Gogh)
ccde10b914 merge bitcoin#25157: Fix -rpcwait with -netinfo returning negative time durations (Kittywhiskers Van Gogh)
484447cc86 merge bitcoin#25100: Switch scheduler to steady_clock (Kittywhiskers Van Gogh)
cc7d2b8d0a merge bitcoin#25102: Remove unused GetTimeSeconds (Kittywhiskers Van Gogh)
8f8e73242d net: use `GetTime<T>()` in leftover `GetTimeSeconds()` usage (Kittywhiskers Van Gogh)
b114718240 merge bitcoin#18642: Use std::chrono for the time to rotate destination of addr messages + tests (Kittywhiskers Van Gogh)
Pull request description:
## Breaking Changes
None expected.
## Checklist
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [x] I have made corresponding changes to the documentation **(note: N/A)**
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
PastaPastaPasta:
utACK e905ae0f4b
UdjinM6:
utACK e905ae0f4b
Tree-SHA512: 022b8fac41315726e622b887ef5f5f5d011c947048d144f6f54c7c596d9e90286b77ec6f91dfc9bdb60ecc21dfa791afe4aba3d97f962eb1e86cd750831275bd
c005011e84 perf: NodesSnapshot, do not hold m_nodes_mutex while shuffling (pasta)
Pull request description:
## Issue being fixed or feature implemented
Upstream, as expected, only holds m_nodes_mutex while needed. We hold it a bit too long 36f5effa17/src/net.h (L1628-L1640)
Not sure how this got introduced. I also don't expect this to be a major contention savior, as there is only one instance in ThreadMessageHandler where we actually do shuffle, but still, might as well fix.
## What was done?
## How Has This Been Tested?
builds
## 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)_
ACKs for top commit:
kwvg:
utACK c005011e84
UdjinM6:
utACK c005011e84
Tree-SHA512: 76a848ace9a746c851e0fc1f66def92d67da92e9f295b7aade5a23f7d76b3eb3c28b7a6ac9d04df6dc252c1f1d9fae364821e9416a1f003a2905a30fc51eb41f
43778b07f5 refactor: pass CWallet reference to CTransactionBuilder (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
Follow-up for #6441
## What was done?
Pass reference to CWallet instead const reference to smart pointer with CWallet to CTransactionBuilder.
## How Has This Been Tested?
Run unit/functional tests
## 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
ACKs for top commit:
UdjinM6:
utACK 43778b07f5
PastaPastaPasta:
utACK 43778b07f5
Tree-SHA512: 81451c714ae3fcf695924da1cd578e832ad48fa678dddd67df12a9f4ffcfdfc4522e34977d3e86efc01eb70cfe5359c752c23db0502f1cd9bae2be59587a1c86
906c2d79ba refactor: add annotation gsl::not_null for ConstructCreditPool (Konstantin Akimov)
f1905ca950 fix: intermittent missing of PoSe ban in feature_llmq_simplepose.py (Konstantin Akimov)
cf84dffc9f fix: bump time for all nodes during mine_quorum in feature_llmq_rotation.py test (Konstantin Akimov)
efd4701d30 fix: intermittent error for feature_llmq_simplepose due to rotating quorums (Konstantin Akimov)
2bafadfc34 feat: put DIP0024 activation to block 1 on RegTest (Konstantin Akimov)
632c4c4bcd feat: re-bury DIP0024 with new height when quorums actually appeared (Konstantin Akimov)
343c74b779 fix: intermittent error in feature_index_prune due to DKG influence (Konstantin Akimov)
de821b9b15 refactor: remove command line argument -bip147height, -dip8params (Konstantin Akimov)
d8ce0a74fa docs: updated comment for DIP0003 activation on RegTest (Konstantin Akimov)
4dafec870c fix: add check that DIP0003 activated before retrieving CbTx for CreditPool (Konstantin Akimov)
26e9813672 fix: assertion in Credit Pool validation during connecting blocks (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
This PR is 7th in the achieving ultimate goal to activate old forks from block 1.
It helps to run unit and functional tests faster; it helps for platform's dev-environment to start faster.
## What was done?
Prior work: #6187, #6189, #6214, #6225, #6269, #6275
This PR:
- simplify DIP0024 activation and activate it from block 1 at RegTest
- removes command lines arguments: -bip147height, -dip8params
- fixed intermittent errors in feature_index_prune.py and feature_llmq_simplepose.py
- fix assertion crash on Regtest if using strange combination of -testactivationheight and -dip3params
## How Has This Been Tested?
Run unit, functional tests.
## Breaking Changes
[regtest only] -dip8params, -bip147height are removed.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] 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
ACKs for top commit:
UdjinM6:
utACK 906c2d79ba
PastaPastaPasta:
utACK 906c2d79ba
Tree-SHA512: fab8a9bc03bb7f220c19dd952a03f8fec0b6ef1362d7308eb77c90e0ba814a241bb2bf36beccf78bb285ede1b6d85ec52fa19b3729ac9b643b420d13fbb63b47
1d36a4026c 80%+: it, pl, th, zh_CN (UdjinM6)
adfdb5998c 90%+: fr (UdjinM6)
bfff3c9c76 100%: ru (UdjinM6)
ed5f02db9f en (UdjinM6)
d373f85f6b dashstrings.cpp (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
## What was done?
## How Has This Been Tested?
## Breaking Changes
## Checklist:
- [ ] 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
- [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
knst:
utACK 1d36a4026c
Tree-SHA512: 5d860d2d78e354eeae2157e5d40e8ff659963a258b9eb216d15ec72c1c589e254c2f246173be9e3d1892f9c18acae6cfecc1f18956460be5ff13c5c84d6d0ba6
3404fa0247 docs: add release notes for v22.0.0 (pasta)
Pull request description:
## Issue being fixed or feature implemented
Release notes for v22
## What was done?
## How Has This Been Tested?
## Breaking Changes
## 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
- [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
Top commit has no ACKs.
Tree-SHA512: b1fca9297814ab3497cc87512eb2a66daff22a17802c6cd87ef9af75c48fbecda8963c332191c3e3e7abdf495817da3e505b12baa646e97d388cb1aee74a67a7
2dabd78956 chore: update man pages for v22 (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
## What was done?
## How Has This Been Tested?
## Breaking Changes
## Checklist:
- [ ] 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
- [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
PastaPastaPasta:
utACK 2dabd78956
knst:
utACK 2dabd78956
Tree-SHA512: fc03af689e9c5be54f150d6727772038c70e1fafad0f450b431203aa0e662873e17095e702c8d970eafdd92e386bb471d7894ba51bb72b166fadefabdfaad116
2712968384 chore: bumped chain assumed sizes based on latest usage (pasta)
Pull request description:
## Issue being fixed or feature implemented
Release process item to bump assumed size
## What was done?
bump sizes as manually calculated
## How Has This Been Tested?
n/a
## 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)_
ACKs for top commit:
knst:
ACK 2712968384
kwvg:
utACK 2712968384
Tree-SHA512: d0131d692f4ccc858dcc4fff3c494b44bf9f702e2e7a9172d166c53dc693d6eb90e9c86ff412b31defa4cff13e21099808d9fb2ea2174032e6fe79339b1a57d4
bbcc0169f1 test: small improvements in feature_governance.py (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
Make debugging test failures a bit easier by adding some more logs and ensuring vote propagation before moving any further.
## What was done?
## How Has This Been Tested?
## Breaking Changes
## Checklist:
- [ ] 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
- [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
PastaPastaPasta:
utACK bbcc0169f1
knst:
utACK bbcc0169f1
Tree-SHA512: d4f7d7b560b1de849c30db598290af26d807a39b39a7532d2fe3d76c74c0d49ee294e4fac533a19a4a02dbf4f46d56f4b5e0116855a392fbbc992068cb17d38e
09058e0d71 feat: mnauth always use basic scheme (pasta)
Pull request description:
## Issue being fixed or feature implemented
Currently, mnauth has to check status of v19 hard fork. While this isn't soo terrible, it's not needed anymore. On mainnet or testnet, any mn you possible connect to will have it's TIP past v19 HF, meaning in practice it will only ever send you basic scheme anyhow. Let's just harden it. I initially guarded this behind a new protocol version, but I do not think that is needed.
## What was done?
## How Has This Been Tested?
## Breaking Changes
This is potentially a breaking change for devnets, which are moving past the v19 hard fork, but on develop v19 activates at block 2 for devnets sooooo, this shouldn't be noticed.
## 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)_
ACKs for top commit:
UdjinM6:
utACK 09058e0d71
knst:
utACK 09058e0d71
Tree-SHA512: 45483904135c90a507f2a1f61c4b06915eb7c02a29eeda254938624a2593aec86540fa360aea498428285b3c0e954c676510dcb128f4ec7062302293c134d517
e08068687c fix: don't shrink window when setting minimum width (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* Closes https://github.com/dashpay/dash/issues/5886
* We've been setting restrictions on the width of the window since [dash#3734](https://github.com/dashpay/dash/pull/3734) (see d351fca6a6 and c0447b0bc6), meaning, the behavior as described in [dash#5886](https://github.com/dashpay/dash/issues/5886) has been present since v0.16 (v0.15 and earlier used the old UI design).
* We do not set any restrictions on height and in that respect, Dash Qt will behave similar to Bitcoin Qt in that it can be resized to be arbitrarily small.
## How Has This Been Tested?
Crossed compiled for `x86_64-w64-mingw32` and tested resulting `dash-qt.exe` on Windows 11 22H2 (Build 22621.431). Followed reproduction steps as mentioned in [dash#5886](https://github.com/dashpay/dash/issues/5886), found width to persist even after closing Options modal using buttons and the close button on the top right.
## Breaking Changes
None expected.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)**
- [x] I have made corresponding changes to the documentation **(note: N/A)**
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
PastaPastaPasta:
generally utACK e08068687c
UdjinM6:
utACK e08068687c
Tree-SHA512: ed3874c830c0ddf1a9e1166b75e3088969b7dfae8bca25af47045727e14a270ca2bb740e2c30b657b26a4b8e542dff8d898e5d5d2c9482d67da15578e0054632
a4378fc2ff fix(qt): emit dataChanged for the whole model in TransactionTableModel (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
Somehow a 500k+ txes wallet is MUCH more responsive with this patch. I'm not sure if it's macos only or not though, pls test.
## What was done?
## How Has This Been Tested?
Run a wallet with 100k+ txes in it and send a tx or try changing units.
## Breaking Changes
## Checklist:
- [ ] 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
- [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
knst:
ACK a4378fc2ff
PastaPastaPasta:
utACK a4378fc2ff
Tree-SHA512: eb0116f99f145ee131bd4a69895c9384dc26fc113ee034a2af74a92b0770b33631455a79e5826dc004f922ac182481eb26c54255274e186a47da3796da9550a6
7d26061170 refactor: move `CConnman`, `PeerManager` out of `CCoinJoinClientQueueManager` ctor (Kittywhiskers Van Gogh)
953ba96ac9 refactor: move `CConnman` out of `CoinJoinWalletManager` ctor (Kittywhiskers Van Gogh)
ac930a84d8 refactor: remove unused `CConnman` from `CDeterministicMNManager` ctor (Kittywhiskers Van Gogh)
a14e604064 refactor: remove `CConnman`, `PeerManager` from `LLMQContext` ctor (Kittywhiskers Van Gogh)
d9e5cc7c9a refactor: move `PeerManager` out of `CInstantSendManager` ctor (Kittywhiskers Van Gogh)
82d1aed1d6 refactor: move `CConnman`, `PeerManager` out of `CSigSharesManager` ctor (Kittywhiskers Van Gogh)
7498a38076 refactor: move `PeerManager` out of `CSigningManager` ctor (Kittywhiskers Van Gogh)
7ebc61e375 refactor: move `CConnman` out of `CQuorumManager` ctor (Kittywhiskers Van Gogh)
c07b522baa refactor: move `CConnman` out of `CDKGSession{,Handler,Manager}` ctor (Kittywhiskers Van Gogh)
01876c7e56 refactor: move `PeerManager` out of `CDKGSession{,Handler,Manager}` ctor (Kittywhiskers Van Gogh)
cc0e771c29 refactor: start BLS thread in `LLMQContext` ctor, move `Start` downwards (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* Depends on https://github.com/dashpay/dash/pull/6425
* Dependency for https://github.com/dashpay/dash/pull/6304
* In order to reduce the logic used in chainstate initialization (that will be split out of `init.cpp` in [bitcoin#23280](https://github.com/bitcoin/bitcoin/pull/23280)), the spinning up of threads (as done in `LLMQContext::Start()`) needs to be moved down.
* They were moved up in [dash#5752](https://github.com/dashpay/dash/pull/5752) as `CBLSWorker` is a part of `LLMQContext` and `CBLSWorker` is needed during chainstate verification. As suggested in dash#5752, an alternate fix to the one already merged in was to move `CBLSWorker` `Start()`/`Stop()` to the constructor, which is done here.
* Another alternate fix is that we move it out of `LLMQContext` entirely and let it remain in `NodeContext` though this approach has not been taken.
* The reason we cannot retain the status quo is because `bitcoin-chainstate` (the binary introduced in [bitcoin#24304](https://github.com/bitcoin/bitcoin/pull/24304) that's built on the code split off in [bitcoin#23280](https://github.com/bitcoin/bitcoin/pull/23280)) aims to be devoid of P2P logic and this is reflected in the source files used to build it ([source](https://github.com/bitcoin/bitcoin/pull/24304/files#diff-4cb884d03ebb901069e4ee5de5d02538c40dd9b39919c615d8eaa9d364bbbd77R794)). (Also, there's no `NodeContext`, [source](https://github.com/bitcoin/bitcoin/pull/24304/files#diff-4cb884d03ebb901069e4ee5de5d02538c40dd9b39919c615d8eaa9d364bbbd77R795-R798))
* This means need to separate P2P and validation components from Dash-specific logic in order for the split to work as expected. This PR is a step in that direction by moving P2P elements (`CConnman` and `PeerManager`) out of constructors.
* As it stands, there are two sources for Dash-specific components to have access to P2P components, initialization (e.g. through `LLMQContext::Start()` or `PeerManagerImpl::ProcessMessage()`).
## Breaking Changes
None expected. While changes are present in initialization order, consensus behaviour should remain unchanged.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
- [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)**
- [x] I have made corresponding changes to the documentation **(note: N/A)**
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
knst:
utACK 7d26061170
UdjinM6:
utACK 7d26061170
Tree-SHA512: 4f12cbda935cad3a186acb31fed513cea489b0d3a55aa80be8e1336d10cbe1579d6d3db862a78a167134650c9e97816732acaf0c85ab759f6555b1b6be99ec02
40070c0337 stats: don't report `network.*hashesPerSecond` if it's zero (Kittywhiskers Van Gogh)
b39c6b9909 fix: avoid potential divide-by-zero in H/s stats calculation (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
It was reported that on occasion, `network.*hashesPerSecond` would report NaN gauge values, which would be dismissed as malformed reporting by Grafana (see below). Those gauges use a simplified version ([source](1ecfb891bc/src/init.cpp (L851-L864))) of `GetNetworkHashPS` ([source](1ecfb891bc/src/rpc/mining.cpp (L61))), crucially, without a check meant to avoid divide-by-zeros ([source](1ecfb891bc/src/rpc/mining.cpp (L89-L91))).
<details>
<summary>Error log (courtesy of PastaPastaPasta):</summary>
```
[...]
graphite-1 | 7 Dec 21:18:05 - DEBUG: Bad line: -nan,g in msg "network.terahashesPerSecond:-nan|g"
graphite-1 | 7 Dec 21:18:05 - DEBUG: Bad line: -nan,g in msg "network.petahashesPerSecond:-nan|g"
graphite-1 | 7 Dec 21:18:05 - DEBUG: Bad line: -nan,g in msg "network.exahashesPerSecond:-nan|g"
graphite-1 | 7 Dec 21:18:10 - DEBUG: Bad line: -nan,g in msg "network.hashesPerSecond:-nan|g"
graphite-1 | 7 Dec 21:18:10 - DEBUG: Bad line: -nan,g in msg "network.terahashesPerSecond:-nan|g"
graphite-1 | 7 Dec 21:18:10 - DEBUG: Bad line: -nan,g in msg "network.petahashesPerSecond:-nan|g"
graphite-1 | 7 Dec 21:18:10 - DEBUG: Bad line: -nan,g in msg "network.exahashesPerSecond:-nan|g"
graphite-1 | 7 Dec 21:18:15 - DEBUG: Bad line: -nan,g in msg "network.hashesPerSecond:-nan|g"
[...]
```
</details>
This has been resolved by adding that check, alongside encapsulating the logic in a lambda and not reporting the gauge values if the estimated hashes per second reported is zero, due to the unlikelihood of it being correct.
## Breaking Changes
None expected.
## Checklist
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
- [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)**
- [x] I have made corresponding changes to the documentation **(note: N/A)**
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
UdjinM6:
utACK 40070c0337
knst:
utACK 40070c0337
Tree-SHA512: 64bcca0f51a8bebb090613d3495ddba481ea6464f9f4a6791d830593fd1401c890eba65869f8719c6c6033a3346af11f16855a95fec5f42722b26a12b9f8b3c9
fixup: drop CChain from mnauth ProcessMessage
feat: let RPC mnauth to generate only BASIC bls messages and fixes for rpc_mnauth.py and p2p_quorum_data.py
refactor: drop unused ConstCBLSPublicKeyVersionWrapper
It's more likely that the stat is broken than the network running with
such low difficulty, the hash rate is reported as zero.
`*hashesPerSecond` are a gauge and we should let the last known good
value linger instead of potentially overwriting it with an improbable
value.
87b3c7f5d1 docs: update supported versions in SECURITY.md (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
Updates the supported versions list in `SECURITY.md`
## Checklist:
- [x] I have performed a self-review of my own code **(note: N/A)**
- [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
- [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)**
- [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)_
ACKs for top commit:
knst:
utACK 87b3c7f5d1
UdjinM6:
utACK 87b3c7f5d1
Tree-SHA512: 2a196b0e07e40a557f87a5137ede7d3984f14f48dedbec742e540c9ff1a9a762eccb317b35d102154745756cb83145ce5577495c2a2e581691edcfb0fa18553c
1ecfb891bc chore: bump MIN_MASTERNODE_PROTO_VERSION to latest proto (pasta)
Pull request description:
## Issue being fixed or feature implemented
Bump minimum master node protocol version to latest protocol.
## What was done?
## How Has This Been Tested?
## Breaking Changes
Masternodes that don't upgrade will end up getting a PoSe ban
## 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)_
ACKs for top commit:
UdjinM6:
utACK 1ecfb891bc
kwvg:
utACK 1ecfb891bc
Tree-SHA512: 18e0620370924c4f9b0c72b9f384a7b627b9d395b4a4a942d1eaabd8f4294869fb5831c8d237b825053a0ca5aa143cfbb4726312877670088333faa04ab10ef2
e50a9be1540c769a99fcdc1f7a109a6bf1c7516b Remove outdated comment on CFeeRate (Murch)
Pull request description:
This comment described how the constructor of CFeeRate was previously indirectly used to parse fee rate arguments from RPCs. The command line input was actually in sat/vB but due to the use of AmountFromValue() it got converted to BTC/vB which then got rectified in the constructor by creating a CFeeRate from that given value and COIN as the transaction size. Since this usage pattern was removed from the codebase some months ago, the comment is now obsolete.
ACKs for top commit:
michaelfolkson:
ACK e50a9be1540c769a99fcdc1f7a109a6bf1c7516b
jonatack:
ACK e50a9be1540c769a99fcdc1f7a109a6bf1c7516b
Tree-SHA512: f17bf0baeeca85a5c7883edadd407da845f6e3af1c949e93116bd67c02e601682a5f7f1ab2497172472e3acf1c4e3c234b01161a77e7d7f028e3551da34777f0
ce690847b69eb80b0232f818152dbb1db7c4c61a cli: describe quality/recency filtering in -addrinfo (Jon Atack)
7c975614c0fc6ff2084a1708a4c1f0368a4bc98f rpc: describe quality/recency filtering in getnodeaddresses (Jon Atack)
Pull request description:
Addresses #24278.
```
$ bitcoin-cli help getnodeaddresses
getnodeaddresses ( count "network" )
Return known addresses, after filtering for quality and recency.
These can potentially be used to find new peers in the network.
The total number of addresses known to the node may be higher.
```
```
$ bitcoin-cli -help | grep -A3 addrinfo
-addrinfo
Get the number of addresses known to the node, per network and total,
after filtering for quality and recency. The total number of
addresses known to the node may be higher.
```
ACKs for top commit:
mzumsande:
Thanks, Code Review ACK ce690847b69eb80b0232f818152dbb1db7c4c61a
prayank23:
reACK ce690847b6
Tree-SHA512: 82d23b15e64a99411eb8e70d7267a1b4f23182fabe072e824277569d9677e392b466be63f00e3d157d7db94bbe032d53f12ad4ab30b55b7b8a629c37d80d1d8c
62cc138ecb9cc7afcbe6fdb42b060a8f149826de Rename wallet-tool to bitcoin-wallet in code comment (Kristaps Kaupe)
0db3ad3ba41a76dff80bcb5f292e587da400ebf1 Mention -signet in bitcoin-wallet help output (Kristaps Kaupe)
Pull request description:
* Mention `-signet` in sentence where there is already `-testnet/-signet` in help output.
* Rename `wallet-tool` to `bitcoin-wallet` in single remaining place in code comments (was already done in #17648 at other places).
ACKs for top commit:
RandyMcMillan:
tACK 62cc138ecb
Tree-SHA512: c5df7811b8200f61943908dcf3b2b788fe991bf00bef28f069ab8784924556ffd5d86fc0ba2ad0b3c3f9be2ba73a34bc67059d7c057bba646c1801ffa3cb2070
fa8dad0e078c577d740a9667636733957586c035 rpc: Fix implicit-integer-sign-change in verifychain (MarcoFalke)
Pull request description:
It doesn't really make sense to treat `DEFAULT_CHECKLEVEL` as unsigned as long as `VerifyDB` accepts a signed integer.
Making it signed also avoids a cast round trip from signed->unsigned->signed in the RPC.
ACKs for top commit:
luke-jr:
utACK fa8dad0e078c577d740a9667636733957586c035
theStack:
Code-review ACK fa8dad0e078c577d740a9667636733957586c035
Tree-SHA512: 75499dbe4ace2962792e5fbec7defb10c25fdbbfde951d5e542a91daa880cc50395da0287173e2c84a28e18267c74af7b44b9f38ce364bcb0216c402f65b7641
