merge #17812, #16702, #16730, #18023: supplying and using asmap to improve IP bucketing in addrman by kwvg · Pull Request #4028 · dashpay/dash

kwvg · 2021-03-09T15:47:36Z

Overview

As part of #4025, component pull requests that are required by TorV3 logic are being broken down into branches wherever feasable. This PR deals with asmap related address logic.

Merge p2p: supplying and using asmap to improve IP bucketing in addrman bitcoin/bitcoin#16702 ("p2p: supplying and using asmap to improve IP bucketing in addrman" by naumenkogs)
Merge Support serialization of std::vector<bool> bitcoin/bitcoin#16730 ("Support serialization of std::vector" by sipa)
Merge config, net, test: asmap feature refinements and functional tests bitcoin/bitcoin#17812 ("config, net, test: asmap feature refinements and functional tests" by fanquake)

Disclosures

Dash-specific changes may not have been tested, only compilation has been ensured. Running the client on (a) testnet(s) may be necessary.
Contains squashed patches bb8958d, 7d2eb46d97bf4ffe88dc97324d5f27de51c2ddaf and 9ad5d1d from UdjinM6

UdjinM6

Thanks! 👍

Looks mostly ok, see below

kwvg · 2021-03-09T18:20:59Z

Certain addrman_tests are failing as of now

error: in "addrman_tests/addrman_tried_collisions": check addrman.size() == i has failed [69 != 73]
error: in "addrman_tests/addrman_evictionworks": check addrman.SelectTriedCollision().ToString() == "[::]:0" has failed
error: in "addrman_tests/addrman_noevict": check addrman.SelectTriedCollision().ToString() == "[::]:0" has failed

UdjinM6 · 2021-03-09T18:51:44Z

Looks like we also need bitcoin#16730 for asmap serialization to work properly.

kwvg · 2021-03-09T18:57:42Z

I've already added it in #4025 at cb608de, it will be merged along with other serialization updates. Right now I'm just splitting the PRs one at a time, I've bunged up the commit order in the original PR, it'll be a little while to sort it out

UdjinM6 · 2021-03-09T19:04:41Z

I mean, 16702 simply doesn't work without 16730.

kwvg · 2021-03-09T19:05:57Z

It doesn't. The PRs aren't in order right now. I'm currently working on it so that it's in the correct order. Just splitting them so that they can be (relatively) atomically reviewed.

EDIT: Nevermind, bitcoin#16730 is a direct prerequisite, resolved in 688985b

UdjinM6 · 2021-03-09T19:17:56Z

Looks good now imo 👍 Let's wait for Gitlab to confirm.

UdjinM6

Seems to be working as expected, slightly tested ACK

xdustinface

Looks good overall 👍 Just two points, see below

PastaPastaPasta · 2021-03-12T19:39:34Z

Note that tests are failing

xdustinface · 2021-03-13T10:36:23Z

@kittywhiskers The last force push moved GetGroup again to the wrong place, no? Was this for some reason or mistake? Also, are you going to add bitcoin#18023 here?

kwvg · 2021-05-05T22:36:23Z

The asmap branch should now be fit for review

This didn't age well... sigh, I'll give it a look

PastaPastaPasta · 2021-05-19T14:26:13Z

You have a linter failure

UdjinM6 · 2021-05-19T14:31:11Z

Pls see bb8958d129c90262030683d8fae9f5ac403627de and 9ad5d1d3b90e84563822f7bd7d4ca604df9fa02c

kwvg · 2021-05-19T18:02:33Z

@UdjinM6, integrated in 067aa67a9d

UdjinM6

LGTM, utACK

PastaPastaPasta

utACK

kwvg mentioned this pull request Mar 9, 2021

partial #8149 and merge #19845, #19954: Complete the BIP155 implementation and upgrade to TORv3 #4025

Closed

UdjinM6 requested changes Mar 9, 2021

View reviewed changes

kwvg force-pushed the asmap branch from f52370a to 4cc529d Compare March 9, 2021 17:46

UdjinM6 requested changes Mar 9, 2021

View reviewed changes

Comment thread test/functional/feature_asmap.py Outdated

Comment thread test/functional/feature_asmap.py Outdated

kwvg force-pushed the asmap branch from 4cc529d to cdf7bab Compare March 9, 2021 18:10

kwvg requested a review from UdjinM6 March 9, 2021 18:13

kwvg force-pushed the asmap branch from cdf7bab to 2c07fb4 Compare March 9, 2021 18:23

kwvg force-pushed the asmap branch from 2c07fb4 to 7c97098 Compare March 9, 2021 19:11

UdjinM6 changed the title ~~merge #17812, #16702: supplying and using asmap to improve IP bucketing in addrman~~ merge #17812, #16702, #16730: supplying and using asmap to improve IP bucketing in addrman Mar 9, 2021

kwvg force-pushed the asmap branch from 7c97098 to 0b1280d Compare March 9, 2021 19:14

UdjinM6 added this to the 17 milestone Mar 9, 2021

UdjinM6 previously approved these changes Mar 10, 2021

View reviewed changes

UdjinM6 requested review from PastaPastaPasta and xdustinface March 10, 2021 23:48

xdustinface requested changes Mar 12, 2021

View reviewed changes

Comment thread src/init.cpp Outdated

Comment thread src/netaddress.h Outdated

Comment thread src/netaddress.h Outdated

kwvg dismissed UdjinM6’s stale review via e07aeab March 12, 2021 17:19

kwvg force-pushed the asmap branch 3 times, most recently from 3e9d088 to d2901c2 Compare March 12, 2021 17:44

kwvg requested a review from xdustinface March 12, 2021 17:44

kwvg force-pushed the asmap branch from d2901c2 to 0b1280d Compare March 13, 2021 04:55

kwvg force-pushed the asmap branch from 0b1280d to 9be6d8a Compare March 14, 2021 07:54

kwvg force-pushed the asmap branch from 550e63c to 74dee9e Compare May 11, 2021 11:33

kwvg requested a review from PastaPastaPasta May 11, 2021 11:34

kwvg force-pushed the asmap branch 2 times, most recently from cc80d54 to d9ef41b Compare May 11, 2021 15:11

PastaPastaPasta reviewed May 15, 2021

View reviewed changes

Comment thread src/init.cpp Outdated

PastaPastaPasta reviewed May 15, 2021

View reviewed changes

Comment thread src/netaddress.cpp Outdated

PastaPastaPasta reviewed May 15, 2021

View reviewed changes

Comment thread src/netaddress.h Outdated

PastaPastaPasta reviewed May 15, 2021

View reviewed changes

Comment thread test/functional/feature_asmap.py Outdated

kwvg force-pushed the asmap branch from d9ef41b to 734024d Compare May 18, 2021 16:51

Merge bitcoin#16730: Support serialization of std::vector<bool>

6a25151

kwvg force-pushed the asmap branch from 734024d to 68aceda Compare May 18, 2021 17:02

kwvg requested a review from PastaPastaPasta May 19, 2021 02:47

kwvg mentioned this pull request May 19, 2021

Merge #19660, #19373, #19841, #13862, #13866, #17280, #17682 and partial #19326, #14978: Auxiliary Backports #4164

Merged

kwvg force-pushed the asmap branch from 68aceda to 354a4b1 Compare May 19, 2021 18:02

kwvg requested a review from UdjinM6 May 19, 2021 18:07

UdjinM6 reviewed May 19, 2021

View reviewed changes

Comment thread src/Makefile.test.include Outdated

kwvg added 3 commits May 20, 2021 00:05

Merge bitcoin#16702: supplying and using asmap to improve IP bucketing

4b2b5f7

Merge bitcoin#17812: asmap feature refinements and functional tests

4010982

Merge bitcoin#18023: Fix some asmap issues

f3819c4

kwvg force-pushed the asmap branch from 354a4b1 to f3819c4 Compare May 19, 2021 18:35

kwvg requested a review from UdjinM6 May 19, 2021 18:36

UdjinM6 approved these changes May 19, 2021

View reviewed changes

PastaPastaPasta approved these changes May 19, 2021

View reviewed changes

PastaPastaPasta merged commit 2f555f8 into dashpay:develop May 19, 2021

kwvg mentioned this pull request May 20, 2021

Merge #19628, #19687: change CNetAddr::ip to have flexible size #4166

Merged

kwvg deleted the asmap branch July 18, 2023 11:38

Uh oh!

Conversation

kwvg commented Mar 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Contents

Disclosures

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kwvg commented Mar 9, 2021

Uh oh!

UdjinM6 commented Mar 9, 2021

Uh oh!

kwvg commented Mar 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

UdjinM6 commented Mar 9, 2021

Uh oh!

kwvg commented Mar 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

UdjinM6 commented Mar 9, 2021

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

xdustinface left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PastaPastaPasta commented Mar 12, 2021

Uh oh!

xdustinface commented Mar 13, 2021

Uh oh!

kwvg commented May 5, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PastaPastaPasta commented May 19, 2021

Uh oh!

UdjinM6 commented May 19, 2021

Uh oh!

kwvg commented May 19, 2021

Uh oh!

Uh oh!

UdjinM6 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 Mar 9, 2021 •

edited

Loading

kwvg commented Mar 9, 2021 •

edited

Loading

kwvg commented Mar 9, 2021 •

edited

Loading