Skip to content

Conversation

achow101
Copy link
Member

Instead of changing some actions' behavior based on IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS), make separate things that work with no private keys. In particular, instead of having bumpfee give out a psbt when there are no private keys, add a psbtbumpfee RPC that always gives out a psbt and just have bumpfee be disabled when there are no private keys. This is mirrored in the GUI bumpfee menu items. Additionally, instead of changing the Send button to Create Unsigned when there are no private keys, just always have a Create Unsigned button and disable Send when there are no private keys. To deal with bumpfee already doing the mutated behavior thing, that behavior is hidden behind a -deprecatedrpc=bumpfee option.

To make the GUI stuff easier to follow, test, and review, this is being based on #17509

Sjors and others added 10 commits April 13, 2020 15:09
This commit does not change behavior.
co-authored-by: Glenn Willen <gwillen@nerdnet.org>
co-authored-by: Glenn Willen <gwillen@nerdnet.org>
Instead of changing Send to make an unsigned tx for wallets with
private keys disabled, have a separate button for that functionality
and disable Send for such wallets.
Instead of changing GUI bumpfee behavior based on private keys,
make a separate menu action to do that.
@DrahtBot
Copy link
Contributor

DrahtBot commented Apr 13, 2020

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@instagibbs
Copy link
Member

concept and light GUI tACK 11f2521

  1. Still means that if someone imports a privkey in the future to a multisig descriptor wallet, the "send" button will be clickable. Not sure how to avoid this aside from explicitly explicitly knowing that it has all pieces of data to complete any transaction it creates.
  2. There's no offer to write the PSBT to disk in the GUI for bump fee, just clipboard
  3. Maybe the PSBT options should be gated behind an Expert option in Options for the GUI?

@jb55
Copy link
Contributor

jb55 commented Apr 14, 2020

Concept ACK

With psbtbumpfee, we can deprecate bumpfee's psbt creation behavior.
So put that behind a -deprecatedrpc
@jonasschnelli
Copy link
Contributor

Concept ACK
Would it be possible to split this into n smaller PRs?

@achow101
Copy link
Member Author

This has been split into #18654 (psbtbumpfee RPC), #18655 (bumpFeePSBT action), and #18656 (make unsigned button).

@achow101 achow101 closed this Apr 15, 2020
meshcollider added a commit that referenced this pull request Aug 13, 2020
…tbumpfee

79d6332 moveonly: Fix indentation in bumpfee RPC (Andrew Chow)
431071c Hide bumpfee's psbt creation behavior behind -deprecatedrpc (Andrew Chow)
4638224 Add psbtbumpfee RPC (Andrew Chow)

Pull request description:

  Adds a new RPC `psbtbumpfee` which always creates a psbt. `bumpfee` will then only be able to create and broadcast fee bumping transactions instead of changing its behavior based on `IsWalletSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)`.

  Split from #18627

ACKs for top commit:
  Sjors:
    re-utACK 79d6332
  meshcollider:
    utACK 79d6332
  fjahr:
    Code review ACK 79d6332

Tree-SHA512: 1c92c4b4461bb30e78be3ee73165f624398ef33996ce36043b61a8931be667030d0fca12fd0b30097b78c56e4e9092c69582b237cbdac51d56f6be23d8c0f1bb
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 14, 2020
…nto psbtbumpfee

79d6332 moveonly: Fix indentation in bumpfee RPC (Andrew Chow)
431071c Hide bumpfee's psbt creation behavior behind -deprecatedrpc (Andrew Chow)
4638224 Add psbtbumpfee RPC (Andrew Chow)

Pull request description:

  Adds a new RPC `psbtbumpfee` which always creates a psbt. `bumpfee` will then only be able to create and broadcast fee bumping transactions instead of changing its behavior based on `IsWalletSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)`.

  Split from bitcoin#18627

ACKs for top commit:
  Sjors:
    re-utACK 79d6332
  meshcollider:
    utACK 79d6332
  fjahr:
    Code review ACK 79d6332

Tree-SHA512: 1c92c4b4461bb30e78be3ee73165f624398ef33996ce36043b61a8931be667030d0fca12fd0b30097b78c56e4e9092c69582b237cbdac51d56f6be23d8c0f1bb
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
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