From 823a03db775c3cef53c552c059a9d661a1793967 Mon Sep 17 00:00:00 2001 From: Sean Gilligan Date: Sun, 30 Jun 2019 15:19:12 -0700 Subject: [PATCH] Move HDPath methods from HDUtils to HDPath All HDPath-related methods in HDUtils are now deprecated and delegate to HDPath. HDPath now contains parsePath code. --- .../crypto/DeterministicHierarchy.java | 13 ++--- .../org/bitcoinj/crypto/DeterministicKey.java | 6 +-- .../org/bitcoinj/crypto/HDKeyDerivation.java | 4 +- .../main/java/org/bitcoinj/crypto/HDPath.java | 24 +++++++++ .../java/org/bitcoinj/crypto/HDUtils.java | 49 +++++++++---------- .../wallet/DeterministicKeyChain.java | 6 +-- .../org/bitcoinj/wallet/KeyChainGroup.java | 12 ++--- .../java/org/bitcoinj/crypto/HDUtilsTest.java | 2 + 8 files changed, 70 insertions(+), 46 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/crypto/DeterministicHierarchy.java b/core/src/main/java/org/bitcoinj/crypto/DeterministicHierarchy.java index 611c52c04..ae27d71b5 100644 --- a/core/src/main/java/org/bitcoinj/crypto/DeterministicHierarchy.java +++ b/core/src/main/java/org/bitcoinj/crypto/DeterministicHierarchy.java @@ -79,13 +79,14 @@ public class DeterministicHierarchy { * @throws IllegalArgumentException if create is false and the path was not found. */ public DeterministicKey get(List path, boolean relativePath, boolean create) { + HDPath inputPath = HDPath.of(path); HDPath absolutePath = relativePath ? rootPath.extend(path) - : HDPath.of(path); + : inputPath; if (!keys.containsKey(absolutePath)) { if (!create) throw new IllegalArgumentException(String.format(Locale.US, "No key found for %s path %s.", - relativePath ? "relative" : "absolute", HDUtils.formatPath(path))); + relativePath ? "relative" : "absolute", inputPath.toString())); checkArgument(absolutePath.size() > 0, "Can't derive the master key: nothing to derive from."); DeterministicKey parent = get(absolutePath.subList(0, absolutePath.size() - 1), false, true); putKey(HDKeyDerivation.deriveChildKey(parent, absolutePath.get(absolutePath.size() - 1))); @@ -101,7 +102,7 @@ public class DeterministicHierarchy { * @param relative whether the path is relative to the root path * @param createParent whether the parent corresponding to path should be created (with any necessary ancestors) if it doesn't exist already * @param privateDerivation whether to use private or public derivation - * @return next newly created key using the child derivation funtcion + * @return next newly created key using the child derivation function * @throws IllegalArgumentException if the parent doesn't exist and createParent is false. */ public DeterministicKey deriveNextChild(List parentPath, boolean relative, boolean createParent, boolean privateDerivation) { @@ -116,14 +117,14 @@ public class DeterministicHierarchy { throw new HDDerivationException("Maximum number of child derivation attempts reached, this is probably an indication of a bug."); } - private ChildNumber getNextChildNumberToDerive(List path, boolean privateDerivation) { + private ChildNumber getNextChildNumberToDerive(HDPath path, boolean privateDerivation) { ChildNumber lastChildNumber = lastChildNumbers.get(path); ChildNumber nextChildNumber = new ChildNumber(lastChildNumber != null ? lastChildNumber.num() + 1 : 0, privateDerivation); - lastChildNumbers.put(HDPath.of(path), nextChildNumber); + lastChildNumbers.put(path, nextChildNumber); return nextChildNumber; } - public int getNumChildren(List path) { + public int getNumChildren(HDPath path) { final ChildNumber cn = lastChildNumbers.get(path); if (cn == null) return 0; diff --git a/core/src/main/java/org/bitcoinj/crypto/DeterministicKey.java b/core/src/main/java/org/bitcoinj/crypto/DeterministicKey.java index 7b58ccbea..94a1c1cd8 100644 --- a/core/src/main/java/org/bitcoinj/crypto/DeterministicKey.java +++ b/core/src/main/java/org/bitcoinj/crypto/DeterministicKey.java @@ -188,10 +188,10 @@ public class DeterministicKey extends ECKey { } /** - * Returns the path of this key as a human readable string starting with M to indicate the master key. + * Returns the path of this key as a human readable string starting with M or m to indicate the master key. */ public String getPathAsString() { - return HDUtils.formatPath(getPath()); + return getPath().toString(); } /** @@ -558,7 +558,7 @@ public class DeterministicKey extends ECKey { throw new IllegalArgumentException("Parent was provided but this key doesn't have one"); if (parent.getFingerprint() != parentFingerprint) throw new IllegalArgumentException("Parent fingerprints don't match"); - path = HDUtils.append(parent.getPath(), childNumber); + path = parent.getPath().extend(childNumber); if (path.size() != depth) throw new IllegalArgumentException("Depth does not match"); } else { diff --git a/core/src/main/java/org/bitcoinj/crypto/HDKeyDerivation.java b/core/src/main/java/org/bitcoinj/crypto/HDKeyDerivation.java index f15393b90..4e2f840fd 100644 --- a/core/src/main/java/org/bitcoinj/crypto/HDKeyDerivation.java +++ b/core/src/main/java/org/bitcoinj/crypto/HDKeyDerivation.java @@ -146,7 +146,7 @@ public final class HDKeyDerivation { public static DeterministicKey deriveChildKeyFromPrivate(DeterministicKey parent, ChildNumber childNumber) throws HDDerivationException { RawKeyBytes rawKey = deriveChildKeyBytesFromPrivate(parent, childNumber); - return new DeterministicKey(HDUtils.append(parent.getPath(), childNumber), rawKey.chainCode, + return new DeterministicKey(parent.getPath().extend(childNumber), rawKey.chainCode, new BigInteger(1, rawKey.keyBytes), parent); } @@ -182,7 +182,7 @@ public final class HDKeyDerivation { public static DeterministicKey deriveChildKeyFromPublic(DeterministicKey parent, ChildNumber childNumber, PublicDeriveMode mode) throws HDDerivationException { RawKeyBytes rawKey = deriveChildKeyBytesFromPublic(parent, childNumber, PublicDeriveMode.NORMAL); - return new DeterministicKey(HDUtils.append(parent.getPath(), childNumber), rawKey.chainCode, + return new DeterministicKey(parent.getPath().extend(childNumber), rawKey.chainCode, new LazyECPoint(ECKey.CURVE.getCurve(), rawKey.keyBytes), null, parent); } diff --git a/core/src/main/java/org/bitcoinj/crypto/HDPath.java b/core/src/main/java/org/bitcoinj/crypto/HDPath.java index 96a5180f2..e5b0a04ed 100644 --- a/core/src/main/java/org/bitcoinj/crypto/HDPath.java +++ b/core/src/main/java/org/bitcoinj/crypto/HDPath.java @@ -16,6 +16,7 @@ package org.bitcoinj.crypto; +import javax.annotation.Nonnull; import java.util.AbstractList; import java.util.ArrayList; import java.util.Arrays; @@ -66,6 +67,29 @@ public class HDPath extends AbstractList { return HDPath.of(Collections.emptyList()); } + /** + * Create an HDPath from a path string. The path string is a human-friendly representation of the deterministic path. For example: + * + * "44H / 0H / 0H / 1 / 1" + * + * Where a letter "H" means hardened key. Spaces are ignored. + */ + public static HDPath parsePath(@Nonnull String path) { + String[] parsedNodes = path.replace("M", "").split("/"); + List nodes = new ArrayList<>(); + + for (String n : parsedNodes) { + n = n.replaceAll(" ", ""); + if (n.length() == 0) continue; + boolean isHard = n.endsWith("H"); + if (isHard) n = n.substring(0, n.length() - 1); + int nodeNumber = Integer.parseInt(n); + nodes.add(new ChildNumber(nodeNumber, isHard)); + } + + return new HDPath(nodes); + } + /** * Is this a path to a private key? * diff --git a/core/src/main/java/org/bitcoinj/crypto/HDUtils.java b/core/src/main/java/org/bitcoinj/crypto/HDUtils.java index 252824c8e..a91fc8351 100644 --- a/core/src/main/java/org/bitcoinj/crypto/HDUtils.java +++ b/core/src/main/java/org/bitcoinj/crypto/HDUtils.java @@ -18,24 +18,19 @@ package org.bitcoinj.crypto; import org.bitcoinj.core.ECKey; -import com.google.common.base.Joiner; -import com.google.common.collect.Iterables; import org.bouncycastle.crypto.digests.SHA512Digest; import org.bouncycastle.crypto.macs.HMac; import org.bouncycastle.crypto.params.KeyParameter; import javax.annotation.Nonnull; import java.nio.ByteBuffer; -import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.List; /** * Static utilities used in BIP 32 Hierarchical Deterministic Wallets (HDW). */ public final class HDUtils { - private static final Joiner PATH_JOINER = Joiner.on("/"); static HMac createHmacSha512Digest(byte[] key) { SHA512Digest digest = new SHA512Digest(); @@ -66,41 +61,43 @@ public final class HDUtils { return bytes; } - /** Append a derivation level to an existing path */ + /** + * Append a derivation level to an existing path + * + * @deprecated Use {@code HDPath#extend} + */ + @Deprecated public static HDPath append(List path, ChildNumber childNumber) { return new HDPath(path).extend(childNumber); } - /** Concatenate two derivation paths */ + /** + * Concatenate two derivation paths + * + * @deprecated Use {@code HDPath#extend} + */ + @Deprecated public static HDPath concat(List path, List path2) { return new HDPath(path).extend(path2); } - /** Convert to a string path, starting with "M/" */ + /** + * Convert to a string path, starting with "M/" + * + * @deprecated Use {@code HDPath#toString} + */ + @Deprecated public static String formatPath(List path) { - return PATH_JOINER.join(Iterables.concat(Collections.singleton("M"), path)); + return HDPath.of(path).toString(); } /** - * The path is a human-friendly representation of the deterministic path. For example: + * Create an HDPath from a path string. * - * "44H / 0H / 0H / 1 / 1" - * - * Where a letter "H" means hardened key. Spaces are ignored. + * @deprecated Use {@code HDPath.parsePath} */ + @Deprecated public static HDPath parsePath(@Nonnull String path) { - String[] parsedNodes = path.replace("M", "").split("/"); - List nodes = new ArrayList<>(); - - for (String n : parsedNodes) { - n = n.replaceAll(" ", ""); - if (n.length() == 0) continue; - boolean isHard = n.endsWith("H"); - if (isHard) n = n.substring(0, n.length() - 1); - int nodeNumber = Integer.parseInt(n); - nodes.add(new ChildNumber(nodeNumber, isHard)); - } - - return new HDPath(nodes); + return HDPath.parsePath(path); } } diff --git a/core/src/main/java/org/bitcoinj/wallet/DeterministicKeyChain.java b/core/src/main/java/org/bitcoinj/wallet/DeterministicKeyChain.java index f3774758e..449e11d56 100644 --- a/core/src/main/java/org/bitcoinj/wallet/DeterministicKeyChain.java +++ b/core/src/main/java/org/bitcoinj/wallet/DeterministicKeyChain.java @@ -412,8 +412,8 @@ public class DeterministicKeyChain implements EncryptableKeyChain { encryptNonLeaf(aesKey, chain, rootKey, getAccountPath().subList(0, i)); } DeterministicKey account = encryptNonLeaf(aesKey, chain, rootKey, getAccountPath()); - externalParentKey = encryptNonLeaf(aesKey, chain, account, HDUtils.concat(getAccountPath(), EXTERNAL_SUBPATH)); - internalParentKey = encryptNonLeaf(aesKey, chain, account, HDUtils.concat(getAccountPath(), INTERNAL_SUBPATH)); + externalParentKey = encryptNonLeaf(aesKey, chain, account, getAccountPath().extend(EXTERNAL_SUBPATH)); + internalParentKey = encryptNonLeaf(aesKey, chain, account, getAccountPath().extend(INTERNAL_SUBPATH)); // Now copy the (pubkey only) leaf keys across to avoid rederiving them. The private key bytes are missing // anyway so there's nothing to encrypt. @@ -503,7 +503,7 @@ public class DeterministicKeyChain implements EncryptableKeyChain { basicKeyChain.importKeys(lookahead); List keys = new ArrayList<>(numberOfKeys); for (int i = 0; i < numberOfKeys; i++) { - HDPath path = HDUtils.append(parentKey.getPath(), new ChildNumber(index - numberOfKeys + i, false)); + HDPath path = parentKey.getPath().extend(new ChildNumber(index - numberOfKeys + i, false)); DeterministicKey k = hierarchy.get(path, false, false); // Just a last minute sanity check before we hand the key out to the app for usage. This isn't inspired // by any real problem reports from bitcoinj users, but I've heard of cases via the grapevine of diff --git a/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java b/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java index 395ed9dcd..6a4407ac7 100644 --- a/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java +++ b/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java @@ -1019,17 +1019,17 @@ public class KeyChainGroup implements KeyBag { // kinds of KeyPurpose are introduced. if (activeChain.getIssuedExternalKeys() > 0) { DeterministicKey currentExternalKey = activeChain.getKeyByPath( - HDUtils.append( - HDUtils.concat(activeChain.getAccountPath(), DeterministicKeyChain.EXTERNAL_SUBPATH), - new ChildNumber(activeChain.getIssuedExternalKeys() - 1))); + activeChain.getAccountPath() + .extend(DeterministicKeyChain.EXTERNAL_SUBPATH) + .extend(new ChildNumber(activeChain.getIssuedExternalKeys() - 1))); currentKeys.put(KeyChain.KeyPurpose.RECEIVE_FUNDS, currentExternalKey); } if (activeChain.getIssuedInternalKeys() > 0) { DeterministicKey currentInternalKey = activeChain.getKeyByPath( - HDUtils.append( - HDUtils.concat(activeChain.getAccountPath(), DeterministicKeyChain.INTERNAL_SUBPATH), - new ChildNumber(activeChain.getIssuedInternalKeys() - 1))); + activeChain.getAccountPath() + .extend(DeterministicKeyChain.INTERNAL_SUBPATH) + .extend(new ChildNumber(activeChain.getIssuedInternalKeys() - 1))); currentKeys.put(KeyChain.KeyPurpose.CHANGE, currentInternalKey); } return currentKeys; diff --git a/core/src/test/java/org/bitcoinj/crypto/HDUtilsTest.java b/core/src/test/java/org/bitcoinj/crypto/HDUtilsTest.java index 717dc5fe0..95ca2fdd0 100644 --- a/core/src/test/java/org/bitcoinj/crypto/HDUtilsTest.java +++ b/core/src/test/java/org/bitcoinj/crypto/HDUtilsTest.java @@ -125,6 +125,7 @@ public class HDUtilsTest { } + // TODO: Move this test to HDPath test class and call HDPath directly @Test public void testFormatPath() { Object[] tv = { @@ -154,6 +155,7 @@ public class HDUtilsTest { } + // TODO: Move this test to HDPath test class and call HDPath directly @Test public void testParsePath() { Object[] tv = {