## Issue being fixed or feature implemented
Help field showed wrong type
## What was done?
Changed height to number instead of string
## How Has This Been Tested?
Locally
## Breaking Changes
N/A
## 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
- [ ] 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)_
## Issue being fixed or feature implemented
fix failures like https://gitlab.com/dashpay/dash/-/jobs/6175160403
## What was done?
use `minimumAmount` option in `listunspent` rpc call to avoid picking
coins that are too small for asset lock txes
## How Has This Been Tested?
run `feature_asset_locks.py`
## 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
## Issue being fixed or feature implemented
We did not previously ship any onion seeds. This results in people
needing to use `addnode` in order to actually get connected
## What was done?
Modified seed creation process to handle a list of onion seeds.
## How Has This Been Tested?
Running with and without onlynet=onion and with dnsseed=0 and deleting
peers.dat
## 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)_
edd0313ae7c94420642081c9172e349080bb9335 test: Improve "invalid_command" subtest in system_tests for Windows (Hennadii Stepanov)
fb1b0590af138e317803893d2cab9dc887f33c5b test: Fix "non-zero exit code" subtest in system_tests for Windows (Hennadii Stepanov)
0aad33db6410ed36fa0f4b96245cacbae7897d2e test: Fix "false" subtest in system_tests for Windows (Hennadii Stepanov)
507c009c1ee68a4c3ad100f765bf854307d5bf39 test: Fix "echo" subtest in the system_tests for Windows (Hennadii Stepanov)
Pull request description:
An attempt to fixbitcoin/bitcoin#23775.
With this PR on Windows 10 Pro 21H1 (build 19043.1348):
```
C:\Users\hebasto\bitcoin>src\test_bitcoin.exe --run_test=system_tests/run_command
Running 1 test case...
*** No errors detected
C:\Users\hebasto\bitcoin>src\test_bitcoin.exe
Running 482 test cases...
*** No errors detected
```
ACKs for top commit:
sipsorcery:
tACK edd0313ae7c94420642081c9172e349080bb9335
Tru3Nrg:
tACK edd0313ae7
Tree-SHA512: 66a4f2372858011ff862b71c6530bedb8bc731b18595636fac9affc9189d9320f212c68b62498f2b57ee7a07f59e842dbec085b76a7419791d1a06c8e80e7744
faa5fa9a78d6d23b4e9adea07fdfb34ead170a2f fuzz: Use LIMITED_WHILE instead of limit_max_ops (MarcoFalke)
Pull request description:
This avoids the local stack variable `limit_max_ops` and makes it easier to grep for limited loops. Also, it is less code.
ACKs for top commit:
theStack:
Code-review ACK faa5fa9a78d6d23b4e9adea07fdfb34ead170a2f 🍷
Zero-1729:
crACK faa5fa9a78d6d23b4e9adea07fdfb34ead170a2f 🥤
Tree-SHA512: b10d89f4ce57bac0d6e9de9db7d4db85bae81bc02536d46a910be8c0e77333f2d9a547c7fe03df57f5ff9fd90b6994b09996d8e148573fb7ecd840d08b5c0c7d
fa35efa84bb614df9629076e45e9e974d9e940f6 ci: Run arm task on arm64 hardware (MarcoFalke)
Pull request description:
It will still run cross-compilation to armhf, but run the binaries on the hardware itself, not qemu.
There shouldn't be any significant difference, other than maybe a slight speedup.
ACKs for top commit:
hebasto:
re-ACK fa35efa84bb614df9629076e45e9e974d9e940f6
Tree-SHA512: d03bb2b55d0a6b5021243eedff6e143f4fad03a1a8913000c2c5c74665e4bb1c77cb30aa112dbddbc0bb7c645bec18f6b8b8ba91049587b464bbe5ec07dd46bd
fa33ed4b3fe422d6a6949cec04d2e14efc9ba3ca fuzz: Limit max ops in tx_pool fuzz targets (MarcoFalke)
Pull request description:
Without a size limit on the input data, the runtime is unbounded. Fix this by picking an upper bound on the maximum number of fuzz operations.
Reproducer from OSS-Fuzz (without bug report):
[clusterfuzz-testcase-tx_pool_standard-5963992253202432.log](https://github.com/bitcoin/bitcoin/files/6822465/clusterfuzz-testcase-tx_pool_standard-5963992253202432.log)
ACKs for top commit:
practicalswift:
cr ACK fa33ed4b3fe422d6a6949cec04d2e14efc9ba3ca
Tree-SHA512: 32098d573880afba12d510ac83519dc886a6c65d5207edb810f92c7c61edf5e2fc9c57e7b7a1ae656c02ce14e3595707dd6b93caf7956beb2bc817609e14d23d
057750c09d0a8331c33966d2cc2285ef82f08af8 ci: Upgrading pip version in macos environment (Tushar Singla)
Pull request description:
During each CI run, in macos native environment, python packages lief and zmq are rebuilt everytime which wastes a lot of resources and time and fixes#22206. The latest version of pip directly fetches pre-built binaries. Through this commit pip version is upgraded in macos environment before installation of these packages.
ACKs for top commit:
MarcoFalke:
cr ACK 057750c09d0a8331c33966d2cc2285ef82f08af8
Tree-SHA512: e61d02e46c8fe6a89119014d025a26aba090f9507d725315680893290f5bbc20a375ef408c71fa8db2f485b44ec91cfa0c140198ca44a9d3e0a57055b6bb9582
37371268d14ed6d5739af5b65d8bdb38b0e8dda2 Mark `CheckTxInputs` `[[nodiscard]]` (out-param `txfee` only set if call is successful). Avoid UUM in fuzzing harness `coins_view`. (practicalswift)
Pull request description:
Mark `CheckTxInputs` `[[nodiscard]]` (out-param `txfee` only set if call is successful).
Avoid use of uninitialised memory (UUM) in fuzzing harness `coins_view`.
ACKs for top commit:
MarcoFalke:
review ACK 37371268d14ed6d5739af5b65d8bdb38b0e8dda2
Tree-SHA512: edada5b2e80ce9ad3bd57b4c445bedefffa0a2d1cc880957d6848e4b7d9fc1ce036cd17f8b18bc03a36fbf84fc29c166cd6ac3dfbfe03e69d6fdbda13697754d
8b7713365134022475e7e5d24d3ca73149bd10e1 qt: Replace disambiguation strings with translator comments (Hennadii Stepanov)
Pull request description:
Since https://github.com/bitcoin/bitcoin/pull/21694 is merged, translator comments is the right way to pass context to translators.
This PR fixes changes were made:
- in #220 before https://github.com/bitcoin/bitcoin/pull/21694
- in https://github.com/bitcoin/bitcoin/pull/21694 on testing purpose
- in #125Closes#288.
ACKs for top commit:
jarolrod:
ACK 8b7713365134022475e7e5d24d3ca73149bd10e1
Tree-SHA512: 466ade35f4969a41fbf3196780b1ae9fa810bab5d2f09077f8631604636cc63b24a901c719f6b5797366d2aa307993d0aa419ce35200c8d0a741a3d81cad3e6b
415fb2e1abac189fcbe8eccf2ea065724d17460f GUI/Intro: Move prune setting below explanation (Luke Dashjr)
2a84c6bcf61429ac507b506a39247b3a66edbcb4 GUI/Intro: Estimate max age of backups that can be restored with pruning (Luke Dashjr)
e2dcd957fa206a28404ff824fb764b8889705fb2 GUI/Intro: Rework UI flow to let the user set prune size in GBs (Luke Dashjr)
f2e5a6b54fa38b10d822609939b8108bd8fe041b GUI/Intro: Abstract GUI-to-option into Intro::getPrune (Luke Dashjr)
62932cc686dc46ce14c026993deea8e561654177 GUI/Intro: Return actual prune setting from showIfNeeded (Luke Dashjr)
Pull request description:
Moved from https://github.com/bitcoin/bitcoin/pull/18728
ACKs for top commit:
ryanofsky:
Code review ACK 415fb2e1abac189fcbe8eccf2ea065724d17460f. Changes since last review: mb/gib suffixes, constexpr QOverload expected_backup_days tweaks, new moveonly layout commit
jarolrod:
Tested ACK 415fb2e.
Talkless:
tACK 415fb2e1abac189fcbe8eccf2ea065724d17460f, tested on Debian Sid with Qt 5.15.2.
hebasto:
ACK 415fb2e1abac189fcbe8eccf2ea065724d17460f, my unresolved comments are not blockers, and they could be resolved in follow ups.
Tree-SHA512: bd4882a9c08e6a6eb14b7fb6366983db8581425b4949fea212785d34d8fad9e32fb81ca8c8cdbfb2c05ea394aaf5a746ba2cf16623795c7252c3bdb61d455f00
cc3971c9ff538a924c1a76ca1352bcaeb24f579f GUI: Write PSBTs to file with binary mode (Andrew Chow)
Pull request description:
As noted in https://github.com/bitcoin/bitcoin/issues/20959, PSBT files should be opened in binary mode as on windows, all newlines are turned into CRLF which produces invalid PSBTs.
Fixes https://github.com/bitcoin/bitcoin/issues/20959
ACKs for top commit:
Talkless:
utACK cc3971c9ff538a924c1a76ca1352bcaeb24f579f.
Tree-SHA512: fee62b66da844017a44d7d6da6d2d2794b097a7dec33fb07711615df1e94dccc76f987ffcbb325ad1f8db2a2dd6eaf514b6cbd2453e7658b9f6c9fb5c4c41dab
bedb8d88bcfbfcadcd23e8f3ff4956340fcb028c Avoid comparision of integers with different signs (Jonas Schnelli)
Pull request description:
Fixes an integer comparison of different signs (which errors out on `-Werror,-Wsign-compare`). Introduced in #21121.
See https://bitcoinbuilds.org/index.php?ansilog=982c61cf-6969-4001-bebc-dc215e5d29a4.log
ACKs for top commit:
MarcoFalke:
review ACK bedb8d88bcfbfcadcd23e8f3ff4956340fcb028c
amitiuttarwar:
ACK bedb8d88bcfbfcadcd23e8f3ff4956340fcb028c
vasild:
ACK bedb8d88bcfbfcadcd23e8f3ff4956340fcb028c
Tree-SHA512: cb22a6239a1fc9d0be5573bf6ae4ec379eb7398c88edc8fa2ae4fd721f37f9ca3724896c1ac16de14a5286888a0b631813da32cb62d177ffbf9b2c31e716a7aa
7ff05358a96f49ae6b7eb265ce301748bfde30a8 [mempool] Remove error suppression on upgrade (Amiti Uttarwar)
Pull request description:
In 0.21, we added unbroadcast txids to mempool.dat (#18038). When users upgraded from 0.21 to 0.22, this would throw a misleading "failed to deserialize mempool data" error even though everything actually loaded properly. So, commit 9c8a55d added a try-block to prevent throwing the error. After upgrading to 0.22, this exception handling is no longer useful, so now we can remove it.
ACKs for top commit:
MarcoFalke:
review ACK 7ff05358a96f49ae6b7eb265ce301748bfde30a8
theStack:
Code review ACK 7ff05358a96f49ae6b7eb265ce301748bfde30a8
Tree-SHA512: 0444eea2b1326904f9855fd0af6669a4990f0427cf7c9293252a5b7049cdcc785bdf9398fd08ed8dedacfdd78e75039ddf1087b3654c558ff52498df15f05daf
faa94cb1675d8bd511eb593176cd07aa59465225 test: Check that invalid peer traffic is accounted for (MarcoFalke)
fae243f0cb92b5648d07d0a5033e2f4de862ae99 test: Remove confusing cast to same type (int to int) (MarcoFalke)
Pull request description:
Couldn't find a test for this and it seems something we should test, so I wrote one.
ACKs for top commit:
vasild:
ACK faa94cb16
practicalswift:
ACK faa94cb1675d8bd511eb593176cd07aa59465225: patch looks correct
Tree-SHA512: efcdd35960045cdfbd14480e16e0d1d09e81eb01670ac541ac2b105e1a63818a157c95853270242234a224880873e79957832bf4231374d7fe81de30f35e3abf
## Issue being fixed or feature implemented
The architecture of bitcoin assumes that there's no any external class
that processes network messages and knows anything about PeerManager
from net_processing; no any external call for PeerManager::Misbehaving
in bitcoin. All logic related to processing messages are located in
net_processing.
Dash has many many extra types of network messages and many of them
processed by external components such as llmq/signing or
coinjoin/client. Current architecture creates multiple circular
dependency.
## What was done?
That's part II of refactorings.
This PR removes PeerManager from several constructor and let LLMQContext
to forget about PeerManager.
Prior work in this PR: https://github.com/dashpay/dash/pull/5782
## What else to do?
Some network messages are processed asynchronously in external
components such as llmq/signing, llmq/instantsend,
llmq/dkgsessionhandler. It doesn't let to refactor them easily, because
they can't just simple return status of processing; status of processing
would be available sometime later and there's need callback or other way
to pass result code without spreading PeerManager over codebase.
## How Has This Been Tested?
- Run unit/functional tests
- run a linter test/lint/lint-circular-dependencies.sh
## 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
- [ ] 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
## Issue being fixed or feature implemented
To prevent spending withdrawal before that's finalized, mined and
chainlocked, next things should be true:
- txes with spending of unlock and unlock themselves do not receive
islocks. That's true, because IS excluded for txes with no inputs.
- When the unlock is removed from mempool, so are the children.
These functionality has no tests, but that's crucial for consensus be
fine.
## What was done?
Implemented checks to be sure that IS is not send for Withdrawal txes.
Functional test for asset_locks.py are refactored a bit to make it
easier to read.
## How Has This Been Tested?
This PR is 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
9913419cc9db5f8ce7afa0c3774468c330136064 test: remove type: comments in favour of actual annotations (fanquake)
Pull request description:
Now that we require Python 3.6+, we should be using variable type
annotations directly rather than `# type:` comments.
Also takes care of the discarded value issue in p2p_message_capture.py.
See: https://github.com/bitcoin/bitcoin/pull/19509/files#r571674446.
ACKs for top commit:
MarcoFalke:
review ACK 9913419cc9db5f8ce7afa0c3774468c330136064
jnewbery:
Code review ACK 9913419cc9db5f8ce7afa0c3774468c330136064
Tree-SHA512: 63aba5eef6c1320578f66cf8a6d85ac9dbab9d30b0d21e6e966be8216e63606de12321320c2958c67933bf68d10f2e76e9c43928e5989614cea34dde4187aad8
bff7c66e67aa2f18ef70139338643656a54444fe Add documentation to contrib folder (Troy Giorshev)
381f77be858d7417209b6de0b7cd23cb7eb99261 Add Message Capture Test (Troy Giorshev)
e4f378a505922c0f544b4cfbfdb169e884e02be9 Add capture parser (Troy Giorshev)
4d1a582549bc982d55e24585b0ba06f92f21e9da Call CaptureMessage at appropriate locations (Troy Giorshev)
f2a77ff97bec09dd5fcc043d8659d8ec5dfb87c2 Add CaptureMessage (Troy Giorshev)
dbf779d5deb04f55c6e8493ce4e12ed4628638f3 Clean PushMessage and ProcessMessages (Troy Giorshev)
Pull request description:
This PR introduces per-peer message capture into Bitcoin Core. 📓
## Purpose
The purpose and scope of this feature is intentionally limited. It answers a question anyone new to Bitcoin's P2P protocol has had: "Can I see what messages my node is sending and receiving?".
## Functionality
When a new debug-only command line argument `capturemessages` is set, any message that the node receives or sends is captured. The capture occurs in the MessageHandler thread. When receiving a message, it is captured as soon as the MessageHandler thread takes the message off of the vProcessMsg queue. When sending, the message is captured just before the message is pushed onto the vSendMsg queue.
The message capture is as minimal as possible to reduce the performance impact on the node. Messages are captured to a new `message_capture` folder in the datadir. Each node has their own subfolder named with their IP address and port. Inside, received and sent messages are captured into two binary files, msgs_recv.dat and msgs_sent.dat, like so:
```
message_capture/203.0.113.7:56072/msgs_recv.dat
message_capture/203.0.113.7:56072/msgs_sent.dat
```
Because the messages are raw binary dumps, included in this PR is a Python parsing tool to convert the binary files into human-readable JSON. This script has been placed on its own and out of the way in the new `contrib/message-capture` folder. Its usage is simple and easily discovered by the autogenerated `-h` option.
## Future Maintenance
I sympathize greatly with anyone who says "the best code is no code".
The future maintenance of this feature will be minimal. The logic to deserialize the payload of the p2p messages exists in our testing framework. As long as our testing framework works, so will this tool.
Additionally, I hope that the simplicity of this tool will mean that it gets used frequently, so that problems will be discovered and solved when they are small.
## FAQ
"Why not just use Wireshark"
Yes, Wireshark has the ability to filter and decode Bitcoin messages. However, the purpose of the message capture added in this PR is to assist with debugging, primarily for new developers looking to improve their knowledge of the Bitcoin Protocol. This drives the design in a different direction than Wireshark, in two different ways. First, this tool must be convenient and simple to use. Using an external tool, like Wireshark, requires setup and interpretation of the results. To a new user who doesn't necessarily know what to expect, this is unnecessary difficulty. This tool, on the other hand, "just works". Turn on the command line flag, run your node, run the script, read the JSON. Second, because this tool is being used for debugging, we want it to be as close to the true behavior of the node as possible. A lot can happen in the SocketHandler thread that would be missed by Wireshark.
Additionally, if we are to use Wireshark, we are at the mercy of whoever it maintaining the protocol in Wireshark, both as to it being accurate and recent. As can be seen by the **many** previous attempts to include Bitcoin in Wireshark (google "bitcoin dissector") this is easier said than done.
Lastly, I truly believe that this tool will be used significantly more by being included in the codebase. It's just that much more discoverable.
ACKs for top commit:
MarcoFalke:
re-ACK bff7c66e67aa2f18ef70139338643656a54444fe only some minor changes: 👚
jnewbery:
utACK bff7c66e67aa2f18ef70139338643656a54444fe
theStack:
re-ACK bff7c66e67aa2f18ef70139338643656a54444fe
Tree-SHA512: e59e3160422269221f70f98720b47842775781c247c064071d546c24fa7a35a0e5534e8baa4b4591a750d7eb16de6b4ecf54cbee6d193b261f4f104e28c15f47
572fd0f7382bd0e6c7acc27dc354fae8489ab0a0 doc: More precise -debug and -debugexclude doc (wodry)
Pull request description:
I wondered how one could enable debug logging with `-debug=<category>` for multiple categories. Found out solution is to specify that option multiple times for each wanted category.
This PR documents this behavior and uses the same wording for the same behavior of `-debugexclude=<category>` to make that also clear and stringent.
ACKs for top commit:
laanwj:
ACK 572fd0f7382bd0e6c7acc27dc354fae8489ab0a0
MarcoFalke:
ACK 572fd0f7382bd0e6c7acc27dc354fae8489ab0a0
theStack:
ACK 572fd0f7382bd0e6c7acc27dc354fae8489ab0a0
Tree-SHA512: 8d93db37602fd5ff4247e7c11478e55b99c0e3d47eaa2bb901937805d8f2a466b3a198b713b760981c5411576b74c52e2909c46c6d3f2e0e04215fd521b65cf7
## Issue being fixed or feature implemented
Should fix failures like
https://gitlab.com/dashpay/dash/-/jobs/6107697452
## What was done?
See inline comments
## How Has This Been Tested?
Run lots of `feature_governance.py` in parallel multiple times
## 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
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
---------
Co-authored-by: Konstantin Akimov <knstqq@gmail.com>
## Issue being fixed or feature implemented
This pull request is a follow-up to
[some](https://github.com/dashpay/dash/pull/5834#discussion_r1470105685)
[feedback](https://github.com/dashpay/dash/pull/5834#discussion_r1467009815)
received on [dash#5834](https://github.com/dashpay/dash/pull/5834) as
the patterns highlighted were present in different parts of the codebase
and hence not corrected within the PR itself but addressed separately.
This is that separate PR 🙂 (with some additional cleanup of my own)
## What was done?
* This pull request will remain a draft until
[dash#5834](https://github.com/dashpay/dash/pull/5834) as it will
introduce more changes that will need to be corrected in this PR.
* Code introduced that is unique to Dash Core (CoinJoin, InstantSend,
etc.) has been excluded from un-Dashification as the purpose of it is to
reduce backport conflicts, which don't apply in those cases.
* `CWallet::CreateTransaction` and the `CreateTransactionTest` fixture
have been excluded as the former originates from
[dash#3668](https://github.com/dashpay/dash/pull/3668) and the latter
from [dash#3667](https://github.com/dashpay/dash/pull/3667) and are
distinct enough to be unique to Dash Core.
* There are certain Dashifications and SegWit-removals that prove
frustrating as it would break compatibility with programs that rely on
the naming of certain keys
* `getrawmempool`, `getmempoolancestors`, `getmempooldescendants` and
`getmempoolentry` return `vsize` which is currently an alias of `size`.
I have been advised to retain `vsize` in lieu of potential future
developments. (this was originally remedied in
219a1d08973e7ccda6e778218b9a8218b4aae034 but has since been dropped)
* `getaddressmempool`, `getaddressutxos` and `getaddressdeltas` all
return a value with the key `satoshis`. This is frustrating to rename to
`duffs` for compatibility reasons.
* `decodepsbt` returns (if applicable) `non_witness_utxo` which is
frustrating to rename simply to `utxo` for the same reason.
* `analyzepsbt` returns (if applicable) `estimated_vsize` which
frustrating to rename to `estimated_size` for the same reason.
## How Has This Been Tested?
## 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)_
## Issue being fixed or feature implemented
HD wallets are old-existsing feature, appeared in Dash years ago, but
enabling HD wallets is not trivial task that requires multiple steps and
command line/rpc calls.
Let's have them enabled by default.
## What was done?
- HD wallets are enabled by default. Currently behavior `dashd`,
`dash-qt` are similar to run with option `-usehd=1`
- the rpc `upgradewallet` do not let to upgrade from non-HD wallet to HD
wallet to don't encourage user use non-crypted wallets (postponed till
v21)
- the initialization of ScriptPubKey is updated to be sure that encypted
HD seed is never written on disk (if passphrase is provided)
- enabled and dashified a script `wallet_upgradewallet.py` which test
compatibility between different versions of wallet
## What is not done?
- wallet tool still does not support passhprase, HD seed can appear on
disk
- there's no dialog that show user a mnemonic phrase and encourage him
to make a paper backup
Before removing a command line 'usehd' (backport bitcoin#11250) need to
make at least one major release for fail-over option (if someone wish to
use non-HD wallets only).
## How Has This Been Tested?
Run unit and functional tests.
Enabled new functional test `wallet_upgradewallet.py` that has been
backported long time ago but waited this PR to be enabled.
## Breaking Changes
HD wallets are created by default.
## 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
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
---------
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
## Issue being fixed or feature implemented
Fix failures like https://gitlab.com/dashpay/dash/-/jobs/6120923632
## What was done?
Handle disconnects and reconnection of the revoked MN in the right
place.
## How Has This Been Tested?
Run multiple `feature_dip3_v19.py` in parallel a few times
## 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
- [ ] 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)_
## Issue being fixed or feature implemented
Shouldn't change the way data is stored on mainnet/testnet nodes since
they use basic bls scheme anyway now. For devnets/regtest (which
activate v19 again and again) this patch should fix potential issues
reading pre-fork data right after the fork.
## What was done?
Pls see individual commits
## How Has This Been Tested?
Run tests, run a node on testnet
## 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)_
db058efeb0821cb5022e3b29e0aff3627d7aaf83 sync: use HasReason() in double lock tests (Vasil Dimov)
a21dc469ccf076ca3b07b1adbd8bf667145f1c44 sync: const-qualify the argument of double_lock_detected() (Vasil Dimov)
6d3689fcf6cff397187028344570489db3e6ecf4 sync: print proper lock order location when double lock is detected (Vasil Dimov)
Pull request description:
Before:
```
Assertion failed: detected double lock at src/sync.cpp:153, details in debug log.
```
After:
```
Assertion failed: detected double lock for 'm' in src/test/sync_tests.cpp:40 (in thread ''), details in debug log.
```
ACKs for top commit:
jonasschnelli:
utACK db058efeb0821cb5022e3b29e0aff3627d7aaf83
ajtowns:
ACK db058efeb0821cb5022e3b29e0aff3627d7aaf83
hebasto:
ACK db058efeb0821cb5022e3b29e0aff3627d7aaf83, tested on Linux Mint 20 (x86_64).
Tree-SHA512: 452ddb9a14e44bb174135b39f2219c76eadbb8a6c0e80d64a25f995780d6dbc7b570d9902616db94dbfabaee197b5828ba3475171a68240ac0958fb203a7acdb
bf100f8170770544fb39ae6802175c564cde532f [net] Cleanup InactivityChecks() and add commenting about time (John Newbery)
06fa85cd50b718fecd69f0481740d2b8714a1397 [net] InactivityCheck() takes a CNode reference (John Newbery)
Pull request description:
This is a pure refactor and should not change any behavior. It clarifies and documents the InactivityCheck() function
This makes #20721 easier to review. In particular, this function uses a mixture of (unmockable) system time and mockable time. It's important to understand where those are being used when reviewing #20721.
#20721 doesn't require this change, so if others don't agree that it's useful and makes review easier, then I'm happy to close this and just do #20721 directly.
ACKs for top commit:
fanquake:
ACK bf100f8170770544fb39ae6802175c564cde532f
MarcoFalke:
review ACK bf100f8170770544fb39ae6802175c564cde532f 💫
Tree-SHA512: 7b001de2a5fbe8a6dc37baeae930db5775290afb2e8a6aecdf13161f1e5b06ef813bc6291d8ee5cefcf1e430c955ea702833a8db84192eebe6e6acf0b9304cb2
fa9f20b6477a206adf5089398803b45d1a114b6f log: Properly log txs rejected from mempool (MarcoFalke)
Pull request description:
Currently `CheckTxInputs` rejections from the mempool are the only rejections that log directly and unconditionally to debug.log instead of leaving it to the caller. This has multiple issues:
* A rejected RPC transaction will log a redundant failure reason to debug log. All other failures are merely reported to the RPC user.
* A rejected p2p transaction will log the failure twice. Once with the `MEMPOOLREJ` flag, and once unconditionally.
* A rejected orphan transaction will log no failure.
Fix all issues by simply returning the state to the caller, like it is done for all other rejections.
The patch includes whitespace fixups to highlight relevant parts of the codebase and simplify review.
ACKs for top commit:
naumenkogs:
utACK fa9f20b6477a206adf5089398803b45d1a114b6f
rajarshimaitra:
Concept ACK. Compiled and ran tests. `fa9f20b`
jnewbery:
code review ACK fa9f20b6477a206adf5089398803b45d1a114b6f
Tree-SHA512: 86cc17b2a9239c01c4fc3f254ad48ee1d3883266966b9811030176338b9ac3deaea7ea5babfb8bbf739d7440154e30011fede8f9313175f199d4a062af6494f7
addf18da951439f696dba163ae1c73458d43ea03 Call SHA256AutoDetect in benchmark setup (Pieter Wuille)
Pull request description:
It seems `SHA256AutoDetect()` was not being called in benchmarks, making the numbers only reflect the naive implementation. Fix this by calling it in bench_bitcoin's setup.
ACKs for top commit:
fjahr:
tested ACK addf18da951439f696dba163ae1c73458d43ea03
pstratem:
ACK addf18da951439f696dba163ae1c73458d43ea03
laanwj:
ACK addf18da951439f696dba163ae1c73458d43ea03
Tree-SHA512: 3ba4b068145942df1429bf5913e3f685511e6ebeae2c1a3f9b8ac0144f6db1c7df456f88f480a2129f3e1602e3bf6a39530bb96e2c74c03ddb19324cec6799c7
56010f92564a94b0ca6c008c0e6f74a19fad4a2a test: hoist p2p values to test framework constants (Jon Atack)
75447f0893f9ad9bf83d182b301d139430d8de1c test: improve msg sends and p2p disconnections in p2p_invalid_messages (Jon Atack)
57960192a5362ff1a7b996995332535f4c2a25c3 test: refactor test_large_inv() into 3 tests with common method (Jon Atack)
e2b21d8a597c536a8617408d43958bfe9f98a442 test: add p2p_invalid_messages logging (Jon Atack)
9fa494dc0969c61d5ef33708a08923cca19ce091 net: update misbehavior logging for oversized messages (Jon Atack)
Pull request description:
...seen while reviewing #19264, #19252, #19304 and #19107:
in `net_processing.cpp`
- make the debug logging for oversized message size misbehavior the same for `addr`, `getdata`, `headers` and `inv` messages
in `p2p_invalid_messages`
- add missing logging
- improve assertions/message sends, move cleanup disconnections outside the assertion scopes
- split a slowish 3-part test into 3 order-independent tests
- add a few p2p constants to the test framework
ACKs for top commit:
troygiorshev:
reACK 56010f92564a94b0ca6c008c0e6f74a19fad4a2a
MarcoFalke:
ACK 56010f9256 🎛
Tree-SHA512: db67b70278f8d4c318907e105af54b54eb3afd15500f9aa0c98034f6fd4bd1cf9ad1663037bd9b237ff4890f3059b37291a6498d8d6ae2cc38efb9f045f73310
c8992e89594a54edf283e4916f794475070b5114 test: Fix fuzzer compilation on macOS fixes#19557 (freenancial)
Pull request description:
fixes#19557
Before the fix:
```
➜ bitcoin git:(fix-fuzzer-macos) make
Making all in src
CXX test/fuzz/addition_overflow-addition_overflow.o
In file included from test/fuzz/addition_overflow.cpp:7:
./test/fuzz/util.h:335:13: error: no matching function for call to 'AdditionOverflow'
if (AdditionOverflow((uint64_t)fuzzed_file->m_offset, random_bytes.size())) {
^~~~~~~~~~~~~~~~
./test/fuzz/util.h:201:16: note: candidate template ignored: deduced conflicting types for parameter 'T' ('unsigned long long' vs. 'unsigned long')
NODISCARD bool AdditionOverflow(const T i, const T j) noexcept
^
./test/fuzz/util.h:346:13: error: no matching function for call to 'AdditionOverflow'
if (AdditionOverflow(fuzzed_file->m_offset, n)) {
^~~~~~~~~~~~~~~~
./test/fuzz/util.h:201:16: note: candidate template ignored: deduced conflicting types for parameter 'T' ('long long' vs. 'long')
NODISCARD bool AdditionOverflow(const T i, const T j) noexcept
^
```
After the fix:
```
➜ bitcoin git:(fix-fuzzer-macos) ./configure --enable-fuzz --with-sanitizers=fuzzer,address,undefined CC=/usr/local/opt/llvm/bin/clang CXX=/usr/local/opt/llvm/bin/clang++ --disable-asm && make clean && make -j5
...
...
CXXLD test/fuzz/uint256_deserialize
Making all in doc/man
make[1]: Nothing to be done for `all'.
make[1]: Nothing to be done for `all-am'.
```
ACKs for top commit:
fanquake:
ACK c8992e89594a54edf283e4916f794475070b5114 - tested that compiling works on macOS.
MarcoFalke:
review ACK c8992e89594a54edf283e4916f794475070b5114
Tree-SHA512: 965cdc61b30db0e2209c91b29f0d42de927a9a5b85e1e70f22d1452e0955f876726c7a8c1d1a5f448f12bf24eec3000802071cd4ae28d8605343fd43d174ca84
1554b54d47d7e24ce2491f57d24e56d38ceb7649 Static asserts for consistency of fee defaults. (Daniel Kraft)
Pull request description:
This adds `static_assert`'s that ensure that the default values given for fee levels in the wallet (minimum fee and incremental feerate increase) are at least as high as the corresponding levels configured in the core node policy. Since the core policy values are enforced by the network, it makes sense for the wallet to be conservative and above (or at least not below) this.
ACKs for top commit:
laanwj:
code review ACK 1554b54d47d7e24ce2491f57d24e56d38ceb7649, these assumptions seem straightforward
Tree-SHA512: 50e5adf082f467062334377f82a3ee75bcfd436afc65bd0eb33c8d0549d6d90fd1f48c31f60cabe523eb59be9efa8ae0879e9e09cd51ca9c1bd466631ce03cf4
4133c8104f522c403c55d26bd03436a8149ff106 guix: use gcc tool wrappers (fanquake)
Pull request description:
This way, correct `--plugin` arguments are passed through.
This is a prerequisite for LTO (see #25391). Split out, to try move things along, as this change is isolated, and should be straight-forward.
ACKs for top commit:
TheCharlatan:
ACK [4133c81](4133c8104f)
hebasto:
ACK 4133c8104f522c403c55d26bd03436a8149ff106
Tree-SHA512: 4311a72a613cf027bd4490caa29604c985ed455589acd972285f13cbdf4806d2184a4dc6f20cb6f47c3fa751d58bfd0bacc257b87d4a804bf5ecf5b240e4a757
4becee396f3bda40832138dd1aaa90368ed31857 guix: combine and document enable_werror (fanquake)
Pull request description:
Combine into `hardened-glibc`.
Document why we don't use `--disable-werror` directly.
https://www.gnu.org/software/libc/manual/html_node/Configuring-and-compiling.html
> By default, the GNU C Library is built with -Werror. If you wish
> to build without this option (for example, if building with a
> newer version of GCC than this version of the GNU C Library was
> tested with, so new warnings cause the build with -Werror to fail),
> you can configure with --disable-werror.
ACKs for top commit:
hebasto:
ACK 4becee396f3bda40832138dd1aaa90368ed31857, the diff is correct.
TheCharlatan:
ACK 4becee396f3bda40832138dd1aaa90368ed31857
Tree-SHA512: 8724415f51b4d72d40c4e797faf52c93a81147fb629332b9388ffd7f113f2b16db3b7496bf3063dd978ac629fd5bde3ec7df4f1ff1ed714cb56f316a9334d119
127c637cf0a80e0ea68a7c5aaa088e5ccc9d3d13 guix: pass --enable-initfini-array to release GCC (fanquake)
Pull request description:
This returns us to pre-Guix behaviour, where the compilers we were using to build releases, were configured with this option.
> [--enable-initfini-array](https://gcc.gnu.org/install/configure.html)
> Force the use of sections .init_array and .fini_array (instead of .init and .fini) for constructors and destructors. Option --disable-initfini-array has the opposite effect. If neither option is specified, the configure script will try to guess whether the .init_array and .fini_array sections are supported and, if they are, use them.
ACKs for top commit:
TheCharlatan:
ACK 127c637cf0a80e0ea68a7c5aaa088e5ccc9d3d13
vincenzopalazzo:
utACK 127c637cf0
Tree-SHA512: fa61227054d52d4dfb4524af3888203a501f680661bdef00bb0970d4e8f7c96cf7f592686c4795be5a0debca267b8e564a4960859297c31f6b261c0729238382
