[RPC] Add generatetoaddress rpc to mine to an address #7671

pull achow101 wants to merge 2 commits into bitcoin:master from achow101:generate-to-addr changing 6 files +107 −33
  1. achow101 commented at 4:52 PM on March 12, 2016: member

    Add a new optional parameter to the generate call to mine to a specific address even if it doesn't exist in the wallet.

  2. in src/rpc/mining.cpp:None in 4ef0344266 outdated
     122 | +            "generate numblocks (address)\n"
     123 |              "\nMine blocks immediately (before the RPC call returns)\n"
     124 |              "\nNote: this function can only be used on the regtest network\n"
     125 |              "\nArguments:\n"
     126 |              "1. numblocks    (numeric, required) How many blocks are generated immediately.\n"
     127 | +            "2. address    (string, optional) The base58 encoded address to send the newly generated bitcoin to.\n"
    


    jonasschnelli commented at 6:49 PM on March 12, 2016:

    Maybe use "address" instead of "base58 encoded address" (it would even be wrong because it's a base58 check encoded ripemd hash".

  3. jonasschnelli commented at 6:50 PM on March 12, 2016: contributor

    Nice! utACK.

  4. jonasschnelli added the label Mining on Mar 12, 2016
  5. achow101 force-pushed on Mar 12, 2016
  6. sipa commented at 6:56 PM on March 12, 2016: member

    I like the concept, but perhaps we can separate that out to a different RPC (which then can function without wallet support)?

    Also, the change in arguments to generate conflicts with #7663.

  7. achow101 commented at 7:02 PM on March 12, 2016: member

    @sipa How about moving it to a new rpc generatetoaddress?

    Done, but there is code duplication since most of it is copied from the generate rpc. I left it duplicated so that it doesn't conflict with #7663.

  8. achow101 force-pushed on Mar 12, 2016
  9. luke-jr commented at 8:22 PM on March 12, 2016: member

    This seems unnecessary. Why not just move GBT->block code from getblocktemplate_proposals.py to blocktools?

  10. achow101 renamed this:
    [RPC] Allow mining to a specific address
    [RPC] Add generatetoaddress rpc to mine to an address
    on Mar 12, 2016
  11. achow101 commented at 8:32 PM on March 12, 2016: member

    This seems unnecessary.

    Why?

    Why not just move GBT->block code from getblocktemplate_proposals.py to blocktools?

    Not sure what you mean by this.

  12. laanwj commented at 11:12 AM on March 14, 2016: member

    Concept ACK Needs rebase after #7507,#7663

    I like the concept, but perhaps we can separate that out to a different RPC (which then can function without wallet support)?

    Sounds like a good idea to me. Mining without wallet support would be awesome for some tests.

  13. achow101 force-pushed on Mar 14, 2016
  14. achow101 force-pushed on Mar 14, 2016
  15. achow101 commented at 7:43 PM on March 14, 2016: member

    Rebased

  16. MarcoFalke commented at 7:58 PM on March 14, 2016: member

    I'd prefer to have rpc tests as part of this pull.

  17. achow101 commented at 8:14 PM on March 14, 2016: member

    @MarcoFalke Where would be the best place to put such tests?

  18. MarcoFalke commented at 8:28 PM on March 14, 2016: member

    @achow101 You can either start from scratch (c.f. https://github.com/bitcoin/bitcoin/blob/48f39058315c908c360acb596923cbc090119480/qa/rpc-tests/disablewallet.py) or extend an existing python script in this folder.

  19. sipa commented at 8:30 PM on March 14, 2016: member

    One of the primary use cases for this, I think, would be to have RPC tests where you get an address from one node, and let another node mine directly to it, without the need to generate 100 blocks in between before you can transfer.

  20. achow101 commented at 9:57 PM on March 14, 2016: member

    I added rpc tests, both with and without wallets.

    The wallet RPC Test mines Bitcoin to another node's address and checks that that node has received the Bitcoin.

    The RPC test without the wallet mines Bitcoin to an arbitrary address and checks that it works. It then mines to an arbitrary invalid address and checks that that fails.

  21. in qa/rpc-tests/wallet.py:None in ef18efd1a5 outdated
     258 | +        # Check that the txid and balance is found by node1
     259 | +        try:
     260 | +            self.nodes[1].gettransaction(cbTxId)
     261 | +        except JSONRPCException,e:
     262 | +            errorString = e.error['message']
     263 | +        assert_equal("Invalid or non-wallet transaction id" in errorString, False)
    


    MarcoFalke commented at 10:15 PM on March 14, 2016:

    Nit:

    assert("Invalid or non-wallet transaction id" not in errorString)
    

    MarcoFalke commented at 10:15 PM on March 14, 2016:

    Actually, you can remove the whole except block?


    achow101 commented at 10:17 PM on March 14, 2016:

    Actually, you can remove the whole except block?

    Why?


    MarcoFalke commented at 10:21 PM on March 14, 2016:

    The error string is always empty, so it's dead code? (Correct me if I am wrong)


    achow101 commented at 10:35 PM on March 14, 2016:

    Uh, I don't think so. There is an error from gettransaction and the error string was defined earlier.

    Anyways, this should work, right?

    except JSONRPCException,e:
                assert("Invalid or non-wallet transaction id" not in e.error['message'])
    
  22. in qa/rpc-tests/disablewallet.py:None in ef18efd1a5 outdated
      42 | +        errorString = ''
      43 | +        try:
      44 | +            self.nodes[0].generatetoaddress(1, '3J98t1WpEZ73CNmQviecrnyiWrnqRhWNLy')
      45 | +        except JSONRPCException,e:
      46 | +            errorString = e.error['message']
      47 | +        assert_equal("Invalid address" in errorString, True)
    


    MarcoFalke commented at 10:20 PM on March 14, 2016:

    Nit: Move it up one line and make it

                raise AssertionError("Must not mine to invalid address!")
            except JSONRPCException,e:
                assert("Invalid address" in e.error['message'])
    
  23. achow101 force-pushed on Mar 14, 2016
  24. achow101 commented at 10:39 PM on March 14, 2016: member

    addressed nits

  25. laanwj commented at 8:24 AM on March 15, 2016: member

    Random Q: Should this also be able to take an arbitrary scriptPubKey?

    One of the primary use cases for this, I think, would be to have RPC tests where you get an address from one node, and let another node mine directly to it,

    Also eventually we want to have at least part of the RPC tests usable without wallet.

  26. achow101 commented at 8:24 PM on March 15, 2016: member

    Random Q: Should this also be able to take an arbitrary scriptPubKey?

    Do you think that it should?

  27. laanwj commented at 9:18 AM on March 16, 2016: member

    Do you think that it should?

    I don't deeply care myself. Just a question that came up with me, usually the discussion of custom scripts comes up immediately after allowing arbitrary addresses, as addresses map to a only a subset of the possible output scripts.

    So to be clear, I'm ok with just leaving it like this.

  28. achow101 commented at 12:36 PM on March 17, 2016: member

    OK, I'll just leave it as is.

  29. sipa commented at 12:40 PM on March 17, 2016: member

    If we want to support that, perhaps we should just define an address format that supports arbitrary scriptPubKeys. That way, it would be available everywhere.

  30. laanwj commented at 1:14 PM on March 21, 2016: member

    Needs rebase.

  31. Create generatetoaddress rpc
    Creates the generatetoaddress rpc which is virtually identical to the generate rpc except that it takes an argument for the address to mine to. It does not rely on wallet functionality.
    
    The mining code shared by generate and generatetoaddress has been moved to another method to reduce duplication.
    fe00ca758a
  32. achow101 force-pushed on Mar 21, 2016
  33. achow101 commented at 2:01 PM on March 21, 2016: member

    rebased

  34. RPC tests for generatetoaddress
    Adds two RPC tests for the generatetoaddress RPC, one in the wallet, and one when the wallet is disabled.
    
    The wallet RPC Test mines Bitcoin to another node's address and checks that that node has received the Bitcoin.
    
    The RPC test without the wallet mines Bitcoin to an arbitrary address and checks that it works. It then mines to an arbitrary invalid address and checks that that fails.
    d5c5c713e6
  35. in qa/rpc-tests/disablewallet.py:None in 8cad9b22c0 outdated
      35 | +        except JSONRPCException,e:
      36 | +            assert("Invalid address" not in e.error['message'])
      37 | +            assert("ProcessNewBlock, block not accepted" not in e.error['message'])
      38 | +            assert("Couldn't create new block" not in e.error['message'])
      39 | +
      40 | +        errorString = ''
    


    MarcoFalke commented at 2:06 PM on March 21, 2016:

    Please remove this line. The variable is unused and I'd prefer not to use it ever again: see https://github.com/bitcoin/bitcoin/commit/3b4324b1edd7b52bbc0e1f350ab04bc41f343f2e#diff-dafb9035fa41af011fcf87041198a05aL309


    achow101 commented at 2:23 PM on March 21, 2016:

    fixed

  36. achow101 force-pushed on Mar 21, 2016
  37. laanwj merged this on Mar 23, 2016
  38. laanwj closed this on Mar 23, 2016

  39. laanwj referenced this in commit e2ebd259fb on Mar 23, 2016
  40. laanwj commented at 12:25 PM on March 23, 2016: member

    ACK fe00ca7

  41. achow101 deleted the branch on Oct 29, 2016
  42. codablock referenced this in commit 89161bf287 on Sep 16, 2017
  43. codablock referenced this in commit b926c3d11a on Sep 19, 2017
  44. codablock referenced this in commit 21ba367c03 on Dec 9, 2017
  45. codablock referenced this in commit 760d58e3dc on Dec 19, 2017
  46. DrahtBot 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: 2026-04-13 15:15 UTC

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