fundrawtransaction #6088

pull TheBlueMatt wants to merge 5 commits into bitcoin:master from TheBlueMatt:frt2 changing 12 files +787 −14
  1. TheBlueMatt commented at 8:54 PM on April 30, 2015: contributor

    This superceedes #5503 and #5524. I rewrote chunks of the first and largely rewrote the second, stealing test cases almost verbatim from both.

  2. jonasschnelli cross-referenced this on Apr 30, 2015 from issue [RPC] fundrawtransaction by jonasschnelli
  3. kanzure cross-referenced this on Apr 30, 2015 from issue Implement watchonly for fundrawtransaction by kanzure
  4. laanwj added the label Wallet on May 1, 2015
  5. laanwj added the label RPC on May 1, 2015
  6. laanwj added this to the milestone 0.11.0 on May 1, 2015
  7. sipa commented at 12:45 PM on May 1, 2015: member

    Could we have a DummySignatureCreator, instead of passing a dummy boolean to TransactionSignatureCreator?

  8. sipa commented at 12:50 PM on May 1, 2015: member

    I had to read the source code to guess what 'fAllowOtherInputs' means. Can you add a comment?

  9. jonasschnelli commented at 12:52 PM on May 1, 2015: contributor

    @sipa: I tried this ("DummySignatorCreator"). But somehow i stopped it because it was getting a inheritance mess. I can't actually remember why exactly.

  10. sipa commented at 12:55 PM on May 1, 2015: member

    @jonasschnelli Let me hack something up.

  11. sipa commented at 1:24 PM on May 1, 2015: member

    See https://github.com/sipa/bitcoin/commits/dummysig

  12. jonasschnelli commented at 2:11 PM on May 1, 2015: contributor

    @sipa: are you sure this would work also for P2SH Multisig inputs?

  13. sipa commented at 2:19 PM on May 1, 2015: member

    @jonasschnelli pretty sure, it should.

    BaseSignatureCreator is about creating individual (DER+nHashType) signatures, and is called by ProduceSignature wherever necessary. For multisig/P2SH, it will be called multiple times as necessary.

  14. jonasschnelli commented at 5:05 PM on May 1, 2015: contributor

    @sipa: Right. I would do so. To make use of https://github.com/sipa/bitcoin/commit/134090be5a0aaad27981ed4d4fe52a843fce337b it would need some adaptation and some changes within SignSignature(). I just tried but had some compiling/casting issues with the DummySignatureChecker class.

    But the current solution (as it is in this PR) without a DummySignatorCreator class works well and basically adds only 8 lines of code. But indeed its not that elegant as sipas proposal.

  15. sipa commented at 5:18 PM on May 1, 2015: member

    I'm fine with fixing it up afterwards.

  16. jgarzik commented at 4:02 PM on May 2, 2015: contributor

    Concept ACK - the dummy sig stuff is ugly and poops all over several function/method sigs

  17. laanwj commented at 8:07 AM on May 6, 2015: member

    The dummy sig business may be ugly, but it was introduced to avoid even uglier solutions to compute signature sizes: either having parallel byte accounting functioning (lots of duplicate hard-to-crosscheck code), or doing real signing then throwing away the result (requires wallet to be unlocked and is just wrong).

  18. laanwj removed this from the milestone 0.11.0 on May 18, 2015
  19. jonasschnelli commented at 1:45 PM on May 18, 2015: contributor

    Needs rebase.

  20. luke-jr commented at 3:15 AM on June 2, 2015: member

    Dummy sign may be ugly, but it also would enable prompting the user with fee etc prior to passphrase being entered...

  21. in src/wallet/rpcdump.cpp:None in 0b05c94ff6 outdated 
     157 |              "\nAdds an address or script (in hex) that can be watched as if it were in your wallet but cannot be used to spend.\n"
 158 |              "\nArguments:\n"
 159 |              "1. \"address\"          (string, required) The address\n"
 160 |              "2. \"label\"            (string, optional, default=\"\") An optional label\n"
 161 |              "3. rescan               (boolean, optional, default=true) Rescan the wallet for transactions\n"
 162 | +            "4. p2sh                 (boolean, optional, default=false) Add the P2SH version of the script as well\n"
    luke-jr commented at 3:16 AM on June 2, 2015:

    This seems nonsensical. What is the use case?

  22. TheBlueMatt commented at 6:58 AM on June 11, 2015: contributor

    So it turns out the watch-only signing never worked anyway (it used the constant "0" for the public key when calculating size of pay-to-pubkey-hash txn), so I walked that back and watchonly-supporting fundrawtransaction will be a separate pull.

  23. TheBlueMatt force-pushed on Jun 11, 2015
  24. TheBlueMatt force-pushed on Jun 11, 2015
  25. TheBlueMatt force-pushed on Jun 11, 2015
  26. Add DummySignatureCreator which just creates zeroed sigs 9b4e7d9a5e
  27. Small tweaks to CCoinControl for fundrawtransaction 2d84e22703
  28. Add FundTransaction method to wallet
    Some code stolen from Jonas Schnelli <jonas.schnelli@include7.ch>
    1e0d1a2ff0
  29. Add fundrawtransaction RPC method 21bbd920e5
  30. fundrawtransaction tests 208589514c
  31. TheBlueMatt force-pushed on Jun 11, 2015
  32. laanwj merged this on Jun 23, 2015
  33. laanwj closed this on Jun 23, 2015
  34. laanwj referenced this in commit 91389e51c7 on Jun 23, 2015
  35. btcdrak commented at 10:57 PM on June 23, 2015: contributor

    ACK

  36. laanwj commented at 5:14 AM on June 24, 2015: member

    Tested ACK (which I forgot to post)

  37. jonasschnelli commented at 6:39 AM on June 24, 2015: contributor

    Post merge ACK

  38. in src/wallet/wallet.cpp:None in 208589514c 
    1753 | +    CReserveKey reservekey(this);
1754 | +    CWalletTx wtx;
1755 | +    if (!CreateTransaction(vecSend, wtx, reservekey, nFeeRet, nChangePosRet, strFailReason, &coinControl, false))
1756 | +        return false;
1757 | +
1758 | +    if (nChangePosRet != -1)
    sipa commented at 8:34 PM on June 24, 2015:

    Any reason for trying to guess the changes made by CreateTransaction, and applying those on the original, rather than just using the constructed result?

  39. sipa commented at 9:03 PM on June 24, 2015: member

    Posthumous untested ACK.

  40. TheBlueMatt cross-referenced this on Jul 10, 2015 from issue Implement watchonly support in fundrawtransaction by TheBlueMatt
  41. laanwj cross-referenced this on Feb 10, 2016 from issue Add createtransaction by promag
  42. str4d cross-referenced this on Feb 14, 2017 from issue Bitcoin 0.12 RPC PRs 1 by str4d
  43. zkbot referenced this in commit 9af55822fb on Feb 15, 2017
  44. zkbot referenced this in commit a7cf698873 on Mar 4, 2017
  45. dagurval cross-referenced this on Dec 27, 2017 from issue Add fundrawtransaction by dagurval
  46. random-zebra cross-referenced this on Jun 4, 2020 from issue [Wallet][RPC] FundTransaction - fundrawtransaction by random-zebra
  47. furszy cross-referenced this on Jun 6, 2020 from issue Base work for the Sapling signatureHash by furszy
  48. furszy referenced this in commit 0724bbbad2 on Jun 28, 2020
  49. random-zebra referenced this in commit 5a092159f6 on Aug 5, 2020
  50. bitcoin locked this on Sep 8, 2021
Contributors
TheBlueMattsipajonasschnellijgarziklaanwjluke-jrbtcdrak
Labels
WalletRPC/REST/ZMQ
Linked (view graph)
#5503 [RPC] fundrawtransaction#5524 Implement watchonly for fundrawtransaction#6415 Implement watchonly support in fundrawtransaction#7498 Add createtransaction
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: 2026-05-20 06:55 UTC