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.

Sign up for GitHub

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

Jump to bottom

legacyrpc: add address type arg to getNewAddress #783

Closed
wants to merge 1 commit into from
Closed

legacyrpc: add address type arg to getNewAddress #783

wants to merge 1 commit into from

Conversation

chappjc
Copy link
Contributor

@chappjc chappjc commented Dec 3, 2021 •
edited
Loading

Resolves #646 and #762
Requires btcsuite/btcd#1776 (see the replace to my btcd repository that would be removed if that is merged). The btcctl from that branch would also be needed for testing this.

This handles the AddressType argument in the getNewAddress and getRawChangeAddress RPC handlers. It recognizes "legacy", "p2sh-segwit", and "bech32" to match Bitcoin Core's RPC options. These correspond to the bip44, bip49 "plus", and bip84 paths. These are the waddrmgr.DefaultKeyScopes.

The validateaddress RPC is already able to recognize these addresses, and it may be used to verify the addresses returned by both getnewaddress and getrawchangeaddress.

This adds btcjson.ErrRPCWalletInvalidAddressType with a code (-5) and message ("unknown address type") to match Bitcoin Core's v22.

This was referenced Dec 3, 2021
Merged
Merged
Closed
@chappjc
Copy link
Contributor Author

chappjc commented Dec 22, 2021

Updated getrawchangeaddress as well.

@chappjc chappjc force-pushed the bech32-getnewaddress branch 3 times, most recently from 7f67f28 to 896c627 Compare March 22, 2022 21:25
@chappjc
Copy link
Contributor Author

chappjc commented Apr 9, 2022

Removed the temporary replace and rebased. I need to re-test on my end though.

CI is bombing now because it started using go 1.18 and go get works differently than it used to. I think #800 is needed.

@chappjc
Copy link
Contributor Author

chappjc commented Apr 10, 2022

All still working as expected on my end after rebasing. Can get addresses of all three type and they validate as ismine.

This was referenced Apr 13, 2022
Closed
Merged
Merged
@Roasbeef
Copy link
Member

This slipped by, approved a CI run.

@chappjc
Copy link
Contributor Author

chappjc commented Apr 27, 2022 •
edited
Loading

Looks like I need to rebase. Also I'd fix up the change to sweepaccount in this PR if you would like to accept the rpcclient changes proposed in btcsuite/btcd#1844

@chappjc
Copy link
Contributor Author

chappjc commented Jun 2, 2022 •
edited
Loading

Now that btcd 0.23 is tagged with rpcclient using btcsuite/btcd#1776, this PR will have to get merged before a new btcwallet release otherwise rpcclient will not be able to get new addresses from btcwallet. See btcsuite/btcd#1843

@chappjc chappjc mentioned this pull request Jun 2, 2022
Closed
@chappjc chappjc force-pushed the bech32-getnewaddress branch 3 times, most recently from 1b3229f to 66ab03b Compare June 7, 2022 13:46
@chappjc
legacyrpc: add address type arg to getNewAddress and getRawChangeAddress
186a4f9 
This handles the AddressType argument in the getNewAddress RPC handler.
It recognizes "legacy", "p2sh-segwit", and "bech32" to match Bitcoin
Core's RPC options.  These correspond to bip44, bip49 "plus", and bip84.

These are the waddrmgr.DefaultKeyScopes.

The validateaddress RPC is already able to recognize these addresses,
and it may be used to verify the addresses returned by getnewaddress.

This adds btcjson.ErrRPCWalletInvalidAddressType with a code (-5) and
message ("unknown address type") to match Bitcoin Core's v22.
@chappjc chappjc mentioned this pull request Jun 7, 2022
Merged
@chappjc
Copy link
Contributor Author

chappjc commented Jun 7, 2022

Just retested with the following commands:

getnewaddress
getnewaddress "default"
getnewaddress "default" "legacy"
getnewaddress "default" "p2sh-segwit"
getnewaddress "default" "bech32"
getrawchangeaddress "default" "legacy"
getrawchangeaddress "default" "p2sh-segwit"
getrawchangeaddress "default" "bech32"

All working as expected.

I had to update to neutrino 0.14.2 locally to build, but that is also done in #805

@Crypt-iQ
Copy link
Contributor

Crypt-iQ commented Jun 7, 2022

I'll pull this into #805 and give commit credit once the neutrino upgrade issue is resolved

@Crypt-iQ
Copy link
Contributor

Crypt-iQ commented Jun 7, 2022

Replaced by #805

@chappjc chappjc closed this Jun 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bech32 not available for address RPC commands
3 participants
@chappjc @Roasbeef @Crypt-iQ