backport: merge bitcoin#20222, #18766, #14033, #19486, #19607, #20291, partial #20187, #20228 (deglobalize addrman and feeEstimator) #5163

kwvg · 2023-01-17T13:30:33Z

Depends on #5169

github-actions · 2023-01-19T10:11:09Z

This pull request has conflicts, please rebase.

github-actions · 2023-02-06T19:59:46Z

This pull request has conflicts, please rebase.

knst

20222 - seems as there is a mixture of changes from 20222 and from 18766.
Better to do as spare commit. So far it looks for me as a merge/conflict mistake when 18766 and 20222 squashed together by mistakes.

~~20187 - seems as big missing chunk (should be marked partial then):~~

        if (!pfrom.IsInboundConn()) {
            // For non-inbound connections, we update the addrman to record
            // connection success so that addrman will have an up-to-date
            // notion of which peers are online and available.
            //
            // While we strive to not leak information about block-relay-only
            // connections via the addrman, not moving an address to the tried
            // table is also potentially detrimental because new-table entries
            // are subject to eviction in the event of addrman collisions.  We
            // mitigate the information-leak by never calling
            // CAddrMan::Connected() on block-relay-only peers; see
            // FinalizeNode().
            //
            // This moves an address from New to Tried table in Addrman,
            // resolves tried-table collisions, etc.

I recommend to do bitcoin#19724 (and may be some others related) prior this backport 20187.
Merge bitcoin#19724: [net] Cleanup connection types- followups

bitcoin#20228 - partial because missing changes from rpc/net.cpp
That rpc would be introduced only in
Merge bitcoin#19658: [rpc] Allow RPC to fetch all addrman records and add records to addrman

Also after bitcoin#20828 would be much easier to backport src/test/fuzz/connman.cpp

I feel like these PR is ahead of time ATM

src/init.cpp

test/functional/feature_fee_estimation.py

src/net_processing.cpp

knst · 2023-02-17T09:31:11Z

src/test/fuzz/connman.cpp

        case 0:
-            random_address = ConsumeAddress(fuzzed_data_provider);
-            break;
-        case 1:


nit: may be better to do bitcoin#20828 before for simplicity?
Merge bitcoin#20828: fuzz: Introduce CallOneOf helper to replace switch-case

i remember backporting bitcoin#20828 and for some reason it caused more problems than it solved, forcing me to manually decrement the cases, which was annoying to work with.

github-actions · 2023-02-20T10:14:35Z

This pull request has conflicts, please rebase.

knst

utACK

src/init.cpp

PastaPastaPasta

utACK for merging via merge commit

UdjinM6

pls see below

src/init.cpp

src/net_processing.cpp

UdjinM6

utACK

knst

LGTM

PastaPastaPasta

utACK for merging via merge commit

…moving the fee estimates global)

…_PROTO_VERSION is greater

…ETHEADERS_VERSION

…block-relay-only peers Contains only daf5553

…cted()

