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

validateaddress and getaddressinfo RPC Update #731

Merged
merged 3 commits into from
Mar 26, 2019

Conversation

tynes
Copy link
Member

@tynes tynes commented Mar 16, 2019

Overview

Update the node rpc method validateaddress to match the latest bitcoind interface.
The new interface is defined here: https://bitcoincore.org/en/doc/0.17.0/rpc/util/validateaddress/

Add the wallet rpc method getaddressinfo
https://bitcoincore.org/en/doc/0.17.0/rpc/wallet/getaddressinfo/

Notably, the properties ismine and iswatchonly are moved to getaddressinfo,
which is a good thing because in bcoin, validateaddress had them
hardcoded to false anyways, and we know of an exchange that
was attempting to use the value ismine for integrations into their system.
ismine now properly returns the correct value for addresses that
fall into the lookahead

Tests are added for p2pkh, p2sh, p2wpkh and p2wsh. I don't love that I had
to add undefined keys to the node-test.js file, but the undefined values
are being included in the object and other tests in the same file have hardcoded
undefined values as well. I think its because the objects aren't going over the
wire so the undefined values aren't being removed from the objects when
they get stringified

See changelog here: https://github.com/bitcoin/bitcoin/blob/53f26cd11dc04a0ab7a3ee72a3469050c4c2bc17/doc/release-notes/release-notes-0.17.0.md
See changes here: bitcoin/bitcoin#10583

Examples

Starting bitcoind with the command:

bitcoind -server -rest -regtest

And starting bcoin with the command:

bcoin --network regtest

getaddressinfo

p2pkh

bitcoind
$ bitcoin-cli -regtest getaddressinfo mkpZhYtJu2r87Js3pDiWJDmPte2NRZ8bJV
{
  "address": "mkpZhYtJu2r87Js3pDiWJDmPte2NRZ8bJV",
  "scriptPubKey": "76a9143a2d4145a4f098523b3e8127f1da87cfc55b8e7988ac",
  "ismine": false,
  "iswatchonly": false,
  "isscript": false,
  "iswitness": false,
  "labels": [
  ]
}
bcoin
$ bwallet-cli --network regtest rpc getaddressinfo mkpZhYtJu2r87Js3pDiWJDmPte2NRZ8bJV
{
  "address": "mkpZhYtJu2r87Js3pDiWJDmPte2NRZ8bJV",
  "scriptPubKey": "76a9143a2d4145a4f098523b3e8127f1da87cfc55b8e7988ac",
  "ismine": false,
  "iswatchonly": false,
  "isscript": false,
  "iswitness": false
}

p2sh

bitcoind
$ bitcoin-cli -regtest getaddressinfo 2NEoayrnYbRVSLmp1Y9buNsVGjWsU36Wagu
{
  "address": "2NEoayrnYbRVSLmp1Y9buNsVGjWsU36Wagu",
  "scriptPubKey": "a914ec7979ddc7ba13eefca73703c8a1da47eae965fa87",
  "ismine": false,
  "iswatchonly": false,
  "isscript": true,
  "iswitness": false,
  "labels": [
  ]
}
bcoin
$ bwallet-cli --network regtest rpc getaddressinfo 2NEoayrnYbRVSLmp1Y9buNsVGjWsU36Wagu
{
  "address": "2NEoayrnYbRVSLmp1Y9buNsVGjWsU36Wagu",
  "scriptPubKey": "a914ec7979ddc7ba13eefca73703c8a1da47eae965fa87",
  "ismine": false,
  "iswatchonly": false,
  "isscript": true,
  "iswitness": false
}

p2wpkh

bitcoind
$ bitcoin-cli -regtest getaddressinfo bcrt1q8gk5z3dy7zv9ywe7synlrk58elz4hrnegvpv6m
{
  "address": "bcrt1q8gk5z3dy7zv9ywe7synlrk58elz4hrnegvpv6m",
  "scriptPubKey": "00143a2d4145a4f098523b3e8127f1da87cfc55b8e79",
  "ismine": false,
  "iswatchonly": false,
  "isscript": false,
  "iswitness": true,
  "witness_version": 0,
  "witness_program": "3a2d4145a4f098523b3e8127f1da87cfc55b8e79",
  "labels": [
  ]
}
bcoin
$ bwallet-cli --network regtest rpc getaddressinfo bcrt1q8gk5z3dy7zv9ywe7synlrk58elz4hrnegvpv6m
{
  "address": "bcrt1q8gk5z3dy7zv9ywe7synlrk58elz4hrnegvpv6m",
  "scriptPubKey": "00143a2d4145a4f098523b3e8127f1da87cfc55b8e79",
  "ismine": false,
  "iswatchonly": false,
  "isscript": false,
  "iswitness": true,
  "witness_version": 0,
  "witness_program": "3a2d4145a4f098523b3e8127f1da87cfc55b8e79"
}

p2wsh

bitcoind
bitcoin-cli -regtest getaddressinfo bcrt1qy48aglvtadmaerzngkedgswms528344p5jlvpahwvl5cgsxjtmlqcz0253
{
  "address": "bcrt1qy48aglvtadmaerzngkedgswms528344p5jlvpahwvl5cgsxjtmlqcz0253",
  "scriptPubKey": "0020254fd47d8beb77dc8c5345b2d441db851478d6a1a4bec0f6ee67e98440d25efe",
  "ismine": false,
  "iswatchonly": false,
  "isscript": true,
  "iswitness": true,
  "witness_version": 0,
  "witness_program": "254fd47d8beb77dc8c5345b2d441db851478d6a1a4bec0f6ee67e98440d25efe",
  "labels": [
  ]
}
bcoin
$ bwallet-cli --network regtest rpc getaddressinfo bcrt1qy48aglvtadmaerzngkedgswms528344p5jlvpahwvl5cgsxjtmlqcz0253
{
  "address": "bcrt1qy48aglvtadmaerzngkedgswms528344p5jlvpahwvl5cgsxjtmlqcz0253",
  "scriptPubKey": "0020254fd47d8beb77dc8c5345b2d441db851478d6a1a4bec0f6ee67e98440d25efe",
  "ismine": false,
  "iswatchonly": false,
  "isscript": true,
  "iswitness": true,
  "witness_version": 0,
  "witness_program": "254fd47d8beb77dc8c5345b2d441db851478d6a1a4bec0f6ee67e98440d25efe"
}

validateaddress

p2pkh

bitcoind
$ bitcoin-cli -regtest validateaddress mkpZhYtJu2r87Js3pDiWJDmPte2NRZ8bJV
{
  "isvalid": true,
  "address": "mkpZhYtJu2r87Js3pDiWJDmPte2NRZ8bJV",
  "scriptPubKey": "76a9143a2d4145a4f098523b3e8127f1da87cfc55b8e7988ac",
  "isscript": false,
  "iswitness": false
}
bcoin
$ bcoin-cli --network regtest rpc validateaddress mkpZhYtJu2r87Js3pDiWJDmPte2NRZ8bJV
{
  "isvalid": true,
  "address": "mkpZhYtJu2r87Js3pDiWJDmPte2NRZ8bJV",
  "scriptPubKey": "76a9143a2d4145a4f098523b3e8127f1da87cfc55b8e7988ac",
  "isscript": false,
  "iswitness": false
}

p2sh

bitcoind
$ bitcoin-cli -regtest validateaddress 2NEoayrnYbRVSLmp1Y9buNsVGjWsU36Wagu
{
  "isvalid": true,
  "address": "2NEoayrnYbRVSLmp1Y9buNsVGjWsU36Wagu",
  "scriptPubKey": "a914ec7979ddc7ba13eefca73703c8a1da47eae965fa87",
  "isscript": true,
  "iswitness": false
}
bcoin
$ bcoin-cli --network regtest rpc validateaddress 2NEoayrnYbRVSLmp1Y9buNsVGjWsU36Wagu
{
  "isvalid": true,
  "address": "2NEoayrnYbRVSLmp1Y9buNsVGjWsU36Wagu",
  "scriptPubKey": "a914ec7979ddc7ba13eefca73703c8a1da47eae965fa87",
  "isscript": true,
  "iswitness": false
}

p2wpkh

bitcoind
$ bitcoin-cli -regtest validateaddress bcrt1q8gk5z3dy7zv9ywe7synlrk58elz4hrnegvpv6m
{
  "isvalid": true,
  "address": "bcrt1q8gk5z3dy7zv9ywe7synlrk58elz4hrnegvpv6m",
  "scriptPubKey": "00143a2d4145a4f098523b3e8127f1da87cfc55b8e79",
  "isscript": false,
  "iswitness": true,
  "witness_version": 0,
  "witness_program": "3a2d4145a4f098523b3e8127f1da87cfc55b8e79"
}
bcoin
$ bcoin-cli --network regtest rpc validateaddress bcrt1q8gk5z3dy7zv9ywe7synlrk58elz4hrnegvpv6m
{
  "isvalid": true,
  "address": "bcrt1q8gk5z3dy7zv9ywe7synlrk58elz4hrnegvpv6m",
  "scriptPubKey": "00143a2d4145a4f098523b3e8127f1da87cfc55b8e79",
  "isscript": false,
  "iswitness": true,
  "witness_version": 0,
  "witness_program": "3a2d4145a4f098523b3e8127f1da87cfc55b8e79"
}

p2wsh

bitcoind
$ bitcoin-cli -regtest validateaddress bcrt1qy48aglvtadmaerzngkedgswms528344p5jlvpahwvl5cgsxjtmlqcz0253
{
  "isvalid": true,
  "address": "bcrt1qy48aglvtadmaerzngkedgswms528344p5jlvpahwvl5cgsxjtmlqcz0253",
  "scriptPubKey": "0020254fd47d8beb77dc8c5345b2d441db851478d6a1a4bec0f6ee67e98440d25efe",
  "isscript": true,
  "iswitness": true,
  "witness_version": 0,
  "witness_program": "254fd47d8beb77dc8c5345b2d441db851478d6a1a4bec0f6ee67e98440d25efe"
}
bcoin
$ bcoin-cli --network regtest rpc validateaddress bcrt1qy48aglvtadmaerzngkedgswms528344p5jlvpahwvl5cgsxjtmlqcz0253
{
  "isvalid": true,
  "address": "bcrt1qy48aglvtadmaerzngkedgswms528344p5jlvpahwvl5cgsxjtmlqcz0253",
  "scriptPubKey": "0020254fd47d8beb77dc8c5345b2d441db851478d6a1a4bec0f6ee67e98440d25efe",
  "isscript": true,
  "iswitness": true,
  "witness_version": 0,
  "witness_program": "254fd47d8beb77dc8c5345b2d441db851478d6a1a4bec0f6ee67e98440d25efe"
}

@tynes tynes requested a review from braydonf March 16, 2019 05:56
@codecov-io

This comment has been minimized.

@tynes tynes added compatibility Incompatibility with other implementations tests Has tests for code change is just an addtional test labels Mar 16, 2019
@tynes tynes added enhancement Improving a current feature wallet Wallet related labels Mar 19, 2019
@tynes tynes changed the title validateaddress rpc update validateaddress and getaddressinfo RPC Update Mar 19, 2019
@tynes tynes added the ready for review Ready to be reviewed label Mar 22, 2019
@tynes
Copy link
Member Author

tynes commented Mar 22, 2019

Realized I need to also update the CHANGELOG.md

Updated the CHANGELOG.md

@braydonf
Copy link
Contributor

Looks good, a few minor nits. It's good to resolve those values from being hardcoded and sometimes incorrect values.The watchonly is defined by the wallet in bcoin, so the per address check doesn't completely map over, however good to have the value be correct.

@braydonf
Copy link
Contributor

braydonf commented Mar 26, 2019

I also get an ischange property in the result from getaddressinfo in the 0.18 branch, worth including?

updates validateaddress to match the changes
in bitcoind bitcoin/bitcoin#10583.
removes the fields ismine and iswatchonly, those
are things that a wallet would know, not a node.
adds segwit related fields, witness_version and
witness_program. test changes to the node rpc method
validateaddress with p2pkh, p2sh, p2wpkh and p2wsh addresses
@tynes tynes force-pushed the validateaddress-upgrade branch from a42580f to 2119591 Compare March 26, 2019 23:08
update the rpc command to better match bitcoind.
add the ismine and iswatchonly fields and
segwit related fields such as witness_version
and witness_program.
@tynes tynes force-pushed the validateaddress-upgrade branch from 2119591 to 2b7c827 Compare March 26, 2019 23:15
@tynes tynes force-pushed the validateaddress-upgrade branch from 2b7c827 to 6850a32 Compare March 26, 2019 23:19
@braydonf braydonf merged commit 6850a32 into bcoin-org:master Mar 26, 2019
braydonf added a commit that referenced this pull request Mar 26, 2019
validateaddress and getaddressinfo RPC Update
@braydonf braydonf removed the ready for review Ready to be reviewed label Mar 26, 2019
tynes pushed a commit to tynes/hsd that referenced this pull request Apr 2, 2019
Adds a new wallet RPC method getaddressinfo that
corresponds to changes with bitcoind. Returns values:

