Implement watchonly for fundrawtransaction #5524

pull kanzure wants to merge 5 commits into bitcoin:master from kanzure:fundrawtransaction-watchonly changing 11 files +531 −26
  1. kanzure commented at 6:31 pm on December 21, 2014: contributor

    Support watchonly in fundrawtransaction, CreateTransaction, SelectCoins and coin selection in general.

    Coin selection is exposed over RPC through fundrawtransaction, and watchonly can be enabled by passing includeWatching true (defaults to false).

    fundrawtransaction will first attempt to fund a transaction without using watchonly, even when includeWatching is enabled. When this fails, an includeWatching attempt is made.

    When an includeWatching attempt at coin selection is performed (in CreateTransaction), and all watchonlys are consumed leaving no funds available for a fee, then CreateTransaction will not include a fee in the transaction.

    Also, includeWatching tests (for fundrawtransaction) added here are passing.

    Reviewers! Here are some things I would especially appreciate eyelooking for: suggestions for more edge cases for the tests, other RPC commands that should be tested, other tests for other features that I should double check for whether or not this makes any impacts, locations or existence of any sort of documentation I should be updating, unintended side effects introduced in CreateTransaction like by multiple parameters that may eventually be used simultaneously in ways that I am failing to anticipate….

    I received a few suggestions from others that SignSignature should be replaced with an estimator in CreateTransaction. However, this estimator apparently does not seem to be necessary for implementing watchonly-support here, so this suggestion may go into another ticket instead.

    This requires #5503 to be merged (I wouldn’t mind this getting merged into #5503).

  2. jonasschnelli commented at 8:28 am on December 22, 2014: contributor

    That was quick. Nice!

    Maybe it makes more sense to reopen this pull when #5503 is stable and got ACK and merged. I prefer to have a easy reviewable code-pice because reviewing and testing is the bottleneck. Also it prevents you from rebasing all the time.

  3. kanzure force-pushed on Dec 22, 2014
  4. jonasschnelli commented at 9:22 am on December 29, 2014: contributor
    Breaks travis. Maybe you close and reopen this pull whenever #5503 makes it into master?
  5. jgarzik added the label RPC on Dec 31, 2014
  6. kanzure force-pushed on Jan 16, 2015
  7. kanzure force-pushed on Jan 16, 2015
  8. kanzure commented at 2:29 pm on January 19, 2015: contributor

    Breaks travis

    Tests are passing now.

  9. kanzure force-pushed on Jan 31, 2015
  10. laanwj added the label Wallet on Feb 27, 2015
  11. [RPC] fundrawtransaction basics 002951a7a3
  12. [RPC] add support for existing vins for `fundrawtransaction` a98e7fe771
  13. [RPC] add simple unittest for `fundrawtransaction` (increase test coverage) 0fd9051f11
  14. [RPC] fundrawtransaction overhaul 
    - fix typo
- fix RPCTypeCheck for fundrawtransaction
    10fd562041
  15. jonasschnelli commented at 12:37 pm on March 11, 2015: contributor

    @kanzure could you rebase on top of #5503?

    Conceptual ACK. Could also go into 0.11 together with #5503 IMO.

  16. Implement watchonly support in fundrawtransaction 
    Support watchonly in fundrawtransaction, CreateTransaction, SelectCoins
and coin selection in general.

Coin selection is exposed over RPC through fundrawtransaction, and
watchonly can be enabled by passing includeWatching true (defaults to
false).

fundrawtransaction will first attempt to fund a transaction without
using watchonly, even when includeWatching is enabled. When this fails,
an includeWatching attempt is made.

When an includeWatching attempt at coin selection is performed (in
CreateTransaction), and all watchonlys are consumed leaving no funds
available for a fee, then CreateTransaction will not include a fee in
the transaction.

Also, includeWatching tests (for fundrawtransaction).

See also: https://github.com/bitcoin/bitcoin/pull/5503
    a3af4c1c11
  17. kanzure force-pushed on Mar 15, 2015
  18. kanzure commented at 5:54 pm on March 15, 2015: contributor

    Rebased on to the fundrawtransaction branch. Travis is reporting an error:

     0Tests successful
 1Running testscript rawtransactions.py...
 2Initializing test directory /tmp/testZdbica
 3JSONRPC error: Expected type int, got str
 4  File "/home/travis/build/bitcoin/bitcoin/bitcoin-x86_64-unknown-linux-gnu/qa/rpc-tests/test_framework.py", line 117, in main
 5    self.run_test()
 6  File "/home/travis/build/bitcoin/bitcoin/bitcoin-x86_64-unknown-linux-gnu/qa/rpc-tests/rawtransactions.py", line 68, in run_test
 7    rawtxfund = self.nodes[2].fundrawtransaction(rawtx)
 8  File "/home/travis/build/bitcoin/bitcoin/bitcoin-x86_64-unknown-linux-gnu/qa/rpc-tests/python-bitcoinrpc/bitcoinrpc/authproxy.py", line 126, in __call__
 9    raise JSONRPCException(response['error'])
10Cleaning up
  19. in src/rpcrawtransaction.cpp: in a3af4c1c11 
    795+                            + HelpExampleCli("sendrawtransaction", "\"signedhex\"") +
796+                            "\nAs a json rpc call\n"
797+                            + HelpExampleRpc("sendrawtransaction", "\"signedhex\"")
798+                            );
799+    
800+    RPCTypeCheck(params, boost::assign::list_of(int_type)(int_type)(array_type));
    dexX7 commented at 11:12 pm on March 15, 2015:
    Should probably be RPCTypeCheck(params, boost::assign::list_of(str_type)(bool_type));.
  20. TheBlueMatt commented at 9:10 pm on April 20, 2015: member
    Why try to fund without watchonly first? If I call the RPC and say I want to include watchonly, it means I want to include watchonly (and get the best fee from the inputs I opted to select).
  21. jonasschnelli commented at 7:56 pm on April 21, 2015: contributor
    I’d like to add this to #5503 but the nFeeRet=0 as well as the issue mentioned by @TheBlueMatt (calling CreateTransaction() two times) are unclear and needs fixing. RPC tests should also send a funded raw transaction and check the response to make sure it passes the mempool checks (i have a feeling that it won’t because of the generated fee)
  22. TheBlueMatt referenced this in commit ec4ae6fe26 on Apr 24, 2015
  23. TheBlueMatt referenced this in commit 929d18d469 on Apr 24, 2015
  24. TheBlueMatt referenced this in commit d90d283111 on Apr 25, 2015
  25. TheBlueMatt referenced this in commit 947d6f72c4 on Apr 25, 2015
  26. TheBlueMatt referenced this in commit 5aa90ca8a1 on Apr 25, 2015
  27. TheBlueMatt referenced this in commit aee48c5a9e on Apr 30, 2015
  28. TheBlueMatt referenced this in commit 0b05c94ff6 on Apr 30, 2015
  29. kanzure commented at 9:31 pm on April 30, 2015: contributor
    Closed in favor of #6088.
  30. kanzure closed this on Apr 30, 2015
  31. TheBlueMatt referenced this in commit aa24321ab3 on Jul 10, 2015
  32. TheBlueMatt referenced this in commit bbf7da345d on Jul 17, 2015
  33. TheBlueMatt referenced this in commit 2ff8c92e21 on Jul 17, 2015
  34. TheBlueMatt referenced this in commit 6bdb474dc9 on Jul 20, 2015
  35. jonasschnelli referenced this in commit 6a65f5b982 on Sep 2, 2015
  36. random-zebra referenced this in commit 239d21d4be on Jun 4, 2020
  37. random-zebra referenced this in commit 4aad15b095 on Jun 5, 2020
  38. random-zebra referenced this in commit 3bc2809474 on Jun 9, 2020
  39. random-zebra referenced this in commit 134c5d2c1a on Jun 28, 2020
  40. random-zebra referenced this in commit 051311e4a8 on Jul 4, 2020
  41. DrahtBot locked this on Sep 8, 2021


kanzure jonasschnelli dexX7 TheBlueMatt

Labels
Wallet RPC/REST/ZMQ

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-29 07:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me