…er pointers, check before `queueman` access, update P2P message map in tests 24f9357 fix: update P2P message map in functional test framework (Kittywhiskers Van Gogh) a432a95 fix: check if `queueman` is initialized before accessing it (Kittywhiskers Van Gogh) 0444e59 trivial: use `std::atomic` to protect connected manager pointers (Kittywhiskers Van Gogh) b7700b3 trivial: use `std::atomic` to protect connected signer pointers (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Dependency for #6838 This pull request contains the following: * Minor follow-up to [dash#6828](#6828) based on feedback received during review also extended to similar connections introduced in [dash#5980](#5980) ([commit](a5be37c#diff-c065d4cd2398ad0dbcef393c5dfc53f465bf44723348892395fffd2fb3bac522R350-R355)) and [dash#6030](#6030) ([commit](805537e#diff-59f8e9f1b35c1428332caab753a818e3b2146e73bb6c998a2aed5f7eddbc6fa1R357-R363)) * A bugfix to avoid potential crash caused by missing `nullptr` check after `CCoinJoinClientQueueManager` was conditionally initialized in [dash#5163](#5163) ([commit](2d2814e#diff-b1e19192258d83199d8adaa5ac31f067af98f63554bfdd679bd8e8073815e69dR2308-R2310)) * Updating the Python mapping of Dash-specific P2P messages to avoid unexpected test failures ([build](https://github.com/dashpay/dash/actions/runs/17707917238/job/50323243018#step:6:328)), observed when working on [dash#6838](#6838) ## 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 **(note: N/A)** - [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 24f9357 Tree-SHA512: 90b0b2536a7704e3f3da4ece05b6ad09c393a4348aaff87682b7547f6a7d6ffede504176fa1f63bd9ad88961fb1e3b113aae5df357c0dfb70df2fc55500c2b5f

kwvg force-pushed the mempool_deglob branch from 3d84d7b to 8814e13 Compare January 17, 2023 14:04

kwvg marked this pull request as draft January 17, 2023 14:08

kwvg force-pushed the mempool_deglob branch from 8814e13 to 884b1e1 Compare January 19, 2023 10:10

github-actions bot added the needs rebase label Jan 19, 2023

kwvg changed the title ~~backport: merge bitcoin#19826, #19848, #14384, #19556, #20228, #20222, #18766, #14033, partial #18861, #20187 (deglobalize mempool and feeEstimator)~~ backport: merge bitcoin#20222, #20228, #14033, partial #20187 (deglobalize addrman and feeEstimator) Jan 19, 2023

kwvg force-pushed the mempool_deglob branch from 884b1e1 to 12d6d8c Compare January 19, 2023 11:00

kwvg force-pushed the mempool_deglob branch from 12d6d8c to c20573f Compare February 5, 2023 10:56

github-actions bot removed the needs rebase label Feb 5, 2023

kwvg force-pushed the mempool_deglob branch from c20573f to cd69258 Compare February 5, 2023 11:48

github-actions bot added the needs rebase label Feb 6, 2023

kwvg force-pushed the mempool_deglob branch from cd69258 to 54feb29 Compare February 15, 2023 20:20

github-actions bot removed the needs rebase label Feb 15, 2023

kwvg force-pushed the mempool_deglob branch 5 times, most recently from 4c0f19f to 9dde804 Compare February 16, 2023 18:50

kwvg marked this pull request as ready for review February 17, 2023 06:24

kwvg requested review from PastaPastaPasta, UdjinM6 and knst February 17, 2023 06:25

knst requested changes Feb 17, 2023

View reviewed changes

kwvg force-pushed the mempool_deglob branch from 9dde804 to 574682d Compare February 18, 2023 08:32

kwvg requested a review from knst February 18, 2023 10:12

github-actions bot added the needs rebase label Feb 20, 2023

knst previously approved these changes Feb 20, 2023

View reviewed changes

src/init.cpp Outdated Show resolved Hide resolved

PastaPastaPasta previously approved these changes Feb 21, 2023

View reviewed changes

UdjinM6 requested changes Feb 21, 2023

View reviewed changes

src/init.cpp Outdated Show resolved Hide resolved

src/net_processing.cpp Show resolved Hide resolved

src/net_processing.cpp Outdated Show resolved Hide resolved

src/net_processing.cpp Outdated Show resolved Hide resolved

src/net_processing.cpp Outdated Show resolved Hide resolved

kwvg dismissed stale reviews from PastaPastaPasta and knst via 30a4993 February 22, 2023 08:09

kwvg force-pushed the mempool_deglob branch from a2ae17f to 30a4993 Compare February 22, 2023 08:09

kwvg changed the title ~~backport: merge bitcoin#20222, #20228, #14033, partial #20187 (deglobalize addrman and feeEstimator)~~ backport: merge bitcoin#20222, #18766, #14033, #19486, #19607, #20291, partial #20187, #20228 (deglobalize addrman and feeEstimator) Feb 22, 2023

kwvg requested review from PastaPastaPasta, UdjinM6 and knst February 22, 2023 16:13

UdjinM6 previously approved these changes Feb 23, 2023

View reviewed changes

knst previously approved these changes Feb 24, 2023

View reviewed changes

PastaPastaPasta previously approved these changes Feb 27, 2023

View reviewed changes

PastaPastaPasta force-pushed the mempool_deglob branch from 30a4993 to f0c4cc6 Compare February 27, 2023 15:16

PastaPastaPasta dismissed stale reviews from knst, UdjinM6, and themself via f0c4cc6 February 27, 2023 17:50

kwvg added 9 commits February 28, 2023 00:11

refactor: replace references to feeEstimator global with references

7ca8214

merge bitcoin#20222: CTxMempool constructor clean up

a5f2012

merge bitcoin#18766: disable fee estimation in blocksonly mode (by re…

2d2814e

…moving the fee estimates global)

merge bitcoin#14033: Drop CADDR_TIME_VERSION checks now that MIN_PEER…

cca796a

…_PROTO_VERSION is greater

merge bitcoin#19486: Remove unused constants CADDR_TIME_VERSION and G…

b513441

…ETHEADERS_VERSION

partial bitcoin#20187: test-before-evict bugfix and improvements for …

698a717

…block-relay-only peers Contains only daf5553

merge bitcoin#19607: Add Peer struct for per-peer data in net processing

07fe6d4

merge bitcoin#20291: Consolidate logic around calling CAddrMan::Conne…

3e993ab

…cted()

partial bitcoin#20228: Make addrman a top-level component

40906b2

UdjinM6 force-pushed the mempool_deglob branch from f0c4cc6 to 40906b2 Compare February 27, 2023 21:11

UdjinM6 added this to the 20 milestone Feb 27, 2023

UdjinM6 merged commit 7e57ae6 into dashpay:develop Feb 27, 2023

kwvg mentioned this pull request Sep 9, 2025

fix: use std::atomic to protect connected manager/signer pointers, check before queueman access, update P2P message map in tests #6842

Merged

5 tasks

backport: merge bitcoin#20222, #18766, #14033, #19486, #19607, #20291, partial #20187, #20228 (deglobalize addrman and feeEstimator) #5163

backport: merge bitcoin#20222, #18766, #14033, #19486, #19607, #20291, partial #20187, #20228 (deglobalize addrman and feeEstimator) #5163

Uh oh!

Conversation

kwvg commented Jan 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 19, 2023

Uh oh!

github-actions bot commented Feb 6, 2023

Uh oh!

knst left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

knst Feb 17, 2023

Choose a reason for hiding this comment

Uh oh!

kwvg Feb 17, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 20, 2023

Uh oh!

knst left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PastaPastaPasta left a comment

Choose a reason for hiding this comment

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

knst left a comment

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kwvg commented Jan 17, 2023 •

edited

Loading

knst left a comment •

edited

Loading