Skip to content

Conversation

fanquake
Copy link
Member

@fanquake fanquake commented Nov 1, 2023

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 1, 2023

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK stickies-v

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

fanquake and others added 2 commits December 11, 2023 15:51
This commits fixes a crash bug that can be caused with the following steps:
- change to the "Transactions" view
- right-click on an arbitrary transaction -> "Show transaction details"
- close the transaction detail window again
- select "Settings" -> "Mask values"

The problem is that the list of opened dialogs, tracked in the member
variable `m_opened_dialogs`, is only ever appended with newly opened
transaction detail dialog pointers, but never removed. This leads to
dangling pointers in the list, and if the "Mask values" menu item is
selected, a crash is caused in the course of trying to close the opened
transaction detail dialogs (see `closeOpenedDialogs()` method). Fix this
by removing the pointer from the list if the corresponding widget is
destroyed.

Github-Pull: bitcoin-core/gui#774
Rebased-From: e26e665
@fanquake fanquake marked this pull request as ready for review January 11, 2024 15:45
@fanquake fanquake requested a review from stickies-v January 12, 2024 11:02
The crash would happen when querying a mempool transaction with verbosity=2, while pruning.

Github-Pull: bitcoin#29003
Rebased-From: 494a926
This fails on master without the previous commit.

Github-Pull: bitcoin#29003
Rebased-From: 9075a44
Copy link
Contributor

@stickies-v stickies-v left a comment

Choose a reason for hiding this comment

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

LGTM 115e31b modulo small release notes issue. Also, PR description needs to be updated to remove the reference to #28919.

@jonatack
Copy link
Member

#29200 would be relevant.

@fanquake
Copy link
Member Author

Also, PR description needs to be updated to remove the reference to #28919.

Updated.

#29200 would be relevant.

There'll be another 25.x backports PR, so it could be included there.

Copy link
Contributor

@stickies-v stickies-v left a comment

Choose a reason for hiding this comment

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

ACK 53bbda5

@fanquake fanquake merged commit 8087626 into bitcoin:25.x Jan 16, 2024
@fanquake fanquake deleted the backports_25_2 branch January 16, 2024 11:23
@bitcoin bitcoin locked and limited conversation to collaborators Jan 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants