bitcoinj/bitcoinj

Serialized XPUB from an Encrypted Wallet doesn't match Unencrypted Wallet after encryption

Opened this issue · 1 comments

Today, I discovered a minor issue with the Serialized XPUB of a watching key, when the account path is something M/44H/1H/0H where account paths such as 'M/0H` or unencrypted wallets are presumably unaffected.

https://github.com/bitcoinj/bitcoinj/blob/master/core/src/main/java/org/bitcoinj/wallet/DeterministicKeyChain.java#L412-L418

        hierarchy = new DeterministicHierarchy(rootKey);
        basicKeyChain.importKey(rootKey);

        for (HDPath path : getAccountPath().ancestors()) {
            encryptNonLeaf(aesKey, chain, rootKey, path);
        }
        DeterministicKey account = encryptNonLeaf(aesKey, chain, rootKey, getAccountPath());

As seen above, the issue is with using the encrypted rootKey as the parent of the account key. This is good for an account path much as M/0H because the parent is M or the root. But for cases where BIP44 or some other standard is used, then the parent key of M/44H/1H/0H will be set to M (root) rather than to M/44H/1H. This results in the encrypted account key in having a depth of 1 instead of 3 and a different parentFingerprint.

Later when a serialized key is generated using DeterministicKey.serializePub58 the resulting XPUB string will have a different depth and a different fingerprint that results in a vastly different string.

This issue does not affect key generation since since the depth and parentFingerprint are not used for that purpose.

A test can be performed as follows by adding a few lines to DeterministicKeyChainTest.masterKeyAccount:

      
        //Simulate Wallet.fromSpendingKeyB58(PARAMS, prv58, secs)
        final String prv58 = watchingKey.serializePrivB58(params);
        assertEquals("xprv9yYQhynAmWWuz62PScx5Q2frBET2F1raaXna5A2E9Lj8XWgmKBL7S98Yand8F736j9UCTNWQeiB4yL5pLZP7JDY2tY8eszGQkiKDwBkezeS", prv58);
  // add these lines below
        final String pub58 = watchingKey.serializePubB58(params);
        assertEquals("xpub69KR9epSNBM57Z2Pc8EPiLmqbXt34Lpc5WHS2UZ5UnYMkQ8hW3T5beByUrCaNN4UpT4CLm5CXUaB1eVU1y6EG4b7dfZbSkfPUVLSkcateKJ", pub58);

        DeterministicKeyChain bip44chainEncrypted = bip44chain.toEncrypted("hello");
        final String pub58encrypted = bip44chainEncrypted.getWatchingKey().serializePubB58(params);
        assertEquals("xpub69KR9epSNBM57Z2Pc8EPiLmqbXt34Lpc5WHS2UZ5UnYMkQ8hW3T5beByUrCaNN4UpT4CLm5CXUaB1eVU1y6EG4b7dfZbSkfPUVLSkcateKJ", pub58encrypted);

Once I determine a solution in my own implementation that is forked from bitcoinj, a PR will be submitted to this repo.

I've run into the same (or at least similar) issue while working on output descriptors (and testing them with BIP44 paths. See Issue #2474.