- address
- ismine
- iswatchonly
- ischange
- isspendable
- isscript
- witness_version
- witness_program

The ismine property previously existed on the node RPC
method validateaddress which was always hardcoded to true.
An additional PR will be added to update validateaddress
to remove the ismine property. isspendable refers to
an address with null data, of version 31.

See PR in bcoin: bcoin-org/bcoin#731
See bitcoind docs:
https://bitcoincore.org/en/doc/0.17.0/rpc/wallet/getaddressinfo/
tynes pushed a commit to tynes/hsd that referenced this pull request Apr 2, 2019
This PR updates the node rpc validateaddress to
better seperate the wallet and the node. The rpc
was returning ismine and iswatch only. These values
were moved to the wallet rpc method getaddressinfo.
This corresponds to a change in bitcoind and bcoin.

The updated validateaddress rpc returns the values:

- isvalid
- address
- ismine
- iswatchonly
- isscript
- isspendable
- witness_version
- witness_program

See PR in bcoin: bcoin-org/bcoin#731
See PR in bitcoind: bitcoin/bitcoin#10583
boymanjor pushed a commit to handshake-org/hsd that referenced this pull request Apr 9, 2019
This PR updates the node rpc validateaddress to
better seperate the wallet and the node. The rpc
was returning ismine and iswatch only. These values
were moved to the wallet rpc method getaddressinfo.
This corresponds to a change in bitcoind and bcoin.

The updated validateaddress rpc returns the values:

- isvalid
- address
- ismine
- iswatchonly
- isscript
- isspendable
- witness_version
- witness_program

See PR in bcoin: bcoin-org/bcoin#731
See PR in bitcoind: bitcoin/bitcoin#10583
tynes pushed a commit to tynes/hsd that referenced this pull request Apr 12, 2019
Adds a new wallet RPC method getaddressinfo that
corresponds to changes with bitcoind. Returns values:

- address
- ismine
- iswatchonly
- ischange
- isspendable
- isscript
- witness_version
- witness_program

The ismine property previously existed on the node RPC
method validateaddress which was always hardcoded to true.
An additional PR will be added to update validateaddress
to remove the ismine property. isspendable refers to
an address with null data, of version 31.

See PR in bcoin: bcoin-org/bcoin#731
See bitcoind docs:
https://bitcoincore.org/en/doc/0.17.0/rpc/wallet/getaddressinfo/
tynes pushed a commit to tynes/hsd that referenced this pull request May 17, 2019
Adds a new wallet RPC method getaddressinfo that
corresponds to changes with bitcoind. Returns values:

- address
- ismine
- iswatchonly
- ischange
- isspendable
- isscript
- witness_version
- witness_program

The ismine property previously existed on the node RPC
method validateaddress which was always hardcoded to true.
An additional PR will be added to update validateaddress
to remove the ismine property. isspendable refers to
an address with null data, of version 31.

See PR in bcoin: bcoin-org/bcoin#731
See bitcoind docs:
https://bitcoincore.org/en/doc/0.17.0/rpc/wallet/getaddressinfo/
tynes pushed a commit to tynes/hsd that referenced this pull request May 19, 2019
Adds a new wallet RPC method getaddressinfo that
corresponds to changes with bitcoind. Returns values:

- address
- ismine
- iswatchonly
- ischange
- isspendable
- isscript
- witness_version
- witness_program

The ismine property previously existed on the node RPC
method validateaddress which was always hardcoded
to true.

Commit 62b4de5
updates `validateaddress` to remove the `ismine`
property because that property can now be found
on the `getaddressinfo` RPC.

The `isspendable` property refers to an address
will null data, which is version 31.

See PR in bcoin: bcoin-org/bcoin#731
See bitcoind docs:
https://bitcoincore.org/en/doc/0.17.0/rpc/wallet/getaddressinfo/
@braydonf braydonf added this to the 2.0.0 milestone Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility Incompatibility with other implementations enhancement Improving a current feature tests Has tests for code change is just an addtional test wallet Wallet related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants