[Wallet] add HD keypath to CKeyMetadata, report over validateaddress #8205

pull jonasschnelli wants to merge 1 commits into bitcoin:master from jonasschnelli:2016/06/hd_metadata changing 4 files +17 −3

jonasschnelli commented at 8:03 AM on June 15, 2016: contributor

Adds the HD keypath (example: `m/0'/0'/1') to the persisted CKeyMetadata object.

Currently, the used keypath scheme is still static and should be made dynamic/configurable in a different PR.
jonasschnelli added the label Wallet on Jun 15, 2016
jonasschnelli force-pushed on Jun 15, 2016

in qa/rpc-tests/wallet.py:None in a552acaf39 outdated

  55 | @@ -56,8 +56,10 @@ def run_test (self):
  56 |          assert_equal(len(self.nodes[2].listunspent()), 0)
  57 |  
  58 |          # Send 21 BTC from 0 to 2 using sendtoaddress call.
  59 | -        self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 11)
  60 | +        addr = self.nodes[2].getnewaddress()
  61 | +        self.nodes[0].sendtoaddress(addr, 11)

paveljanik commented at 8:16 AM on June 15, 2016:

Can you please fix the comment above? You are sending 11 BTC but the comment says 21 BTC.

jonasschnelli commented at 8:17 AM on June 15, 2016:

Fixed.

jonasschnelli force-pushed on Jun 15, 2016
MarcoFalke commented at 9:03 AM on June 15, 2016: member

validateaddress doc needs update as well.
jonasschnelli force-pushed on Jun 15, 2016
[Wallet] add HD keypath to CKeyMetadata, report over validateaddress a66870a1c6
jonasschnelli force-pushed on Jun 15, 2016
jonasschnelli commented at 9:15 AM on June 15, 2016: contributor

@MarcoFalke: thanks. Added mentioning in the validateaddress help.
in qa/rpc-tests/wallet.py:None in a66870a1c6
```
  59 | -        self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 11)
  60 | +        # Send 11 BTC from 0 to 2 using sendtoaddress call.
  61 | +        addr = self.nodes[2].getnewaddress()
  62 | +        self.nodes[0].sendtoaddress(addr, 11)
  63 |          self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 10)
  64 | +        assert(self.nodes[2].validateaddress(addr)["hdkeypath"] == "m/0'/0'/1'")
```
MarcoFalke commented at 9:18 AM on June 15, 2016:

Not sure about this one but I feel we could create a fresh rpc-test/hd-wallet.py or wallet-hd.py, as I expect the hd functionality will be extended further.

In the future it would be easier to extend the hd test if it was a separate file and we wouldn't need to fiddle with the already bloated wallet.py

jonasschnelli commented at 9:21 AM on June 15, 2016:

Yes. As soon as we have an bip32 import function (start a wallet with given xpriv) this makes sense. Without that, it's hard to cover the new hd features in tests. For this little change I guess it would be an overkill to start the a new test-script.

MarcoFalke commented at 6:21 PM on July 6, 2016:

I think it is wrong to use wallet.py to test HD behavior. Instead, we should set -usehd=0 for the wallet tests and create a separate file for HD test. Otherwise, the legacy wallet is completely untested.

MarcoFalke commented at 6:21 PM on July 6, 2016:

Please rebase on #8309 when merged and move this check over to the other test script.
luke-jr commented at 2:19 PM on July 8, 2016: member

I think this conflicts with key origin which is for better or worse being used in the wild.
jonasschnelli commented at 10:16 AM on July 9, 2016: contributor

Closing in favor of #8323
jonasschnelli closed this on Jul 9, 2016
DrahtBot locked this on Sep 8, 2021

Contributors

Labels