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

achow101 · 2016-03-12T16:52:57Z

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

jonasschnelli · 2016-03-12T18:49:19Z

src/rpc/mining.cpp

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

jonasschnelli · 2016-03-12T18:50:28Z

Nice!
utACK.

sipa · 2016-03-12T18:56:54Z

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.

achow101 · 2016-03-12T19:02:12Z

@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.

luke-jr · 2016-03-12T20:22:51Z

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

achow101 · 2016-03-12T20:32:08Z

This seems unnecessary.

Why?

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

Not sure what you mean by this.

laanwj · 2016-03-14T11:12:55Z

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.

achow101 · 2016-03-14T19:43:20Z

Rebased

maflcko · 2016-03-14T19:58:10Z

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

achow101 · 2016-03-14T20:14:10Z

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

maflcko · 2016-03-14T20:28:25Z

@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.

sipa · 2016-03-14T20:30:23Z

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.

achow101 · 2016-03-14T21:57:37Z

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.

maflcko · 2016-03-14T22:15:14Z

qa/rpc-tests/wallet.py

Nit:

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

Actually, you can remove the whole except block?

Actually, you can remove the whole except block?

Why?

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

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'])

achow101 · 2016-03-14T22:39:45Z

addressed nits

laanwj · 2016-03-15T08:24:32Z

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.

achow101 · 2016-03-15T20:24:22Z

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

Do you think that it should?

laanwj · 2016-03-16T09:18:59Z

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.

achow101 · 2016-03-17T12:36:05Z

OK, I'll just leave it as is.

sipa · 2016-03-17T12:40:28Z

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.

laanwj · 2016-03-21T13:14:19Z

Needs rebase.

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.

achow101 · 2016-03-21T14:01:57Z

rebased

maflcko · 2016-03-21T14:06:51Z

qa/rpc-tests/disablewallet.py

Please remove this line. The variable is unused and I'd prefer not to use it ever again: see 3b4324b#diff-dafb9035fa41af011fcf87041198a05aL309

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.

d5c5c71 RPC tests for generatetoaddress (Andrew C) fe00ca7 Create generatetoaddress rpc (Andrew C)

laanwj · 2016-03-23T12:25:18Z

ACK fe00ca7

…ress d5c5c71 RPC tests for generatetoaddress (Andrew C) fe00ca7 Create generatetoaddress rpc (Andrew C)

jonasschnelli reviewed Mar 12, 2016
View reviewed changes

jonasschnelli added the Mining label Mar 12, 2016

achow101 force-pushed the generate-to-addr branch from 4ef0344 to 983d9a5 Compare March 12, 2016 18:55

achow101 force-pushed the generate-to-addr branch from 983d9a5 to fd85325 Compare March 12, 2016 19:53

achow101 changed the title ~~[RPC] Allow mining to a specific address~~ [RPC] Add generatetoaddress rpc to mine to an address Mar 12, 2016

achow101 mentioned this pull request Mar 13, 2016

Add a regression test mode goatpig/BitcoinArmory#22

Closed

achow101 force-pushed the generate-to-addr branch 2 times, most recently from 0115c8f to 0422b5c Compare March 14, 2016 19:42

maflcko reviewed Mar 14, 2016
View reviewed changes

achow101 force-pushed the generate-to-addr branch from ef18efd to 3b0f6b1 Compare March 14, 2016 22:39

achow101 force-pushed the generate-to-addr branch from 3b0f6b1 to 8cad9b2 Compare March 21, 2016 14:01

maflcko reviewed Mar 21, 2016
View reviewed changes

achow101 force-pushed the generate-to-addr branch from 8cad9b2 to d5c5c71 Compare March 21, 2016 14:22

laanwj merged commit d5c5c71 into bitcoin:master Mar 23, 2016

laanwj added a commit that referenced this pull request Mar 23, 2016

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

e2ebd25

d5c5c71 RPC tests for generatetoaddress (Andrew C) fe00ca7 Create generatetoaddress rpc (Andrew C)

laanwj mentioned this pull request Mar 23, 2016

TODO for release notes 0.13.0 #7678

Closed

16 tasks

mruddy mentioned this pull request Apr 3, 2016

RPC: fix generatetoaddress failing to parse address #7797

Merged

achow101 deleted the generate-to-addr branch October 29, 2016 03:34

sickpig mentioned this pull request Feb 22, 2017

Bundle together deadalnix's PR 213 to 218 BitcoinUnlimited/BitcoinUnlimited#313

Merged

codablock pushed a commit to codablock/dash that referenced this pull request Dec 19, 2017

Merge bitcoin#7671: [RPC] Add generatetoaddress rpc to mine to an add…

760d58e

…ress d5c5c71 RPC tests for generatetoaddress (Andrew C) fe00ca7 Create generatetoaddress rpc (Andrew C)

dagurval mentioned this pull request Feb 12, 2018

[RPC] Add generatetoaddress rpc to mine to an address bitcoinxt/bitcoinxt#329

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

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

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

Uh oh!

Conversation

achow101 commented Mar 12, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Mar 12, 2016

Uh oh!

sipa commented Mar 12, 2016

Uh oh!

achow101 commented Mar 12, 2016

Uh oh!

luke-jr commented Mar 12, 2016

Uh oh!

achow101 commented Mar 12, 2016

Uh oh!

laanwj commented Mar 14, 2016

Uh oh!

achow101 commented Mar 14, 2016

Uh oh!

maflcko commented Mar 14, 2016

Uh oh!

achow101 commented Mar 14, 2016

Uh oh!

maflcko commented Mar 14, 2016

Uh oh!

sipa commented Mar 14, 2016

Uh oh!

achow101 commented Mar 14, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

achow101 commented Mar 14, 2016

Uh oh!

laanwj commented Mar 15, 2016

Uh oh!

achow101 commented Mar 15, 2016

Uh oh!

laanwj commented Mar 16, 2016

Uh oh!

achow101 commented Mar 17, 2016

Uh oh!

sipa commented Mar 17, 2016 via email

Uh oh!

laanwj commented Mar 21, 2016

Uh oh!

achow101 commented Mar 21, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laanwj commented Mar 23, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants