Commit Graph

2572 Commits

Author SHA1 Message Date
pasta
0a9a83fa4f
Merge #6282: refactor: drop some of circular dependencies over dkgsession
74a5a9f984 style: apply clang-format (Konstantin Akimov)
db9798f5e4 refactor: move call GetQuorumMembers inside Init() (Konstantin Akimov)
9593566802 refactor: move pQuorumBaseBlockIndex from Init() to constructor in CDKGSession (Konstantin Akimov)
9f3eb6bbb8 perf: check DIP0003 before CDKGSession initialization (Konstantin Akimov)
7f815cb501 refactor: remove unused constructor of CDKGLogger (Konstantin Akimov)
c82672af94 refactor: remove dependency of CDKGSession on PeerManager (Konstantin Akimov)
fb78b0cc94 refactor: remove retBan flag from ReceiveMessage (Konstantin Akimov)
d26d4ab0bc refactor: remove dependency of dkgsessionmgr on dkgsession (Konstantin Akimov)
d361b11e5b refactor: moved including llmq/dkgsession.h from dkgsessionmgr.h to cpp file (Konstantin Akimov)
e77aeb321b refactor: removed including quorums.h from chainlocks.h (Konstantin Akimov)
6f7068ef42 refactor: remove exceeding evodb.h from headers (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  We have 72 circular dependencies of dash specific code. This PR removes 2 of them, over dkgsession.

  ## What was done?
  Refactor dkgsession initialization, message processing, dropped unused arguments, re-distributed code between functions and modules... See each commit.
  Also optimized headers: excluded evo/evodb.h and llmq/quorums.h from the headers where they are not needed.

  ## How Has This Been Tested?
  Run `test/lint/lint-circular-dependencies.sh`
  Run unit/functional tests

  ## 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

ACKs for top commit:
  UdjinM6:
    utACK 74a5a9f984
  PastaPastaPasta:
    utACK 74a5a9f984

Tree-SHA512: f3adabe6a7bc6c4dcae6430be6857b31a0722d7f605d6651f3ff93c6fcb350a7312a9a0ecbda8ac131ffef6fbf2499700112de3990c7512ddb057bde7cc42665
2024-09-25 09:08:24 -05:00
pasta
6d615243e8
Merge #6264: backport: Merge bitcoin#23142, (partial) bitcoin-core/gui#409, 23333, 23755, (partial) 22981
71689fe6dc (partial) Merge bitcoin/bitcoin#22981: doc: Fix incorrect C++ named args (fanquake)
2b71a9b030 Merge bitcoin/bitcoin#23755: rpc: Quote user supplied strings in error messages (MarcoFalke)
5a441b38de (partial) Merge bitcoin-core/gui#409: Fix window title of wallet loading window (Hennadii Stepanov)
49c87e93a6 Merge bitcoin/bitcoin#23142: Return false on corrupt tx rather than asserting (W. J. van der Laan)

Pull request description:

  backports from bitcoin

ACKs for top commit:
  UdjinM6:
    utACK 71689fe6dc
  knst:
    utACK 71689fe6dc

Tree-SHA512: c68e2a1be5669f4fd8b02001ea81310b41fcac2cc5cc660e67b5140b334669c9a071a4bd5b33232580215607f323af5f87218a3465493675a633e112984296eb
2024-09-24 08:59:35 -05:00
pasta
8f8cd61fd6
Merge #6274: backport: bitcoin#23723, 23547, 24153, 23591
073d6d6b2a Merge bitcoin/bitcoin#23723: test: Replace hashlib.new with named constructor (MarcoFalke)
674dcf9a55 Merge bitcoin/bitcoin#23547: Bugfix: RPC/mining: Fail properly in estimatesmartfee if smart fee data is unavailable (MarcoFalke)
546e548755 Merge bitcoin/bitcoin#24153: test: remove unused sanitizer suppressions (fanquake)
78b06a4dd2 Merge bitcoin/bitcoin#23591: refactor: Use underlying type of isminetype for isminefilter (W. J. van der Laan)

Pull request description:

  backporting

ACKs for top commit:
  UdjinM6:
    utACK 073d6d6b2a
  knst:
    utACK 073d6d6b2a

Tree-SHA512: 5c5af5b795ec86f2b98cf9884e1275b8d3a5e7942f8b6632d74ecb799b1b7fe34071c052ac9af15abac14bad1b886ead5d0478f5e03fe0b461b7b40a7defef9e
2024-09-24 08:56:13 -05:00
pasta
60a7902b43
Merge #6272: trivial: remove some legacy code, make global a unique_ptr, simplify get_next_work test, make UBSan CI halt on error (misc. changes)
4e621037c5 test: move `EXPECTED_STDERR_NO_GOV`{`_PRUNE`} and use it more (Kittywhiskers Van Gogh)
77ce6af5c1 chore: remove ancient setting migration logic (Kittywhiskers Van Gogh)
061aa05cf0 chore: remove old llmq db migration code (Kittywhiskers Van Gogh)
438cb85ece ci: set UBSan to halt on error and provide more information (Kittywhiskers Van Gogh)
3a1743fc7f trivial: avoid unneeded copy when iterating through `mapDenomCount` (Kittywhiskers Van Gogh)
cba650953a test: simplify `pow_test`'s `get_next_work` block index construction (Kittywhiskers Van Gogh)
dacf859218 refactor: make `pdsNotificationInterface` a `unique_ptr`, rename (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  Collection of miscellaneous changes collected from work on earlier pull requests that don't fit into pull requests in the immediate future but are nonetheless useful.

  ## 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 **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  knst:
    re-utACK 4e621037c5
  UdjinM6:
    utACK 4e621037c5

Tree-SHA512: 5af4e5afdc34840905ffbf375f53cb12b682053cc135ff190dd02d245da9903a7f3e6af6fb1f1727546bf5034cbe42243477342f775e05d15bcfc5d5957616e9
2024-09-24 08:51:32 -05:00
pasta
3b8f24431c
Merge #6280: backport: merge bitcoin#26519, #25880, #26982, #26584, #27577, #28419, #28196, #28433, #28489, #28525 (BIP324 backports)
63b13aa519 merge bitcoin#28525: Drop v2 garbage authentication packet (Kittywhiskers Van Gogh)
662394cd49 merge bitcoin#28489: fix incorrect assumption in v2transport_test (Kittywhiskers Van Gogh)
98782c62df merge bitcoin#28433: Follow-up to BIP324 connection support (Kittywhiskers Van Gogh)
f9825168fb merge bitcoin#28196: BIP324 connection support (Kittywhiskers Van Gogh)
3087275039 merge bitcoin#28419: introduce and use `ConsumePrivateKey` helper (Kittywhiskers Van Gogh)
dccd395a4e merge bitcoin#27577: give seednodes time before falling back to fixed seeds (Kittywhiskers Van Gogh)
eb4f01f931 merge bitcoin#26584: include local ("unreachable") peers in -netinfo table (Kittywhiskers Van Gogh)
10dc874136 merge bitcoin#26982: bitcoin#25880 fixups (Kittywhiskers Van Gogh)
a36f8f2a1a merge bitcoin#25880: Make stalling timeout adaptive during IBD (Kittywhiskers Van Gogh)
1d77f3ff55 merge bitcoin#26519: Add getpeerinfo test for missing version message (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * When backporting [bitcoin#25880](https://github.com/bitcoin/bitcoin/pull/25880), changes needed to be made to `p2p_ibd_stalling.py` to help it pass.
    * DIP3 activation had to be delayed to a block beyond the range of the test. This is to prevent block rejection arising from a missing DIP3-compliant coinbase (done with `-dip3params=2000:2000`)
    * Mock time was disabled to ensure nodes in the test do not resort to direct fetching (with mock time enabled, nodes would be considered close apart in time, which would prevent the primary node from fetching in parallel, which is behavior this test relies on) (done with `self.disable_mocktime = True`)
    * The nodes connected do not report compressed headers support (the test relies on sending `headers` messages and reworking it to use compressed headers has little benefit) (done with `services = NODE_NETWORK | NODE_BLOOM`)

  * When backporting [bitcoin#28196](https://github.com/bitcoin/bitcoin/pull/28196), in the `v2transport_test` unit test, references to `4000000` were substituted with `MAX_PROTOCOL_MESSAGE_LENGTH` as Dash messages have a protocol limit of 3MiB ([source](d754799580/src/net.h (L79-L80))) while Bitcoin messages have a protocol limit of 4MB ([source](225718eda8/src/net.h (L62-L63))).
    * Additionally note that short message IDs as defined in the BIP324 spec ([source](22660ad307/bip-0324.mediawiki (v2-bitcoin-p2p-message-structure))) have not been changed to include Dash-specific messages, meaning, Dash-specific messages will always take 13 bytes.
    * As `FEEFILTER` is not supported by Dash, it has been replaced with a blank string in the short IDs table. It was not removed as doing so would disturb the table's arrangement as specified in spec and require readjustment of tests to account for the change in layout.

  ## 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 **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 63b13aa519
  knst:
    utACK 63b13aa519

Tree-SHA512: c41d2c6b1f145be513e285c8f91d00ac31fff4e4d24f611a1fdab24c8740f453b0bb28912021cdf8be4f5ce93dcff8579864727ee14b8e45894b56df524ab48d
2024-09-24 08:46:47 -05:00
Konstantin Akimov
c82672af94
refactor: remove dependency of CDKGSession on PeerManager 2024-09-24 12:50:29 +07:00
Konstantin Akimov
d26d4ab0bc
refactor: remove dependency of dkgsessionmgr on dkgsession 2024-09-24 12:50:14 +07:00
pasta
312134e39e
Merge #6239: feat: increase the number of block headers able to be downloaded at once to 8000 in protocol version 70235
48c7f98b1a doc: drop trailing whitespace (pasta)
697743d77b test: add missing import (UdjinM6)
cfe99fd289 docs: add release notes for 6239 (pasta)
a6bbaacfaa fix: GetHeadersLimit is used for getheaders(2) and headers(2), refactor it to accept `compressed` instead of `msg_type` (UdjinM6)
b224f3f6ca bump p2p_version in tests (PastaPastaPasta)
b423f42aae refactor: sort imports (UdjinM6)
f6c68ba71b refactor: simplify _compute_requested_block_headers (UdjinM6)
07876b2c4a use `MAX_HEADERS_UNCOMPRESSED_RESULT` not `MAX_HEADERS_UNCOMPRESSED_RESULTS` ; use `MAX_HEADERS_UNCOMPRESSED_RESULT` in RPC to avoid breaking changes (pasta)
b137280df4 change to _COMPRESSED or _UNCOMPRESSED (pasta)
303bc7af99 fix: increase it for headers2 only (UdjinM6)
e23410ffdd trivial: rename `MAX_HEADERS_RESULTS_NEW` to `MAX_HEADERS_RESULTS` (Kittywhiskers Van Gogh)
bcf0320691 trivial: move the headers limit determination to `GetHeadersLimit()` (Kittywhiskers Van Gogh)
993c7c0f90 feat: increase the number of block headers able to be downloaded at once to 8000 in protocol version `70234` (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  We did some testing quite a while ago that found that sending 8000 headers at a time could speed stuff up. But we wanted to wait until compressed headers were implemented. Well, they've been implemented!

  ## What was done?
  Bump 2000 -> 8000 triggered by protocol version

  ## How Has This Been Tested?
  Hasn't, we should setup a few nodes running this and sync them from each other

  ## Breaking Changes
  New protocol version, not breaking but should add notes? I should probably add release notes

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    light ACK 48c7f98b1a
  knst:
    utACK 48c7f98b1a

Tree-SHA512: 54c68b9496131ab7f32504d44398d776a151df809d0120d093bbabb18904a783bd9b58796820209f5d75552df5476e30eaa09d68f7c5057882f94b5766a64f4c
2024-09-23 11:50:26 -05:00
MarcoFalke
073d6d6b2a
Merge bitcoin/bitcoin#23723: test: Replace hashlib.new with named constructor
fa1b63c01887adff83f16b1bbba3bd159dc51104 test: Replace hashlib.new with named constructor (MarcoFalke)

Pull request description:

  A small refactor that doesn't matter too much, but it using the named constructor is nice because:
  * It clarifies that it is a built-in function
  * It is (trivially) faster and less code.

ACKs for top commit:
  Zero-1729:
    ACK fa1b63c01887adff83f16b1bbba3bd159dc51104
  w0xlt:
    ACK fa1b63c

Tree-SHA512: d23dc4552c1e6fc1f90f8272e47e4efcbe727f0b66a6f6a264db8a50ee6cb6d57a2809befcb95fda6725136672268633817a03dd1859f2298d20e3f9e0ca4a7f
2024-09-21 18:32:53 +05:30
Kittywhiskers Van Gogh
4e621037c5
test: move EXPECTED_STDERR_NO_GOV{_PRUNE} and use it more 2024-09-20 12:29:02 +00:00
Kittywhiskers Van Gogh
dccd395a4e
merge bitcoin#27577: give seednodes time before falling back to fixed seeds 2024-09-20 12:24:22 +00:00
Kittywhiskers Van Gogh
10dc874136
merge bitcoin#26982: bitcoin#25880 fixups 2024-09-20 12:24:22 +00:00
Kittywhiskers Van Gogh
a36f8f2a1a
merge bitcoin#25880: Make stalling timeout adaptive during IBD
We need to disable mocktime so that the node doesn't resort to direct
fetching. We also need to delay DIP3's activation so that blocks don't
get rejected for not having a valid DIP3-compliant coinbase.
2024-09-20 12:24:22 +00:00
Kittywhiskers Van Gogh
1d77f3ff55
merge bitcoin#26519: Add getpeerinfo test for missing version message
Co-Authored-By: UdjinM6 <UdjinM6@users.noreply.github.com>
2024-09-20 12:24:22 +00:00
Konstantin Akimov
f44edde8fe
tests: use only 2 MN and 2 Evo nodes in feature_asset_locks.py to be sure that is enough 2024-09-18 18:57:39 +07:00
pasta
d754799580
Merge #6270: fix: remove mn_rr fork usage from functional test of EHF
056d869571 refactor: use testdummy in feature_mnehf functional test, removed useless checks (Konstantin Akimov)
0351469bb5 refactor: removed duplicated meaningless condition from Check mnhftx (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Using mn_rr in feature_mnehf.py is a blocker for burying mn_rr fork.

  ## What was done?
  Removed useless conditions, uses testdummy fork instead mn_rr in ehf functional test.

  ## How Has This Been Tested?
  Run it `test/functional/feature_mnehf.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

ACKs for top commit:
  UdjinM6:
    utACK 056d869571
  PastaPastaPasta:
    utACK 056d869571

Tree-SHA512: aa016dc21fee3afdb5a4172e757cdd0b92867c23eb5241287f641d3c97d363ab1c46eba06423481ecc7ba05f6c0010b65eadfe77d4a1fd6074706cc6f3a71309
2024-09-16 21:44:08 -05:00
fanquake
546e548755
Merge bitcoin/bitcoin#24153: test: remove unused sanitizer suppressions
fa3bdbd37be81b613e48a10aa77dfd3bcede61e1 test: remove unused sanitizer suppressions (MarcoFalke)

Pull request description:

  Looks like those are not needed (anymore)

ACKs for top commit:
  fanquake:
    ACK fa3bdbd37be81b613e48a10aa77dfd3bcede61e1

Tree-SHA512: 4bedb6363aba8ea7763291ee0cd074e6bfd77e691bb32999c3959393864dc396bacba1eced2b10d9d600b66e8b83b91f7bc6692331dbd113bbaa87e72d11e2e8
2024-09-14 20:24:02 +05:30
Konstantin Akimov
056d869571
refactor: use testdummy in feature_mnehf functional test, removed useless checks 2024-09-14 12:08:26 +07:00
Konstantin Akimov
42dffe8541
chore: improve logging of functional tests feature_governance.py
The comments are converted to logs for better understanding of progress
2024-09-13 16:07:49 +07:00
Konstantin Akimov
922b796800
feat: simplify and speedup feature_governance.py test by generating less blocks 2024-09-13 15:50:58 +07:00
MarcoFalke
2b71a9b030
Merge bitcoin/bitcoin#23755: rpc: Quote user supplied strings in error messages
fa24a3df8796cbf4eeb35d950a4c848d605e5b22 rpc: Quote user supplied strings in error messages (MarcoFalke)

Pull request description:

  I can't see a downside doing this and this fixes a fuzzing crash

  Background:

  This is a follow-up to commit 926fc2a0d4ff64cf2ff8e1dfa64eca2ebd24e090, which introduced the "starts_with-hack". Maybe an alternative to the hack would be to assign a unique error code to internal bugs? However, I think this can be done in an separate pull request and the changes here make sense even on their own.

ACKs for top commit:
  fanquake:
    ACK fa24a3df8796cbf4eeb35d950a4c848d605e5b22 - to fix the fuzzers.

Tree-SHA512: d998626406a64396a037a6d1fce22fce3dadb7567c2f9638e450ebe8fb8ae77d134e15dd02555326732208f698d77b0028bc62be9ceee9c43282b61fe95fccbd
2024-09-12 18:57:19 +05:30
Kittywhiskers Van Gogh
c798b496cd
merge bitcoin#27529: fix feature_addrman.py on big-endian systems 2024-09-11 18:34:07 +00:00
Kittywhiskers Van Gogh
1adb635ec6
merge bitcoin#25678: skip querying dns seeds if -onlynet disables IPv4 and IPv6 2024-09-11 16:18:46 +00:00
pasta
96685be685
Merge #5167: refactor(stats): modernize statsd::StatsdClient, make global unique_ptr
cc998abec1 fmt: apply `clang-format-diff.py` suggestions (Kittywhiskers Van Gogh)
0401c581eb stats: `const`-ify variables and arguments (Kittywhiskers Van Gogh)
9f96723774 stats: stop using error codes, switch over to `bool` (Kittywhiskers Van Gogh)
1a81979c1e stats: initialize socket after we have a valid socket address (Kittywhiskers Van Gogh)
dbbfc8d766 stats: use `Socks` wrapper, use `CService` to generate our `sockaddr` (Kittywhiskers Van Gogh)
2def905044 stats: move init logic into constructor (Kittywhiskers Van Gogh)
4bc727cd6c stats: clean up randomization code, move `FastRandomContext` inward (Kittywhiskers Van Gogh)
840241eefd stats: cleanup error logging, improve code sanity (Kittywhiskers Van Gogh)
85890ddb13 docs: add copyright notice to source file, update notice in header (Kittywhiskers Van Gogh)
a9d1b1494d stats: move `_StatsdClientData` variables into `StatsdClient` (Kittywhiskers Van Gogh)
30c30c1397 stats: fetch all arguments needed when constructing `g_stats_client` (Kittywhiskers Van Gogh)
5133d88415 stats: s/statsClient/g_stats_client/g (Kittywhiskers Van Gogh)
f81951dd00 stats: make `statsClient` a `std::unique_ptr`, denote as global variable (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  Support for transmitting stats to a Statsd server has been courtesy of Statoshi ([repo](https://github.com/jlopp/statoshi)), implemented Dec, 2020 by [dash#2515](https://github.com/dashpay/dash/pull/2515) but since then, it hasn't gotten much attention aside from benefiting from codebase-wide changes and the occasional compiler appeasement. This pull request aims to give our statistics code some TLC.

  Changes include:

  * Limiting initialization to solely during construction and moving the responsibility of fetching arguments outside of `statsd::StatsdClient`.
  * Using the RAII `Socks` wrapper as early as possible (we still need to construct a raw socket ourselves but this is done in the initializer and control is moved to the wrapper and everywhere else, the wrapper is used)
  * Utilizing existing networking code to generate the socket address
    * This lets us trivially allow IPv6 connections as the responsibility to construct it safely is moved to `CService`.
  * Using `std::string` and our string manipulation capabilities (replacing `snprintf` with `strprintf`), replacing platform-specific types (replacing `short` with `uint16_t`).

  ## Breaking Changes

  None observed.

  ## Checklist:

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)**
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  PastaPastaPasta:
    utACK [cc998ab](cc998abec1)
  UdjinM6:
    utACK cc998abec1

Tree-SHA512: 433c92160d6ac7ebb8582ada3cbb65ead7913618266b773619a528c90dfe0e286aafa46dc3b0bca62f246938e5948a732080e2cddba942d3627f007ca6efcc1f
2024-09-11 11:17:52 -05:00
pasta
6019a8708f
Merge #6260: backport: merge bitcoin#20018, #22141, #22221, #22653, #23054, #23398, #23175, #22362, #23769, #23936, #24238, #24331 (auxiliary backports: part 15)
6c7335e002 merge bitcoin#24331: Revert back `MoveFileExW` call for MinGW-w64 (Kittywhiskers Van Gogh)
15e794bdd8 merge bitcoin#24238: use arc4random on OpenBSD (Kittywhiskers Van Gogh)
e039aecbdc merge bitcoin#23936: Add and use EnsureArgsman helper (Kittywhiskers Van Gogh)
b4bfacfd24 merge bitcoin#23769: Disallow copies of CChain (Kittywhiskers Van Gogh)
5b66688160 merge bitcoin#22362: Drop only invalid entries when reading banlist.json (Kittywhiskers Van Gogh)
109c963f6a merge bitcoin#23175: Add CJDNS network to -addrinfo and -netinfo (Kittywhiskers Van Gogh)
d57c96ea37 merge bitcoin#23398: add return message to savemempool RPC (Kittywhiskers Van Gogh)
22e59fb464 merge bitcoin#23054: Use C++11 member initializer in CTxMemPoolEntry (Kittywhiskers Van Gogh)
d158063b6d merge bitcoin#22653: Rename JoinErrors and re-use it (Kittywhiskers Van Gogh)
e24324d266 merge bitcoin#22221: Pass block reference instead of pointer to PeerManagerImpl::BlockRequested (Kittywhiskers Van Gogh)
68657efc03 merge bitcoin#22141: net processing: Remove hash and fValidatedHeaders from QueuedBlock (Kittywhiskers Van Gogh)
c0e6792e27 merge bitcoin#20018: ProcessAddrFetch(-seednode) is unnecessary if -connect is specified (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * When backporting [bitcoin#23054](https://github.com/bitcoin/bitcoin/pull/23054), `sigOpCount` and `nSigOpCountWithAncestors` were switched from `unsigned int` to `int64_t`. This change was done to prevent integer narrowing when accepting the `int64_t` taken from the constructor.

    This isn't a problem upstream as the same changes were as part of [bitcoin#8149](https://github.com/bitcoin/bitcoin/pull/8149/files#diff-8a2230436880b65a205db9299ab2e4e4adb1d4069146791b5db566f3fb752adaL90-L107), which was omitted but the type changes remain valid as sigop count cannot be a negative number.

  ## Breaking Changes

  None observed.

  ## 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 **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  PastaPastaPasta:
    utACK 6c7335e002
  UdjinM6:
    utACK 6c7335e002

Tree-SHA512: 29cae42dd82235305d3558562bae346170e742ce0b65897e396b331294b39cad0dd831fa9a09b34780a67844e55292e5b4e784cf544a894cc3f8897afe617ca1
2024-09-11 09:53:26 -05:00
Kittywhiskers Van Gogh
840241eefd
stats: cleanup error logging, improve code sanity
- Use `uint16_t` instead of `short`, `int64_t` instead of `size_t`
- Get rid of the `errmsg` buffer and use `LogPrintf` to report errors
- Use `strprintf` instead of `snprintf`
- Rephrase networking error logs to allow inclusion of error strings
2024-09-11 14:33:55 +00:00
pasta
4d312b5456
Merge #6252: test: Improve tests robustness
0c5179462e test: rework feature_governance_cl.py (UdjinM6)
11ac0819da feat: bump_mocktime also bumps schedulers now (UdjinM6)
1937f503fe feat: regtest-only: do not auto-reset mnsync after 1h (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  1. The original idea behind forced reset was to help desktop clients which go into sleep/hibernation mode from time to time to sync with no issues once they are online again. For regtest however it doesn't do anything good and only causes issues.
  2. We rely on schedulers a lot, bumping them should let nodes behave more like on a real network.
  3. Forcing mnsync to skip governance votes doesn't always work as we'd expect cause we don't control connection creation. To make it more deterministic create a proposal that should get into one superblock only. This way both the proposal and the trigger will expire/be deleted after another superblock meaning we can be 100% sure the isolated node never gets any of them and can only sync thanks to chainlock.

  ## What was done?

  ## How Has This Been Tested?
  run 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
  - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  knst:
    utACK 0c5179462e

Tree-SHA512: 9a40eeaba880f3f27f86736e92afa19a4ecb2a2d157bc42b65dd9da0d4109c9cd1d83a5abdf1ec16be2f8a8b31821fb700a7f0d2265c094fd4fdff7f18bc6ec7
2024-09-11 08:19:02 -05:00
UdjinM6
697743d77b
test: add missing import 2024-09-11 08:11:08 -05:00
PastaPastaPasta
b224f3f6ca
bump p2p_version in tests
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
2024-09-10 12:56:59 -05:00
UdjinM6
0c5179462e
test: rework feature_governance_cl.py 2024-09-10 18:36:40 +03:00
UdjinM6
11ac0819da
feat: bump_mocktime also bumps schedulers now 2024-09-10 18:36:40 +03:00
UdjinM6
b423f42aae
refactor: sort imports 2024-09-10 10:25:06 -05:00
UdjinM6
f6c68ba71b
refactor: simplify _compute_requested_block_headers 2024-09-10 10:25:06 -05:00
pasta
07876b2c4a
use MAX_HEADERS_UNCOMPRESSED_RESULT not MAX_HEADERS_UNCOMPRESSED_RESULTS ; use MAX_HEADERS_UNCOMPRESSED_RESULT in RPC to avoid breaking changes 2024-09-10 10:25:05 -05:00
pasta
b137280df4
change to _COMPRESSED or _UNCOMPRESSED 2024-09-10 10:25:05 -05:00
UdjinM6
303bc7af99
fix: increase it for headers2 only 2024-09-10 10:25:03 -05:00
pasta
993c7c0f90
feat: increase the number of block headers able to be downloaded at once to 8000 in protocol version 70234 2024-09-10 10:23:36 -05:00
pasta
1464e69058
Merge #6148: feat: broadcast dsq messages using the inventory system
57fb0874ef feat: broadcast dsq messages using the inventory system (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  DSQ messages are 142 bytes.

  Previously, assuming a relatively highly connected masternode hosting 100 connection, each round of coinjoin will result in 14.2KB (100*142) of inbound and outbound traffic each.

  ## What was done?
  Now, using the inventory system, a message will first use 36 bytes per peer (sending and receiving), plus the size of a `getdata` message and the actual message itself. As a result, bandwidth usage for 1 round of mixing would be closer to 36 * 100 + 142 (dsq) + 36 (getdata) = ~3.8KB, a reduction of around ~73%

  ## How Has This Been Tested?
  Has not been; @UdjinM6 especially please review well :)

  ## Breaking Changes
  Does introduce a new protocol version, but in a backwards compatible way. I don't think this would need to be delayed to v22 for any reason.

  ## 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)_

Top commit has no ACKs.

Tree-SHA512: 3dc39a339cba29d8cf207cec76ecace5ad0e11d1892ca0f65f9253a2b1d90313da21c6c178c2476756c5566ece0fab777006cd609b7984df906a9206c25d921d
2024-09-10 10:20:59 -05:00
pasta
57fb0874ef
feat: broadcast dsq messages using the inventory system
DSQ messages are 142 bytes.

Previously, assuming a relatively highly connected masternode hosting 100 connection, each round of coinjoin will result in 14.2KB (100*142) of inbound and outbound traffic each.

Now, using the inventory system, a message will first use 36 bytes per peer (sending and receiving), plus the size of a `getdata` message and the actual message itself. As a result, bandwidth usage for 1 round of mixing would be closer to 36 * 100 + 142 (dsq) + 36 (getdata) = ~3.8KB, a reduction of around ~73%
2024-09-10 09:23:22 -05:00
pasta
6d426515a5
Merge #6225: feat: bury v20 fork - fire up test chains by first block - 4/n
23812555b1 fix: possible deadlock during calculation of signals for historical blocks during re-index (Konstantin Akimov)
1087489fd4 feat: bury v20 deployment (Konstantin Akimov)
64cedb30bd feat: actually test something EHF unit tests (Konstantin Akimov)
762a808b8c chore: drop irrelevant bip9 code from feature_llmq_rotation.py (Konstantin Akimov)
7735631aad fix: remove v20 from test feature_llmq_evo as far as mn_rr used (Konstantin Akimov)
ca83b26815 fix: crash in CreditPool: it meant to check that DIP0003 is activated (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  V20 is activated on mainnet: time to bury it!

  https://github.com/dashpay/dash/issues/6186

  ## What was done?
  Hard-fork v20 is buried and it requires to implement multiple fixes, simplifications, refactoring:
   - some tests for EHF moved from functional tests to unit tests
   - fixed crash in Credit Pool if DIP3 is not activated yet
   - added a requirement for v20 activation for `CMNHFManager::GetSignalsStage`
   - removed useless code from functional test feature_llmq_rotation
   - renamed variables "v20" to "mn_rr" in feature_llmq_evo.py so far as actually used fork is mn_rr

  ## How Has This Been Tested?
  Some unit and functional tests to succeed.

  Done reindex  (just in case):

      src/qt/dash-qt -reindex  -assumevalid=0
      src/qt/dash-qt -reindex  -assumevalid=0 -testnet

  ## 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

ACKs for top commit:
  UdjinM6:
    ACK 23812555b1
  PastaPastaPasta:
    utACK 23812555b1

Tree-SHA512: eec35745baa695f3f286d39b6a61fa0a9f34820b13d1dd4cfbd1efe707850283892c39bf7fe49c49c812e0c02465d64df11480b3f12aa7f21b59a71eeae7260e
2024-09-10 08:50:23 -05:00
Kittywhiskers Van Gogh
d57c96ea37
merge bitcoin#23398: add return message to savemempool RPC 2024-09-08 16:24:36 +00:00
Kittywhiskers Van Gogh
c0e6792e27
merge bitcoin#20018: ProcessAddrFetch(-seednode) is unnecessary if -connect is specified 2024-09-08 15:35:20 +00:00
pasta
f222f798ab
Merge #6155: fix: avoid voting for the same trigger multiple times
e92aad7cff test: make sure MNs don't vote twice even when they are allowed to (UdjinM6)
3d75390e4e fix: correct conditions for YES voting (UdjinM6)
ec1392c6de chore: make clang-format and linter happy (UdjinM6)
a6320865c4 fix: avoid voting for the same trigger multiple times (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  We just had a sb voting period and I noticed that the network is way too spammy and produces too many votes (10x+ the expected numbers). It turns out that relying on `ProcessVoteAndRelay` on mainnet is not enough because rate-check expires too soon and MNs are able to vote again and again. On testnet it was never an issue because the voting period there is really short.

  ## What was done?
  Check known votes to make sure we never voted earlier.

  ## How Has This Been Tested?
  Run tests, run a MN on mainnet and check logs.

  ## 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
  - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  PastaPastaPasta:
    utACK e92aad7cff

Tree-SHA512: 142e23d3a19fa9527fa5257eb790e558d3507a7a857f17c6e02fd58eeb5643fcfb48d824d227e0ea7cd3dae6a6d7d871b3af88b13077f5af074ed1911e42bb28
2024-09-05 12:49:44 -05:00
pasta
93e62f3e78
Merge #6232: backport: merge bitcoin#22006, #22955, #23375, #23302, #22902, #23724, #26944, #23907, #24358, #25360, #25794, #26945, #27458, #28110 (userspace static defined tracing)
0a7a234bd3 merge bitcoin#28110: correct Fedora systemtap dep (Kittywhiskers Van Gogh)
c92eb67c40 merge bitcoin#27458: Detect USDT the same way how it is used in the code (Kittywhiskers Van Gogh)
f4a53ba8ce merge bitcoin#26945: systemtap 4.8 (Kittywhiskers Van Gogh)
88696129f3 merge bitcoin#25794: don't rely on block_connected USDT event order in tests (Kittywhiskers Van Gogh)
457bbd3f8b merge bitcoin#25360: SystemTap 4.7 (RISC-V support) (Kittywhiskers Van Gogh)
f3b219ad0d merge bitcoin#24358: USDT tracepoint interface tests (Kittywhiskers Van Gogh)
5b10a5a2fd merge bitcoin#23907: utxocache tracepoints follow up (Kittywhiskers Van Gogh)
c3d7e3a192 merge bitcoin#26944: fix systemtap download URL (Kittywhiskers Van Gogh)
264e02fcc7 merge bitcoin#23724: add systemtap's sys/sdt.h as depends for GUIX builds with USDT tracepoints (Kittywhiskers Van Gogh)
6cc596b99a merge bitcoin#22902: utxocache tracepoints (Kittywhiskers Van Gogh)
644a47ef9a merge bitcoin#23302: drop GetHash().ToString() argument from the `validation:block_connected` tracepoint (Kittywhiskers Van Gogh)
bfdc9ad364 merge bitcoin#23375: more deterministic coin selection for coinbase UTXOs (oldest first) (Kittywhiskers Van Gogh)
5718716cd2 merge bitcoin#22955: Rename fBlocksOnly, Add test (Kittywhiskers Van Gogh)
cacc31213b merge bitcoin#22006: first tracepoints and documentation on User-Space, Statically Defined Tracing (USDT) (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * In [bitcoin#22955](https://github.com/bitcoin/bitcoin/pull/22955), `fBlocksOnly` has not been renamed to `reject_tx_invs` as Dash uses the inventory system beyond relaying transaction data with the block-relay-only blocklist have a greater set of prohibited messages. This renders the new name misleading but coining a new name may be a source of confusion, making retaining the legacy name desirable.

    * Additionally, because the word "transaction" isn't hardcoded into the log message (instead opting to use `CInv::GetCommand()`'s output instead, which for transactions, are "tx"), the expected log message in `p2p_blocksonly.py` has been adjusted accordingly.

  * [bitcoin#24358](https://github.com/bitcoin/bitcoin/pull/24358) depends on [bitcoin#22955](https://github.com/bitcoin/bitcoin/pull/22955) and [bitcoin#23375](https://github.com/bitcoin/bitcoin/pull/23375) in order to work as without the latter backport, `interface_usdt_utxocache.py` will return the following error

    <details>

    <summary>Test run:</summary>

    ```
    debian@debian:~/dash$ sudo ./test/functional/interface_usdt_utxocache.py
    [sudo] password for debian:
    2024-08-26T17:08:05.234000Z TestFramework (INFO): Initializing test directory /tmp/dash_func_test_n5rb0xy4
    2024-08-26T17:08:07.023000Z TestFramework (INFO): testing the utxocache:uncache tracepoint API
    2024-08-26T17:08:07.026000Z TestFramework (ERROR): Assertion failed
    Traceback (most recent call last):
      File "/home/debian/dash/test/functional/test_framework/test_framework.py", line 160, in main
        self.run_test()
      File "/home/debian/dash/./test/functional/interface_usdt_utxocache.py", line 149, in run_test
        self.test_uncache()
      File "/home/debian/dash/./test/functional/interface_usdt_utxocache.py", line 172, in test_uncache
        invalid_tx = self.wallet.create_self_transfer(
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/home/debian/dash/test/functional/test_framework/wallet.py", line 166, in create_self_transfer
        assert_equal(mempool_valid, tx_info['allowed'])
      File "/home/debian/dash/test/functional/test_framework/util.py", line 51, in assert_equal
        raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
    AssertionError: not(True == False)
    2024-08-26T17:08:07.533000Z TestFramework (INFO): Stopping nodes
    2024-08-26T17:08:08.535000Z TestFramework (WARNING): Not cleaning up dir /tmp/dash_func_test_n5rb0xy4
    2024-08-26T17:08:08.535000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/dash_func_test_n5rb0xy4/test_framework.log
    2024-08-26T17:08:08.535000Z TestFramework (ERROR):
    2024-08-26T17:08:08.535000Z TestFramework (ERROR): Hint: Call /home/debian/dash/test/functional/combine_logs.py '/tmp/dash_func_test_n5rb0xy4' to consolidate all logs
    2024-08-26T17:08:08.536000Z TestFramework (ERROR):
    2024-08-26T17:08:08.536000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
    2024-08-26T17:08:08.536000Z TestFramework (ERROR): https://github.com/dashpay/dash/issues
    2024-08-26T17:08:08.536000Z TestFramework (ERROR):
    ```

    </details>

    with the underlying error that has been alluded to in d2c4904

    ```
    {'txid': '44b58b10e69321dacf00724f1893c9ecb50fc1f89ed7c70a6c0b3c08f7dc750f', 'allowed': False, 'reject-reason': 'bad-txns-premature-spend-of-coinbase'}
    ```

  * In [bitcoin#24358](https://github.com/bitcoin/bitcoin/pull/24358), all `interface_usdt_*.py` tests needed minor syntax changes to account for how we invoke certain RPCs. `interface_usdt_utxocache.py` required tweaking with the blocks needed to be eligible for pruning (`450` vs. `350` upstream) and stop the node explicitly to account for the governance validation warning .

    This is important as the placement assumes that `test_flush()` is the last test, should this change, the node may need to be manually started before continuing on with the tests after.

  * Some `TRACE*` entries may not exactly match the backports they come from because the variables or functions they access may have been amended by a backport done _after_ the backport that introduce the trace entry.

  ## Breaking Changes

  None observed.

  ## Checklist:

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 0a7a234bd3
  PastaPastaPasta:
    utACK [0a7a234](0a7a234bd3)

Tree-SHA512: 8df11392dd8d152c18d55ac70a446d1eec336bdf1a984cbf41c3202c353358180e05ba4b7182ec2962ea09eefa41d1dc3cd383d358f9b3dec57ce8b67c6e6afd
2024-09-05 12:13:17 -05:00
pasta
b38c4d3c1b
Merge #6229: fix: creditOutputs in AssetLock tx json output should be an array of objects, not debug strings
9876c2d78b docs: add partial release notes (UdjinM6)
b330318db7 refactor: drop circular dependency (UdjinM6)
e54fe42ce8 refactor: use `key_to_p2pkh_script` in more places (UdjinM6)
3ed6246889 test: check `creditOutputs` format (UdjinM6)
ba0e64505b fix: `creditOutputs` in AssetLock tx json output should be an array of objects, not debug strings (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  Txout-s in `creditOutputs` for AssetLock txes should be shown the way txout-s are shown in other places. We should not be using debug strings there.

  Example: `getrawtransaction 50757f651f335e22c5a810bd05c1e5aac0d95b132f6454e2a72683f88e3983f3 1`

  develop:
  ```
    "assetLockTx": {
      "version": 1,
      "creditOutputs": [
        "CTxOut(nValue=0.01000000, scriptPubKey=76a914cdfca4ae1cf2333056659a2c)"
      ]
    },
  ```
  This PR:
  ```
    "assetLockTx": {
      "version": 1,
      "creditOutputs": [
        {
          "value": 0.01000000,
          "valueSat": 1000000,
          "scriptPubKey": {
            "asm": "OP_DUP OP_HASH160 cdfca4ae1cf2333056659a2c8dc656f36d228402 OP_EQUALVERIFY OP_CHECKSIG",
            "hex": "76a914cdfca4ae1cf2333056659a2c8dc656f36d22840288ac",
            "address": "yf6c2VSpWGXUgmjQSHRpfEcTPsbqN4oL4c",
            "type": "pubkeyhash"
          }
        }
      ]
    },
  ```
  kudos to @coolaj86 for finding the issue

  ## What was done?
  Change `CAssetLockPayload::ToJson()` output to be closer to [`TxToUniv()`](https://github.com/dashpay/dash/blob/develop/src/core_write.cpp#L262-L272)

  NOTE: `refactor: use key_to_p2pkh_script in more places` commit is a bit unrelated but I decided to add it anyway to make it easier to follow assetlock creation vs getrawtransaction rpc check.

  ## How Has This Been Tested?
  Try example above, run tests

  ## Breaking Changes
  RPC output is different for AssetLock txes

  ## 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
  - [ ] I have assigned this pull request to a milestone

ACKs for top commit:
  PastaPastaPasta:
    utACK 9876c2d78b

Tree-SHA512: 158c98ac9e4979bb29c4f54cb1b71806f22aaec92218d92cd2b2e9b9f74df721563e7a6c5f517ea358ac74659fa79f51d1b683002a1cdceb1b8ee80f8fd79375
2024-09-05 09:38:29 -05:00
Kittywhiskers Van Gogh
88696129f3
merge bitcoin#25794: don't rely on block_connected USDT event order in tests 2024-09-04 18:46:14 +00:00
Kittywhiskers Van Gogh
f3b219ad0d
merge bitcoin#24358: USDT tracepoint interface tests 2024-09-04 18:46:14 +00:00
Kittywhiskers Van Gogh
bfdc9ad364
merge bitcoin#23375: more deterministic coin selection for coinbase UTXOs (oldest first) 2024-09-04 18:46:13 +00:00
Kittywhiskers Van Gogh
5718716cd2
merge bitcoin#22955: Rename fBlocksOnly, Add test
`fBlocksOnly` has not been renamed as Dash uses the inv system for
relaying far more than transaction data and the blocklist of invs in
block-relay-only mode extends beyond what is conveyed by `reject_tx_invs`
2024-09-04 18:46:13 +00:00
pasta
18625ae559
Merge #6243: backport: merge bitcoin#22226, #22831, #23084, #22872, #23477, #23492, #23713, #23780, #23826, #23373, #24201, #24665, #22910, partial bitcoin#23025 (addrman backports: part 3)
f032119456 merge bitcoin#22910: Encapsulate asmap in NetGroupManager (Kittywhiskers Van Gogh)
8020bfa8c1 merge bitcoin#24665: document clang tidy named args (Kittywhiskers Van Gogh)
40a22e457a merge bitcoin#24201: Avoid InitError when downgrading peers.dat (Kittywhiskers Van Gogh)
cdcaf2278c merge bitcoin#23373: Parse command line arguments from unit and fuzz tests, make addrman consistency check ratio easier to change (Kittywhiskers Van Gogh)
b30f0fa441 test: remove `connman` local from `BasicTestingSetup` (Kittywhiskers Van Gogh)
df43565464 merge bitcoin#23826: Make AddrMan unit tests use public interface, extend coverage (Kittywhiskers Van Gogh)
c14a54089f merge bitcoin#23780: update `addrman_tests.cpp` to use output from `AddrMan::Good()` (Kittywhiskers Van Gogh)
8b2db6bce4 merge bitcoin#23713: refactor addrman_tried_collisions test to directly check for collisions (Kittywhiskers Van Gogh)
5b5dd39f45 merge bitcoin#23492: tidy up addrman unit tests (Kittywhiskers Van Gogh)
aba0ebd400 merge bitcoin#23477: tidy up unit tests (Kittywhiskers Van Gogh)
cdc8321c4d merge bitcoin#22872: improve checkaddrman logging with duration in milliseconds (Kittywhiskers Van Gogh)
8d22fe9945 merge bitcoin#23084: avoid non-determinism in asmap-addrman test (Kittywhiskers Van Gogh)
ba4696718e partial bitcoin#23025: update nanobench add `-min_time` (Kittywhiskers Van Gogh)
c28b05c5ca merge bitcoin#22831: add addpeeraddress "tried", test addrman checks on restart with asmap (Kittywhiskers Van Gogh)
c4fe6085c8 merge bitcoin#22226: add unittest core dump instructions (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * In [bitcoin#22831](https://github.com/bitcoin/bitcoin/pull/22831), when restarting the node in `rpc_net.py`'s `test_addpeeraddress()`, existing commands need to be appended to `extra_args` to ensure they're retained when the node is restarted (default behavior is to overwrite the argument list with `extra_args`) to prevent the test from hanging and then failing due to missing fast DIP3 activation params from the arguments list.

    Missing arguments result in a block database sanity check failure on restart due to `bad-qc-premature` arising from the activation height being higher than the height of a block with a quorum commitment.

  * `NodeContext` was moved from `TestingSetup` to `BasicTestingSetup` in [bitcoin#18571](https://github.com/bitcoin/bitcoin/pull/18571) ([dash#4844](https://github.com/dashpay/dash/pull/4844)) but `connman` as a `BasicTestingSetup` variable still stuck around (despite `NodeContext`'s presence making this unnecessary).

    To prepare for [bitcoin#22910](https://github.com/bitcoin/bitcoin/pull/22910), the remnant variable has been replaced with `m_node.connman` and adjustments have been made to that effect.

  ## Breaking Changes

  None observed.

  ## Checklist:

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK f032119456
  PastaPastaPasta:
    utACK f032119456

Tree-SHA512: b29c292ecda54cda8301ea804b433f80476a1cdbb72bd48740cc9b2e885a4ff52350e5e42f112426856282bd6d961f0e37f1b23020c52f07238413070bbc504a
2024-09-04 12:44:49 -05:00