Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: Add generatetodescriptor RPC #16943

Merged
merged 1 commit into from Oct 30, 2019

Conversation

@MarcoFalke
Copy link
Member

MarcoFalke commented Sep 23, 2019

The existing generatetoaddress RPC can only generate to scriptPubKeys that can be represented by an address. However, raw scripts (such as OP_TRUE) or P2PK can not be represented by an address, which complicates testing.

@MarcoFalke

This comment has been minimized.

Copy link
Member Author

MarcoFalke commented Sep 23, 2019

This has also been requested by @harding (https://twitter.com/hrdng/status/1101924904990056448). And everyone who liked that tweet, obviously.

@MarcoFalke MarcoFalke force-pushed the MarcoFalke:1909-rpcMiningDescriptor branch from fa20684 to fa64e2d Sep 23, 2019
@instagibbs

This comment has been minimized.

Copy link
Member

instagibbs commented Sep 23, 2019

concept ACK

@DrahtBot

This comment has been minimized.

Copy link
Contributor

DrahtBot commented Sep 23, 2019

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #16365 (Log RPC parameters (arguments) if -debug=rpcparams by LarryRuane)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Copy link
Member

promag left a comment

Concept ACK.

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Sep 25, 2019

I'm a bit ambivalent about extending the internal miner after so much effort over time to reduce it or even get rid of it. But if this helps testing, sure, concept ACK.

@fjahr

This comment has been minimized.

Copy link
Contributor

fjahr commented Sep 26, 2019

tested ACK
Ran tests and also did some manual testing.

src/rpc/mining.cpp Outdated Show resolved Hide resolved
@MarcoFalke MarcoFalke force-pushed the MarcoFalke:1909-rpcMiningDescriptor branch from fa64e2d to fada009 Sep 26, 2019
@@ -961,6 +1004,7 @@ static const CRPCCommand commands[] =


{ "generating", "generatetoaddress", &generatetoaddress, {"nblocks","address","maxtries"} },
{ "generating", "generatetodescriptor", &generatetodescriptor, {"num_blocks","descriptor","maxtries"} },

This comment has been minimized.

Copy link
@laanwj

laanwj Sep 30, 2019

Member

As it is test-only, it should probably be hidden from the normal help.

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Oct 2, 2019

Author Member

generatetoaddress isn't hidden, so I used the same. Otherwise it would also be missing from the gui-autocomplete.

@laanwj laanwj added this to the 0.20.0 milestone Oct 1, 2019
@MarcoFalke MarcoFalke removed this from the 0.20.0 milestone Oct 18, 2019
@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Oct 30, 2019

Needs update after #17192, I guess.

@MarcoFalke MarcoFalke force-pushed the MarcoFalke:1909-rpcMiningDescriptor branch from fada009 to fa144e6 Oct 30, 2019
@MarcoFalke

This comment has been minimized.

Copy link
Member Author

MarcoFalke commented Oct 30, 2019

Rebased for CHECK_NONFATAL

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Oct 30, 2019

ACK fa144e6

laanwj added a commit that referenced this pull request Oct 30, 2019
fa144e6 rpc: Add generatetodescriptor (MarcoFalke)

Pull request description:

  The existing `generatetoaddress` RPC can only generate to scriptPubKeys that can be represented by an address. However, raw scripts (such as `OP_TRUE`) or P2PK can not be represented by an address, which complicates testing.

ACKs for top commit:
  laanwj:
    ACK fa144e6

Tree-SHA512: aee934ab7e33f07c81f3b4c8ec23e7b6ddf63a1f4b86051af0bd76b75d8da1f51627cc682e5c6e42582340ca576bbf8ff724bdd43f87128ccecfa91e52d30ae7
@laanwj laanwj merged commit fa144e6 into bitcoin:master Oct 30, 2019
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@MarcoFalke MarcoFalke deleted the MarcoFalke:1909-rpcMiningDescriptor branch Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.