Commit Graph

27772 Commits

Author SHA1 Message Date
pasta
94da0de6ef
refactor: implement minimal RelayInv which doesn't rely on m_connman
Vast majority of usages of RelayInv don't use the minProtoVersion, we may as well have these not contribute to contention of m_nodes_mutex / use m_connman
2024-12-01 22:36:39 -06:00
pasta
c2b83dc80e
Merge #6309: backport: Merge bitcoin#24237, 23631, 24609
e237301b76 Merge bitcoin/bitcoin#24609: Clarify -maxtimeadjustment that only outbound peers influence timedata (MarcoFalke)
e101fd8e3b Merge bitcoin/bitcoin#23631: p2p: Don't use timestamps from inbound peers for Adjusted Time (Vijay)
abfa8c0bd4 Merge bitcoin/bitcoin#24237: test: Avoid testing negative block heights (MarcoFalke)

Pull request description:

  bitcoin backports

ACKs for top commit:
  UdjinM6:
    utACK e237301b76

Tree-SHA512: 5ecb1c3504a92a2b84962809aa349e5b102455984748a65bee8f2fb6c39e1ba85c6a0063780030f73806546017efffca21959de6d96ba65b4a2db9416d8fd6ed
2024-12-01 22:35:08 -06:00
pasta
ceee455fac
Merge #6433: fix: possible bug due to using unspecified behavior of std::move
6f75a7f601 refactor: use swap instead assign+clean in GetAndClearDirtyGovernanceObjectHashes (Konstantin Akimov)
83e0bb6e35 fix: potential bug due to unspecified state of std::vector after move (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  All standard library objects that have been moved from are placed in a "valid but unspecified state", meaning the object's class invariants hold (so functions without preconditions, such as the assignment operator, can be safely used on the object after it was moved from). See example:

  ```
  std::vector<int> v = {2, 3, 3};
  std::vector<int> u = std::move(v); // the value of v is unspecified
  v.clear(); // we are good now
  ```

  Instead, let's have swap better!
  ```
  WITH_LOCK(cs_pendingSigns, v.swap(pendingSigns));
  ```

  ## What was done?
  Fixed `CSigSharesManager::SignPendingSigShares`, `CSigningManager::ProcessPendingReconstructedRecoveredSigs` and refactored `CMasternodeMetaMan::GetAndClearDirtyGovernanceObjectHashes` for unification.

  ## How Has This Been Tested?
  Run unit and functional 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

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

Tree-SHA512: 5dd8664dbe9ce78329dfae24f6e8b67b7032ff7d2c066da0e01c4ed1b13bf76359a0307ee76e5b006820318693f067b505e59408614f47ee2fa8a979a1e0dc4d
2024-12-01 21:40:32 -06:00
MarcoFalke
e237301b76
Merge bitcoin/bitcoin#24609: Clarify -maxtimeadjustment that only outbound peers influence timedata
1bba72d824224f8a2625f529963d8982a00dfe14 Clarify in -maxtimeadjustment that only outbound peers influence time data (Jon Atack)

Pull request description:

  #23631 changed our adjusted time to only take into account time from outbound peers.

  Update `-maxtimeadjustment` to clarify this for users.

ACKs for top commit:
  MarcoFalke:
    cr ACK 1bba72d824224f8a2625f529963d8982a00dfe14
  mzumsande:
    code Review ACK 1bba72d824224f8a2625f529963d8982a00dfe14
  brunoerg:
    crACK 1bba72d824224f8a2625f529963d8982a00dfe14

Tree-SHA512: ad610ab3038fb83134e21d31cca952ef9ac926e88992ff93023b7010f2499f9a4d952e8e98a0ec56f8949872d966e5ffdd01a81e6b6115768f1992bd81be7a56
2024-11-27 12:01:37 +05:30
Vijay
e101fd8e3b
Merge bitcoin/bitcoin#23631: p2p: Don't use timestamps from inbound peers for Adjusted Time
0c85dc30e6b628f7538a67776c7eefcb84ef4f82 p2p: Don't use timestamps from inbound peers (Martin Zumsande)

Pull request description:

  `GetAdjustedTime()` (used e.g. in validation and addrman) returns a time with an offset that is influenced by timestamps that our peers have sent us in their version message.

  Currently, timestamps from all peers are used for this.
  However, I think that it would make sense to ignore the timedata samples from inbound peers, making it much harder for others to influence the Adjusted Time in a targeted way.
  With the extra feeler connections (every 2 minutes on average) and extra block-relay-only connections (every 5 minutes on average) there are also now plenty of opportunities to gather a meaningful number of timedata samples from outbound peers.

  There are some measures in place to prevent abuse: the `-maxtimeadjustment` parameter with a default of 70 minutes, warnings in cases of large deviations, only using the first 200 samples ([explanation](383d350bd5/src/timedata.cpp (L57-L72))), but I think that only using samples from outbound connections in the first place would be an additional safety measure that would make sense.

  See also issue #4521 for further context and links: There have been several discussions in the past about replacing or abolishing the existing timedata system.

ACKs for top commit:
  jnewbery:
    Concept and code review ACK 0c85dc30e6b628f7538a67776c7eefcb84ef4f82
  naumenkogs:
    ACK 0c85dc30e6b628f7538a67776c7eefcb84ef4f82
  vasild:
    ACK 0c85dc30e6b628f7538a67776c7eefcb84ef4f82

Tree-SHA512: 2d6375305bcae034d68b58b7a07777b40ac430dfed554c88e681a048c527536691e1b7d08c0ef995247d356f8e81aa0a4b983bf2674faf6a416264e5f1af0a96
2024-11-27 11:57:57 +05:30
MarcoFalke
abfa8c0bd4
Merge bitcoin/bitcoin#24237: test: Avoid testing negative block heights
fad81548fa03861c244397201d6b6e6cbf883c38 test: Avoid testing negative block heights (MarcoFalke)

Pull request description:

  A negative chain height is only used to denote an empty chain, not the height of any block.

  So stop testing that and remove a suppression.

ACKs for top commit:
  brunoerg:
    crACK fad81548fa03861c244397201d6b6e6cbf883c38

Tree-SHA512: 0f9e91617dfb6ceda99831e6cf4b4bf0d951054957c159b1a05a178ab6090798fae7368edefe12800da24585bcdf7299ec3534f4d3bbf5ce6a6eca74dd3bb766
2024-11-27 11:57:53 +05:30
Konstantin Akimov
6f75a7f601
refactor: use swap instead assign+clean in GetAndClearDirtyGovernanceObjectHashes 2024-11-27 01:21:53 +07:00
Konstantin Akimov
83e0bb6e35
fix: potential bug due to unspecified state of std::vector after move 2024-11-27 01:21:52 +07:00
pasta
8a05f0caaa
Merge #6305: backport: Merge bitcoin#24434, 23136
379aefab10 Merge bitcoin/bitcoin#24434: Add missed word to error message (laanwj)
5187ec0b69 Merge bitcoin/bitcoin#23136: test: update fee rate assertion helper in the functional test framework (MarcoFalke)

Pull request description:

  bitcoin backports

ACKs for top commit:
  knst:
    utACK 379aefab10
  PastaPastaPasta:
    utACK 379aefab10

Tree-SHA512: c61e295f9543878fa7a56dcb42cebb740cf1d38ae870f8ec78bbbb0dd643713e6415d6bd14bbdaeba61a2be8eaa8c4648edc76754b6f32cc89845af9ed8ca994
2024-11-26 11:29:51 -06:00
pasta
1155ff2ccf
Merge #6399: backport: merge bitcoin#23235, #23104, #24770, #24830, #24464, #24757, #25202, #25217, #25292, #25614, partial bitcoin#22766 (logging backports)
1621696a6f log: restore `LogPrintLevel` messages from prior backports (Kittywhiskers Van Gogh)
52a1263989 merge bitcoin#25614: Severity-based logging, step 2 (Kittywhiskers Van Gogh)
21470fdeb3 merge bitcoin#25292: Add LogPrintLevel to lint-format-strings, drop LogPrint-vs-LogPrintf section in dev notes (Kittywhiskers Van Gogh)
026409e4ff merge bitcoin#25217: update lint-logs.py to detect LogPrintLevel, mention WalletLogPrintf (Kittywhiskers Van Gogh)
b046e091c9 merge bitcoin#25202: Use severity-based logging for leveldb/libevent messages, reverse LogPrintLevel order (Kittywhiskers Van Gogh)
7697b73257 revert dash#2794: Disable logging of libevent debug messages (Kittywhiskers Van Gogh)
ff6304f5f3 merge bitcoin#24757: add `DEBUG_LOCKCONTENTION` to `--enable-debug` and CI (Kittywhiskers Van Gogh)
88592f30a3 merge bitcoin#24464: Add severity level to logs (Kittywhiskers Van Gogh)
d3e837ad22 merge bitcoin#24830: Allow -proxy="" setting values (Kittywhiskers Van Gogh)
0e01d5b5f3 partial bitcoin#22766: Clarify and disable unused ArgsManager flags (Kittywhiskers Van Gogh)
a9cfbd1048 fix: don't use non-existent `PrintLockContention` in `SharedEnter` (Kittywhiskers Van Gogh)
f331cbe8c8 merge bitcoin#24770: Put lock logging behind DEBUG_LOCKCONTENTION preprocessor directive (Kittywhiskers Van Gogh)
d9cc2ea178 merge bitcoin#23104: Avoid breaking single log lines over multiple lines in the log file (Kittywhiskers Van Gogh)
479ae82ecc merge bitcoin#23235: Reduce unnecessary default logging (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * This pull request's primary purpose is to restore `LogPrintLevel`s from backports in [dash#6333](https://github.com/dashpay/dash/pull/6333) that were changed to `LogPrint`s as they were backported before `LogPrintLevel` was backported.
  * ~~`clang-format` suggestions for `LogPrintLevel` have to be ignored in order to prevent the linter from tripping due to a "missing newline" ([build](https://gitlab.com/dashpay/dash/-/jobs/8398818860#L54)).~~ Resolved by applying diff ([source](https://github.com/dashpay/dash/pull/6399#issuecomment-2488992710)).
  * `SharedLock` was introduced in [dash#5961](https://github.com/dashpay/dash/pull/5961) and `PrintLockContention` was removed in [dash#6046](https://github.com/dashpay/dash/pull/6046) but the changes in the latter were not extended to the former. This has been corrected as part of this pull request.

  ## Breaking Changes

  None expected.

  ## Checklist

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

Top commit has no ACKs.

Tree-SHA512: f2d0ef8ce5cb1091c714a2169e89deb33fa71ff174ce4e6147b3ad421f57a84183d2a9e76736c0b064b2cc70fb3f2e545c42b8562cf36fdce18c3fb61307c364
2024-11-26 11:15:23 -06:00
pasta
f98b0d5d6d
Merge #6393: docs: mention building for some HOSTs only in release-process.md
87c31ad67a Update doc/release-process.md (UdjinM6)
55d74630b4 docs: mention building for some HOSTs only in `release-process.md` (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  https://github.com/dashpay/guix.sigs/pull/73

  #6390 follow-up

  ## What was done?

  ## How Has This Been Tested?

  ## Breaking Changes

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

Top commit has no ACKs.

Tree-SHA512: b4a2cadf5899a8aea6612b4ff9c0e9f9c530a9e2344eb090967fbcf9a2ab219aff02f11f86434e4082f84c401d578cf2d033b6838c94705f532beca4ab604986
2024-11-26 08:27:05 -06:00
pasta
0cb7485923
Merge #6426: fix: respect SENDDSQUEUE message, move DSQ relay into net processing / peerman
dafa7363a3 fix: respect SENDDSQUEUE message, move DSQ relay into net processing / peerman (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  in #6148, I broke the functionality where a peer must opt in / opt out of DSQUEUE messages. This was mostly ok, and not immediately detected, as with this bug, simply everyone would receive DSQ messages over inventory (or classically, old proto versions were not affected by this bug). But this still would result in quite a bit of wasted bandwidth for peers which may not care about DSQ at all.

  ## What was done?
  This commit should restore the prior functionality, where a node should send the SENDDSQUEUE message if they wish to receive DSQs. Once they've sent that, depending on their protocol version, they will either have the messages pushed to them as available, or on modern protocols, they will thereafter receive DSQs over the inventory system.

  NOTE: I also refactor the code in this commit, moving some network proccessing into.... wait for it... net_processing.cpp! This allowed us to remove some dependencies in coinjoin.h. DSQ messages are now relayed to peers by calling peer_manager.RelayDSQ

  ## How Has This Been Tested?
  I have not yet mixed on testnet with this; we should include it in rc.2 and test

  ## Breaking Changes
  Slightly breaking for v22.0.x (so rc.1), as they in theory could be relying on this new logic of always receiving the DSQ inv. But I don't think anyone besides core is using this new protocol.

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

ACKs for top commit:
  UdjinM6:
    light ACK dafa7363a3
  kwvg:
    utACK dafa7363a3

Tree-SHA512: 18f9b0dfe05cde19db451653db9bb9a00352efd1bc37adffd83f74958010475f2782b1111b1c0d2dd967e7a851c3c4795fa55033b4bd0cc810aa293e754ce314
2024-11-26 08:26:10 -06:00
laanwj
379aefab10
Merge bitcoin/bitcoin#24434: Add missed word to error message
ff33c5ae633191df94759a5f3deead43863cd8fd Add missed word to error message (Hennadii Stepanov)

Pull request description:

  Reported by Transifex translator [mmazi](https://www.transifex.com/user/profile/mmazi/).

ACKs for top commit:
  laanwj:
    ACK ff33c5ae633191df94759a5f3deead43863cd8fd

Tree-SHA512: 487512956dbaa42a33057362fc438f45ded211493221646b23101784902e75e6d72ef5b5184f175362e0d821a8409badc58abab4e06e86d4934b064dc5bb7050
2024-11-24 20:31:54 +05:30
MarcoFalke
5187ec0b69
Merge bitcoin/bitcoin#23136: test: update fee rate assertion helper in the functional test framework
b658d7d5c5339739dc19bf961d84186469a818d5 test: update assert_fee_amount() in test_framework/util.py (Jon Atack)

Pull request description:

  Follow-up to 42e1b5d979 (#12486).
  - update call to `round()` with our utility function `satoshi_round()` to avoid intermittent test failures
  - rename `fee_per_kB` to `feerate_BTC_kvB` for precision
  - store division result in `feerate_BTC_vB`

  Possibly resolves #19418.

ACKs for top commit:
  meshcollider:
    utACK b658d7d5c5339739dc19bf961d84186469a818d5

Tree-SHA512: f124ded98c913f98782dc047a85a05d3fdf5f0585041fa81129be562138f6261ec1bd9ee2af89729028277e75b591b0a7ad50244016c2b2fa935c6e400523183
2024-11-24 20:31:54 +05:30
Kittywhiskers Van Gogh
1621696a6f
log: restore LogPrintLevel messages from prior backports 2024-11-24 08:59:00 +00:00
Kittywhiskers Van Gogh
52a1263989
merge bitcoin#25614: Severity-based logging, step 2 2024-11-24 08:59:00 +00:00
Kittywhiskers Van Gogh
21470fdeb3
merge bitcoin#25292: Add LogPrintLevel to lint-format-strings, drop LogPrint-vs-LogPrintf section in dev notes 2024-11-24 08:58:59 +00:00
Kittywhiskers Van Gogh
026409e4ff
merge bitcoin#25217: update lint-logs.py to detect LogPrintLevel, mention WalletLogPrintf 2024-11-24 08:58:59 +00:00
Kittywhiskers Van Gogh
b046e091c9
merge bitcoin#25202: Use severity-based logging for leveldb/libevent messages, reverse LogPrintLevel order
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
2024-11-24 08:58:59 +00:00
Kittywhiskers Van Gogh
7697b73257
revert dash#2794: Disable logging of libevent debug messages
reverts:
- a1e4ac21fd
2024-11-24 08:58:59 +00:00
Kittywhiskers Van Gogh
ff6304f5f3
merge bitcoin#24757: add DEBUG_LOCKCONTENTION to --enable-debug and CI 2024-11-24 08:58:59 +00:00
Kittywhiskers Van Gogh
88592f30a3
merge bitcoin#24464: Add severity level to logs 2024-11-24 08:58:59 +00:00
Kittywhiskers Van Gogh
d3e837ad22
merge bitcoin#24830: Allow -proxy="" setting values 2024-11-24 08:58:59 +00:00
Kittywhiskers Van Gogh
0e01d5b5f3
partial bitcoin#22766: Clarify and disable unused ArgsManager flags
excludes:
- c5d7e34bd9a4ad752c5ec88032420e2e90ab17ab
2024-11-24 08:58:59 +00:00
pasta
dafa7363a3
fix: respect SENDDSQUEUE message, move DSQ relay into net processing / peerman
in 6148, I broke the functionality where a peer must opt in / opt out of DSQUEUE messages. This was mostly ok, and not immediately detected, as with this bug, simply everyone would receive DSQ messages over inventory (or classically, old proto versions were not affected by this bug). But this still would result in quite a bit of wasted bandwidth for peers which may not care about DSQ at all.

This commit should restore the prior functionality, where a node should send the SENDDSQUEUE message if they wish to receive DSQs. Once they've sent that, depending on their protocol version, they will either have the messages pushed to them as available, or on modern protocols, they will thereafter receive DSQs over the inventory system.

NOTE: I also refactor the code in this commit, moving some network proccessing into.... wait for it... net_processing.cpp! This allowed us to remove some dependencies in coinjoin.h. DSQ messages are now relayed to peers by calling peer_manager.RelayDSQ
2024-11-23 00:01:11 -06:00
pasta
f9d044d5ec
Merge #6423: refactor: create helper function RelayRecoveredSig inside peerman
86e92c376a refactor: drop unused CConnman from CSigningManager (Konstantin Akimov)
4668db60a2 refactor: create helper function RelayRecoveredSig inside peerman (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  High m_nodes_mutex lock contention during high load

  ## What was done?
  this commit should have a few benefits:
  1. previous logic using ForEachNode results in locking m_nodes_mutex, a highly contended RecursiveMutex, AND m_peer_mutex(in GetPeerRef)
  2. prior also resulted in calling .find over the m_peer_map for each node. Basically old logic was (probably) O(n(nlogn) the new logic results in acquiring m_peer_mutex once and looping over the list of peers, (probably) O(n)
  3. Moves networking logic out of llmq/ and into actual net_processing.cpp

  ## How Has This Been Tested?
  Hasn't really yet; it builds, but I need to run tests / maybe deploy to testnet mn

  ## Breaking Changes

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

ACKs for top commit:
  knst:
    utACK 86e92c376a
  UdjinM6:
    utACK 86e92c376a

Tree-SHA512: ca9d6ac22f8b72b117188147044c499ae62722283c6291633067b99726e6a6abc52e5c8cf3bdcd0d8fed0ad8d9086b000f628c9a932dfe89153e912b563eda5a
2024-11-22 16:27:31 -06:00
UdjinM6
87c31ad67a
Update doc/release-process.md
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
2024-11-23 01:27:04 +03:00
pasta
f526937790
Merge #6415: refactor: use self.wait_until in all the dash specific "wait_until_x" logic in order to actually apply the timeout scaling settings
3ba602672c refactor: use self.wait_until in all the dash specific "wait_until_x" logic in order to actually apply the timeout scaling settings (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  Currently we use the raw helper, but that means that timeout scaling isn't applying.. I think this may be a cause of a lot of the functional test failures that we see in tsan / ubsan.

  ## What was done?

  ## How Has This Been Tested?
  hasn't; wait for CI

  ## Breaking Changes

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

ACKs for top commit:
  knst:
    utACK 3ba602672c
  UdjinM6:
    utACK 3ba602672c

Tree-SHA512: 935498f4b296b1abcac8be686cce396b61b654ef62da46de9a23a0f24ad31254f4938a581a6a4e2533576db0e0120861fd690bd9019e893b30990f21d1e48168
2024-11-22 15:57:01 -06:00
pasta
69995eee5f
Merge #6422: perf: add in quorumBaseBlockIndexCache to reduce cs_main contention
3d67771f89 refactor: add in quorumBaseBlockIndexCache to reduce cs_main contention (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  subset of https://github.com/dashpay/dash/pull/6418; only includes the new quorumBaseBlockIndexCache, doesn't include the caching of the chain-tip, as that introduced regressions I'm still debugging.

  ## What was done?
  introduce a LRU cache for quorumHash -> const CBlockIndex*; this should significantly reduce cs_main contention during high transaction load.

  ## How Has This Been Tested?
  Ran tests locally; let's see CI happy, and I also intend to run this on a testnet MN first and see the level of contention reduction

  ## Breaking Changes
  None

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

ACKs for top commit:
  UdjinM6:
    utACK 3d67771f89
  knst:
    utACK 3d67771f89

Tree-SHA512: dbb4bdafed095397ca0e12dbd8bba25c108d199538387c71b1ff4285af821f9d9ad0ad4426407a015528270f3c163fa66ce91755efb1c8a7a90fd7cb70a918bc
2024-11-22 14:32:02 -06:00
Konstantin Akimov
86e92c376a
refactor: drop unused CConnman from CSigningManager 2024-11-22 12:58:25 -06:00
pasta
3ba602672c
refactor: use self.wait_until in all the dash specific "wait_until_x" logic in order to actually apply the timeout scaling settings 2024-11-22 12:34:34 -06:00
pasta
4668db60a2
refactor: create helper function RelayRecoveredSig inside peerman
this commit should have a few benefits:
1. previous logic using ForEachNode results in locking m_nodes_mutex, a highly contended RecursiveMutex, AND m_peer_mutex(in GetPeerRef)
2. prior also resulted in calling .find over the m_peer_map for each node. Basically old logic was (probably) O(n(nlogn) the new logic results in acquiring m_peer_mutex once and looping over the list of peers, (probably) O(n)
3. Moves networking logic out of llmq/ and into actual net_processing.cpp
2024-11-22 12:29:40 -06:00
pasta
0398b1cd72
Merge #6424: refactor: move expensive CInv initialization out of hot loop
b65f0bab7f refactor: move expensive CInv initialization out of hot loop (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  Not sure how we introduced this one, but it appears we are calling a CInv construction (pretty cheap) and a GetDSTX ((relatively) EXPENSIVE) for each peer we are connected to in RelayTransaction... I can hope and pray that the compiler somehow was magically optimizing this for us, but I really doubt it

  ## What was done?
  Move the initialization out of the loop

  ## How Has This Been Tested?

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

ACKs for top commit:
  knst:
    utACK b65f0bab7f
  UdjinM6:
    utACK b65f0bab7f
  kwvg:
    utACK b65f0bab7f

Tree-SHA512: f556789042ab9265d5c4d87c3ba2910138ce43ffa69c90ed208c9a3bcd861343f201ce2f00aeb541f345c9ca686dac7227df8d4833cf7fbdf61c36260f627864
2024-11-22 12:22:55 -06:00
pasta
f77e3816d9
Merge #6411: chore: update nMinimumChainWork, defaultAssumeValid, checkpointData, chainTxData for mainnet and testnet
31243ca313 chore: update nMinimumChainWork, defaultAssumeValid, checkpointData, chainTxData for mainnet and testnet (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  Bump chainparams in prep for v22

  ## What was done?

  ## How Has This Been Tested?

  ## Breaking Changes

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

ACKs for top commit:
  UdjinM6:
    utACK 31243ca313

Tree-SHA512: 6a01bbbaefb69437e053340b968e0ce68e2bd9e5e5fd2900864d88ffc21c5cd3f9c2a50d8f5b19f16515cd4c62a4a8fc2d981dd6d0456e86ef96f40e350e786a
2024-11-21 14:29:44 -06:00
pasta
c79cffe4d7
Merge #6420: refactor: several trivial refactorings for test_framework and dead code removing
a34caecdff refactor: replace infinite loop with break to while in forcemninfo (Konstantin Akimov)
4ec385d020 refactor: simplify loop of node connection in test_framework (Konstantin Akimov)
fa4ba4d169 refactor: moved functions do_connect, remove_masternode closer to theirs usages (Konstantin Akimov)
8eb5d852c7 refactor: drop dead constant SPECIALTX_TYPE from quorum commitment (Konstantin Akimov)

Pull request description:

  ## What was done?
  see each commit

  ## How Has This Been Tested?
  Run unit and functional 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

ACKs for top commit:
  UdjinM6:
    utACK a34caecdff

Tree-SHA512: d998839c992ab1ff36c88ef10236b2a7811b2edb81a9211a659065bf06bebc2f9174ef3268b1508b489a7b9b29a3f3e0b9fc096ff055cce6c4cacbfaf5877cd2
2024-11-21 14:24:33 -06:00
pasta
b65f0bab7f
refactor: move expensive CInv initialization out of hot loop 2024-11-21 13:54:37 -06:00
Konstantin Akimov
a34caecdff
refactor: replace infinite loop with break to while in forcemninfo 2024-11-22 01:32:27 +07:00
Konstantin Akimov
4ec385d020
refactor: simplify loop of node connection in test_framework 2024-11-22 01:32:27 +07:00
Konstantin Akimov
fa4ba4d169
refactor: moved functions do_connect, remove_masternode closer to theirs usages 2024-11-22 01:32:27 +07:00
Konstantin Akimov
8eb5d852c7
refactor: drop dead constant SPECIALTX_TYPE from quorum commitment 2024-11-22 01:32:27 +07:00
pasta
3d67771f89
refactor: add in quorumBaseBlockIndexCache to reduce cs_main contention 2024-11-21 12:31:59 -06:00
pasta
0f39da9631
Merge #6417: refactor: introduce cs_pendingSigns
397a157e8d refactor: introduce cs_pendingSigns (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  Much more minimal version of https://github.com/dashpay/dash/pull/6410; data from testnet while MNs were doing lots of instantsend locking showed minimal contention over signing_shares cs, however, the contention that did exist, most was a result of AsyncSign conflicting with something else. This should resolve the vast majority of contention in signing_shares while minimizing the complication / risk of introducing dead locks compared to 6410

  ## What was done?
  introduce cs_pendingSigns

  ## How Has This Been Tested?
  Built locally

  ## Breaking Changes
  None

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

ACKs for top commit:
  knst:
    utACK 397a157e8d
  UdjinM6:
    utACK 397a157e8d

Tree-SHA512: aac741c274449cf9c22625ac95e964d275dd1454647eaa2fc064c90b44a215e5509c9f9b8aceccbd49002edc21cbea883900f202204aa2138c104f4989ae5e26
2024-11-21 11:37:57 -06:00
pasta
242dc519ca
Merge #6404: fix(qt): avoid potential precision loss in amounts on Governance Tab
c48efdac83 fix(qt): avoid potential precision loss in amounts on Governance Tab (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  https://github.com/dashpay/dash/pull/6403#discussion_r1847834174

  ## What was done?

  ## How Has This Been Tested?

  ## Breaking Changes

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

ACKs for top commit:
  knst:
    utACK c48efdac83
  PastaPastaPasta:
    utACK c48efdac83

Tree-SHA512: 705bc154e10150e32e3ff04d4a90de14e29ffd195cc9f94a753dd1b4588f2730d6526cd66d851005b956e0d074d39250d142baa946bea8df95dbc92718931762
2024-11-20 23:24:36 -06:00
pasta
8a14482190
Merge #6407: fix: dataraces
5078baea2b fix(test): wait for chainlock before mining a block we expect to include said chainlock (pasta)
f39c1e6f4c fix: guard m_can_tx_relay behind m_tx_relay_mutex; make it private; add additional annotations (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  See each commit; fixes two bugs, both discovered while running feature_llmq_chainlocks.py with tsan / debug.

  one a datarace in net_processing.cpp and the other in the test I was using to ensure this fix was correct, feature_llmq_chainlocks

  ## What was done?
  ### net_processing.cpp
  You can see the datarace here: https://gist.github.com/PastaPastaPasta/c966a9f805758b34524085e3d52ea7f8

  We simply guard it with an existing mutex that is always locked in close proximity.

  ### feature_llmq_chainlocks.py
  Most of the time, while generating the cycle quorum, there is sufficient time to generate a chainlock; however, this is racey, and I've observed locally where the block gets generated before a chainlock is present and as such `test_coinbase_best_cl` fails. We should instead wait for the chainlock first, and then mine the block. This was we can ensure the mined block will include that chainlock.

  This was observed locally maybe 1/10 times or so

  ## How Has This Been Tested?
  ran feature_llmq_chainlocks.py ~40 times locally with tsan / debug

  ## Breaking Changes
  None

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

ACKs for top commit:
  kwvg:
    utACK 5078baea2b
  knst:
    utACK 5078baea2b
  UdjinM6:
    utACK 5078baea2b

Tree-SHA512: b346fc60809df72d0161f625073dce7062bd2641d35e4f80160fac9afeec63707de552e2856940ac2604875908ae3b98a225d352de36bfbfc6ee3fbe1e1538ff
2024-11-20 22:09:02 -06:00
pasta
9d87d9df72
Merge #6414: chore: bump seeds for v22
5741d5da28 chore: update seeds (Kittywhiskers Van Gogh)
2d732fc66e chore: drop defunct onion seeds, add new existing onion hosts as seeds (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  Update onion seeds for upcoming version

  ## What was done?
  Update onion seeds to reflect latest status

  ## How Has This Been Tested?
  As of today, all of these are able to be connected to on port 9999; I've not actually connected to all of them and verified they're on latest core or something like that; but their addresses my core knows about, and are able to be trivially connected to

  ## Breaking Changes

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

ACKs for top commit:
  kwvg:
    utACK 5741d5da28
  UdjinM6:
    utACK 5741d5da28

Tree-SHA512: 541bcc510b2ebf6de08ac91f2b7e5b1c910536dca9bab0b38930d6e5cfa1cd05e8c014ba4c74c14c43ed211cda3275fffb5baaf1489bbd1221567130d804b0ec
2024-11-20 13:59:11 -06:00
pasta
397a157e8d
refactor: introduce cs_pendingSigns 2024-11-20 13:54:33 -06:00
pasta
5adf92cb3c
Merge #6412: chore: bump develop to be 22.1.0
d728879329 chore: bump develop to be 22.1.0 (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  Develop is now for 22.1

  ## What was done?

  ## How Has This Been Tested?

  ## Breaking Changes

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

ACKs for top commit:
  kwvg:
    utACK d728879329
  UdjinM6:
    utACK d728879329

Tree-SHA512: 3ce6d37c43925c59a581cb7d518279c23cbf412b5511a3fa46dc1dd479da8a5d6bd3c40615f88e3db5bd6452ad3d9246901057055db91e3387e9d7c452e5c730
2024-11-20 12:02:46 -06:00
pasta
3c80dfc3f5
Merge #6403: feat(qt): Governance Tab should respect selected units and settings
90923ae7c7 feat(qt): Governance Tab should respect selected units and settings (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  Governance Tab ignores selected units and settings atm. Let's fix this.

  Display settings:
  <img width="607" alt="Screenshot 2024-11-19 at 00 49 35" src="https://github.com/user-attachments/assets/4ee777b4-8104-4e82-801a-c412573aa505">

  develop:
  <img width="965" alt="Screenshot 2024-11-19 at 00 47 35" src="https://github.com/user-attachments/assets/8aa69db0-c8a7-4e31-b733-e765e7f7f510">
  <img width="956" alt="Screenshot 2024-11-19 at 00 48 02" src="https://github.com/user-attachments/assets/1213fd66-24a2-40b8-bb32-87f3310f0086">

  this PR:

  <img width="956" alt="Screenshot 2024-11-19 at 00 49 04" src="https://github.com/user-attachments/assets/a09f8134-eddd-4e9c-ab86-1b5c4afeb994">
  <img width="958" alt="Screenshot 2024-11-19 at 00 49 17" src="https://github.com/user-attachments/assets/f9c34938-c56f-4198-8613-26f6e8c08c71">

  ## What was done?

  ## How Has This Been Tested?

  ## Breaking Changes

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

ACKs for top commit:
  PastaPastaPasta:
    ACK 90923ae7c7
  knst:
    utACK 90923ae7c7

Tree-SHA512: 90887821674ff989156718f39ffccda4d30bcf3126627443cb3dab9ce00869d436606eb8a6cd2fc498bd6d8107f6697863bf7df1e41200505c2d7c3081d2b21a
2024-11-20 11:38:34 -06:00
pasta
8a86fd898e
Merge #6416: refactor: drop unused parameter mninfos_valid in test framework mine_ cycle_quorum
0072318421 refactor: drop unused parameter mninfos_valid in test framework mine_cycle_quorum (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  This param is unused

  ## What was done?
  Removed

  ## How Has This Been Tested?
  see ci

  ## Breaking Changes

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

ACKs for top commit:
  knst:
    utACK 0072318421
  UdjinM6:
    utACK 0072318421

Tree-SHA512: e66e2b23b95b98e843fdb1d58419ba0798e8ba4115ef6c52e2e4681b04ea3e43efe73cee0ac96758560914567f822d107169ca0d6212210e07cc81b317a3e2eb
2024-11-20 11:34:21 -06:00
Kittywhiskers Van Gogh
5741d5da28
chore: update seeds 2024-11-20 11:33:06 -06:00