Skip to content

[0.21] test: Backports #21614

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.

Sign up for GitHub

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

Jump to bottom
Merged
fanquake merged 4 commits into from
Apr 16, 2021
Merged

[0.21] test: Backports #21614

fanquake merged 4 commits into from
Apr 16, 2021

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Apr 6, 2021

Backport tests

@fanquake fanquake added this to the 0.21.1 milestone Apr 6, 2021
@fanquake
Copy link
Member

fanquake commented Apr 6, 2021

Should/can this include a backport for #21489?

This was referenced Apr 6, 2021
Merged
Merged
@maflcko
Copy link
Member Author

maflcko commented Apr 6, 2021

Good idea. Done

ajtowns
ajtowns reviewed Apr 7, 2021
View reviewed changes
src/test/fuzz/versionbits.cpp Outdated
Copy link
Contributor

@ajtowns ajtowns Apr 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This } // namespace line needs to be removed for the backport.

(EDIT: the patch that's being backported moved it lower from earlier in the file, but the previous pr that was backported has moved that same line earlier in the file. git just sees the original line being deleted in both patches, and considers it redundant and fine to omit, rather than marking it as a conflict)

Copy link
Member Author

@maflcko maflcko Apr 7, 2021

Choose a reason for hiding this comment

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

Correct. Fixed.

@maflcko maflcko force-pushed the 2104-21Backports branch from 7ed4ddb to 4546ac5 Compare April 7, 2021 05:26
@ajtowns ajtowns mentioned this pull request Apr 7, 2021
Merged
@fanquake fanquake requested a review from vasild April 16, 2021 00:32
@vasild
Copy link
Contributor

vasild commented Apr 16, 2021

ACK 4546ac5af1c4105a0cf78a407fefcb189822bcf5

I verified that the corresponding commits are same in master and in the backport (modulo mechanical diffs due to diverged code bases).

@maflcko maflcko modified the milestones: 0.21.1, 0.21.2 Apr 16, 2021
@maflcko
Copy link
Member Author

maflcko commented Apr 16, 2021

Moved milestone due to lack of interest

@jnewbery
Copy link
Contributor

jnewbery commented Apr 16, 2021

ACK 4546ac5af1c4105a0cf78a407fefcb189822bcf5

Verified cherrypick/rebase.

vasild and others added 4 commits April 16, 2021 11:20
@vasild
test: use pointers in denialofservice_tests/peer_discouragement
dfeb6c1 
This is a non-functional change that replaces the `CNode` on-stack
variables with `CNode` pointers.

The reason for this is that it would allow us to add those `CNode`s
to `CConnman::vNodes[]` which in turn would allow us to check that they
are disconnected properly - a `CNode` object must be in
`CConnman::vNodes[]` in order for its `fDisconnect` flag to be set.

If we store pointers to the on-stack variables in `CConnman` then it
would crash at the end, trying to `delete` them.

Github-Pull: bitcoin#21571
Rebased-From: 4d6e246
@vasild
test: also check disconnect in denialofservice_tests/peer_discouragement
b765f41 
Use `CConnmanTest` instead of `CConnman` and add the nodes to it
so that their `fDisconnect` flag is set during disconnection.

Github-Pull: bitcoin#21571
Rebased-From: 637bb6d
@vasild
test: make sure non-IP peers get discouraged and disconnected
79cdb4a 
Github-Pull: bitcoin#21571
Rebased-From: 81747b2
@ajtowns
fuzz: cleanups for versionbits fuzzer
b8af67e 
Github-Pull: bitcoin#21489
Rebased-From: aa7f418
@maflcko
Copy link
Member Author

maflcko commented Apr 16, 2021

Rebased

@jnewbery
Copy link
Contributor

jnewbery commented Apr 16, 2021

ACK b8af67e

Verified rebase.

@ajtowns
Copy link
Contributor

ajtowns commented Apr 16, 2021

ACK b8af67e ; visually compared individual commits to originals, checked original commits are in master

vasild
vasild approved these changes Apr 16, 2021
View reviewed changes
Copy link
Contributor

@vasild vasild left a comment

Choose a reason for hiding this comment

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

ACK b8af67e

Part of the last commit is already applied via b529222.

@fanquake fanquake modified the milestones: 0.21.2, 0.21.1 Apr 16, 2021
@fanquake fanquake merged commit f8bbee4 into bitcoin:0.21 Apr 16, 2021
@maflcko maflcko deleted the 2104-21Backports branch April 17, 2021 05:56
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@ajtowns ajtowns ajtowns left review comments

@vasild vasild vasild approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Backport

Projects

None yet

Milestone

0.21.1

Development

Successfully merging this pull request may close these issues.

5 participants

@maflcko @fanquake @vasild @jnewbery @ajtowns