[RPC]Move transaction combining from signrawtransaction to new RPC #10571

pull achow101 wants to merge 1 commits into bitcoin:master from achow101:combineraw-rpc changing 4 files +154 −55
  1. achow101 commented at 5:40 am on June 10, 2017: member

    Create a combinerawtransaction RPC which accepts a json array of hex raw transactions to combine them into one transaction. Signrawtransaction is changed to no longer combine transactions and only accept one transaction at a time.

    The tests have been updated to test this. Tests for the signrawtransaction merge have also been removed.

    This is part of #10570

  2. fanquake added the label RPC/REST/ZMQ on Jun 10, 2017
  3. achow101 force-pushed on Jun 10, 2017
  4. achow101 force-pushed on Jun 12, 2017
  5. sipa commented at 10:20 pm on June 12, 2017: member
    Concept ACK. However, this does break backward compatibility with an undocumented but apparently intentional and tested feature. Does this require deprecation first?
  6. achow101 force-pushed on Jun 19, 2017
  7. achow101 force-pushed on Jun 19, 2017
  8. jnewbery commented at 2:55 pm on June 28, 2017: member
    Concept feedback: combining transactions seems like a utility. Is it possible to add it to bitcoin-tx rather than add a utility-only RPC?
  9. sipa commented at 2:56 pm on June 28, 2017: member
    It is not a utility-only RPC. It needs access to the UTXO set.
  10. achow101 force-pushed on Jun 30, 2017
  11. laanwj added this to the milestone 0.15.0 on Jul 6, 2017
  12. achow101 force-pushed on Jul 7, 2017
  13. achow101 force-pushed on Jul 7, 2017
  14. sipa commented at 2:10 am on July 15, 2017: member
    utACK c710eb9eaffa50daddfedb50a93036085b3296c7
  15. gmaxwell approved
  16. gmaxwell commented at 3:43 pm on July 17, 2017: contributor
    ACK
  17. in src/rpc/rawtransaction.cpp:587 in c710eb9eaf outdated
    582+    UniValue txs = request.params[0].get_array();
    583+    std::vector<CMutableTransaction> txVariants(txs.size());
    584+
    585+    for (unsigned int idx = 0; idx < txs.size(); idx++) {
    586+        if (!DecodeHexTx(txVariants[idx], txs[idx].get_str(), true))
    587+            throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed for tx " + idx);
    


    morcos commented at 4:01 pm on July 17, 2017:
    nit: braces
  18. in src/rpc/rawtransaction.cpp:592 in c710eb9eaf outdated
    586+        if (!DecodeHexTx(txVariants[idx], txs[idx].get_str(), true))
    587+            throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed for tx " + idx);
    588+    }
    589+
    590+    if (txVariants.empty())
    591+        throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Missing transactions");
    


    morcos commented at 4:01 pm on July 17, 2017:
    nit: braces
  19. in test/functional/rawtransactions.py:147 in c710eb9eaf outdated
    142+        self.sync_all()
    143+        self.nodes[0].generate(1)
    144+        self.sync_all()
    145+
    146+        #THIS IS A INCOMPLETE FEATURE
    147+        #NODE2 HAS TWO OF THREE KEY AND THE FUNDS SHOULD BE SPENDABLE AND COUNT AT BALANCE CALCULATION
    


    morcos commented at 6:16 pm on July 17, 2017:
    This is copy-paste error leading to incorrect documentation of the test. This test is a 2of2 multisig for which node 2 has only 1 key.
  20. morcos approved
  21. morcos commented at 6:22 pm on July 17, 2017: member

    utACK c710eb9

    although at least the documentation in the RPC test should probably be corrected

  22. achow101 force-pushed on Jul 17, 2017
  23. achow101 commented at 6:47 pm on July 17, 2017: member
    Nits addressed and rebased.
  24. morcos commented at 7:08 pm on July 17, 2017: member
    @achow101 It’s better if you address the nits separately from rebasing (was rebasing needed?) otherwise its hard to review just the differential and reACK.
  25. in test/functional/rawtransactions.py:147 in 10238696e7 outdated
    142+        self.sync_all()
    143+        self.nodes[0].generate(1)
    144+        self.sync_all()
    145+
    146+        #THIS IS A INCOMPLETE FEATURE
    147+        #NODE2 HAS ONE OF TWO KEY AND THE FUNDS SHOULD BE SPENDABLE AND COUNT AT BALANCE CALCULATION
    


    morcos commented at 7:09 pm on July 17, 2017:
    This comment is still incorrect. The funds should not be spendable there is nothing incomplete here.
  26. achow101 commented at 7:11 pm on July 17, 2017: member
    @morcos sorry, I rebased out of habit.
  27. achow101 commented at 7:16 pm on July 17, 2017: member
    Now the comment should be right.
  28. morcos commented at 7:31 pm on July 17, 2017: member

    heh thanks… and i think its ok to squash, especially if the old commit can still be found, b/c then a diff btwn old and new can show the differences. it’s the rebase that makes it difficult

    utACK f293282

  29. achow101 force-pushed on Jul 17, 2017
  30. morcos commented at 7:57 pm on July 17, 2017: member
    utACK 3cd7a5b
  31. sipa commented at 10:20 pm on July 17, 2017: member
    utACK 3cd7a5bc31e1a3cdd1cc740a884f69d9c6689e2f
  32. Move transaction combining from signrawtransaction to new RPC
    Create a combinerawtransaction RPC which accepts a json array of hex raw
    transactions to combine them into one transaction. Signrawtransaction is changed
    to no longer combine transactions and only accept one transaction at a time.
    6b4f231f5f
  33. in src/rpc/rawtransaction.cpp:587 in 3cd7a5bc31 outdated
    582+    UniValue txs = request.params[0].get_array();
    583+    std::vector<CMutableTransaction> txVariants(txs.size());
    584+
    585+    for (unsigned int idx = 0; idx < txs.size(); idx++) {
    586+        if (!DecodeHexTx(txVariants[idx], txs[idx].get_str(), true)) {
    587+            throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed for tx " + idx);
    


    laanwj commented at 3:23 pm on July 18, 2017:
    This won’t work, you’re adding a string and an integer. Use strprintf?

    achow101 commented at 5:59 pm on July 18, 2017:
    Fixed
  34. achow101 force-pushed on Jul 18, 2017
  35. laanwj merged this on Jul 20, 2017
  36. laanwj closed this on Jul 20, 2017

  37. laanwj referenced this in commit adf170daf9 on Jul 20, 2017
  38. achow101 deleted the branch on Aug 29, 2017
  39. sipa referenced this in commit ffc6e48b29 on Feb 20, 2018
  40. PastaPastaPasta referenced this in commit 0c60a6820f on Sep 16, 2019
  41. PastaPastaPasta referenced this in commit 6478562b3d on Sep 18, 2019
  42. codablock referenced this in commit 555eb28f6e on Sep 20, 2019
  43. UdjinM6 referenced this in commit ed0731530d on Sep 20, 2019
  44. UdjinM6 referenced this in commit 7b99bc5c06 on Sep 20, 2019
  45. codablock referenced this in commit c054f8f5dc on Sep 22, 2019
  46. codablock referenced this in commit 8308012e18 on Sep 23, 2019
  47. barrystyle referenced this in commit dd946c5491 on Jan 22, 2020
  48. PastaPastaPasta referenced this in commit 3f66e7da20 on Dec 11, 2020
  49. PastaPastaPasta referenced this in commit fc96339839 on Dec 12, 2020
  50. PastaPastaPasta referenced this in commit 38ca3c214e on Dec 12, 2020
  51. PastaPastaPasta referenced this in commit b4f67a5e9e on Dec 15, 2020
  52. PastaPastaPasta referenced this in commit a2f107424b on Dec 15, 2020
  53. PastaPastaPasta referenced this in commit d977daaff0 on Dec 18, 2020
  54. MarcoFalke locked this on Sep 8, 2021

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 01:12 UTC

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