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
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
d296005194 fix(qt): allow refreshing wallet data without crashing (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
We re-use `refreshWallet` method to optimise loading for huge wallets https://github.com/dashpay/dash/pull/5453.
However gui121 backport (via 25f87b9434) added a condition that prevents this logic. Fix it by allowing explicit wallet refresh (force=true).
## What was done?
## How Has This Been Tested?
Open a wallet with 10k+ txes, do `rescanblockchain` via rpc console
## 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 d296005194c7bd8c65006ddf1b25052b2655c923;
Tree-SHA512: d308b3fe9c4fbbfbf2e2339aa14c825aa6f69c72d1f04dab7a14dc1c8721138beca47c7b3801db9782d6cecf2c54023a19a6d22e04b84615f9bddb0b8ec1696c
dfe86b4fb2 fix: follow-up fixes (UdjinM6)
a254a7b70c refactor: sping LogAcceptCategory and LogAcceptDebug (Konstantin Akimov)
82238e6be2 refactor: Set log level in `LogAcceptCategory()` to `Debug` by default (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
#6399 introduced severity-based logging via b046e091c9. We use `LogAcceptCategory()` in quite a few places and it's always `BCLog::Level::Debug` so log level is kind of redundant for us and just makes it harder to read the code.
## What was done?
~Set log level in `LogAcceptCategory()` to `BCLog::Level::Debug` by default~. Introduce `LogAcceptDebug()` which is `LogAcceptCategory()` with `Debug` level. Simplify corresponding Dash-specific code.
## How Has This Been Tested?
## Breaking Changes
n/a
## 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:
kwvg:
utACK dfe86b4fb2
Tree-SHA512: 167da533af088c4a3bfca22abd223a2314848ec79af10f117368f6d94a4a7faa3b009477a7af455ff890f8001622494c1e3a05112f9a7204321cc237278bf387
24dcce979b fix: store ready queues on the mixing masternode (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
We normally do not re-relay/store "ready" `dsq`-s because these are ment to be relayed between mixing clients and the mixing masternode only. I works ok when you simply send `dsq` messages because no extra steps are required. However since 70235 we send `dsq` _`inv`-s_ first and we send actual `dsq`-s only if they are requested via `getdata`. The problem here is that `ProcessGetData()` queries `vecCoinJoinQueue` via `GetQueueFromHash()` to get the data to send back but there is none because we never saved it.
## What was done?
## How Has This Been Tested?
To test this patch you need a MN but you can test 2 cases _without this patch_ to indirectly test the idea:
1. try mixing on develop: almost no mixing txes, maybe 10 or so in an hour if you are lucky to mix on an old MN that often
2. ignore old MNs (`MIN_PEER_PROTO_VERSION = 70235`): 0 mixing txes, no matter how long you wait
3. pretend being an old client to receive no-inv `dsq` only (`PROTOCOL_VERSION = 70233`): no issues, mixing on these nodes is as fast as usual, several txes per block (need a couple of nodes like that on the network so that a mixing session could be completed, I'm running one node for now so that anyone could join and test it)
I'm running a testnet MN with this fix and applied "ignore old mn" patch on my local machine. Local wallet got mixing tx when it finally hit the patched mn. Also confirmed this via MN`debug.log` (`Create`/`Relay`/`CommitFinalTransaction` in logs).
## 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 24dcce979b
Tree-SHA512: 69cee5401d26eec3f66166a754b8020e7f550dac4a0fdea8ec48ea1082f1286e647ac0a26a189c4d39e1a9da4e7ac36f71913684b13ea0fb4b3cfe831174970e
Alternate fix as proposed in dash#5752, needed because dependencies for
threaded logic will be pulled out of ctor in upcoming commits and that
needs `Start` to be pushed downwards so we can avoid having to pass
`unique_ptr` references.
2fa480a878 partial bitcoin#25288: Reliably don't start itself (lint-all.py runs all tests twice) (Kittywhiskers Van Gogh)
bda1e03b24 merge bitcoin#24982: Port `lint-all.sh` to `lint-all.py` (Kittywhiskers Van Gogh)
b054a0d894 merge bitcoin#24840: port `lint-shell.sh` to python (Kittywhiskers Van Gogh)
973ca7b46f merge bitcoin#23506: Make more shell scripts verifiable by the `shellcheck` tool (Kittywhiskers Van Gogh)
694c1a4582 merge bitcoin#24929: convert shell locale linter test to Python (Kittywhiskers Van Gogh)
2a7d32a5e6 merge bitcoin#24916: Convert lint-python-utf8-encoding.sh to Python (Kittywhiskers Van Gogh)
0321fa053a merge bitcoin#24915: Convert lint-circular-dependencies.sh to Python (Kittywhiskers Van Gogh)
e3dc4b1e27 merge bitcoin#24902: Convert lint-include-guards.sh to Python (Kittywhiskers Van Gogh)
fc48a134b5 merge bitcoin#23524: Fix typos in endif header comments (Kittywhiskers Van Gogh)
1f8c3b5e95 merge bitcoin#24794: Convert Python linter to Python (Kittywhiskers Van Gogh)
110b6ac3dc partial revert dash#4807: enable more multi-threading and caching in linters (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* Depends on https://github.com/dashpay/dash/pull/6428
* The introduction in `flake8-cached` for `lint-python.sh` in [dash#4807](https://github.com/dashpay/dash/pull/4807) was reverted as this logic wasn't going to be ported over to the Python replacement as the `flake8-cached` repo has been archived since April 2023 ([source](https://github.com/jnoortheen/flake8-cached)) and we don't use it in CI through GitLab ([build](https://gitlab.com/dashpay/dash/-/jobs/8456994796#L144)) or GitHub Actions ([build](https://github.com/dashpay/dash/actions/runs/11981121905/job/33406844883#step:7:75)).
* [bitcoin#25288](https://github.com/bitcoin/bitcoin/pull/25288) has been marked as partial as the change of the glob pattern from `{mod_path}/lint-*` to `{mod_path}/lint-*.py` as we still have `lint-cppcheck-dash.sh` around ([source](b88d9910a8/test/lint/lint-cppcheck-dash.sh)) (and the original `cppcheck` linter upstream was removed in [bitcoin#25091](https://github.com/bitcoin/bitcoin/pull/25091)).
A Python port of that linter would allow for completing [bitcoin#25288](https://github.com/bitcoin/bitcoin/pull/25288).
## 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
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
UdjinM6:
utACK 2fa480a878
PastaPastaPasta:
utACK 2fa480a878
Tree-SHA512: 48ddf11be11232df26051b39dfadac9f363d2f201b9f303cad6ddd54550e2f1881947061155da9d4eaf3f5a87cdd371368dc36b4d70eb81ff4c48a7a93af63ae
bb5d70c8d6 refactor: convert IsInvInFilter to accept a const Peer&, introduce const version of GetTxRelay (pasta)
30fc76c397 fix: simplify logic in AskPeersForTransactions and remove erroneous negative EXCLUSIVE_LOCKS_REQUIRED (pasta)
090ae9237e refactor: move CInstantSendManager::AskNodesForLockedTx into PeerManager (pasta)
Pull request description:
## Issue being fixed or feature implemented
Instantsend manager currently relies on CConnMan, which is not needed. The function AskNodesForLockedTx is all networking logic anyhow, and these no reason why this logic would be contained to instantsend processing. Move it into net_processing instead.
## What was done?
**This does change the logic!** We no longer prioritize asking MNs. This is probably fine? I don't specifically recall why we wanted to ask MNs besides potentially that they may be higher performing or better connected? We can potentially restore this logic once we bring masternode connection logic into Peer
Does also change logic, by short-circuiting once peersToAsk is full.
This commit has the added benefit of reducing contention on m_nodes_mutex due to no-longer calling connman.ForEachNode not once but twice
This may slightly increase contention on m_peer_mutex; but that should be an ok tradeoff for not only removing dependencies, but also reducing contention on a much more contested RecursiveMutex
## How Has This Been Tested?
Built, local tests
## 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:
ACK bb5d70c8d6
UdjinM6:
utACK bb5d70c8d6
Tree-SHA512: c06e4f80a1d19c109fb12b626028b3655c315f203ef569e1af976b8530f4b36172d42cf83c91080ecfddbe740e394f379adc3f98ae4f4e3811e1c8614ea4a7f4
f25a93647b build: stop tracking cmake dependency relic_conf.h.in (Kittywhiskers Van Gogh)
62fa66524c Squashed 'src/dashbls/' changes from 4e070243ae..7e747e8a07 (Kittywhiskers Van Gogh)
b1b3840ac5 revert: stop tracking cmake dependency relic_conf.h.in (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* Closes https://github.com/dashpay/dash/issues/6343
* Includes [bls-signatures#102](https://github.com/dashpay/bls-signatures/pull/102) and [bls-signatures#104](https://github.com/dashpay/bls-signatures/pull/104)
## Breaking Changes
None expected.
## 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 **(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 f25a93647b
UdjinM6:
utACK f25a93647b
Tree-SHA512: 394a02a50f57538e9d12f836fd1ea1598d8a20e2d0079fcb44bb317a42a64a638a1ef906222f2d3bab06d2c0b8cfac43c6e0055d87fbdb86abe680c53ecd6b7a
excluded:
- f26a496dfd0a7ce3833a10075027d7d5b0345e32 (change in glob pattern)
We still have shell scripts that end in `.sh`, so we can't
restrict the glob to only cover files that end in `.py`.
f8cfda60f1 refactor: simplify and optimize creation of rotation IS quorum in functional tests (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
Mining rotation quorum currently is not trivial. All over a codebase used this template to get a rotating quorum:
```
self.move_to_next_cycle()
self.log.info("Cycle H height:" + str(self.nodes[0].getblockcount()))
self.move_to_next_cycle()
self.log.info("Cycle H+C height:" + str(self.nodes[0].getblockcount()))
self.move_to_next_cycle()
self.log.info("Cycle H+2C height:" + str(self.nodes[0].getblockcount()))
self.mine_cycle_quorum(llmq_type_name='llmq_test_dip0024', llmq_type=103)
```
The performance of this code is not great also because generating 3 or 4 batches of blocks requires time to sync nodes after each batch.
## What was done?
This PR move instantly to 3-4 DKG sessions forward and starts from cycling quorum.
It's only one cycling quorum (instant-send), so, extra params are also removed.
To get it just call:
```
self.mine_cycle_quorum() # first time, which is usually used to get IS feature working in functional tests
self.mine_cycle_quorum(is_first=False) # if you don't need preparing 3 DKG sessions in advance
```
## How Has This Been Tested?
Run unit and functional tests
By my benchmark it gives roughly 10 seconds for each functional test that uses rotation quorum.
## Breaking Changes
N/A
## 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:
PastaPastaPasta:
utACK f8cfda60f1b47c358f8fbca3eaccb6f7a7fbf310; I wish that is_first could be automatically detected instead of manually set.
UdjinM6:
utACK f8cfda60f1
Tree-SHA512: 993f42b54c0c11ba146e164cc6db8b5c3f302dd78488aa2572b3ed199d25c266d8d1ff1a027a4e450eab2349b691c19fcd8f5859e2c423ccf2357db29ee3d536
852f55e23c merge bitcoin#24932: Convert lint-locale-dependence.sh to Python (Kittywhiskers Van Gogh)
f745b7f7ef merge bitcoin#24802: convert format strings linter test to python (Kittywhiskers Van Gogh)
a0b051b4ef partial revert dash#4807: enable more multi-threading and caching in linters (Kittywhiskers Van Gogh)
7864c21645 merge bitcoin#24895: Convert lint-includes.sh to Python (Kittywhiskers Van Gogh)
f63bafe253 merge bitcoin#24853: Convert lint-git-commit-check.sh to Python (Kittywhiskers Van Gogh)
d463d7d397 fix: clone full history on GitHub Actions build job, track `develop` (Kittywhiskers Van Gogh)
d23b6614e8 merge bitcoin#24844: Convert lint-whitespace.sh to Python (Kittywhiskers Van Gogh)
fe165168ba merge bitcoin#24849: Convert lint-logs.sh to Python (Kittywhiskers Van Gogh)
dbfd0b04e1 merge bitcoin#24766: convert spellchecking lint test to python (Kittywhiskers Van Gogh)
5c23addddd merge bitcoin#24778: Convert Python dead code linter test to Python (Kittywhiskers Van Gogh)
829dcfb936 partial bitcoin#23462: Enable SC2046 and SC2086 shellcheck rules (Kittywhiskers Van Gogh)
df6be0e8c0 partial bitcoin#23212: enable mypy import checking (Kittywhiskers Van Gogh)
f77eca9256 fix: make sure that parallelized `lint-all` runs incl. python scripts (Kittywhiskers Van Gogh)
e220175d0f merge bitcoin#22861: Update test README and lint script (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* Dependency for https://github.com/dashpay/dash/pull/6429
* Even though support for Python scripts was extended to `lint-all` in [bitcoin#24762](f226e8dc1f) ([dash#6023](https://github.com/dashpay/dash/pull/6023)), the changes needed were not extended to parallelized linting added in [dash#4637](https://github.com/dashpay/dash/pull/4637).
This meant the match expression didn't include non-shell scripts and additionally, `parallel` interpreted all scripts as Bash scripts. This has been resolved in this PR.
* [bitcoin#23212](https://github.com/bitcoin/bitcoin/pull/23212) is partial as `--ignore-missing-imports` has not been removed from `mypy` arguments as the version of `mypy` used in the PR (0.910) isn't syntax aware to `imports` like [here](f9d044d5ec/test/functional/test_framework/crypto/bip324_cipher.py (L16-L17)) and [here](f9d044d5ec/test/functional/test_framework/crypto/muhash.py (L9)).
<details>
<summary>Lint errors:</summary>
```
dash@96b217d539f7:/src/dash$ ./test/lint/lint-python.sh
Consider install flake8-cached for cached flake8 results.
[...]
test/functional/test_framework/crypto/muhash.py:9: error: Skipping analyzing ".chacha20": found module but no type hints or library stubs [import]
test/functional/test_framework/crypto/ellswift.py:15: error: Cannot find implementation or library stub for module named "test_framework.crypto.secp256k1" [import]
test/functional/test_framework/crypto/bip324_cipher.py:16: error: Skipping analyzing ".chacha20": found module but no type hints or library stubs [import]
test/functional/test_framework/crypto/bip324_cipher.py:17: error: Skipping analyzing ".poly1305": found module but no type hints or library stubs [import]
test/functional/test_framework/messages.py:29: error: Cannot find implementation or library stub for module named "test_framework.crypto.siphash" [import]
test/functional/test_framework/messages.py:32: error: Cannot find implementation or library stub for module named "dash_hash" [import]
test/functional/test_framework/script.py:20: error: Cannot find implementation or library stub for module named "test_framework.crypto.ripemd160" [import]
test/functional/test_framework/key.py:16: error: Cannot find implementation or library stub for module named "test_framework.crypto" [import]
test/functional/test_framework/v2_p2p.py:9: error: Cannot find implementation or library stub for module named "test_framework.crypto.bip324_cipher" [import]
test/functional/test_framework/v2_p2p.py:10: error: Cannot find implementation or library stub for module named "test_framework.crypto.chacha20" [import]
test/functional/test_framework/v2_p2p.py:11: error: Cannot find implementation or library stub for module named "test_framework.crypto.ellswift" [import]
test/functional/test_framework/v2_p2p.py:12: error: Cannot find implementation or library stub for module named "test_framework.crypto.hkdf" [import]
test/functional/p2p_v2_encrypted.py:22: error: Cannot find implementation or library stub for module named "test_framework.crypto.chacha20" [import]
test/functional/feature_utxo_set_hash.py:12: error: Cannot find implementation or library stub for module named "test_framework.crypto.muhash" [import]
Found 17 errors in 12 files (checked 275 source files)
```
</details>
And upgrading to the latest version of `mypy` used upstream (1.4.1, [source](f7144b24be/ci/lint/04_install.sh (L52))) brings syntax awareness but new errors.
<details>
<summary>Lint errors:</summary>
```
dash@96b217d539f7:/src/dash$ ./test/lint/lint-python.sh
Consider install flake8-cached for cached flake8 results.
[...]
test/functional/test_framework/coverage.py:23: error: Incompatible default for argument "coverage_logfile" (default has type "None", argument has type "str") [assignment]
test/functional/test_framework/coverage.py:23: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True
test/functional/test_framework/coverage.py:23: note: Use https://github.com/hauntsaninja/no_implicit_optional to automatically upgrade your codebase
test/functional/test_framework/util.py:322: error: Incompatible default for argument "timeout" (default has type "None", argument has type "int") [assignment]
test/functional/test_framework/util.py:322: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True
test/functional/test_framework/util.py:322: note: Use https://github.com/hauntsaninja/no_implicit_optional to automatically upgrade your codebase
test/functional/test_framework/util.py:322: error: Incompatible default for argument "coveragedir" (default has type "None", argument has type "str") [assignment]
test/functional/test_framework/messages.py:32: error: Cannot find implementation or library stub for module named "dash_hash" [import]
test/functional/test_framework/test_framework.py:122: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs [annotation-unchecked]
test/functional/test_framework/test_framework.py:123: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs [annotation-unchecked]
test/functional/test_framework/test_framework.py:124: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs [annotation-unchecked]
test/functional/test_framework/test_framework.py:125: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs [annotation-unchecked]
test/functional/p2p_message_capture.py:48: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs [annotation-unchecked]
Found 6 errors in 5 files (checked 275 source files)
```
</details>
* [bitcoin#23462](https://github.com/bitcoin/bitcoin/pull/23462) is partial as neither `SC2046` nor `SC2086` have been enabled. This backport was done for the sole purpose of backporting changes to shell scripts that would be replaced with their Python counterparts in later backports.
The necessary changes needed to enable `SC2046` and `SC2086` will be done in a later pull request.
* `actions/checkout` does not do a full clone nor tracks the default branch by default. It is already documented that `git merge-base` (used by `lint-git-commit-check.py` and `lint-whitespace.py`, [source](4aaa314a57/test/lint/lint-git-commit-check.py (L45))) doesn't play nice with it (see [actions/checkout#423](https://github.com/actions/checkout/discussions/423)).
No such issue exists on GitLab ([build](https://gitlab.com/dashpay/dash/-/jobs/8456260939#L113)), but it remains present on GitHub Actions ([build](https://github.com/dashpay/dash/actions/runs/11996049800/job/33440106874?pr=6428#step:7:103)). This PR does a full clone, switches to the `develop` branch, then switches back to the intended commit, as a workaround (see working build [here](https://github.com/dashpay/dash/pull/6428#issuecomment-2495980410))
* Changes to `run-lint-format-strings.py` made in [dash#4807](https://github.com/dashpay/dash/pull/4807) were reverted as they were interfering with `lint-format-strings.py`
<details>
<summary>Lint error:</summary>
```
dash@96b217d539f7:/src/dash$ ./test/lint/lint-format-strings.py
Traceback (most recent call last):
File "/src/dash/test/lint/run-lint-format-strings.py", line 308, in <module>
main()
File "/src/dash/test/lint/run-lint-format-strings.py", line 304, in main
sys.exit(max(exit_codes))
ValueError: max() arg is an empty sequence
Traceback (most recent call last):
File "/src/dash/test/lint/run-lint-format-strings.py", line 308, in <module>
main()
File "/src/dash/test/lint/run-lint-format-strings.py", line 304, in main
sys.exit(max(exit_codes))
ValueError: max() arg is an empty sequence
```
</details>
## 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
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
knst:
utACK 852f55e23c
UdjinM6:
utACK 852f55e23c
Tree-SHA512: e508311c00249e7e48f91ca23e6f62117c07497f9c4471d07659b233e43a9f4d09ee2bfe0dd76f0c9746209cdba5895cb002a708924daa19d7aa76869815a7d9
