Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backport!: Merge bitcoin#19915, 19469, 19464, 19512, 22495 #5511

Merged
merged 5 commits into from
Mar 22, 2024

Conversation

vijaydasmp
Copy link

@vijaydasmp vijaydasmp commented Jul 26, 2023

bitcoin backports

@vijaydasmp vijaydasmp changed the title backport: backport : Merge bitcoin#20791, 20377, 20813 Jul 26, 2023
@vijaydasmp vijaydasmp changed the title backport : Merge bitcoin#20791, 20377, 20813 backport: Merge bitcoin#20791, 20377, 20813 Jul 26, 2023
@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#20791, 20377, 20813 backport: Merge bitcoin#20791, 20377, 19915, 20584 Jul 27, 2023
@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#20791, 20377, 19915, 20584 backport: Merge bitcoin#20791, 20377, 19915, 20584, 19469, 19725, 19770, 20755 Jul 28, 2023
@github-actions
Copy link

This pull request has conflicts, please rebase.

@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#20791, 20377, 19915, 20584, 19469, 19725, 19770, 20755 backport: Merge bitcoin#20791, 20377, 19915, 20584, 19469, 19725, 19770, 20755, 19724 Jul 29, 2023
@vijaydasmp vijaydasmp force-pushed the bp22_10 branch 2 times, most recently from e503e46 to 58e6162 Compare July 29, 2023 17:27
@github-actions
Copy link

github-actions bot commented Aug 3, 2023

This pull request has conflicts, please rebase.

@vijaydasmp vijaydasmp force-pushed the bp22_10 branch 2 times, most recently from 5cafb2e to b82842b Compare August 10, 2023 15:15
@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#20791, 20377, 19915, 20584, 19469, 19725, 19770, 20755, 19724 backport: Merge bitcoin#19316, 20791, 20377, 19915, 20584, 19469, 19725, 19770, 20755, 19724 Aug 10, 2023
@vijaydasmp vijaydasmp force-pushed the bp22_10 branch 4 times, most recently from eff96a9 to 55f061e Compare August 12, 2023 18:05
@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#19316, 20791, 20377, 19915, 20584, 19469, 19725, 19770, 20755, 19724 backport: Merge bitcoin#19316, 20377, 19915, 19469, 19725,19724 Aug 12, 2023
@github-actions
Copy link

This pull request has conflicts, please rebase.

@thephez thephez added the RPC Some notable changes to RPC params/behaviour/descriptions label Aug 29, 2023
@vijaydasmp vijaydasmp requested a review from UdjinM6 March 16, 2024 01:44
@UdjinM6 UdjinM6 mentioned this pull request Mar 16, 2024
5 tasks
@UdjinM6
Copy link

UdjinM6 commented Mar 16, 2024

Looks good but should be merged after #5939 to avoid issues

Copy link

This pull request has conflicts, please rebase.

