From 146f082dfb683495f372c34fce82efee5335a5c0 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Thu, 23 Oct 2014 15:42:34 +0200 Subject: [PATCH] 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 f0bde28e..545b3989 100644 --- a/core/src/main/java/org/bitcoinj/core/Wallet.java +++ b/core/src/main/java/org/bitcoinj/core/Wallet.java @@ -4391,7 +4391,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 44cb5b3b..7e2915d6 100644 --- a/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java +++ b/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java @@ -67,7 +67,7 @@ public class KeyChainGroup implements KeyBag { private BasicKeyChain basic; private NetworkParameters params; - private final List chains; + protected final List chains; private final EnumMap currentKeys; // The map keys are the watching keys of the followed chains and values are the following chains diff --git a/core/src/test/java/org/bitcoinj/core/WalletTest.java b/core/src/test/java/org/bitcoinj/core/WalletTest.java index 3f6f8afb..ef9b72a3 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; @@ -2393,10 +2394,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()); @@ -2411,6 +2419,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)