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 coverage for purpose arg in listlabels #26730

Closed

Conversation

brunoerg
Copy link
Contributor

This PR adds test coverage for listlabels command when specifying the purpose (send and receive).

RPCHelpMan listlabels()
{
return RPCHelpMan{"listlabels",
"\nReturns the list of all labels, or labels that are assigned to addresses with a specific purpose.\n",
{
{"purpose", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Address purpose to list labels for ('send','receive'). An empty string is the same as not providing this argument."},
},

@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 20, 2022

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

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK kristapsk
Concept ACK yusufsahinhamza, rserranon

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

@DrahtBot DrahtBot added the Tests label Dec 20, 2022
@brunoerg brunoerg force-pushed the 2022-12-wallet-labels-verbose branch from c4aa77e to c467cff Compare December 20, 2022 14:15
@yusufsahinhamza
Copy link
Contributor

Concept ACK

Copy link

@rserranon rserranon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK, Test ACK

Comment on lines +87 to +89
node2_addr = self.nodes[1].getnewaddress()
node.setlabel(node2_addr, "node2_addr")
assert_equal(node.listlabels(purpose="send"), ["node2_addr"])
Copy link

@rserranon rserranon Jan 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not clear to me why node2_addr is a send address, I tried to replicate it in regtest but all my new addresses are receive addresses, maybe is worth clarifying it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

node = self.nodes[0] -> so, node refers to self.nodes[0].

Here we are getting a new address from self.nodes[1], so it will receive btc using this address. For this reason, it can be used by self.nodes[0] to send Bitcoins.

So, node2_addr for self.nodes[1] is "receive" and for self.nodes[0] is "send".

Copy link
Contributor

@kristapsk kristapsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK c467cff

maflcko pushed a commit to bitcoin-core/gui that referenced this pull request Jan 11, 2023
…`listlabels`

c467cff test: add coverage for `purpose` arg in `listlabels` (brunoerg)

Pull request description:

  This PR adds test coverage for `listlabels` command when specifying the `purpose` (send and receive).

  https://github.com/bitcoin/bitcoin/blob/dcdfd72861c09a7945b9facc3726177a2d06fa64/src/wallet/rpc/addresses.cpp#L698-L704

ACKs for top commit:
  kristapsk:
    ACK c467cff

Tree-SHA512: 7e7143c1264692f7b22952e7c70dbe9ed3f5dcd2e3b69962a47be9f9c21b3f4a9089ca87962fbc8ff9116e7d2dbeb7f36d6a132c9ac13724a255cfe1b32373a8
@maflcko maflcko closed this Jan 11, 2023
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 11, 2023
…els`

c467cff test: add coverage for `purpose` arg in `listlabels` (brunoerg)

Pull request description:

  This PR adds test coverage for `listlabels` command when specifying the `purpose` (send and receive).

  https://github.com/bitcoin/bitcoin/blob/dcdfd72861c09a7945b9facc3726177a2d06fa64/src/wallet/rpc/addresses.cpp#L698-L704

ACKs for top commit:
  kristapsk:
    ACK c467cff

Tree-SHA512: 7e7143c1264692f7b22952e7c70dbe9ed3f5dcd2e3b69962a47be9f9c21b3f4a9089ca87962fbc8ff9116e7d2dbeb7f36d6a132c9ac13724a255cfe1b32373a8
@bitcoin bitcoin locked and limited conversation to collaborators Jan 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants