Skip to content

fix: keeping reference to local variable in Coin Join unit tests #5174

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
PastaPastaPasta merged 1 commit into from
Jan 31, 2023
Merged

fix: keeping reference to local variable in Coin Join unit tests #5174

PastaPastaPasta merged 1 commit into from
Jan 31, 2023

Conversation

@knst
Copy link
Collaborator

@knst knst commented Jan 30, 2023

It become a visible UB (crash) after backport bitcoin#19848

What was done?

Increased life-term for object under reference

How Has This Been Tested?

Run unit/functional tests with and without bitcoin#19848

Breaking Changes

No breaking changes

Checklist:

  • I have performed a self-review of my own code
  • I have assigned this pull request to a milestone

@knst knst added this to the 19 milestone Jan 30, 2023
@knst knst mentioned this pull request Jan 30, 2023
Merged
2 tasks
PastaPastaPasta
PastaPastaPasta approved these changes Jan 31, 2023
View reviewed changes
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 for squash merge

UdjinM6
UdjinM6 approved these changes Jan 31, 2023
View reviewed changes
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

@PastaPastaPasta
Copy link
Member

PastaPastaPasta commented Jan 31, 2023 •
edited
Loading

Please justify why this is a backport candidate?

justification

It's backport candidate because there is UB in code and it's just a pure luck, that it is not noticeable till now yet.
But it can start to crash if moon will change phase or someone will use newer/older version compiler or non-standard flags or whatever. (edited)

@PastaPastaPasta PastaPastaPasta merged commit 9a03334 into dashpay:develop Jan 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@UdjinM6 UdjinM6 UdjinM6 approved these changes

@PastaPastaPasta PastaPastaPasta PastaPastaPasta approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

19

Development

Successfully merging this pull request may close these issues.

3 participants

@knst @PastaPastaPasta @UdjinM6