PastaPastaPasta added a commit that referenced this pull request Mar 19, 2024
c9ffb72 fix: avoid `hSocket` double lock (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  It's is locked in `CloseSocketDisconnect()` already.

  To be merged before #5511 or 19915 backport is going to cause issues otherwise.

  ## What was done?
  Assert the lock is held already, instead of locking it again.

  ## How Has This Been Tested?
  Run tests, run a node on testnet and drop connections to peers

  ## 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:
  PastaPastaPasta:
    utACK c9ffb72

Tree-SHA512: 6042d0683bf9b1326a74d73b5b44896a0470865b88e1c686d6eefe55c0d419b11a134474499bb6c9c308af69b2f7c4a60070d4535072304330cb640f91f5e367
UdjinM6
UdjinM6 previously approved these changes Mar 19, 2024
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@vijaydasmp
Copy link
Author

Hello @knst @PastaPastaPasta, requesting review

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK a39e85d

src/rpc/net.cpp Outdated Show resolved Hide resolved
@PastaPastaPasta
Copy link
Member

conflicts with #5933

Copy link

This pull request has conflicts, please rebase.

@PastaPastaPasta
Copy link
Member

I rebased it for you. Range diff here:

 -:  ---------- >  1:  10fa7a66b6 Merge #19538: ci: Add tsan suppression for race in DatabaseBatch
 -:  ---------- >  2:  2489f29f0e Merge #19830: test: Add tsan supp for leveldb::DBImpl::DeleteObsoleteFiles
 -:  ---------- >  3:  59d5a4ef39 Merge #19773: wallet: Avoid recursive lock in IsTrusted
 -:  ---------- >  4:  5821a1d23a Merge #14582: wallet: always do avoid partial spends if fees are within a specified range
 -:  ---------- >  5:  6a6d379711 Merge #19756: tests: add sync_all to fix race condition in wallet groups test
 -:  ---------- >  6:  eb4270deae Merge #19743: -maxapsfee follow-up
 -:  ---------- >  7:  1707f01309 fix: follow-up changes from bitcoin/bitcoin#22220 for maxapsfee
 -:  ---------- >  8:  44929bad82 Merge #16404: qa: Test ZMQ notification after chain reorg
 -:  ---------- >  9:  a1c2386153 Merge #17445: zmq: Fix due to invalid argument and multiple notifiers
 -:  ---------- > 10:  0ce66fd477 Merge #19507: Expand functional zmq transaction tests
 -:  ---------- > 11:  2f788aa76d fix: change port to use for zmq in interface_zmq_dash.py
 -:  ---------- > 12:  5a6b8b6b1f partial Merge bitcoin/bitcoin#27053: wallet: reuse change dest when re-creating TX with avoidpartialspends
 -:  ---------- > 13:  2dd6082e54 refactor: move special transaction processing into helper class
 -:  ---------- > 14:  0d6f736a19 fix: add missing entity for destruction in DashTestSetupClose
 -:  ---------- > 15:  667852c851 refactor: cleanup CDSNotificationInterface member names, add asserts
 -:  ---------- > 16:  67cfee70f8 refactor: remove CMNHFManager::GetSignalsStage alias from CChainState
 -:  ---------- > 17:  f0451fb98d refactor: remove CCreditPoolManager global, move to NodeContext
 -:  ---------- > 18:  a53046c4a9 refactor: remove CDSTXManager global and alias, move to CJContext
 -:  ---------- > 19:  c427305805 refactor: remove CNetFulfilledRequestManager global, move to NodeContext
 -:  ---------- > 20:  991c9ec1a2 refactor: accept NodeContext arg into BlockAssembler instead of managers
 -:  ---------- > 21:  e9de972982 refactor: move GetListAtChainTip() calls out of CGovernanceObject
 -:  ---------- > 22:  da39b73f01 refactor: move GetListAtChainTip() calls out of CGovernanceVote
 -:  ---------- > 23:  055dbba1fa refactor: move GetListAtChainTip() calls out of llmq::utils::*, misc changes
 -:  ---------- > 24:  6bd23f40aa refactor: pass CDeterministicMNManager by ref to CGovernanceManager
 -:  ---------- > 25:  d731f4127e refactor: pass CDeterministicMNManager by ref to LLMQContext members
 -:  ---------- > 26:  e628d7517a refactor: pass CDeterministicMNManager by ref to CJContext members
 -:  ---------- > 27:  d4aa891735 refactor: more passing CDeterministicMNManager by ref
 -:  ---------- > 28:  f8d1853903 refactor: even more passing CDeterministicMNManager by ref
 -:  ---------- > 29:  e432122cbd net: Move CConnman/NetEventsInterface after CNode in header file
 -:  ---------- > 30:  140e91fdca merge bitcoin#20864: Move SocketSendData lock annotation to header
 -:  ---------- > 31:  fa6847d2fb refactor: drop circular dependencies over deterministicmns in validationinterface
 1:  8ff7971286 = 32:  03d7538acf Merge #19915: p2p, refactor: Use Mutex type for some mutexes in CNode class
 2:  0c99d0a1c8 = 33:  5924442462 Merge #19469: rpc: deprecate banscore field in getpeerinfo
 3:  a965e553f9 ! 34:  ff739237b2 Merge #19464: net: remove -banscore configuration option
    @@ src/test/denialofservice_tests.cpp: BOOST_AUTO_TEST_CASE(DoS_banning)
          peerLogic->Misbehaving(dummyNode2.GetId(), 50);
          {
     @@ src/test/denialofservice_tests.cpp: BOOST_AUTO_TEST_CASE(DoS_banscore)
    -                                        m_node.llmq_ctx, false);
    +                                        ::deterministicMNManager, m_node.cj_ctx, m_node.llmq_ctx, false);
      
          banman->ClearBanned();
     -    gArgs.ForceSetArg("-banscore", "111"); // because 11 is my favorite number
 4:  8575a6b4d5 ! 35:  cc41ffe65e Merge #19512: p2p: banscore updates to gui, tests, release notes
    @@ src/test/denialofservice_tests.cpp: BOOST_AUTO_TEST_CASE(DoS_banning)
     -    auto banman = std::make_unique<BanMan>(GetDataDir() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME);
     -    auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman);
     -    auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), *m_node.scheduler,
    --                                       *m_node.chainman, *m_node.mempool, *m_node.govman, *m_node.sporkman, m_node.cj_ctx,
    --                                       m_node.llmq_ctx, false);
    +-                                       *m_node.chainman, *m_node.mempool, *m_node.govman, *m_node.sporkman,
    +-                                       ::deterministicMNManager, m_node.cj_ctx, m_node.llmq_ctx, false);
     -
     -    banman->ClearBanned();
     -    CAddress addr1(ip(0xa0b0c001), NODE_NONE);
 5:  a39e85dc1c = 36:  6145ba1a07 Merge bitcoin/bitcoin#22495: p2p: refactor: tidy up `PeerManagerImpl::Misbehaving(...)`

@UdjinM6
Copy link

UdjinM6 commented Mar 22, 2024

rebased via GH to double-check CI

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

MarcoFalke and others added 5 commits March 22, 2024 11:08
…n CNode class

0e51a35 refactor: Use Mutex type for some mutexes in CNode class (Hennadii Stepanov)

Pull request description:

  No need the `RecursiveMutex` type for the `CNode::cs_vSend`, `CNode::cs_hSocket` and `CNode::cs_vRecv`.

  Related to bitcoin#19303.

ACKs for top commit:
  jnewbery:
    utACK 0e51a35
  MarcoFalke:
    review ACK 0e51a35 🔊

Tree-SHA512: 678ee5e3c15ad21a41cb86ec7179741bd505a138638fdc07f41d6d677c38fbf2208219bfc0509e3675e721fc8d8816e858070db7b87c5d72ad93aae81f7e1636
41d55d3 doc: getpeerinfo banscore deprecation release note (Jon Atack)
dd54e37 test: getpeerinfo banscore deprecation test (Jon Atack)
8c7647b rpc: deprecate banscore field in rpc getpeerinfo (Jon Atack)

Pull request description:

  Per bitcoin#19219 (comment) and bitcoin#19219 (comment), this PR deprecates returning the `banscore` field in the `getpeerinfo` RPC, updates the help, adds a test, and updates the release notes. Related to bitcoin#19464.

ACKs for top commit:
  fanquake:
    ACK 41d55d3

Tree-SHA512: 8eca08332581e2fe191a2aafff6ba89ce39413f0491ed0de8b86577739f0ec430b1a8fbff2914b0f3138a229563dfcc1981c0cf5b7dd6061b5c48680a28423bc
06059b0 net: rename DEFAULT_BANSCORE_THRESHOLD to DISCOURAGEMENT_THRESHOLD (Jon Atack)
1d4024b net: remove -banscore configuration option (Jon Atack)

Pull request description:

  per bitcoin#19219 (comment), bitcoin#19219 (comment) and bitcoin#19219 (comment). Edit: now split into 3 straightforward PRs:
  - net: remove -banscore configuration option (this PR)
  - rpc: deprecate banscore field in getpeerinfo (bitcoin#19469, *merged*)
  - gui: no longer display banscores (TBA in the gui repo)

ACKs for top commit:
  MarcoFalke:
    review ACK 06059b0 📙
  vasild:
    ACK 06059b0

Tree-SHA512: 03fad249986e0896697033fbb8ba2cbfaae7d7603b1fb2a38b3d41db697630d238623f4d732b9098c82af249ce5a1767dd432b7ca0fec10544e23d24fbd57c50
fa108d6 test: update tests for peer discouragement (Jon Atack)
1a9f462 gui, doc: rm Ban Score in GUI Peers window/release notes updates (Jon Atack)

Pull request description:

  This is the third `-banscore` PR in the mini-series described in bitcoin#19464. See that PR for the intention and reasoning.

  - no longer display "Ban Score" in the GUI peers window and add a release note, plus release note fixups per bitcoin#19464 (review)
  - update tests (`src/test/denialofservice_tests.cpp` and `test/functional/p2p_leak.py`) from banning to discouragement and per bitcoin#19464 (comment)

ACKs for top commit:
  jnewbery:
    ACK fa108d6
  laanwj:
    ACK fa108d6

Tree-SHA512: 58a449b3f47b8cb5490b34e4442ee8675bfad1ce48af4e4fd5c67715b0c1a596fb8e731d42e576b4c3b64627f76e0a68cbb1da9ea9f588a5932fe119baf40d50
…ving(...)`

8858e88 p2p: refactor: tidy up `PeerManagerImpl::Misbehaving(...)` (Sebastian Falbesoner)

Pull request description:

  This simple refactoring PR has the goal to improve the readability of the `Misbehaving` method by

  - introducing constant variables `score_before` and  `score_now` (to avoid repeatedly calculating the former)
  - deduplicating calls to LogPrint(), eliminates else-branch

ACKs for top commit:
  jnewbery:
    utACK 8858e88
  rajarshimaitra:
    tACK bitcoin@8858e88

Tree-SHA512: 1d4dd5ac1d16ee9595edf4fa46e4960915a203641d74e6c33cffaba62ea71328834309a4451256fb45daf759f0cf6f4f199c46815afff6c89c0746e2ad4d4092
@PastaPastaPasta PastaPastaPasta merged commit 2397209 into dashpay:develop Mar 22, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RPC Some notable changes to RPC params/behaviour/descriptions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants