From db519475b2e8b24bd042d959cf45c460ca2303d8 Mon Sep 17 00:00:00 2001 From: Jarl Fransson Date: Tue, 21 Oct 2014 13:41:02 +0200 Subject: [PATCH 01/20] When deserializing client payment channel state, if there was an existing close transaction, it was deserialized from wrong data. --- .../channels/StoredPaymentChannelClientStates.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/protocols/channels/StoredPaymentChannelClientStates.java b/core/src/main/java/org/bitcoinj/protocols/channels/StoredPaymentChannelClientStates.java index 01d352c5..98ed1b2c 100644 --- a/core/src/main/java/org/bitcoinj/protocols/channels/StoredPaymentChannelClientStates.java +++ b/core/src/main/java/org/bitcoinj/protocols/channels/StoredPaymentChannelClientStates.java @@ -300,8 +300,10 @@ public class StoredPaymentChannelClientStates implements WalletExtension { ECKey.fromPrivate(storedState.getMyKey().toByteArray()), Coin.valueOf(storedState.getValueToMe()), Coin.valueOf(storedState.getRefundFees()), false); - if (storedState.hasCloseTransactionHash()) - channel.close = containingWallet.getTransaction(new Sha256Hash(storedState.toByteArray())); + if (storedState.hasCloseTransactionHash()) { + Sha256Hash closeTxHash = new Sha256Hash(storedState.getCloseTransactionHash().toByteArray()); + channel.close = containingWallet.getTransaction(closeTxHash); + } putChannel(channel, false); } } finally { From 77ace479d9da741aa323fdeced69c6749121a18d Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Wed, 22 Oct 2014 15:42:29 +0200 Subject: [PATCH 02/20] Key rotation: remove the enabled setting. It's no longer useful and defaulted to off, which is dangerous and can lead to bugs. --- core/src/main/java/org/bitcoinj/core/Wallet.java | 10 +--------- core/src/test/java/org/bitcoinj/core/WalletTest.java | 3 --- tools/src/main/java/org/bitcoinj/tools/WalletTool.java | 1 - 3 files changed, 1 insertion(+), 13 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/core/Wallet.java b/core/src/main/java/org/bitcoinj/core/Wallet.java index 16fe2802..a4baf892 100644 --- a/core/src/main/java/org/bitcoinj/core/Wallet.java +++ b/core/src/main/java/org/bitcoinj/core/Wallet.java @@ -187,7 +187,6 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha // UNIX time in seconds. Money controlled by keys created before this time will be automatically respent to a key // that was created after it. Useful when you believe some keys have been compromised. private volatile long vKeyRotationTimestamp; - private volatile boolean vKeyRotationEnabled; protected transient CoinSelector coinSelector = new DefaultCoinSelector(); @@ -451,7 +450,7 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha public void upgradeToDeterministic(@Nullable KeyParameter aesKey) throws DeterministicUpgradeRequiresPassword { lock.lock(); try { - keychain.upgradeToDeterministic(vKeyRotationEnabled ? vKeyRotationTimestamp : 0, aesKey); + keychain.upgradeToDeterministic(vKeyRotationTimestamp, aesKey); } finally { lock.unlock(); } @@ -4295,11 +4294,6 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha saveNow(); } - /** Toggles key rotation on and off. Note that this state is not serialized. */ - public void setKeyRotationEnabled(boolean enabled) { - vKeyRotationEnabled = enabled; - } - /** Returns whether the keys creation time is before the key rotation time, if one was set. */ public boolean isKeyRotating(ECKey key) { long time = vKeyRotationTimestamp; @@ -4357,8 +4351,6 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha checkState(lock.isHeldByCurrentThread()); List results = Lists.newLinkedList(); // TODO: Handle chain replays here. - if (!vKeyRotationEnabled) return results; - // Snapshot volatiles so this method has an atomic view. long keyRotationTimestamp = vKeyRotationTimestamp; if (keyRotationTimestamp == 0) return results; // Nothing to do. diff --git a/core/src/test/java/org/bitcoinj/core/WalletTest.java b/core/src/test/java/org/bitcoinj/core/WalletTest.java index c3c82ae4..4b2ebc36 100644 --- a/core/src/test/java/org/bitcoinj/core/WalletTest.java +++ b/core/src/test/java/org/bitcoinj/core/WalletTest.java @@ -2288,7 +2288,6 @@ public class WalletTest extends TestWithWallet { wallet = new Wallet(params); // Watch out for wallet-initiated broadcasts. MockTransactionBroadcaster broadcaster = new MockTransactionBroadcaster(wallet); - wallet.setKeyRotationEnabled(true); // Send three cents to two different random keys, then add a key and mark the initial keys as compromised. ECKey key1 = new ECKey(); key1.setCreationTimeSeconds(Utils.currentTimeSeconds() - (86400 * 2)); @@ -2375,7 +2374,6 @@ public class WalletTest extends TestWithWallet { // A day later, we get compromised. Utils.rollMockClock(86400); wallet.setKeyRotationTime(Utils.currentTimeSeconds()); - wallet.setKeyRotationEnabled(true); List txns = wallet.maybeDoMaintenance(null, false).get(); assertEquals(1, txns.size()); @@ -2400,7 +2398,6 @@ public class WalletTest extends TestWithWallet { } MockTransactionBroadcaster broadcaster = new MockTransactionBroadcaster(wallet); - wallet.setKeyRotationEnabled(true); Date compromise = Utils.now(); Utils.rollMockClock(86400); diff --git a/tools/src/main/java/org/bitcoinj/tools/WalletTool.java b/tools/src/main/java/org/bitcoinj/tools/WalletTool.java index 084ad256..622a4bf0 100644 --- a/tools/src/main/java/org/bitcoinj/tools/WalletTool.java +++ b/tools/src/main/java/org/bitcoinj/tools/WalletTool.java @@ -448,7 +448,6 @@ public class WalletTool { rotationTimeSecs = options.valueOf(dateFlag).getTime() / 1000; } log.info("Setting wallet key rotation time to {}", rotationTimeSecs); - wallet.setKeyRotationEnabled(true); wallet.setKeyRotationTime(rotationTimeSecs); KeyParameter aesKey = null; if (wallet.isEncrypted()) { From ea7c29e38b90ad337b66084b0b9449d10385da9b Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Wed, 22 Oct 2014 19:29:54 +0200 Subject: [PATCH 03/20] Key rotation: construct new HD chain based on the oldest possible key, a la upgrade, with a fresh random HD chain only being created if all random keys are rotating. --- .../main/java/org/bitcoinj/core/Wallet.java | 25 ++++++----- .../wallet/AllRandomKeysRotating.java | 7 +++ .../org/bitcoinj/wallet/KeyChainGroup.java | 17 +++++--- .../java/org/bitcoinj/core/WalletTest.java | 43 ++++++++++++++++--- 4 files changed, 71 insertions(+), 21 deletions(-) create mode 100644 core/src/main/java/org/bitcoinj/wallet/AllRandomKeysRotating.java diff --git a/core/src/main/java/org/bitcoinj/core/Wallet.java b/core/src/main/java/org/bitcoinj/core/Wallet.java index a4baf892..5581cc69 100644 --- a/core/src/main/java/org/bitcoinj/core/Wallet.java +++ b/core/src/main/java/org/bitcoinj/core/Wallet.java @@ -4273,15 +4273,12 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha } /** - *

When a key rotation time is set, and money controlled by keys created before the given timestamp T will be + *

When a key rotation time is set, any money controlled by keys created before the given timestamp T will be * automatically respent to any key that was created after T. This can be used to recover from a situation where * a set of keys is believed to be compromised. You can stop key rotation by calling this method again with zero - * as the argument, or by using {@link #setKeyRotationEnabled(boolean)}. Once set up, calling - * {@link #maybeDoMaintenance(org.spongycastle.crypto.params.KeyParameter, boolean)} will create and possibly - * send rotation transactions: but it won't be done automatically (because you might have to ask for the users - * password).

- * - *

Note that this method won't do anything unless you call {@link #setKeyRotationEnabled(boolean)} first.

+ * as the argument. Once set up, calling {@link #maybeDoMaintenance(org.spongycastle.crypto.params.KeyParameter, boolean)} + * will create and possibly send rotation transactions: but it won't be done automatically (because you might have + * to ask for the users password).

* *

The given time cannot be in the future.

*/ @@ -4311,7 +4308,7 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha * @param andSend if true, send the transactions via the tx broadcaster and return them, if false just return them. * @return A list of transactions that the wallet just made/will make for internal maintenance. Might be empty. */ - public ListenableFuture> maybeDoMaintenance(@Nullable KeyParameter aesKey, boolean andSend) { + public ListenableFuture> maybeDoMaintenance(@Nullable KeyParameter aesKey, boolean andSend) throws DeterministicUpgradeRequiresPassword { List txns; lock.lock(); try { @@ -4347,7 +4344,7 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha } // Checks to see if any coins are controlled by rotating keys and if so, spends them. - private List maybeRotateKeys(@Nullable KeyParameter aesKey) { + private List maybeRotateKeys(@Nullable KeyParameter aesKey) throws DeterministicUpgradeRequiresPassword { checkState(lock.isHeldByCurrentThread()); List results = Lists.newLinkedList(); // TODO: Handle chain replays here. @@ -4363,8 +4360,14 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha } } if (allChainsRotating) { - log.info("All HD chains are currently rotating, creating a new one"); - keychain.createAndActivateNewHDChain(); + log.info("All HD chains are currently rotating, attempting to create a new one from the next oldest non-rotating key material ..."); + try { + keychain.upgradeToDeterministic(keyRotationTimestamp, aesKey); + log.info(" ... upgraded to HD again, based on next best oldest key."); + } catch (AllRandomKeysRotating rotating) { + log.info(" ... no non-rotating random keys available, generating entirely new HD tree: backup required after this."); + keychain.createAndActivateNewHDChain(); + } } // Because transactions are size limited, we might not be able to re-key the entire wallet in one go. So diff --git a/core/src/main/java/org/bitcoinj/wallet/AllRandomKeysRotating.java b/core/src/main/java/org/bitcoinj/wallet/AllRandomKeysRotating.java new file mode 100644 index 00000000..b7a60ace --- /dev/null +++ b/core/src/main/java/org/bitcoinj/wallet/AllRandomKeysRotating.java @@ -0,0 +1,7 @@ +package org.bitcoinj.wallet; + +/** + * Indicates that an attempt was made to upgrade a random wallet to deterministic, but there were no non-rotating + * random keys to use as source material for the seed. Add a non-compromised key first! + */ +public class AllRandomKeysRotating extends RuntimeException {} diff --git a/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java b/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java index 1a6ff59e..cf49060c 100644 --- a/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java +++ b/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java @@ -630,12 +630,14 @@ public class KeyChainGroup implements KeyBag { * and you should provide the users encryption key. * @return the DeterministicKeyChain that was created by the upgrade. */ - public DeterministicKeyChain upgradeToDeterministic(long keyRotationTimeSecs, @Nullable KeyParameter aesKey) throws DeterministicUpgradeRequiresPassword { - checkState(chains.isEmpty()); + public DeterministicKeyChain upgradeToDeterministic(long keyRotationTimeSecs, @Nullable KeyParameter aesKey) throws DeterministicUpgradeRequiresPassword, AllRandomKeysRotating { checkState(basic.numKeys() > 0); checkArgument(keyRotationTimeSecs >= 0); - ECKey keyToUse = basic.findOldestKeyAfter(keyRotationTimeSecs); - checkArgument(keyToUse != null, "All keys are considered rotating, so we cannot upgrade deterministically."); + // Subtract one because the key rotation time might have been set to the creation time of the first known good + // key, in which case, that's the one we want to find. + ECKey keyToUse = basic.findOldestKeyAfter(keyRotationTimeSecs - 1); + if (keyToUse == null) + throw new AllRandomKeysRotating(); if (keyToUse.isEncrypted()) { if (aesKey == null) { @@ -658,7 +660,12 @@ public class KeyChainGroup implements KeyBag { throw new IllegalStateException("AES Key was provided but wallet is not encrypted."); } - log.info("Auto-upgrading pre-HD wallet using oldest non-rotating private key"); + if (chains.isEmpty()) { + log.info("Auto-upgrading pre-HD wallet to HD!"); + } else { + log.info("Wallet with existing HD chain is being re-upgraded due to change in key rotation time."); + } + log.info("Instantiating new HD chain using oldest non-rotating private key (address: {})", keyToUse.toAddress(params)); byte[] entropy = checkNotNull(keyToUse.getSecretBytes()); // Private keys should be at least 128 bits long. checkState(entropy.length >= DeterministicSeed.DEFAULT_SEED_ENTROPY_BITS / 8); diff --git a/core/src/test/java/org/bitcoinj/core/WalletTest.java b/core/src/test/java/org/bitcoinj/core/WalletTest.java index 4b2ebc36..f2f76f1d 100644 --- a/core/src/test/java/org/bitcoinj/core/WalletTest.java +++ b/core/src/test/java/org/bitcoinj/core/WalletTest.java @@ -78,6 +78,7 @@ public class WalletTest extends TestWithWallet { @Override public void setUp() throws Exception { super.setUp(); + // TODO: Move these fields into the right tests so we don't create two wallets for every test case. encryptedWallet = new Wallet(params); myEncryptedAddress = encryptedWallet.freshReceiveKey().toAddress(params); encryptedWallet.encrypt(PASSWORD1); @@ -96,7 +97,6 @@ public class WalletTest extends TestWithWallet { createMarriedWallet(threshold, numKeys, true); } - private void createMarriedWallet(int threshold, int numKeys, boolean addSigners) throws BlockStoreException { wallet = new Wallet(params); blockStore = new MemoryBlockStore(params); @@ -2302,11 +2302,8 @@ public class WalletTest extends TestWithWallet { assertEquals(0, broadcaster.size()); assertFalse(wallet.isKeyRotating(key1)); - // We got compromised! We have an old style random-only wallet. So let's upgrade to HD: for that we need a fresh - // random key that's not rotating as the wallet won't create a new seed for us, it'll just refuse to upgrade. + // We got compromised! Utils.rollMockClock(1); - ECKey key3 = new ECKey(); - wallet.importKey(key3); wallet.setKeyRotationTime(compromiseTime); assertTrue(wallet.isKeyRotating(key1)); wallet.maybeDoMaintenance(null, true); @@ -2381,6 +2378,42 @@ public class WalletTest extends TestWithWallet { assertNotEquals(watchKey1, watchKey2); } + @SuppressWarnings("ConstantConditions") + @Test + public void keyRotationHD2() throws Exception { + // Check we handle the following scenario: a weak random key is created, then some good random keys are created + // but the weakness of the first isn't known yet. The wallet is upgraded to HD based on the weak key. Later, we + // find out about the weakness and set the rotation time to after the bad key's creation date. A new HD chain + // should be created based on the oldest known good key and the old chain + bad random key should rotate to it. + + // We fix the private keys just to make the test deterministic (last byte differs). + Utils.setMockClock(); + ECKey badKey = ECKey.fromPrivate(Utils.HEX.decode("00905b93f990267f4104f316261fc10f9f983551f9ef160854f40102eb71cffdbb")); + badKey.setCreationTimeSeconds(Utils.currentTimeSeconds()); + Utils.rollMockClock(86400); + ECKey goodKey = ECKey.fromPrivate(Utils.HEX.decode("00905b93f990267f4104f316261fc10f9f983551f9ef160854f40102eb71cffdcc")); + goodKey.setCreationTimeSeconds(Utils.currentTimeSeconds()); + + // Do an upgrade based on the bad key. + KeyChainGroup kcg = new KeyChainGroup(params); + kcg.importKeys(badKey, goodKey); + Utils.rollMockClock(86400); + wallet = new Wallet(params, kcg); // This avoids the automatic HD initialisation + wallet.upgradeToDeterministic(null); + DeterministicKey badWatchingKey = wallet.getWatchingKey(); + assertEquals(badKey.getCreationTimeSeconds(), badWatchingKey.getCreationTimeSeconds()); + sendMoneyToWallet(wallet, CENT, badWatchingKey.toAddress(params), AbstractBlockChain.NewBlockType.BEST_CHAIN); + + // Now we set the rotation time to the time we started making good keys. This should create a new HD chain. + wallet.setKeyRotationTime(goodKey.getCreationTimeSeconds()); + List txns = wallet.maybeDoMaintenance(null, false).get(); + assertEquals(1, txns.size()); + Address output = txns.get(0).getOutput(0).getAddressFromP2PKHScript(params); + ECKey usedKey = wallet.findKeyFromPubHash(output.getHash160()); + assertEquals(goodKey.getCreationTimeSeconds(), usedKey.getCreationTimeSeconds()); + assertEquals("mrM3TpCnav5YQuVA1xLercCGJH4DXujMtv", usedKey.toAddress(params).toString()); + } + @Test(expected = IllegalArgumentException.class) public void importOfHDKeyForbidden() throws Exception { wallet.importKey(wallet.freshReceiveKey()); From ea02436f965bcc09bd6b32a21dfc98f490934dda Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Wed, 22 Oct 2014 19:32:29 +0200 Subject: [PATCH 04/20] Rename maybeDoMaintenance to doMaintenance and add a bit more docs. --- .../src/main/java/org/bitcoinj/core/Wallet.java | 17 ++++++++++------- .../test/java/org/bitcoinj/core/WalletTest.java | 12 ++++++------ .../java/org/bitcoinj/tools/WalletTool.java | 2 +- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/core/Wallet.java b/core/src/main/java/org/bitcoinj/core/Wallet.java index 5581cc69..ca7150e6 100644 --- a/core/src/main/java/org/bitcoinj/core/Wallet.java +++ b/core/src/main/java/org/bitcoinj/core/Wallet.java @@ -4276,7 +4276,7 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha *

When a key rotation time is set, any money controlled by keys created before the given timestamp T will be * automatically respent to any key that was created after T. This can be used to recover from a situation where * a set of keys is believed to be compromised. You can stop key rotation by calling this method again with zero - * as the argument. Once set up, calling {@link #maybeDoMaintenance(org.spongycastle.crypto.params.KeyParameter, boolean)} + * as the argument. Once set up, calling {@link #doMaintenance(org.spongycastle.crypto.params.KeyParameter, boolean)} * will create and possibly send rotation transactions: but it won't be done automatically (because you might have * to ask for the users password).

* @@ -4298,17 +4298,20 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha } /** - * A wallet app should call this from time to time if key rotation is enabled in order to let the wallet craft and - * send transactions needed to re-organise coins internally. A good time to call this would be after receiving coins - * for an unencrypted wallet, or after sending money for an encrypted wallet. If you have an encrypted wallet and - * just want to know if some maintenance needs doing, call this method with doSend set to false and look at the - * returned list of transactions. + * A wallet app should call this from time to time in order to let the wallet craft and send transactions needed + * to re-organise coins internally. A good time to call this would be after receiving coins for an unencrypted + * wallet, or after sending money for an encrypted wallet. If you have an encrypted wallet and just want to know + * if some maintenance needs doing, call this method with andSend set to false and look at the returned list of + * transactions. Maintenance might also include internal changes that involve some processing or work but + * which don't require making transactions - these will happen automatically unless the password is required + * in which case an exception will be thrown. * * @param aesKey the users password, if any. * @param andSend if true, send the transactions via the tx broadcaster and return them, if false just return them. * @return A list of transactions that the wallet just made/will make for internal maintenance. Might be empty. + * @throws org.bitcoinj.wallet.DeterministicUpgradeRequiresPassword if key rotation requires the users password. */ - public ListenableFuture> maybeDoMaintenance(@Nullable KeyParameter aesKey, boolean andSend) throws DeterministicUpgradeRequiresPassword { + public ListenableFuture> doMaintenance(@Nullable KeyParameter aesKey, boolean andSend) throws DeterministicUpgradeRequiresPassword { List txns; lock.lock(); try { diff --git a/core/src/test/java/org/bitcoinj/core/WalletTest.java b/core/src/test/java/org/bitcoinj/core/WalletTest.java index f2f76f1d..3f0ec47f 100644 --- a/core/src/test/java/org/bitcoinj/core/WalletTest.java +++ b/core/src/test/java/org/bitcoinj/core/WalletTest.java @@ -2306,7 +2306,7 @@ public class WalletTest extends TestWithWallet { Utils.rollMockClock(1); wallet.setKeyRotationTime(compromiseTime); assertTrue(wallet.isKeyRotating(key1)); - wallet.maybeDoMaintenance(null, true); + wallet.doMaintenance(null, true); Transaction tx = broadcaster.waitForTransactionAndSucceed(); final Coin THREE_CENTS = CENT.add(CENT).add(CENT); @@ -2323,12 +2323,12 @@ public class WalletTest extends TestWithWallet { // Now receive some more money to the newly derived address via a new block and check that nothing happens. sendMoneyToWallet(wallet, CENT, toAddress, AbstractBlockChain.NewBlockType.BEST_CHAIN); - assertTrue(wallet.maybeDoMaintenance(null, true).get().isEmpty()); + assertTrue(wallet.doMaintenance(null, true).get().isEmpty()); assertEquals(0, broadcaster.size()); // Receive money via a new block on key1 and ensure it shows up as a maintenance task. sendMoneyToWallet(wallet, CENT, key1.toAddress(params), AbstractBlockChain.NewBlockType.BEST_CHAIN); - wallet.maybeDoMaintenance(null, true); + wallet.doMaintenance(null, true); tx = broadcaster.waitForTransactionAndSucceed(); assertNotNull(wallet.findKeyFromPubHash(tx.getOutput(0).getScriptPubKey().getPubKeyHash())); log.info("Unexpected thing: {}", tx); @@ -2372,7 +2372,7 @@ public class WalletTest extends TestWithWallet { Utils.rollMockClock(86400); wallet.setKeyRotationTime(Utils.currentTimeSeconds()); - List txns = wallet.maybeDoMaintenance(null, false).get(); + List txns = wallet.doMaintenance(null, false).get(); assertEquals(1, txns.size()); DeterministicKey watchKey2 = wallet.getWatchingKey(); assertNotEquals(watchKey1, watchKey2); @@ -2406,7 +2406,7 @@ public class WalletTest extends TestWithWallet { // Now we set the rotation time to the time we started making good keys. This should create a new HD chain. wallet.setKeyRotationTime(goodKey.getCreationTimeSeconds()); - List txns = wallet.maybeDoMaintenance(null, false).get(); + List txns = wallet.doMaintenance(null, false).get(); assertEquals(1, txns.size()); Address output = txns.get(0).getOutput(0).getAddressFromP2PKHScript(params); ECKey usedKey = wallet.findKeyFromPubHash(output.getHash160()); @@ -2436,7 +2436,7 @@ public class WalletTest extends TestWithWallet { Utils.rollMockClock(86400); wallet.freshReceiveKey(); wallet.setKeyRotationTime(compromise); - wallet.maybeDoMaintenance(null, true); + wallet.doMaintenance(null, true); Transaction tx = broadcaster.waitForTransactionAndSucceed(); final Coin valueSentToMe = tx.getValueSentToMe(wallet); diff --git a/tools/src/main/java/org/bitcoinj/tools/WalletTool.java b/tools/src/main/java/org/bitcoinj/tools/WalletTool.java index 622a4bf0..b977a852 100644 --- a/tools/src/main/java/org/bitcoinj/tools/WalletTool.java +++ b/tools/src/main/java/org/bitcoinj/tools/WalletTool.java @@ -455,7 +455,7 @@ public class WalletTool { if (aesKey == null) return; } - Futures.getUnchecked(wallet.maybeDoMaintenance(aesKey, true)); + Futures.getUnchecked(wallet.doMaintenance(aesKey, true)); } private static void encrypt() { From 1a55f8d2d529b2c485499e152e778379532d6d01 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Wed, 22 Oct 2014 19:43:16 +0200 Subject: [PATCH 05/20] Add maybeDoMaintenance back as a deprecated alias. --- .../src/main/java/org/bitcoinj/core/Wallet.java | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/core/Wallet.java b/core/src/main/java/org/bitcoinj/core/Wallet.java index ca7150e6..60a7768c 100644 --- a/core/src/main/java/org/bitcoinj/core/Wallet.java +++ b/core/src/main/java/org/bitcoinj/core/Wallet.java @@ -4297,6 +4297,12 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha return time != 0 && key.getCreationTimeSeconds() < time; } + /** @deprecated Renamed to doMaintenance */ + @Deprecated + public ListenableFuture> maybeDoMaintenance(@Nullable KeyParameter aesKey, boolean andSend) throws DeterministicUpgradeRequiresPassword { + return doMaintenance(aesKey, andSend); + } + /** * A wallet app should call this from time to time in order to let the wallet craft and send transactions needed * to re-organise coins internally. A good time to call this would be after receiving coins for an unencrypted @@ -4363,10 +4369,15 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha } } if (allChainsRotating) { - log.info("All HD chains are currently rotating, attempting to create a new one from the next oldest non-rotating key material ..."); try { - keychain.upgradeToDeterministic(keyRotationTimestamp, aesKey); - log.info(" ... upgraded to HD again, based on next best oldest key."); + if (keychain.getImportedKeys().isEmpty()) { + log.info("All HD chains are currently rotating and we have no random keys, creating fresh HD chain ..."); + keychain.createAndActivateNewHDChain(); + } else { + log.info("All HD chains are currently rotating, attempting to create a new one from the next oldest non-rotating key material ..."); + keychain.upgradeToDeterministic(keyRotationTimestamp, aesKey); + log.info(" ... upgraded to HD again, based on next best oldest key."); + } } catch (AllRandomKeysRotating rotating) { log.info(" ... no non-rotating random keys available, generating entirely new HD tree: backup required after this."); keychain.createAndActivateNewHDChain(); From bb138e70c335d2d9d889264c19e7bea2d6e74496 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Wed, 22 Oct 2014 20:03:15 +0200 Subject: [PATCH 06/20] Key rotation: also unit test the creation time of a fresh key. --- core/src/test/java/org/bitcoinj/core/WalletTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/test/java/org/bitcoinj/core/WalletTest.java b/core/src/test/java/org/bitcoinj/core/WalletTest.java index 3f0ec47f..bd7bf854 100644 --- a/core/src/test/java/org/bitcoinj/core/WalletTest.java +++ b/core/src/test/java/org/bitcoinj/core/WalletTest.java @@ -2411,6 +2411,7 @@ public class WalletTest extends TestWithWallet { Address output = txns.get(0).getOutput(0).getAddressFromP2PKHScript(params); ECKey usedKey = wallet.findKeyFromPubHash(output.getHash160()); assertEquals(goodKey.getCreationTimeSeconds(), usedKey.getCreationTimeSeconds()); + assertEquals(goodKey.getCreationTimeSeconds(), wallet.freshReceiveKey().getCreationTimeSeconds()); assertEquals("mrM3TpCnav5YQuVA1xLercCGJH4DXujMtv", usedKey.toAddress(params).toString()); } From 9fa4afb5fe0dd313bf66275d485b46a41ca76f77 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Wed, 22 Oct 2014 21:51:34 +0200 Subject: [PATCH 07/20] Fix WalletTemplate now that checkpoints are included. --- wallettemplate/src/main/java/wallettemplate/Main.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/wallettemplate/src/main/java/wallettemplate/Main.java b/wallettemplate/src/main/java/wallettemplate/Main.java index 472f0953..aec9ce97 100644 --- a/wallettemplate/src/main/java/wallettemplate/Main.java +++ b/wallettemplate/src/main/java/wallettemplate/Main.java @@ -117,14 +117,7 @@ public class Main extends Application { // or progress widget to keep the user engaged whilst we initialise, but we don't. if (params == RegTestParams.get()) { bitcoin.connectToLocalHost(); // You should run a regtest mode bitcoind locally. - } else if (params == MainNetParams.get()) { - // Checkpoints are block headers that ship inside our app: for a new user, we pick the last header - // in the checkpoints file and then download the rest from the network. It makes things much faster. - // Checkpoint files are made using the BuildCheckpoints tool and usually we have to download the - // last months worth or more (takes a few seconds). - bitcoin.setCheckpoints(getClass().getResourceAsStream("checkpoints")); } else if (params == TestNet3Params.get()) { - bitcoin.setCheckpoints(getClass().getResourceAsStream("org.bitcoin.test.checkpoints")); // As an example! bitcoin.useTor(); } From d4ffd63525b0f53c85a67b02eb02298927ad2917 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Wed, 22 Oct 2014 21:51:49 +0200 Subject: [PATCH 08/20] Make basicCategoryStepTest independent of actual min fee level. --- .../java/org/bitcoinj/core/WalletTest.java | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/core/src/test/java/org/bitcoinj/core/WalletTest.java b/core/src/test/java/org/bitcoinj/core/WalletTest.java index bd7bf854..5097e635 100644 --- a/core/src/test/java/org/bitcoinj/core/WalletTest.java +++ b/core/src/test/java/org/bitcoinj/core/WalletTest.java @@ -1956,43 +1956,45 @@ public class WalletTest extends TestWithWallet { // Generate a ton of small outputs StoredBlock block = new StoredBlock(makeSolvedTestBlock(blockStore, notMyAddr), BigInteger.ONE, 1); int i = 0; - while (i <= CENT.divide(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE)) { - Transaction tx = createFakeTxWithChangeAddress(params, Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, myAddress, notMyAddr); + Coin tenThousand = Coin.valueOf(10000); + while (i <= 100) { + Transaction tx = createFakeTxWithChangeAddress(params, tenThousand, myAddress, notMyAddr); tx.getInput(0).setSequenceNumber(i++); // Keep every transaction unique wallet.receiveFromBlock(tx, block, AbstractBlockChain.NewBlockType.BEST_CHAIN, i); } + Coin balance = wallet.getBalance(); // Create a spend that will throw away change (category 3 type 2 in which the change causes fee which is worth more than change) - SendRequest request1 = SendRequest.to(notMyAddr, CENT.add(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE).subtract(SATOSHI)); + SendRequest request1 = SendRequest.to(notMyAddr, balance.subtract(SATOSHI)); wallet.completeTx(request1); assertEquals(SATOSHI, request1.tx.getFee()); assertEquals(request1.tx.getInputs().size(), i); // We should have spent all inputs // Give us one more input... - Transaction tx1 = createFakeTxWithChangeAddress(params, Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, myAddress, notMyAddr); + Transaction tx1 = createFakeTxWithChangeAddress(params, tenThousand, myAddress, notMyAddr); tx1.getInput(0).setSequenceNumber(i++); // Keep every transaction unique wallet.receiveFromBlock(tx1, block, AbstractBlockChain.NewBlockType.BEST_CHAIN, i); // ... and create a spend that will throw away change (category 3 type 1 in which the change causes dust output) - SendRequest request2 = SendRequest.to(notMyAddr, CENT.add(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE).subtract(SATOSHI)); + SendRequest request2 = SendRequest.to(notMyAddr, balance.subtract(SATOSHI)); wallet.completeTx(request2); assertEquals(SATOSHI, request2.tx.getFee()); assertEquals(request2.tx.getInputs().size(), i - 1); // We should have spent all inputs - 1 // Give us one more input... - Transaction tx2 = createFakeTxWithChangeAddress(params, Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, myAddress, notMyAddr); + Transaction tx2 = createFakeTxWithChangeAddress(params, tenThousand, myAddress, notMyAddr); tx2.getInput(0).setSequenceNumber(i++); // Keep every transaction unique wallet.receiveFromBlock(tx2, block, AbstractBlockChain.NewBlockType.BEST_CHAIN, i); // ... and create a spend that will throw away change (category 3 type 1 in which the change causes dust output) // but that also could have been category 2 if it wanted - SendRequest request3 = SendRequest.to(notMyAddr, CENT.add(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE).subtract(SATOSHI)); + SendRequest request3 = SendRequest.to(notMyAddr, CENT.add(tenThousand).subtract(SATOSHI)); wallet.completeTx(request3); assertEquals(SATOSHI, request3.tx.getFee()); assertEquals(request3.tx.getInputs().size(), i - 2); // We should have spent all inputs - 2 // - SendRequest request4 = SendRequest.to(notMyAddr, CENT.add(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE).subtract(SATOSHI)); + SendRequest request4 = SendRequest.to(notMyAddr, balance.subtract(SATOSHI)); request4.feePerKb = Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.divide(request3.tx.bitcoinSerialize().length); wallet.completeTx(request4); assertEquals(SATOSHI, request4.tx.getFee()); @@ -2000,24 +2002,24 @@ public class WalletTest extends TestWithWallet { // Give us a few more inputs... while (wallet.getBalance().compareTo(CENT.multiply(2)) < 0) { - Transaction tx3 = createFakeTxWithChangeAddress(params, Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, myAddress, notMyAddr); + Transaction tx3 = createFakeTxWithChangeAddress(params, tenThousand, myAddress, notMyAddr); tx3.getInput(0).setSequenceNumber(i++); // Keep every transaction unique wallet.receiveFromBlock(tx3, block, AbstractBlockChain.NewBlockType.BEST_CHAIN, i); } // ...that is just slightly less than is needed for category 1 - SendRequest request5 = SendRequest.to(notMyAddr, CENT.add(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE).subtract(SATOSHI)); + SendRequest request5 = SendRequest.to(notMyAddr, CENT.add(tenThousand).subtract(SATOSHI)); wallet.completeTx(request5); assertEquals(SATOSHI, request5.tx.getFee()); assertEquals(1, request5.tx.getOutputs().size()); // We should have no change output // Give us one more input... - Transaction tx4 = createFakeTxWithChangeAddress(params, Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, myAddress, notMyAddr); + Transaction tx4 = createFakeTxWithChangeAddress(params, tenThousand, myAddress, notMyAddr); tx4.getInput(0).setSequenceNumber(i); // Keep every transaction unique wallet.receiveFromBlock(tx4, block, AbstractBlockChain.NewBlockType.BEST_CHAIN, i); // ... that puts us in category 1 (no fee!) - SendRequest request6 = SendRequest.to(notMyAddr, CENT.add(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE).subtract(SATOSHI)); + SendRequest request6 = SendRequest.to(notMyAddr, CENT.add(tenThousand).subtract(SATOSHI)); wallet.completeTx(request6); assertEquals(ZERO, request6.tx.getFee()); assertEquals(2, request6.tx.getOutputs().size()); // We should have a change output From cd25e673f1fd7c0e7652a49ed4d2a33081b7d739 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Wed, 22 Oct 2014 21:52:04 +0200 Subject: [PATCH 09/20] 10x fee drop, now most miners seem to have upgraded to 0.9+ --- core/src/main/java/org/bitcoinj/core/Transaction.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/core/Transaction.java b/core/src/main/java/org/bitcoinj/core/Transaction.java index 28702b03..94b7d405 100644 --- a/core/src/main/java/org/bitcoinj/core/Transaction.java +++ b/core/src/main/java/org/bitcoinj/core/Transaction.java @@ -90,16 +90,16 @@ public class Transaction extends ChildMessage implements Serializable { /** * If fee is lower than this value (in satoshis), a default reference client will treat it as if there were no fee. - * Currently this is 10000 satoshis. + * Currently this is 1000 satoshis. */ - public static final Coin REFERENCE_DEFAULT_MIN_TX_FEE = Coin.valueOf(10000); + public static final Coin REFERENCE_DEFAULT_MIN_TX_FEE = Coin.valueOf(1000); /** * Any standard (ie pay-to-address) output smaller than this value (in satoshis) will most likely be rejected by the network. * This is calculated by assuming a standard output will be 34 bytes, and then using the formula used in - * {@link TransactionOutput#getMinNonDustValue(Coin)}. Currently it's 5460 satoshis. + * {@link TransactionOutput#getMinNonDustValue(Coin)}. Currently it's 546 satoshis. */ - public static final Coin MIN_NONDUST_OUTPUT = Coin.valueOf(5460); + public static final Coin MIN_NONDUST_OUTPUT = Coin.valueOf(546); // These are serialized in both bitcoin and java serialization. private long version; From afb05867a915a80ded23e4e787ce32c458acaa70 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Wed, 22 Oct 2014 22:00:31 +0200 Subject: [PATCH 10/20] Fix off by one in DKC.getKeys(false). Resolves #253 --- .../java/org/bitcoinj/wallet/DeterministicKeyChain.java | 4 ++-- .../org/bitcoinj/wallet/DeterministicKeyChainTest.java | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/wallet/DeterministicKeyChain.java b/core/src/main/java/org/bitcoinj/wallet/DeterministicKeyChain.java index 1829a0ef..8a04a3c2 100644 --- a/core/src/main/java/org/bitcoinj/wallet/DeterministicKeyChain.java +++ b/core/src/main/java/org/bitcoinj/wallet/DeterministicKeyChain.java @@ -1186,8 +1186,8 @@ public class DeterministicKeyChain implements EncryptableKeyChain { DeterministicKey parent = detkey.getParent(); if (parent == null) continue; if (detkey.getPath().size() <= treeSize) continue; - if (parent.equals(internalKey) && detkey.getChildNumber().i() > issuedInternalKeys) continue; - if (parent.equals(externalKey) && detkey.getChildNumber().i() > issuedExternalKeys) continue; + if (parent.equals(internalKey) && detkey.getChildNumber().i() >= issuedInternalKeys) continue; + if (parent.equals(externalKey) && detkey.getChildNumber().i() >= issuedExternalKeys) continue; issuedKeys.add(detkey); } return issuedKeys; diff --git a/core/src/test/java/org/bitcoinj/wallet/DeterministicKeyChainTest.java b/core/src/test/java/org/bitcoinj/wallet/DeterministicKeyChainTest.java index a0bfde96..d13ba6c8 100644 --- a/core/src/test/java/org/bitcoinj/wallet/DeterministicKeyChainTest.java +++ b/core/src/test/java/org/bitcoinj/wallet/DeterministicKeyChainTest.java @@ -72,6 +72,14 @@ public class DeterministicKeyChainTest { key3.sign(Sha256Hash.ZERO_HASH); } + @Test + public void getKeys() throws Exception { + chain.getKey(KeyChain.KeyPurpose.RECEIVE_FUNDS); + chain.getKey(KeyChain.KeyPurpose.CHANGE); + chain.maybeLookAhead(); + assertEquals(2, chain.getKeys(false).size()); + } + @Test public void signMessage() throws Exception { ECKey key = chain.getKey(KeyChain.KeyPurpose.RECEIVE_FUNDS); From d2ea42bfceb16bdc65d1d4296df9fde8d8dc9a5e Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Wed, 22 Oct 2014 22:13:05 +0200 Subject: [PATCH 11/20] doMaintenance: don't trigger signing if bool param is false, as an optimisation. --- .../src/main/java/org/bitcoinj/core/Wallet.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/core/Wallet.java b/core/src/main/java/org/bitcoinj/core/Wallet.java index 60a7768c..81f72361 100644 --- a/core/src/main/java/org/bitcoinj/core/Wallet.java +++ b/core/src/main/java/org/bitcoinj/core/Wallet.java @@ -4313,16 +4313,16 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha * in which case an exception will be thrown. * * @param aesKey the users password, if any. - * @param andSend if true, send the transactions via the tx broadcaster and return them, if false just return them. + * @param signAndSend if true, send the transactions via the tx broadcaster and return them, if false just return them. * @return A list of transactions that the wallet just made/will make for internal maintenance. Might be empty. * @throws org.bitcoinj.wallet.DeterministicUpgradeRequiresPassword if key rotation requires the users password. */ - public ListenableFuture> doMaintenance(@Nullable KeyParameter aesKey, boolean andSend) throws DeterministicUpgradeRequiresPassword { + public ListenableFuture> doMaintenance(@Nullable KeyParameter aesKey, boolean signAndSend) throws DeterministicUpgradeRequiresPassword { List txns; lock.lock(); try { - txns = maybeRotateKeys(aesKey); - if (!andSend) + txns = maybeRotateKeys(aesKey, signAndSend); + if (!signAndSend) return Futures.immediateFuture(txns); } finally { lock.unlock(); @@ -4353,7 +4353,7 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha } // Checks to see if any coins are controlled by rotating keys and if so, spends them. - private List maybeRotateKeys(@Nullable KeyParameter aesKey) throws DeterministicUpgradeRequiresPassword { + private List maybeRotateKeys(@Nullable KeyParameter aesKey, boolean sign) throws DeterministicUpgradeRequiresPassword { checkState(lock.isHeldByCurrentThread()); List results = Lists.newLinkedList(); // TODO: Handle chain replays here. @@ -4389,14 +4389,14 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha // fully done, at least for now (we may still get more transactions later and this method will be reinvoked). Transaction tx; do { - tx = rekeyOneBatch(keyRotationTimestamp, aesKey, results); + tx = rekeyOneBatch(keyRotationTimestamp, aesKey, results, sign); if (tx != null) results.add(tx); } while (tx != null && tx.getInputs().size() == KeyTimeCoinSelector.MAX_SIMULTANEOUS_INPUTS); return results; } @Nullable - private Transaction rekeyOneBatch(long timeSecs, @Nullable KeyParameter aesKey, List others) { + private Transaction rekeyOneBatch(long timeSecs, @Nullable KeyParameter aesKey, List others, boolean sign) { lock.lock(); try { // Build the transaction using some custom logic for our special needs. Last parameter to @@ -4428,7 +4428,8 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha rekeyTx.setPurpose(Transaction.Purpose.KEY_ROTATION); SendRequest req = SendRequest.forTx(rekeyTx); req.aesKey = aesKey; - signTransaction(req); + if (sign) + signTransaction(req); // KeyTimeCoinSelector should never select enough inputs to push us oversize. checkState(rekeyTx.bitcoinSerialize().length < Transaction.MAX_STANDARD_TX_SIZE); return rekeyTx; From 9239387ca7754347951c7361d6daf6acf1c0d876 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Thu, 23 Oct 2014 00:07:01 +0200 Subject: [PATCH 12/20] Key rotation: add saveNow call after new HD chains might have been added. --- core/src/main/java/org/bitcoinj/core/Wallet.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/main/java/org/bitcoinj/core/Wallet.java b/core/src/main/java/org/bitcoinj/core/Wallet.java index 81f72361..092476a7 100644 --- a/core/src/main/java/org/bitcoinj/core/Wallet.java +++ b/core/src/main/java/org/bitcoinj/core/Wallet.java @@ -4382,6 +4382,7 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha log.info(" ... no non-rotating random keys available, generating entirely new HD tree: backup required after this."); keychain.createAndActivateNewHDChain(); } + saveNow(); } // Because transactions are size limited, we might not be able to re-key the entire wallet in one go. So From 6c5d51f55a7a34137ed4b83b35cdb8670491f29b Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Thu, 23 Oct 2014 15:42:05 +0200 Subject: [PATCH 13/20] WalletTool: allow rotation time to be specified in seconds. --- tools/src/main/java/org/bitcoinj/tools/WalletTool.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/src/main/java/org/bitcoinj/tools/WalletTool.java b/tools/src/main/java/org/bitcoinj/tools/WalletTool.java index b977a852..3ea00151 100644 --- a/tools/src/main/java/org/bitcoinj/tools/WalletTool.java +++ b/tools/src/main/java/org/bitcoinj/tools/WalletTool.java @@ -446,6 +446,8 @@ public class WalletTool { long rotationTimeSecs = Utils.currentTimeSeconds(); if (options.has(dateFlag)) { rotationTimeSecs = options.valueOf(dateFlag).getTime() / 1000; + } else if (options.has(unixtimeFlag)) { + rotationTimeSecs = options.valueOf(unixtimeFlag); } log.info("Setting wallet key rotation time to {}", rotationTimeSecs); wallet.setKeyRotationTime(rotationTimeSecs); From 9532fa31a32bb4d287e6d1825dcafd9be35cbf88 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Thu, 23 Oct 2014 15:42:34 +0200 Subject: [PATCH 14/20] Key rotation: fix bug that could cause multiple identical key chains to be created over and over if the key rotation time was equal to the time of the oldest best key, with test coverage. --- .../main/java/org/bitcoinj/core/Wallet.java | 2 +- .../org/bitcoinj/wallet/KeyChainGroup.java | 2 +- .../java/org/bitcoinj/core/WalletTest.java | 21 ++++++++++++++++++- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/core/Wallet.java b/core/src/main/java/org/bitcoinj/core/Wallet.java index 092476a7..4f106bcc 100644 --- a/core/src/main/java/org/bitcoinj/core/Wallet.java +++ b/core/src/main/java/org/bitcoinj/core/Wallet.java @@ -4363,7 +4363,7 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha // We might have to create a new HD hierarchy if the previous ones are now rotating. boolean allChainsRotating = true; for (DeterministicKeyChain chain : keychain.getDeterministicKeyChains()) { - if (chain.getEarliestKeyCreationTime() > vKeyRotationTimestamp) { + if (chain.getEarliestKeyCreationTime() >= vKeyRotationTimestamp) { allChainsRotating = false; break; } diff --git a/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java b/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java index cf49060c..a65778f7 100644 --- a/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java +++ b/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java @@ -68,7 +68,7 @@ public class KeyChainGroup implements KeyBag { private BasicKeyChain basic; private NetworkParameters params; - private final LinkedList chains; + protected final LinkedList chains; private final EnumMap currentKeys; private EnumMap currentAddresses; diff --git a/core/src/test/java/org/bitcoinj/core/WalletTest.java b/core/src/test/java/org/bitcoinj/core/WalletTest.java index 5097e635..455a8881 100644 --- a/core/src/test/java/org/bitcoinj/core/WalletTest.java +++ b/core/src/test/java/org/bitcoinj/core/WalletTest.java @@ -52,6 +52,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import static org.bitcoinj.core.Coin.*; import static org.bitcoinj.core.Utils.HEX; @@ -2397,10 +2398,17 @@ public class WalletTest extends TestWithWallet { goodKey.setCreationTimeSeconds(Utils.currentTimeSeconds()); // Do an upgrade based on the bad key. - KeyChainGroup kcg = new KeyChainGroup(params); + final AtomicReference> fChains = new AtomicReference>(); + KeyChainGroup kcg = new KeyChainGroup(params) { + + { + fChains.set(chains); + } + }; kcg.importKeys(badKey, goodKey); Utils.rollMockClock(86400); wallet = new Wallet(params, kcg); // This avoids the automatic HD initialisation + assertTrue(fChains.get().isEmpty()); wallet.upgradeToDeterministic(null); DeterministicKey badWatchingKey = wallet.getWatchingKey(); assertEquals(badKey.getCreationTimeSeconds(), badWatchingKey.getCreationTimeSeconds()); @@ -2415,6 +2423,17 @@ public class WalletTest extends TestWithWallet { assertEquals(goodKey.getCreationTimeSeconds(), usedKey.getCreationTimeSeconds()); assertEquals(goodKey.getCreationTimeSeconds(), wallet.freshReceiveKey().getCreationTimeSeconds()); assertEquals("mrM3TpCnav5YQuVA1xLercCGJH4DXujMtv", usedKey.toAddress(params).toString()); + DeterministicKeyChain c = fChains.get().get(1); + assertEquals(c.getEarliestKeyCreationTime(), goodKey.getCreationTimeSeconds()); + assertEquals(2, fChains.get().size()); + + // Commit the maint txns. + wallet.commitTx(txns.get(0)); + + // Check next maintenance does nothing. + assertTrue(wallet.doMaintenance(null, false).get().isEmpty()); + assertEquals(c, fChains.get().get(1)); + assertEquals(2, fChains.get().size()); } @Test(expected = IllegalArgumentException.class) From fbccfbbe0ee26a7fe6ed1c6688e83ee63d245d1d Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Thu, 23 Oct 2014 17:11:29 +0200 Subject: [PATCH 15/20] Default risk analysis: fix an off-by-one error in dust output comparisons. --- core/src/main/java/org/bitcoinj/wallet/DefaultRiskAnalysis.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/bitcoinj/wallet/DefaultRiskAnalysis.java b/core/src/main/java/org/bitcoinj/wallet/DefaultRiskAnalysis.java index da8a0116..455cbb38 100644 --- a/core/src/main/java/org/bitcoinj/wallet/DefaultRiskAnalysis.java +++ b/core/src/main/java/org/bitcoinj/wallet/DefaultRiskAnalysis.java @@ -151,7 +151,7 @@ public class DefaultRiskAnalysis implements RiskAnalysis { * Checks the output to see if the script violates a standardness rule. Not complete. */ public static RuleViolation isOutputStandard(TransactionOutput output) { - if (MIN_ANALYSIS_NONDUST_OUTPUT.compareTo(output.getValue()) > 0) + if (output.getValue().compareTo(MIN_ANALYSIS_NONDUST_OUTPUT) < 0) return RuleViolation.DUST; for (ScriptChunk chunk : output.getScriptPubKey().getChunks()) { if (chunk.isPushData() && !chunk.isShortestPossiblePushData()) From 47cdf5f70e126bc3cc6b6542346f6759d9d229c9 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Thu, 23 Oct 2014 17:12:14 +0200 Subject: [PATCH 16/20] Log full tx when considered risky. --- core/src/main/java/org/bitcoinj/core/Wallet.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/bitcoinj/core/Wallet.java b/core/src/main/java/org/bitcoinj/core/Wallet.java index 4f106bcc..ba8b8379 100644 --- a/core/src/main/java/org/bitcoinj/core/Wallet.java +++ b/core/src/main/java/org/bitcoinj/core/Wallet.java @@ -1473,7 +1473,7 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha RiskAnalysis analysis = riskAnalyzer.create(this, tx, dependencies); RiskAnalysis.Result result = analysis.analyze(); if (result != RiskAnalysis.Result.OK) { - log.warn("Pending transaction {} was considered risky: {}", tx.getHashAsString(), analysis); + log.warn("Pending transaction was considered risky: {}\n{}", analysis, tx); return true; } return false; From ee08ba4d5d34d834af9c3053ed140c57ce5901f0 Mon Sep 17 00:00:00 2001 From: Wojciech Langiewicz Date: Thu, 23 Oct 2014 15:34:20 +0200 Subject: [PATCH 17/20] Update toString() in ECKey to include private key in WIF format, adds helper methods with tests. --- .../main/java/org/bitcoinj/core/ECKey.java | 27 ++++++++++++++----- .../org/bitcoinj/crypto/DeterministicKey.java | 2 +- .../java/org/bitcoinj/core/ECKeyTest.java | 16 +++++++++-- 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/core/ECKey.java b/core/src/main/java/org/bitcoinj/core/ECKey.java index 52523c72..e10f6478 100644 --- a/core/src/main/java/org/bitcoinj/core/ECKey.java +++ b/core/src/main/java/org/bitcoinj/core/ECKey.java @@ -1117,23 +1117,36 @@ public class ECKey implements EncryptableItem, Serializable { @Override public String toString() { - return toString(false); + return toString(false, null); } /** * Produce a string rendering of the ECKey INCLUDING the private key. * Unless you absolutely need the private key it is better for security reasons to just use {@link #toString()}. */ - public String toStringWithPrivate() { - return toString(true); + public String toStringWithPrivate(NetworkParameters params) { + return toString(true, params); } - private String toString(boolean includePrivate) { + public String getPrivateKeyAsHex() { + return Utils.HEX.encode(getPrivKeyBytes()); + } + + public String getPublicKeyAsHex() { + return Utils.HEX.encode(pub.getEncoded()); + } + + public String getPrivateKeyAsWiF(NetworkParameters params) { + return getPrivateKeyEncoded(params).toString(); + } + + private String toString(boolean includePrivate, NetworkParameters params) { final ToStringHelper helper = Objects.toStringHelper(this).omitNullValues(); - helper.add("pub", Utils.HEX.encode(pub.getEncoded())); + helper.add("pub HEX", getPublicKeyAsHex()); if (includePrivate) { try { - helper.add("priv", Utils.HEX.encode(getPrivKey().toByteArray())); + helper.add("priv HEX", getPrivateKeyAsHex()); + helper.add("priv WIF", getPrivateKeyAsWiF(params)); } catch (IllegalStateException e) { // TODO: Make hasPrivKey() work for deterministic keys and fix this. } @@ -1156,7 +1169,7 @@ public class ECKey implements EncryptableItem, Serializable { builder.append("\n"); if (includePrivateKeys) { builder.append(" "); - builder.append(toStringWithPrivate()); + builder.append(toStringWithPrivate(params)); builder.append("\n"); } } diff --git a/core/src/main/java/org/bitcoinj/crypto/DeterministicKey.java b/core/src/main/java/org/bitcoinj/crypto/DeterministicKey.java index ff49b71b..e0d4bf7f 100644 --- a/core/src/main/java/org/bitcoinj/crypto/DeterministicKey.java +++ b/core/src/main/java/org/bitcoinj/crypto/DeterministicKey.java @@ -482,7 +482,7 @@ public class DeterministicKey extends ECKey { builder.append("\n"); if (includePrivateKeys) { builder.append(" "); - builder.append(toStringWithPrivate()); + builder.append(toStringWithPrivate(params)); builder.append("\n"); } } diff --git a/core/src/test/java/org/bitcoinj/core/ECKeyTest.java b/core/src/test/java/org/bitcoinj/core/ECKeyTest.java index 2eb7de24..be2d2171 100644 --- a/core/src/test/java/org/bitcoinj/core/ECKeyTest.java +++ b/core/src/test/java/org/bitcoinj/core/ECKeyTest.java @@ -317,9 +317,21 @@ public class ECKeyTest { @Test public void testToString() throws Exception { ECKey key = ECKey.fromPrivate(BigInteger.TEN).decompress(); // An example private key. + NetworkParameters params = MainNetParams.get(); + assertEquals("ECKey{pub HEX=04a0434d9e47f3c86235477c7b1ae6ae5d3442d49b1943c2b752a68e2a47e247c7893aba425419bc27a3b6c7e693a24c696f794c2ed877a1593cbee53b037368d7, isEncrypted=false}", key.toString()); + assertEquals("ECKey{pub HEX=04a0434d9e47f3c86235477c7b1ae6ae5d3442d49b1943c2b752a68e2a47e247c7893aba425419bc27a3b6c7e693a24c696f794c2ed877a1593cbee53b037368d7, priv HEX=000000000000000000000000000000000000000000000000000000000000000a, priv WIF=5HpHagT65TZzG1PH3CSu63k8DbpvD8s5ip4nEB3kEsreBoNWTw6, isEncrypted=false}", key.toStringWithPrivate(params)); + } - assertEquals("ECKey{pub=04a0434d9e47f3c86235477c7b1ae6ae5d3442d49b1943c2b752a68e2a47e247c7893aba425419bc27a3b6c7e693a24c696f794c2ed877a1593cbee53b037368d7, isEncrypted=false}", key.toString()); - assertEquals("ECKey{pub=04a0434d9e47f3c86235477c7b1ae6ae5d3442d49b1943c2b752a68e2a47e247c7893aba425419bc27a3b6c7e693a24c696f794c2ed877a1593cbee53b037368d7, priv=0a, isEncrypted=false}", key.toStringWithPrivate()); + @Test + public void testGetPrivateKeyAsHex() throws Exception { + ECKey key = ECKey.fromPrivate(BigInteger.TEN).decompress(); // An example private key. + assertEquals("000000000000000000000000000000000000000000000000000000000000000a", key.getPrivateKeyAsHex()); + } + + @Test + public void testGetPublicKeyAsHex() throws Exception { + ECKey key = ECKey.fromPrivate(BigInteger.TEN).decompress(); // An example private key. + assertEquals("04a0434d9e47f3c86235477c7b1ae6ae5d3442d49b1943c2b752a68e2a47e247c7893aba425419bc27a3b6c7e693a24c696f794c2ed877a1593cbee53b037368d7", key.getPublicKeyAsHex()); } @Test From af20c37a8dfc5f1c49fd48ff353043bf6a38ee66 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Tue, 21 Oct 2014 13:05:02 +0200 Subject: [PATCH 18/20] Use finer grained locking around the wallet keychain, to allow for fast reading of keys/the current receive address even if the wallet is busy auto saving or processing large transactions. This helps reduce UI hangs/lag on Android. --- .../main/java/org/bitcoinj/core/Wallet.java | 355 ++++++++++-------- 1 file changed, 197 insertions(+), 158 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/core/Wallet.java b/core/src/main/java/org/bitcoinj/core/Wallet.java index ba8b8379..c18ecaee 100644 --- a/core/src/main/java/org/bitcoinj/core/Wallet.java +++ b/core/src/main/java/org/bitcoinj/core/Wallet.java @@ -17,23 +17,6 @@ package org.bitcoinj.core; -import org.bitcoinj.core.TransactionConfidence.ConfidenceType; -import org.bitcoinj.crypto.*; -import org.bitcoinj.params.UnitTestParams; -import org.bitcoinj.script.Script; -import org.bitcoinj.script.ScriptBuilder; -import org.bitcoinj.script.ScriptChunk; -import org.bitcoinj.signers.MissingSigResolutionSigner; -import org.bitcoinj.signers.LocalTransactionSigner; -import org.bitcoinj.signers.TransactionSigner; -import org.bitcoinj.store.UnreadableWalletException; -import org.bitcoinj.store.WalletProtobufSerializer; -import org.bitcoinj.utils.BaseTaggableObject; -import org.bitcoinj.utils.ExchangeRate; -import org.bitcoinj.utils.ListenerRegistration; -import org.bitcoinj.utils.Threading; -import org.bitcoinj.wallet.*; -import org.bitcoinj.wallet.WalletTransaction.Pool; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Objects; import com.google.common.base.Objects.ToStringHelper; @@ -44,21 +27,40 @@ import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.SettableFuture; import com.google.protobuf.ByteString; - +import net.jcip.annotations.GuardedBy; import org.bitcoin.protocols.payments.Protos.PaymentDetails; +import org.bitcoinj.core.TransactionConfidence.ConfidenceType; +import org.bitcoinj.crypto.*; +import org.bitcoinj.params.UnitTestParams; +import org.bitcoinj.script.Script; +import org.bitcoinj.script.ScriptBuilder; +import org.bitcoinj.script.ScriptChunk; +import org.bitcoinj.signers.LocalTransactionSigner; +import org.bitcoinj.signers.MissingSigResolutionSigner; +import org.bitcoinj.signers.TransactionSigner; +import org.bitcoinj.store.UnreadableWalletException; +import org.bitcoinj.store.WalletProtobufSerializer; +import org.bitcoinj.utils.BaseTaggableObject; +import org.bitcoinj.utils.ExchangeRate; +import org.bitcoinj.utils.ListenerRegistration; +import org.bitcoinj.utils.Threading; +import org.bitcoinj.wallet.*; import org.bitcoinj.wallet.Protos.Wallet.EncryptionType; +import org.bitcoinj.wallet.WalletTransaction.Pool; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.spongycastle.crypto.params.KeyParameter; import javax.annotation.Nullable; -import javax.annotation.concurrent.GuardedBy; import java.io.*; import java.util.*; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; import static com.google.common.base.Preconditions.*; @@ -106,7 +108,10 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha private static final long serialVersionUID = 2L; private static final int MINIMUM_BLOOM_DATA_LENGTH = 8; + // Ordering: lock > keychainLock. Keychain is protected separately to allow fast querying of current receive address + // even if the wallet itself is busy e.g. saving or processing a big reorg. Useful for reducing UI latency. protected final ReentrantLock lock = Threading.lock("wallet"); + protected final ReentrantReadWriteLock keychainLock = Threading.factory.newReentrantReadWriteLock("wallet-keychain"); // The various pools below give quick access to wallet-relevant transactions by the state they're in: // @@ -151,7 +156,7 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha // The key chain group is not thread safe, and generally the whole hierarchy of objects should not be mutated // outside the wallet lock. So don't expose this object directly via any accessors! - @GuardedBy("lock") protected KeyChainGroup keychain; + @GuardedBy("keychainLock") protected KeyChainGroup keychain; // A list of scripts watched by this wallet. private Set