0d9046586b merge bitcoin#21254: Avoid connecting to real network when running tests (Kittywhiskers Van Gogh)
2f672bdb3a merge bitcoin#20721: Move ping data to net_processing (Kittywhiskers Van Gogh)
0d46acbfef merge bitcoin#21165: Use mocktime in test_seed_peers (Kittywhiskers Van Gogh)
8f40769385 merge bitcoin#19884: No delay in adding fixed seeds if -dnsseed=0 and peers.dat is empty (Kittywhiskers Van Gogh)
446076d094 test: add missing `dnsseed=0` in configuration (Kittywhiskers Van Gogh)
081d8db4d5 mempool: remove stray boost::optional usage (Kittywhiskers Van Gogh)
bcd383c2d6 merge bitcoin#20724: Cleanup of -debug=net log messages (Kittywhiskers Van Gogh)
8c63868d9b merge bitcoin#19972: Add fuzzing harness for node eviction logic (Kittywhiskers Van Gogh)
18f2dc0865 partial bitcoin#19829: Move block inventory state to net_processing (Kittywhiskers Van Gogh)
ec77bd3ab4 net: move nLast{Block,TX}Time to match upstream location (Kittywhiskers Van Gogh)
f635e4a95f merge bitcoin#20624: Remove nStartingHeight check from block relay (Kittywhiskers Van Gogh)
9f1a3e5f18 merge bitcoin#20477: Add unit testing of node eviction logic (Kittywhiskers Van Gogh)
2d838a627c merge bitcoin#20146: Send post-verack handshake messages at most once (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* [bitcoin#19884](https://github.com/bitcoin/bitcoin/pull/19884) doesn't seem to play nice on its own and necessitated two more backports (namely [bitcoin#21165](https://github.com/bitcoin/bitcoin/pull/21165) and [bitcoin#21254](https://github.com/bitcoin/bitcoin/pull/21254)) before it did.
* Trying to find why that is has been time consuming but there doesn't seem to be a concrete answer. If running the daemon normally (`dashd --regtest --dnsseed=1`), the functionality behaves as expected but the test still fails.
* The closest explanation is that our OOO backports with relation to mocking time could explain why it isn't working as expected due to debug statements I added that always shown the time delta between each "enough time has passed" check was 0 seconds even when the log was advancing forward in time.
* The usage of `dnsseed=0` stems from [bitcoin#16551](https://github.com/bitcoin/bitcoin/pull/16551) (link to diff in commit comment), a backport that was skipped due to complexity. Though, some aspects of the PR have made it with [dash#3946](https://github.com/dashpay/dash/pull/3946).
* [bitcoin#19829](https://github.com/bitcoin/bitcoin/pull/19829) does away with `CConnman::ForEachNode` usage in `PeerManagerImpl::UpdatedBlockTip` ([source](https://github.com/bitcoin/bitcoin/pull/19829/files#diff-6875de769e90cec84d2e8a9c1b962cdbcda44d870d42e4215827e599e11e90e3R1318-R1328)). Dash cannot do the same and continues to use `ForEachNode` as we use the `CNode::CanRelay` check, which is not accessible through the `Peer` struct.
* It would be valuable to find values that are Dash-specific (like `m_masternode_connection`) and migrate them to `Peer` to avoid Dash-specific patches and closer alignment with upstream.
## Breaking Changes
Potential change in behaviour in the GUI and RPC.
In RPC, `getpeerinfo` will now display `pingwait` and `startingheight` if `fStateStats` is true (earlier behaviour was unconditional). `startingheight` has been placed below `banscore` (earlier behaviour placed it above). In the GUI (Qt), `peerHeight` and `peerPingWait` are subject to similar conditionality as mentioned earlier.
No changes in protocol or consensus. Changes are primarily related to refactoring, cleaning up, improving networking code and adding a new flag (`-fixedseeds`).
## 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 **(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:
re-utACK 0d9046586b
Tree-SHA512: 79eedf47387a8715fc9f20c6bc051d4eae832266454445043e1478dc36daafc1679e002623917af43cf923735217622e3985f664123a1de23fadfdfece7e9b6b
81738d2881253f28b69666ada2a01ebb353f503a test: Remove suppression no longer needed with headers-only Boost.Test (Hennadii Stepanov)
Pull request description:
It appears, that moving to [headers-only](https://github.com/bitcoin/bitcoin/pull/24301) Boost.Test makes the removed suppression unneeded even without [bumping](https://github.com/bitcoin/bitcoin/pull/24383) boost version.
ACKs for top commit:
MarcoFalke:
cr ACK 81738d2881253f28b69666ada2a01ebb353f503a
Tree-SHA512: e60443f79a2e38cc78fceeff5c2956d622e8a10730129f9c27c14aef59bc6fa0894b8011e6191530443bf3165f78da978bc08ad04248ddb65e2da373264afa6a
0bd882b7405414b5355e69a9fdcd7a533e504b6b refactor: remove RecursiveMutex cs_nBlockSequenceId (Sebastian Falbesoner)
Pull request description:
The RecursiveMutex `cs_nBlockSequenceId` is only used at one place in `CChainState::ReceivedBlockTransactions()` to atomically read-and-increment the nBlockSequenceId member:
83daf47898/src/validation.cpp (L2973-L2976)
~~For this simple use-case, we can make the member `std::atomic` instead to achieve the same result (see https://en.cppreference.com/w/cpp/atomic/atomic/operator_arith).~~
~~This is related to #19303. As suggested in the issue, I first planned to change the `RecursiveMutex` to `Mutex` (still possible if the change doesn't get Concept ACKs), but using a Mutex for this simple operation seems to be overkill. Note that at the time when this mutex was introduced (PR #3370, commit 75f51f2a63) `std::atomic` were not used in the codebase yet -- according to `git log -S std::atomic` they have first appeared in 2016 (commit 7e908c7b82), probably also because the compilers didn't support them properly earlier.~~
At this point, the cs_main lock is set, hence we can use a plain int for the member and mark it as guarded by cs_main.
ACKs for top commit:
Zero-1729:
ACK 0bd882b
promag:
Code review ACK 0bd882b7405414b5355e69a9fdcd7a533e504b6b.
hebasto:
ACK 0bd882b7405414b5355e69a9fdcd7a533e504b6b
Tree-SHA512: 435271ac8f877074099ddb31436665b500e555f7cab899e5c8414af299b154d1249996be500e8fdeff64e4639bcaf7386e12510b738ec6f20e415e7e35afaea9
8733a8e84c4b2e484f6ed6159fcf5f29a360d42e the result of CWallet::IsHDEnabled() was initialized with true. (Saibato)
Pull request description:
the result of CWallet::IsHDEnabled() was initialized with true.
But in case of no keys or a blank hd wallet the iterator would be skipped
and not set to false but true, since the loop would be not entered.
That had resulted in a wrong return and subsequent false HD and watch-only
icon display in GUi when reloading a wallet after closing.
ACKs for top commit:
achow101:
ACK 8733a8e84c4b2e484f6ed6159fcf5f29a360d42e
hebasto:
ACK 8733a8e84c4b2e484f6ed6159fcf5f29a360d42e
theStack:
utACK 8733a8e84c4b2e484f6ed6159fcf5f29a360d42e
meshcollider:
utACK 8733a8e84c4b2e484f6ed6159fcf5f29a360d42e
Tree-SHA512: 79b976594f7174d05c29fe3819037ead59aaef27498d95415ceba74d633a8e035f6b03b521000ac3370684a8cb09319d8be1a443ce2d29b3ff4089e399f6b719
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
faa94961d6e38392ba068381726ed4e033367b03 test: Add temporary logging to debug #20975 (MarcoFalke)
Pull request description:
to be reverted after a fix
ACKs for top commit:
laanwj:
Code review ACK faa94961d6
Tree-SHA512: 1f3103fcf4cad0af54e26c4d257bd824b128b5f2d2b81c302e861a829fd55d6a099fa476b79b30a71fe98975ae604b9e3ff31fd48a51d442389a9bd515e60ba0
a09e260c15 refacotor: simply RecursiveMutex -> Mutex (pasta)
Pull request description:
## What was done?
This PR is a simpler version of https://github.com/dashpay/dash/pull/5954; it is simply a recursiveMutex -> Mutex PR; as I think some of the scope minimization in the other PR introduced test failures (appears there are undocumented mutex dependancies)
## How Has This Been Tested?
CI TBD
## Breaking Changes
None
## 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:
knst:
utACK a09e260c15
Tree-SHA512: 4ef5aa94ecb1a55f30b642d5e0328e2d2a0adcf2ccc2ab6489a576a420c915339be023828785c3243f019bb73ac2cd77be7dd4397e14ec874cba68999a9e54dc
We could use std::optional<std::reference_wrapper<const CActiveMasternodeManager>>
but then we'd also have to contend with accessing the value with mn_activeman.
value().get().
We assert m_mn_activeman is present instead of fast-failing when it isn't
because of the fMasternodeMode fast-fail check. If we are in masternode
mode, m_mn_activeman should point to a valid target. If it doesn't,
something's gone wrong.
Using unique_ptr is a relic of activeMasternodeInfo, as it was accessible
independent of CActiveMasternodeManager. As it is now only accessible
if CActiveMasternodeManager is initialized, we can make it part of its
constructor and make them const.
Removes the assertion and sanity checks to see if key pair is valid.
External logic should not be able to mutate the CActiveMasternodeManager
state (i.e. CActiveMasternodeInfo). Access is brokered through getter
functions.
A later commit will be moving activeMasternodeInfo into activeMasternodeManager
and that is only conditionally initialized if the node is in masternode
mode, which will render access attempts outside of masternode mode invalid.
We need to adjust behaviour to account for that.
915e34112b5a4c2ef391d7e3706603bcd6f62a2a qt: fix issue when disabling the auto-enabled blank wallet checkbox (Jarol Rodriguez)
Pull request description:
As detailed by #151, On `master` a user can create the confusing scenario where you have a disabled `Encrypt Wallet` checkbox and a selected `Disable Private Keys` checkbox after unselecting the auto-enabled `Blank Wallet` checkbox.
This commit makes it so that when the `Blank Wallet` checkbox is auto-selected after the user selects `Disable Private keys`, unselecting it will also unselect the `Disable Private Keys` checkbox, which in turn re-enables the `Encrypt Wallet` checkbox.
Below are screenshots comparing the behavior of selecting `Disable Private Keys` then unselecting the `Blank Wallet` between `master` and this `PR`:
**Master:**
| Select `Disable Private Keys` | Unselect `Blank Wallet` |
| ----------------------------- | ------------------------ |
|  |  |
**PR:**
| Select `Disable Private Keys` | Unselect `Blank Wallet` |
| ----------------------------- | ------------------------ |
|  |  |
ACKs for top commit:
hebasto:
ACK 915e34112b5a4c2ef391d7e3706603bcd6f62a2a
Talkless:
ACK 915e34112b5a4c2ef391d7e3706603bcd6f62a2a
Tree-SHA512: ce6ecbc35b94a08cabf0b8a24dbdfc874d82cc8918cc8623dce8172c7fc9c75d63a13b036bae5f7ab2c090f8d020574a542285d1651600813faf5d91e2506a8d
173ef8980d921c9c0e46257747ab1165965e3ced build: Small libxcb.mk improvements (Hennadii Stepanov)
5129b36573cb07d8bcbac493506c46e26d7239eb build: Clean remnants of QTBUG-34748 fix (Hennadii Stepanov)
Pull request description:
Hope, this PR will make [transit](https://github.com/bitcoin/bitcoin/pull/21376) to Qt 5.12.10 neater.
A fix for [QTBUG-34748](https://bugreports.qt.io/browse/QTBUG-34748) was introduced in #5915 (v0.11.0, Qt 5.2.1).
[QTBUG-34748](https://bugreports.qt.io/browse/QTBUG-34748) was [fixed](b19b080894) in Qt 5.3.0.
The separated [`fix-xcb-include-order.patch`](bb44d9e754/depends/patches/qt/fix-xcb-include-order.patch), provided by #5915, was dropped in #12971 while bumping Qt to 5.9.4 (5.9.6). But `libxcb.mk` remained unchanged.
This PR reverts #5915 for `libxcb.mk` as well.
ACKs for top commit:
practicalswift:
cr ACK 173ef8980d921c9c0e46257747ab1165965e3ced: patch looks correct
fanquake:
ACK 173ef8980d921c9c0e46257747ab1165965e3ced
Tree-SHA512: 9815a7e532ff4aa08f9623ded8d5708eca1c9c73ac7a2684419a18c125da7627b44ac3191f2e7978946942c8d0580e73b1a93df624986fb2a13791a68ce1e025
bf57b15496 refactor: remove unneeded variable (pasta)
358ae3283d refactor: add some const (pasta)
4804aad2c9 refactor: a few more structured bindings (pasta)
ee6dc2d6ab refactor: remove some nested code blocks (pasta)
c2ac444abf refactor: create a immediately executed llamda; use some structed bindings inside of ti (pasta)
Pull request description:
## Issue being fixed or feature implemented
Resolve a lot of sonarlint and clang-tidy warnings. Please see individual commits
## What was done?
## How Has This Been Tested?
`make check`
## Breaking Changes
Should be 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)_
Top commit has no ACKs.
Tree-SHA512: 7481aaf5e5fae42fa896a9b770f41d9d3e1fa395569d2f9fffc952ec556737898283c5a4929063975852dd58fa857cd8374e78dd518983550870fd2683ad5f79
