Skip to content

rpc: correct getnewaddress/getrawchangeaddress address_type helptext#23676

Merged
maflcko merged 1 commit intobitcoin:masterfrom
brianddk:rpc_bech32m_doc_update
Dec 7, 2021
Merged

rpc: correct getnewaddress/getrawchangeaddress address_type helptext#23676
maflcko merged 1 commit intobitcoin:masterfrom
brianddk:rpc_bech32m_doc_update

Conversation

@brianddk
Copy link
Contributor

@brianddk brianddk commented Dec 5, 2021

RPC calls getnewaddress/getrawchangeaddress support the address_type of bech32m but it is omitted in the RPCHelpMan help text.

The createmultisig and addmultisigaddress help text was not updated since bech32m is not yet supported in these.

@brianddk
correct rpc address_type helptext
5767208 
added address_type of `bech32m` to rpc calls `getnewaddress`/`getrawchangeaddress`
@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 5, 2021 •
edited
Loading

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #23667 (Split up rpcwallet by meshcollider)

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.

@DrahtBot DrahtBot mentioned this pull request Dec 5, 2021
Merged
@shaavan
Copy link
Contributor

shaavan commented Dec 5, 2021

Concept ACK

I agree with the updates done in this PR.

However, I tried to verify the following statement:

The createmultisig and addmultisigaddress help text was not updated since bech32m is not yet supported in these.

But I was not able to find any trusted material. It would help me correctly review the PR if you shared some article explaining this statement. Thank You.

@brianddk
Copy link
Contributor Author

brianddk commented Dec 5, 2021

@shaavan , there is exception handling in both createmultisig and addmultisigaddress that will explicitly fail the command if an address_type of bech32m is provided.

References:

shaavan
shaavan approved these changes Dec 6, 2021
View reviewed changes
Copy link
Contributor

@shaavan shaavan left a comment

Choose a reason for hiding this comment

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

ACK 5767208

Thanks for the references, @brianddk.

I have checked that the changes made in this PR are correct and complete.

@maflcko maflcko merged commit 9a53ba4 into bitcoin:master Dec 7, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 7, 2021
Merge bitcoin#23676: rpc: correct getnewaddress/`getrawchangeaddres…
102a99c 
…s` address_type helptext

5767208 correct rpc address_type helptext (brianddk)

Pull request description:

  RPC calls `getnewaddress`/`getrawchangeaddress` support the address_type of `bech32m` but it is omitted in the `RPCHelpMan` help text.

  The `createmultisig` and `addmultisigaddress` help text was not updated since `bech32m` is not yet supported in these.

ACKs for top commit:
  shaavan:
    ACK 5767208

Tree-SHA512: 3c0cfb96019ca6d316c4a2fe27786d1b621c49b31b3aa61068bad737a5a0ceed89babad704b9923f9aedcabfa670d752916803bdf22236403061ddf9295a2637
RandyMcMillan pushed a commit to RandyMcMillan/mempool-tab that referenced this pull request Dec 23, 2021
Merge bitcoin/bitcoin#23676: rpc: correct getnewaddress/`getrawchan…
d64c6d5 
…geaddress` address_type helptext

66ba3b8 correct rpc address_type helptext (brianddk)

Pull request description:

  RPC calls `getnewaddress`/`getrawchangeaddress` support the address_type of `bech32m` but it is omitted in the `RPCHelpMan` help text.

  The `createmultisig` and `addmultisigaddress` help text was not updated since `bech32m` is not yet supported in these.

ACKs for top commit:
  shaavan:
    ACK 66ba3b8

Tree-SHA512: 3c0cfb96019ca6d316c4a2fe27786d1b621c49b31b3aa61068bad737a5a0ceed89babad704b9923f9aedcabfa670d752916803bdf22236403061ddf9295a2637
@bitcoin bitcoin locked and limited conversation to collaborators Dec 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@shaavan shaavan shaavan approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

RPC/REST/ZMQ Wallet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@brianddk @DrahtBot @shaavan @maflcko