Skip to content
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.

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

wallet: load flags before everything else #21127

Merged
merged 1 commit into from
Feb 13, 2021

Conversation

Sjors
Copy link
Member

@Sjors Sjors commented Feb 9, 2021

Load and set wallet flags before processing other records. That way we can take them into account while processing those other records.

Suggested here:
#16546 (comment)

@achow101
Copy link
Member

achow101 commented Feb 9, 2021

ACK afdfb1d

@Sjors
Copy link
Member Author

Sjors commented Feb 9, 2021

Appeased linter.

@laanwj
Copy link
Member

laanwj commented Feb 10, 2021

Code review ACK 9305862

@Sjors
Copy link
Member Author

Sjors commented Feb 12, 2021

@instagibbs?

@gruve-p
Copy link
Contributor

gruve-p commented Feb 12, 2021

ACK 9305862

@achow101
Copy link
Member

ACK 9305862

@laanwj laanwj merged commit 43981ee into bitcoin:master Feb 13, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 13, 2021
9305862 wallet: load flags before everything else (Sjors Provoost)

Pull request description:

  Load and set wallet flags before processing other records. That way we can take them into account while processing those other records.

  Suggested here:
  bitcoin#16546 (comment)

ACKs for top commit:
  laanwj:
    Code review ACK 9305862
  gruve-p:
    ACK bitcoin@9305862
  achow101:
    ACK 9305862

Tree-SHA512: 7104523e369ce3c670571fe5e8b52c67b9ca92b8e36a2da5eb6f9f8bf8ed0544897007257204b68f6f371d682b3ef0d0635d36e6e8416ac74af1999d9fbc869c
@Sjors Sjors deleted the 2021/02/wallet_flags branch February 14, 2021 14:24
laanwj added a commit that referenced this pull request Feb 23, 2021
f75e0c1 doc: add external-signer.md (Sjors Provoost)
d4b0107 rpc: send: support external signer (Sjors Provoost)
245b445 rpc: signerdisplayaddress (Sjors Provoost)
7ebc7c0 wallet: ExternalSigner: add GetDescriptors method (Sjors Provoost)
fc5da52 wallet: add GetExternalSigner() (Sjors Provoost)
259f52c test: external_signer wallet flag is immutable (Sjors Provoost)
2655197 rpc: add external_signer option to createwallet (Sjors Provoost)
2700f09 rpc: signer: add enumeratesigners to list external signers (Sjors Provoost)
07b7c94 rpc: add external signer RPC files (Sjors Provoost)
8ce7767 wallet: add ExternalSignerScriptPubKeyMan (Sjors Provoost)
157ea7c wallet: add external_signer flag (Sjors Provoost)
f3e6ce7 test: add external signer test (Sjors Provoost)
8cf543f wallet: add -signer argument for external signer command (Sjors Provoost)
f7eb7ec test: framework: add skip_if_no_external_signer (Sjors Provoost)
87a9794 configure: add --enable-external-signer (Sjors Provoost)

Pull request description:

  Big picture overview in [this gist](https://gist.github.com/Sjors/29d06728c685e6182828c1ce9b74483d).

  This PR lets `bitcoind` call an arbitrary command `-signer=<cmd>`, e.g. a hardware wallet driver,  where it can fetch public keys, ask to display an address, and sign a transaction (using PSBT under the hood).

  It's design to work with https://github.com/bitcoin-core/HWI, which supports multiple hardware wallets. Any command with the same arguments and return values will work. It simplifies the manual procedure described [here](https://github.com/bitcoin-core/HWI/blob/master/docs/bitcoin-core-usage.md).

  Usage is documented in [doc/external-signer.md](
  https://github.com/Sjors/bitcoin/blob/2019/08/hww-box2/doc/external-signer.md), which also describes what protocol a different signer binary should conform to.

  Use `--enable-external-signer` to opt in, requires Boost::Process:

  ```
  Options used to compile and link:
    with wallet     = yes
    with gui / qt   = no
    external signer = yes
  ```

  It adds the following RPC methods:
  * `enumeratesigners`: asks <cmd> for a list of signers (e.g. devices) and their master key fingerprint
  * `signerdisplayaddress <address>`:  asks <cmd> to display an address

  It enhances the following RPC methods:
  * `createwallet`: takes an additional `external_signer` argument and fetches keys from device
  * `send`: automatically sends transaction to device and waits

  Usage TL&DR:
  * clone HWI repo somewhere and launch `bitcoind -signer=../HWI/hwi.py`
  * check if you can see your hardware device: `bitcoin-cli enumeratesigners`
  * create wallet and auto import keys `bitcoin-cli createwallet "hww" true true "" true true true`
  * display address on device: `bitcoin-cli signerdisplayaddress ...`
  * to spend, use `send` RPC and approve transaction on device

  Prerequisites:
  - [x] #21127 load wallet flags before everything else
  - [x] #21182 remove mostly pointless BOOST_PROCESS macro

  Potentially useful followups:
  - GUI support: bitcoin-core/gui#4
  - bumpfee support
  - (automatically) verify (a subset of) keys on the device after import, through message signing

ACKs for top commit:
  laanwj:
    re-ACK f75e0c1

Tree-SHA512: 7db8afd54762295c1424c3f01d8c587ec256a72f34bd5256e04b21832dabd5dc212be8ab975ae3b67de75259fd569a561491945750492f417111dc7b6641e77f
@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.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants