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
37fbdee1d9 feat: do not mine extra quorum in feature_dip3_v19.py (Konstantin Akimov)
70ba007f1a feat: do not mine extra quorum in feature_llmq_evo.py (Konstantin Akimov)
129385d349 feat: remove regular masternodes from feature_asset_locks tests (Konstantin Akimov)
8ea45bbf69 fix: make helper get_merkle_root works with no masternodes (Konstantin Akimov)
7e0c2ca5a5 fix: make dynamic masternode register even without is-quorum (Konstantin Akimov)
Pull request description:
This PR has important fixes to implement "single-node quorum" feature.
## Issue being fixed or feature implemented
Without IS-quorum you can not dynamically add a new MN or Evo node.
For evo nodes adding it dynamically is the only way.
## What was done?
Fixed a function that dynamically adds a masternode without Instant Send quorums; use it in a functional test feature_asset_locks.py
As side effect it improves performance of functional tests which do not wait more IS lock significantly; for feature_asset_locks.py it gave ~20 seconds per run.
## How Has This Been Tested?
See updates in `feature_asset_locks.py`, `feature_llmq_evo.py`, `feature_dip3_v19.py`
## 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:
UdjinM6:
utACK 37fbdee1d9
PastaPastaPasta:
utACK 37fbdee1d9
Tree-SHA512: 2da036ccd9842946f20450606621108fa8cd8fae31927ac2d1fc5f02bf9c2f6955a67e23ef76141a02299510c8cbf206fcd4947b0ec41b776fe80f70947afc14
fad81548fa03861c244397201d6b6e6cbf883c38 test: Avoid testing negative block heights (MarcoFalke)
Pull request description:
A negative chain height is only used to denote an empty chain, not the height of any block.
So stop testing that and remove a suppression.
ACKs for top commit:
brunoerg:
crACK fad81548fa03861c244397201d6b6e6cbf883c38
Tree-SHA512: 0f9e91617dfb6ceda99831e6cf4b4bf0d951054957c159b1a05a178ab6090798fae7368edefe12800da24585bcdf7299ec3534f4d3bbf5ce6a6eca74dd3bb766
1621696a6f log: restore `LogPrintLevel` messages from prior backports (Kittywhiskers Van Gogh)
52a1263989 merge bitcoin#25614: Severity-based logging, step 2 (Kittywhiskers Van Gogh)
21470fdeb3 merge bitcoin#25292: Add LogPrintLevel to lint-format-strings, drop LogPrint-vs-LogPrintf section in dev notes (Kittywhiskers Van Gogh)
026409e4ff merge bitcoin#25217: update lint-logs.py to detect LogPrintLevel, mention WalletLogPrintf (Kittywhiskers Van Gogh)
b046e091c9 merge bitcoin#25202: Use severity-based logging for leveldb/libevent messages, reverse LogPrintLevel order (Kittywhiskers Van Gogh)
7697b73257 revert dash#2794: Disable logging of libevent debug messages (Kittywhiskers Van Gogh)
ff6304f5f3 merge bitcoin#24757: add `DEBUG_LOCKCONTENTION` to `--enable-debug` and CI (Kittywhiskers Van Gogh)
88592f30a3 merge bitcoin#24464: Add severity level to logs (Kittywhiskers Van Gogh)
d3e837ad22 merge bitcoin#24830: Allow -proxy="" setting values (Kittywhiskers Van Gogh)
0e01d5b5f3 partial bitcoin#22766: Clarify and disable unused ArgsManager flags (Kittywhiskers Van Gogh)
a9cfbd1048 fix: don't use non-existent `PrintLockContention` in `SharedEnter` (Kittywhiskers Van Gogh)
f331cbe8c8 merge bitcoin#24770: Put lock logging behind DEBUG_LOCKCONTENTION preprocessor directive (Kittywhiskers Van Gogh)
d9cc2ea178 merge bitcoin#23104: Avoid breaking single log lines over multiple lines in the log file (Kittywhiskers Van Gogh)
479ae82ecc merge bitcoin#23235: Reduce unnecessary default logging (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* This pull request's primary purpose is to restore `LogPrintLevel`s from backports in [dash#6333](https://github.com/dashpay/dash/pull/6333) that were changed to `LogPrint`s as they were backported before `LogPrintLevel` was backported.
* ~~`clang-format` suggestions for `LogPrintLevel` have to be ignored in order to prevent the linter from tripping due to a "missing newline" ([build](https://gitlab.com/dashpay/dash/-/jobs/8398818860#L54)).~~ Resolved by applying diff ([source](https://github.com/dashpay/dash/pull/6399#issuecomment-2488992710)).
* `SharedLock` was introduced in [dash#5961](https://github.com/dashpay/dash/pull/5961) and `PrintLockContention` was removed in [dash#6046](https://github.com/dashpay/dash/pull/6046) but the changes in the latter were not extended to the former. This has been corrected as part of this pull request.
## 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)_
Top commit has no ACKs.
Tree-SHA512: f2d0ef8ce5cb1091c714a2169e89deb33fa71ff174ce4e6147b3ad421f57a84183d2a9e76736c0b064b2cc70fb3f2e545c42b8562cf36fdce18c3fb61307c364
b658d7d5c5339739dc19bf961d84186469a818d5 test: update assert_fee_amount() in test_framework/util.py (Jon Atack)
Pull request description:
Follow-up to 42e1b5d979 (#12486).
- update call to `round()` with our utility function `satoshi_round()` to avoid intermittent test failures
- rename `fee_per_kB` to `feerate_BTC_kvB` for precision
- store division result in `feerate_BTC_vB`
Possibly resolves#19418.
ACKs for top commit:
meshcollider:
utACK b658d7d5c5339739dc19bf961d84186469a818d5
Tree-SHA512: f124ded98c913f98782dc047a85a05d3fdf5f0585041fa81129be562138f6261ec1bd9ee2af89729028277e75b591b0a7ad50244016c2b2fa935c6e400523183
3ba602672c refactor: use self.wait_until in all the dash specific "wait_until_x" logic in order to actually apply the timeout scaling settings (pasta)
Pull request description:
## Issue being fixed or feature implemented
Currently we use the raw helper, but that means that timeout scaling isn't applying.. I think this may be a cause of a lot of the functional test failures that we see in tsan / ubsan.
## What was done?
## How Has This Been Tested?
hasn't; wait for CI
## 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
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
knst:
utACK 3ba602672c
UdjinM6:
utACK 3ba602672c
Tree-SHA512: 935498f4b296b1abcac8be686cce396b61b654ef62da46de9a23a0f24ad31254f4938a581a6a4e2533576db0e0120861fd690bd9019e893b30990f21d1e48168
5078baea2b fix(test): wait for chainlock before mining a block we expect to include said chainlock (pasta)
f39c1e6f4c fix: guard m_can_tx_relay behind m_tx_relay_mutex; make it private; add additional annotations (pasta)
Pull request description:
## Issue being fixed or feature implemented
See each commit; fixes two bugs, both discovered while running feature_llmq_chainlocks.py with tsan / debug.
one a datarace in net_processing.cpp and the other in the test I was using to ensure this fix was correct, feature_llmq_chainlocks
## What was done?
### net_processing.cpp
You can see the datarace here: https://gist.github.com/PastaPastaPasta/c966a9f805758b34524085e3d52ea7f8
We simply guard it with an existing mutex that is always locked in close proximity.
### feature_llmq_chainlocks.py
Most of the time, while generating the cycle quorum, there is sufficient time to generate a chainlock; however, this is racey, and I've observed locally where the block gets generated before a chainlock is present and as such `test_coinbase_best_cl` fails. We should instead wait for the chainlock first, and then mine the block. This was we can ensure the mined block will include that chainlock.
This was observed locally maybe 1/10 times or so
## How Has This Been Tested?
ran feature_llmq_chainlocks.py ~40 times locally with tsan / debug
## Breaking Changes
None
## 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)_
ACKs for top commit:
kwvg:
utACK 5078baea2b
knst:
utACK 5078baea2b
UdjinM6:
utACK 5078baea2b
Tree-SHA512: b346fc60809df72d0161f625073dce7062bd2641d35e4f80160fac9afeec63707de552e2856940ac2604875908ae3b98a225d352de36bfbfc6ee3fbe1e1538ff
0072318421 refactor: drop unused parameter mninfos_valid in test framework mine_cycle_quorum (pasta)
Pull request description:
## Issue being fixed or feature implemented
This param is unused
## What was done?
Removed
## How Has This Been Tested?
see ci
## 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
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
knst:
utACK 0072318421
UdjinM6:
utACK 0072318421
Tree-SHA512: e66e2b23b95b98e843fdb1d58419ba0798e8ba4115ef6c52e2e4681b04ea3e43efe73cee0ac96758560914567f822d107169ca0d6212210e07cc81b317a3e2eb
Most of the time, while generating the cycle quorum, there is sufficient time to generate a chainlock; however, this is racey, and I've observed locally where the block gets generated before a chainlock is present and as such `test_coinbase_best_cl` fails. We should instead wait for the chainlock first, and then mine the block. This was we can ensure the mined block will include that chainlock.
This was observed locally maybe 1/10 times or so
fa097d074bc1afcc2a52976796bb618f7c6a68b3 addrman: Log too low compat value (MarcoFalke)
Pull request description:
Before this patch, when writing a negative `lowest_compatible` value, it would be read as a positive value. For example `-32` will be read as `224`. There is generally nothing wrong with that. Though, similarly there shouldn't be anything wrong with refusing to read a negative value. I find the code after this patch more logical than before. Also, this allows dropping a file-wide sanitizer suppression.
In practice none of this should ever happen. Bitcoin Core would never write a negative `lowest_compatible` in normal operation, unless the file storage is later corrupted by external influence.
ACKs for top commit:
mzumsande:
re-ACK fa097d074bc1afcc2a52976796bb618f7c6a68b3
Tree-SHA512: 9aae7b8fe666f52f667f149667025e0160cef1a793cc4d392e36608f65c2bee8096da429235118f40a3368f327aabe30f3732ae78c5874648ea6f423f2687b65
2d05df04fd refactor: use irange (pasta)
4cc3ee6286 test: update feature_asset_locks.py to reflect changes (pasta)
bfe1d2768d refactor: activate DEPLOYMENT_WITHDRAWALS sooner on regtest (pasta)
c8342e1b55 refactor: pull LimitBlocksToTrace into CChainParams (reduce regtest to 100) (pasta)
Pull request description:
## Issue being fixed or feature implemented
Minimize number of blocks needed to be mined in asset locks test
Based on CI test goes from about 270s -> 213s
## What was done?
Reduce number of blocks needed by reducing hard fork activation points
## How Has This Been Tested?
Ran test locally, built
## Breaking Changes
Breaking for regtests, nothing else.
## 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:
utACK 2d05df04fd
UdjinM6:
utACK 2d05df04fd
Tree-SHA512: b52ec2570912ccb578b14baa4ef5c6e69454db7d83e7476c4451bca62b0c57e4c961e4bfafc9e5b0b4eebd6226840ac00505d3787316d7bdaeb64cb66b08fb30
