Skip to content

[23.x] GUI backports #24596

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
merged 4 commits into from
Mar 24, 2022
Merged

[23.x] GUI backports #24596

merged 4 commits into from
Mar 24, 2022

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Mar 17, 2022 •
edited
Loading

Sjors added 3 commits March 17, 2022 07:28
@Sjors @hebasto
move-only: helper function to present PSBT
fc421d4 
This commit does not change behavior.

Review hint:
git show --color-moved --color-moved-ws=allow-indentation-change

Github-Pull: bitcoin-core/gui#555
Rebased-From: 026b5b4
@Sjors @hebasto
refactor: helper function signWithExternalSigner()
9406946 
Does not change behavior.

Review hint:
git show --color-moved --color-moved-ws=allow-indentation-change

Github-Pull: bitcoin-core/gui#555
Rebased-From: 4b5a6cd
@Sjors @hebasto
gui: restore Send for external signer
642f272 
Before this change the send confirmation dialog would keep the Send option disabled. The Create Unsigned choice would actually send. This is potentially confusing.

With this change the Create Unsigned button will not attempt to sign and always produce a PSBT. The Send button will attempt to sign, and only return a PSBT if more signatures are needed.

When using an external signer, the Create Unsigned option only appears when PSBT controls are enabled in the wallet settings.

This commit maintains the pre-existing behavior of filling the PSBT (without signing) even when not using an external signer.

Closes bitcoin#551

Co-authored-by: Jon Atack <jon@atack.com>

Github-Pull: bitcoin-core/gui#555
Rebased-From: 2efdfb8
@hebasto hebasto added this to the 23.0 milestone Mar 17, 2022
@hebasto hebasto mentioned this pull request Mar 17, 2022
Merged
@Sjors
Copy link
Member

Sjors commented Mar 17, 2022

ACK 642f272

@gruve-p
Copy link
Contributor

gruve-p commented Mar 17, 2022

ACK 642f272

@luke-jr
Copy link
Member

luke-jr commented Mar 17, 2022

Approach NACK. This should just be a merge of the exact same commits/PR; it doesn't need a separate cherry-pick.

@hebasto
Copy link
Member Author

hebasto commented Mar 17, 2022

Approach NACK. This should just be a merge of the exact same commits/PR; it doesn't need a separate cherry-pick.

As usual, the backport.py script was used to prepare this PR.

@Sjors
Copy link
Member

Sjors commented Mar 17, 2022

I guess a new merge commit, rather than cherry-picking, has the benefit of preserving my commit signatures.

I don't know how easy it is to implement that in the backport script. I could also make a cherry-pick PR myself :-)

@luke-jr
Copy link
Member

luke-jr commented Mar 17, 2022

As usual, the backport.py script was used to prepare this PR.

Point is, that's not needed (and therefore shouldn't be used) for this PR.

@vasild @hebasto
options: flip listenonion to false if not listening
70f2c57 
If the user has unchecked "Allow incoming connections" in
`Settings->Options...->Network` then `fListen=false` is saved in
`~/.config/Bitcoin/Bitcoin-Qt.conf`. This flips `-listen` to `false`
during startup, but leaves `-listenonion` to `true`.

This flipping of `-listen` is done in `OptionsModel::Init()` after
`InitParameterInteraction()` has been executed which would have flipped
`-listenonion`, should it have seen `-listen` being `false`
(this is a difference between `bitcoind` and `bitcoin-qt`).

Fixes: bitcoin-core/gui#567

Github-Pull: bitcoin-core/gui#568
Rebased-From: 7f90dc2
@hebasto
Copy link
Member Author

hebasto commented Mar 23, 2022 •
edited
Loading

Added backport of bitcoin-core/gui#568.

@hebasto hebasto changed the title [23.x] GUI backport [23.x] GUI backports Mar 23, 2022
@hebasto hebasto mentioned this pull request Mar 23, 2022
Merged
@fanquake
Copy link
Member

@Sjors @gruve-p @vasild @mzumsande want to (re)-review so we can get these couple of backports in.

@Sjors
Copy link
Member

Sjors commented Mar 23, 2022

utACK 70f2c57

@gruve-p
Copy link
Contributor

gruve-p commented Mar 23, 2022

ACK 70f2c57

@maflcko maflcko merged commit 7d03cf6 into bitcoin:23.x Mar 24, 2022
@hebasto hebasto deleted the 220317-23.x-backport branch March 24, 2022 07:14
@maflcko
Copy link
Member

maflcko commented Mar 24, 2022

I can see why it is minimally cleaner (from a git perspective) to not cherry-pick unless required, but I don't think this is reason enough to NACK this pull, since there won't be any difference for end-users (except for the git hash that is embedded in the binary).

@vasild
Copy link
Contributor

vasild commented Mar 24, 2022

ACK 70f2c57

@luke-jr, how could this be a merge instead of cherry-picks? I do not think this is possible, but maybe I am missing something. What exact git commands would create such a merge? There are intermediate, undesired, commits in master, a merge would drag them into 23.x too.

@maflcko
Copy link
Member

maflcko commented Mar 24, 2022

I think this would only be possible for pull requests which happen to be (or are intentionally) based on any commit pre-branch-off.

@bitcoin bitcoin locked and limited conversation to collaborators Mar 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
Backport GUI
Projects
None yet
Milestone
23.0
Development

Successfully merging this pull request may close these issues.
7 participants
@hebasto @Sjors @gruve-p @luke-jr @fanquake @maflcko @vasild