Commit Graph

25085 Commits

Author SHA1 Message Date
Kittywhiskers Van Gogh
f8e4bbb5ce
docs: add missing block of description style not picked up by Doxygen
The changes are a portion of bitcoin#17873, the rest of which is merged
into `developer-notes.md`, leaving this change out for some reason.
2024-02-19 10:17:12 -06:00
Kittywhiskers Van Gogh
bfa8b97731
docs: update Doxygen documentation URLs to doxygen.nl domain
These changes were introduced in bitcoin#15514 (Update Transifex links),
which ordinarily do not apply to Dash as it uses its own Transifex
account but not mentioned in the name are updates to Doxygen URLs.
2024-02-19 10:17:11 -06:00
Kittywhiskers Van Gogh
c15c7bb9be
partial bitcoin#20161: Minor taproot follow-ups
includes:
- 1d22300b99cda0504bb1f457d94468fa2c33c4e2
2024-02-19 10:17:11 -06:00
Kittywhiskers Van Gogh
9aeef44d62
merge bitcoin#21100: remove unused function xor_bytes 2024-02-19 10:17:11 -06:00
Kittywhiskers Van Gogh
663b3c7450
partial bitcoin#20292: Fix intermittent feature_taproot issue
includes:
- 50eb0c2512842b96a0128a7d592a357665f6e006 (only changes to test/
  functional/test_framework/key.py)
2024-02-19 10:17:11 -06:00
Kittywhiskers Van Gogh
94bd52d5e0
partial bitcoin#23394: Taproot wallet test vectors
includes:
- ca83ffc2ea5fe08f16fff7df71c040d067f2afb0 (only changes to test/
  functional/test_framework/key.py)
2024-02-19 10:17:10 -06:00
Kittywhiskers Van Gogh
3960c1bccf
merge bitcoin#27538: remove modinv python util helper function 2024-02-19 10:17:10 -06:00
Kittywhiskers Van Gogh
13c8dc535c
partial bitcoin#19953: Implement BIP 340-342 validation
includes:
- 3c226639eb134314a0640d34e4ccb6148dbde22f
- f06e6d03452cf5e0b1a0863afb08c9e6d3ef452e (only changes to test/
  functional/test_framework/key.py)
2024-02-19 10:17:10 -06:00
Kittywhiskers Van Gogh
5aeb24e020
merge bitcoin#22029: Improve transport deserialization fuzz test coverage 2024-02-19 10:17:07 -06:00
PastaPastaPasta
3be3c1ddec
trivial: use WITH_LOCK when possible (#5879) 2024-02-16 17:16:12 -06:00
thephez
c5fcab9849
docs: rpc help correction (#5877)
## 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)_
2024-02-16 10:50:39 -06:00
UdjinM6
fa4ced95ee
fix: intermittent failures in feature_asset_locks.py (#5875)
## 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
2024-02-15 12:17:14 -06:00
PastaPastaPasta
015e30fa0b
feat: add onion_seeds to seed creation; do seed creation (#5866)
## 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)_
2024-02-14 22:31:24 -06:00
PastaPastaPasta
854bccd3f5
Merge pull request #5870 from PastaPastaPasta/develop-trivial-2024-02-12
backport: trivial 2024 02 12
2024-02-14 14:58:30 -06:00
W. J. van der Laan
09e2a9ebed
Merge bitcoin/bitcoin#23781: test: Fix system_tests/run_command on Windows
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 fix bitcoin/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
2024-02-14 14:57:38 -06:00
MarcoFalke
e05c215f34
Merge bitcoin/bitcoin#22769: fuzz: Use LIMITED_WHILE instead of limit_max_ops
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
2024-02-14 14:57:38 -06:00
MarcoFalke
81b53a4795
Merge bitcoin/bitcoin#22749: ci: Run arm task on arm64 hardware
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
2024-02-14 14:57:37 -06:00
fanquake
306d61db68
Merge bitcoin/bitcoin#22746: doc: Update GitHub documentation links
470c6aa280f42558499db95b3ebca06a179d3514 doc: Update GitHub documentation links (Suriyaa Rocky Sundararuban)

Pull request description:

  Removing URL redirecting for GitHub documentation links.

ACKs for top commit:
  fanquake:
    ACK 470c6aa280f42558499db95b3ebca06a179d3514 - updates look correct.

Tree-SHA512: 5359aaddf1a1adabd0dc0e36cffcdb7b1ed83d9022314804655e010c84bf6143b7a2c8553a4e6c7793f02973868ad4b7554431ad61df82130ce47af74ebf0e99
2024-02-14 14:57:37 -06:00
Hennadii Stepanov
65b7ec3366
Merge bitcoin-core/gui#379: Prompt to reset settings when settings.json cannot be read
1ee6d0b01a517893967379677029fb5417978247 gui: Prompt to reset settings when settings.json cannot be read (Russell Yanofsky)

Pull request description:

  Currently the GUI shows confusing error messages when `settings.json` can't be read or written on startup. This causes the unrecoverable read error described in bitcoin/bitcoin#21340 and write error described bitcoin/bitcoin#21974. Current error read message looks like:

  ![current](https://user-images.githubusercontent.com/7133040/124977362-638ffc80-dffe-11eb-9edd-89135a9bc602.png)

  This PR tries to clarify the error dialog, and adds an option to just clear the settings and reset them to default:

  ![new-read-error](https://user-images.githubusercontent.com/7133040/124977636-b669b400-dffe-11eb-8d35-02eda95f48c0.png)
  ![new-read-details](https://user-images.githubusercontent.com/7133040/124977644-bb2e6800-dffe-11eb-9209-11c1c3d7be40.png)

  Additionally the PR also shows a slightly better error message when there is an error trying to write the settings file. This error probably should occur less frequently, but it is easy to improve, and it should be good to make the write error consistent with the read error. The new write error dialog looks like:

  ![new-write-error](https://user-images.githubusercontent.com/7133040/124978016-3bed6400-dfff-11eb-9d79-9b2e9bbc4369.png)

  ![new-write-details](https://user-images.githubusercontent.com/7133040/124978025-3db72780-dfff-11eb-8df5-741f75a402d9.png)

ACKs for top commit:
  jarolrod:
    ACK 1ee6d0b01a517893967379677029fb5417978247
  Zero-1729:
    ACK 1ee6d0b01a517893967379677029fb5417978247
  hebasto:
    ACK 1ee6d0b01a517893967379677029fb5417978247, tested on Linux Mint 20.2 (Qt 5.12.8).

Tree-SHA512: fb57a0a0d032e3f8219fff49a4de69b4c962bf0b448544ccf9d8d4d45c5bd209e23653d4f13300b9e534b9c03de159498bef1658e95defe3ab6a8ecac57d592c
2024-02-14 14:57:37 -06:00
MarcoFalke
3b317db95f
Merge bitcoin/bitcoin#22454: fuzz: Limit max ops in tx_pool fuzz targets
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
2024-02-14 14:57:37 -06:00
fanquake
14a24acd4d
Merge bitcoin/bitcoin#22353: ci: Upgrading pip version in macos environment
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
2024-02-14 14:57:36 -06:00
MarcoFalke
3550fff5a4
Merge bitcoin/bitcoin#22065: Mark CheckTxInputs [[nodiscard]]. Avoid UUM in fuzzing harness coins_view.
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
2024-02-14 14:57:36 -06:00
Hennadii Stepanov
3520ac271e
Merge bitcoin-core/gui#332: Replace disambiguation strings with translator comments
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 #125

  Closes #288.

ACKs for top commit:
  jarolrod:
    ACK 8b7713365134022475e7e5d24d3ca73149bd10e1

Tree-SHA512: 466ade35f4969a41fbf3196780b1ae9fa810bab5d2f09077f8631604636cc63b24a901c719f6b5797366d2aa307993d0aa419ce35200c8d0a741a3d81cad3e6b
2024-02-14 14:57:36 -06:00
Hennadii Stepanov
292170cfeb
Merge bitcoin-core/gui#125: Enable changing the autoprune block space size in intro dialog
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:

  ![Screenshot_20200911_095102](https://user-images.githubusercontent.com/1095675/92933661-0c4cea00-f436-11ea-9853-2456091ffab3.png)

  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
2024-02-14 14:57:36 -06:00
MarcoFalke
69431fd66a
Merge bitcoin-core/gui#188: Write PSBTs to file with binary mode
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
2024-02-14 14:57:35 -06:00
fanquake
9a02781412
Merge #21228: test: Avoid comparision of integers with different signs
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
2024-02-14 14:57:35 -06:00
MarcoFalke
0ad10660d1
Merge #20854: [mempool] Remove unnecessary try-block
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
2024-02-14 14:57:35 -06:00
fanquake
9c2aefe908
Merge #20028: test: Check that invalid peer traffic is accounted for
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
2024-02-14 14:57:34 -06:00
Konstantin Akimov
4becf980fa
refactor: remove circular dependencies through net_processing (2/N) (#5792)
## 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
2024-02-14 12:32:54 -06:00
Konstantin Akimov
dd27e11813
test: asset unlock (withdrawal) should not be Instant-Send locked (#5862)
## 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
2024-02-14 10:47:18 -06:00
PastaPastaPasta
6c061322ed
Merge pull request #5766 from vijaydasmp/bp22_25
backport: Merge bitcoin#21029, 21060, 19509, 21107, 25117
2024-02-14 10:34:35 -06:00
Vijay Das Manikpuri
639705e43b
Merge bitcoin/bitcoin#25117: test: Check msg type in msg capture is followed by zeros 2024-02-14 10:34:11 -06:00
MarcoFalke
74577eeeee
Merge #21107: test: remove type: comments in favour of actual annotations
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
2024-02-14 10:34:10 -06:00
MarcoFalke
9a92452a5c
Merge #19509: Per-Peer Message Capture
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
2024-02-14 10:34:10 -06:00
fanquake
a26d4b2cd2
Merge #21060: doc: More precise -debug and -debugexclude doc
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
2024-02-14 10:34:10 -06:00
fanquake
3f204ccda8
Merge #21029: bitcoin-cli: Correct docs (no "generatenewaddress" exists)
71430aec4304f71f8a11e6f0fea486e41fe3a9e3 bitcoin-cli: Correct docs (no "generatenewaddress" exists) (Luke Dashjr)

Pull request description:

ACKs for top commit:
  jonatack:
    ACK 71430aec4304f71f8a11e6f0fea486e41fe3a9e3

Tree-SHA512: 45ee7a51080598141fac4446563bdf99a59bfaa0ee00d9e092511087a968e8535d9208683ed589be1c975531be6d0319d33720f1f0dc1c3635c7d5ea6d726a41
2024-02-14 10:34:09 -06:00
UdjinM6
69913377a1
fix: intermittent failures in feature_governance.py (#5868)
## 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>
2024-02-13 07:49:34 -06:00
PastaPastaPasta
2f6528292f
Merge pull request #5867 from UdjinM6/clang16tsan
backport: bitcoin#27298 (use clang 16)
2024-02-12 10:18:47 -06:00
UdjinM6
685b1f0379
ci: use clang-16 2024-02-10 14:47:41 +03:00
fanquake
6a282f4407
Merge bitcoin/bitcoin#27298: ci: Use TSan new runtime (llvm-16, take 3)
faf4aca15a319a26aaec7127455f6db97c7039cc ci: Use TSan new runtime (llvm-16, take 3) (MarcoFalke)

Pull request description:

  The previous two attempts failed:
  * llvm-14: Failed in https://github.com/bitcoin/bitcoin/pull/24572
  * llvm-15: Failed in https://github.com/bitcoin/bitcoin/pull/26775

  However, now that the bug is known and fixed, it should be good to go. See also https://github.com/bitcoin/bitcoin/pull/26775#issuecomment-1380590669

ACKs for top commit:
  fanquake:
    ACK faf4aca15a319a26aaec7127455f6db97c7039cc - I still see [this](https://github.com/bitcoin/bitcoin/pull/27298#issuecomment-1480041089) failure on aarch64, but that isn't really a regression, as running this tests was already broken for me. I'll open a separate issue, and we can follow up.

Tree-SHA512: 372b53c4d42ca7f527dae4a2b5bc5ab33c816930daf7a3479d20ea7749159a0b19cfd8d76244b95b03130e4a3d12ddbbb74668b8f7e9fc272cf1084f53b7ff9b
2024-02-10 14:43:56 +03:00
Kittywhiskers Van Gogh
d9c549e541
trivial: add missing rpc help messages, remove segwit references, dashify help text, undashify code comments (#5852)
## 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)_
2024-02-09 11:40:38 -06:00
Konstantin Akimov
8dba6559f6
feat: enable HD wallets by default (#5807)
## 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>
2024-02-09 11:36:14 -06:00
UdjinM6
19681d0f45
fix: intermittent failure in feature_dip3_v19.py (#5863)
## 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)_
2024-02-09 11:35:33 -06:00
Alessandro Rezzi
8ea9c40655
fix: unlock proregtx collateral on error (#5838) 2024-02-08 12:18:27 -06:00
Alessandro Rezzi
1821d92b66 test: add activate_ehf_by_name
To avoid copy and pasting future ehf deployments activation functions
2024-02-08 16:55:32 +01:00
Alessandro Rezzi
de38dca242 feat(consensus): Generalize ehf activation
Try to sign any ehf deployment that can be activated and that hasn't been mined on chain yet.
Also when receiving a new recovered signature try to match it with any ehf deployment which hasn't been mined on chain yet
2024-02-08 16:55:27 +01:00
UdjinM6
f3f4c16c35
fix: Store BLSVerificationVector on disk using basic bls scheme (#5480)
## 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)_
2024-02-07 12:34:54 -06:00
UdjinM6
3a2bcb1bf1
feat(rpc): Tweak protx info to be able to show info for a specific block (#4738) 2024-02-07 12:34:16 -06:00
PastaPastaPasta
08b22ddc1e
Merge pull request #5770 from vijaydasmp/bp22_26_1
backport: Merge bitcoin#19370, 19562, 19538, 19272, 19214, 19429, 18990, 20927, 20507
2024-02-07 10:28:00 -06:00
MarcoFalke
18500eac2f
Merge #20507: sync: print proper lock order location when double lock is detected
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
2024-02-07 10:27:43 -06:00