UI external signer support (e.g. hardware wallet) #16549

pull Sjors wants to merge 35 commits into bitcoin:master from Sjors:2019/08/hww-qt changing 72 files +1839 −127
  1. Sjors commented at 5:38 pm on August 5, 2019: member

    Big picture overview in this gist.

    This PR adds GUI support for external signers. It consists of 7 commits on top of #16546 (RPC).

    The UX isn’t amazing - especially the blocking calls - but it works.

    First we adds a GUI setting for the signer script (e.g. path to HWI):

    Then we add an external signer checkbox to the wallet creation dialog:

    It’s checked by default if HWI detects a device. It also grabs the name. It then creates a fresh wallet and imports the keys.

    You can verify an address on the device (blocking…):

    Sending, including coin selection, Just Works(tm) as long the device is present. It’s a bit weird though: the device will prompt for signing first and the QT confirmation dialog appears after.

    External signer support is enabled by default when the GUI is configured and Boost::Process is present.

  2. DrahtBot added the label Build system on Aug 5, 2019
  3. DrahtBot added the label Docs on Aug 5, 2019
  4. DrahtBot added the label GUI on Aug 5, 2019
  5. DrahtBot added the label RPC/REST/ZMQ on Aug 5, 2019
  6. DrahtBot added the label Scripts and tools on Aug 5, 2019
  7. DrahtBot added the label Tests on Aug 5, 2019
  8. DrahtBot added the label Utils/log/libs on Aug 5, 2019
  9. DrahtBot added the label Wallet on Aug 5, 2019
  10. DrahtBot commented at 9:04 pm on August 5, 2019: member

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

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #19267 (ci: Upgrade most ci configs to focal by MarcoFalke)
    • #19162 (ci: tsan gui by MarcoFalke)
    • #19136 (wallet: add dumpwalletdescriptor RPC by achow101)
    • #19099 (refactor: Move wallet methods out of chain.h and node.h by ryanofsky)
    • #19077 (wallet: Add sqlite as an alternative wallet database and use it for new descriptor wallets by achow101)
    • #19064 (refactor: Cleanup thread ctor calls by hebasto)
    • #19046 (Replace CWallet::Set* functions that use memonly with Add/Load variants by achow101)
    • #19013 (test: add v0.20.0 to backwards compatibility test by Sjors)
    • #18870 (build: Allow BDB between 4.8 and 5.3 without –with-incompatible-bdb by achow101)
    • #18789 (qt: Add Create Unsigned button to SendConfirmationDialog by achow101)
    • #18788 (tests: Update more tests to work with descriptor wallets by achow101)
    • #18750 (build: optionally skip external warnings by vasild)
    • #18654 (rpc: separate bumpfee’s psbt creation function into psbtbumpfee by achow101)
    • #18531 (rpc: Assert that RPCArg names are equal to CRPCCommand ones by MarcoFalke)
    • #18354 (Use shared pointers only in validation interface by bvbfan)
    • #18351 (Updated text on send confirmation dialog by dannmat)
    • #18202 (refactor: consolidate sendmany and sendtoaddress code by Sjors)
    • #18077 (net: Add NAT-PMP port forwarding support by hebasto)
    • #15937 (Add loadwallet and createwallet load_on_startup options by ryanofsky)
    • #15936 (Unify bitcoin-qt and bitcoind persistent settings by ryanofsky)
    • #15719 (Wallet passive startup by ryanofsky)
    • #15382 (util: add runCommandParseJSON by Sjors)

    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.

  11. DrahtBot added the label Needs rebase on Aug 6, 2019
  12. jonasschnelli commented at 11:41 am on August 12, 2019: contributor
    This PR has 146 commits,… is this intentional?
  13. Sjors commented at 1:18 pm on August 12, 2019: member
    @jonasschnelli see the list in #16546 for all the all the experimental stuff this PR is based on. The actual change is only a few commits (Github messes up the ordering), compared to RPC-only functionality.
  14. fanquake removed the label Build system on Aug 14, 2019
  15. fanquake removed the label Docs on Aug 14, 2019
  16. fanquake removed the label Scripts and tools on Aug 14, 2019
  17. fanquake removed the label Tests on Aug 14, 2019
  18. fanquake removed the label Utils/log/libs on Aug 14, 2019
  19. Sjors force-pushed on Sep 16, 2019
  20. Sjors commented at 5:19 pm on September 16, 2019: member

    Cleaned it up a bit and tweaked the timestamps so it’s more clear that only the most recent 4 commits are relevant.

    139 commits may seem a bit intimidating, but it’s really just a matter of reviewing:

    1. runCommandParseJSON, so the GUI can call HWI: #15382
    2. Wallet boxes #16341
    3. Native descriptor wallets #16528, which in turn needs a rebase on #16341 and the merged #15450 (create wallet)
    4. External signer RPC support: #16546
  21. Sjors force-pushed on Sep 18, 2019
  22. Sjors force-pushed on Oct 29, 2019
  23. DrahtBot removed the label Needs rebase on Oct 29, 2019
  24. Sjors force-pushed on Oct 29, 2019
  25. Sjors force-pushed on Oct 29, 2019
  26. DrahtBot added the label Needs rebase on Oct 30, 2019
  27. Sjors force-pushed on Oct 30, 2019
  28. Sjors force-pushed on Oct 31, 2019
  29. Sjors force-pushed on Oct 31, 2019
  30. DrahtBot removed the label Needs rebase on Oct 31, 2019
  31. DrahtBot added the label Needs rebase on Oct 31, 2019
  32. Sjors force-pushed on Nov 7, 2019
  33. DrahtBot removed the label Needs rebase on Nov 7, 2019
  34. DrahtBot added the label Needs rebase on Nov 8, 2019
  35. Sjors force-pushed on Dec 9, 2019
  36. DrahtBot removed the label Needs rebase on Dec 9, 2019
  37. DrahtBot added the label Needs rebase on Dec 16, 2019
  38. Sjors force-pushed on Jan 30, 2020
  39. DrahtBot removed the label Needs rebase on Jan 30, 2020
  40. Sjors force-pushed on Jan 30, 2020
  41. Sjors force-pushed on Jan 30, 2020
  42. Sjors force-pushed on Jan 30, 2020
  43. DrahtBot added the label Needs rebase on Feb 4, 2020
  44. Sjors force-pushed on Feb 21, 2020
  45. Sjors commented at 9:38 pm on February 21, 2020: member
    Rebased. Sending is a bit of a hack; I’ll clean that up after #18027.
  46. DrahtBot removed the label Needs rebase on Feb 21, 2020
  47. DrahtBot added the label Needs rebase on Feb 25, 2020
  48. Sjors force-pushed on Feb 25, 2020
  49. DrahtBot removed the label Needs rebase on Feb 25, 2020
  50. DrahtBot added the label Needs rebase on Mar 9, 2020
  51. Sjors force-pushed on Mar 23, 2020
  52. Sjors force-pushed on Mar 27, 2020
  53. DrahtBot removed the label Needs rebase on Mar 27, 2020
  54. Sjors force-pushed on Apr 1, 2020
  55. DrahtBot added the label Needs rebase on Apr 6, 2020
  56. Sjors force-pushed on Apr 22, 2020
  57. Sjors force-pushed on Apr 22, 2020
  58. DrahtBot removed the label Needs rebase on Apr 22, 2020
  59. DrahtBot added the label Needs rebase on Apr 23, 2020
  60. Sjors force-pushed on Apr 23, 2020
  61. DrahtBot removed the label Needs rebase on Apr 23, 2020
  62. Sjors force-pushed on Apr 27, 2020
  63. Sjors force-pushed on Apr 27, 2020
  64. DrahtBot commented at 11:09 am on May 1, 2020: member

    🐙 This pull request conflicts with the target branch and needs rebase.

  65. DrahtBot added the label Needs rebase on May 1, 2020
  66. Sjors marked this as a draft on May 4, 2020
  67. Sjors renamed this:
    [WIP] UI external signer support (e.g. hardware wallet)
    UI external signer support (e.g. hardware wallet)
    on May 4, 2020
  68. Sjors commented at 7:35 pm on May 4, 2020: member
    Github lets you mark existing pull requests as draft now!
  69. Sjors force-pushed on May 7, 2020
  70. fjahr commented at 11:44 am on May 11, 2020: member

    Concept ACK

    I am not a huge GUI user but I compared the way this works here with #16546 and it looks good to me.

  71. Sjors force-pushed on May 18, 2020
  72. Sjors force-pushed on May 22, 2020
  73. jonasschnelli commented at 7:31 am on May 29, 2020: contributor
    I haven’t followed the recent HWW work,… but is this still à jour with the concept?
  74. Sjors commented at 6:37 pm on May 29, 2020: member
    I’m keeping this PR up to date and it actually works, so it is the concept :-)
  75. Sjors force-pushed on Jun 10, 2020
  76. [depends] boost: patch unused variable in boost_process
    Co-Authored-By: fanquake <fanquake@gmail.com>
    4037f85033
  77. configure: add ax_boost_process
    Co-authored-by: Luke Dashjr <luke-jr+git@utopios.org>
    2436908651
  78. [build] make boost-process opt-in 459e567992
  79. [build] msvc: add boost::process
    * AppVeyor boost-process vcpkg package.
    * Tell Boost linter to ignore it
    * Add HAVE_BOOST_PROCESS for MSVC build (bitcoin_config.h)
    97f65def30
  80. [doc] include Doxygen comments for HAVE_BOOST_PROCESS 4a093579e1
  81. [ci] use boost::process
    Explictly opt-out on win64, in case the default changes.
    f97f9e2bd4
  82. [util] add runCommandParseJSON 48c57c7548
  83. [rpc] [DO NOT MERGE] sendmany: return PSBT for wallets without private keys
    TODO: replace with #16378 once its prerequisites are merged.
    
    In addition we check to see if the PSBT is complete, in preperation for a ScriptPubKeyMan that can use an external signer.
    fc46eb2202
  84. configure: add --enable-external-signer
    This prepares external signer support to be disabled by default.
    It adds a configure option to enable this feature and to check
    if Boost::Process is present.
    
    It can also be enabled using --with-boost-process
    
    This also exposes ENABLE_EXTERNAL_SIGNER to the test suite via test/config.ini
    
    fixup! e192a9dedf1bc5dcddb928466eca9a93c83e5c91
    edcf22fc0b
  85. [build] msvc: define ENABLE_EXTERNAL_SIGNER 1532fb3ac5
  86. [doc] include Doxygen comments for ENABLE_EXTERNAL_SIGNER 599c285abd
  87. [test] framework: add skip_if_no_external_signer b66180c034
  88. [wallet] add -signer argument for external signer command
    Create basic ExternalSigner class with contructor. A Signer(<cmd>)
    is added to CWallet on load if -signer=<cmd> is set.
    267ee5adf9
  89. [test] add external signer test
    Includes a mock to mimick the HWI interace.
    1357381b78
  90. [wallet] add external_signer flag 749bd05feb
  91. [wallet] add ExternalSignerScriptPubKeyMan 1be8ac458d
  92. Sjors force-pushed on Jun 16, 2020
  93. [rpc] add external signer RPC files a468f7a382
  94. [rpc] signer: add enumeratesigners to list external signers dd34019b8e
  95. [rpc] add external_signer option to createwallet d9c27f0a79
  96. [test] external_signer wallet flag is immutable 5ec6357b62
  97. [wallet] ExternalSigner: add getDescriptors method c59a7483fd
  98. [wallet] add GetExternalSigner() d8591fb41c
  99. [wallet] fetch keys from external signer upon creation 489f37b9c3
  100. Add Fingerprint() to Descriptor f481c5fdb5
  101. [rpc] signerdisplayaddress e31b152670
  102. [rpc] sendtoaddress: support external signer 23c992d390
  103. [doc] add external-signer.md 36f8792ede
  104. [gui] options: add external signer path 7218c328b1
  105. [gui] create wallet with external signer 3c21dd566c
  106. Add displayAddress to wallet interface 26a9baffe9
  107. [gui] display address on external signer b5c46cb99e
  108. [gui] node: get external signer list ee8b40eb2e
  109. [gui] wallet creation detects external signer c168ed0028
  110. [gui] send: use external signer 6f9259c790
  111. Sjors force-pushed on Jun 16, 2020
  112. [build] enable external signer by default for GUI e306aaa3c0
  113. Sjors commented at 6:13 pm on June 18, 2020: member
    Moving to the new GUI repo: bitcoin-core/gui#4
  114. Sjors closed this on Jun 18, 2020

  115. Sjors deleted the branch on Apr 11, 2021
  116. DrahtBot locked this on Aug 16, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-09-28 22:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me