diff --git a/core/src/main/java/org/bitcoinj/core/Wallet.java b/core/src/main/java/org/bitcoinj/core/Wallet.java index a7560786..af4ba05a 100644 --- a/core/src/main/java/org/bitcoinj/core/Wallet.java +++ b/core/src/main/java/org/bitcoinj/core/Wallet.java @@ -939,6 +939,9 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha } else if (script.isSentToAddress()) { byte[] pubkeyHash = script.getPubKeyHash(); keychain.markPubKeyHashAsUsed(pubkeyHash); + } else if (script.isPayToScriptHash() && keychain.isMarried()) { + Address a = Address.fromP2SHScript(tx.getParams(), script); + keychain.markP2SHAddressAsUsed(a); } } catch (ScriptException e) { // Just means we didn't understand the output of this transaction: ignore it. diff --git a/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java b/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java index a65778f7..f4549e1d 100644 --- a/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java +++ b/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java @@ -69,9 +69,10 @@ public class KeyChainGroup implements KeyBag { private BasicKeyChain basic; private NetworkParameters params; protected final LinkedList chains; + // currentKeys is used for normal, non-multisig/married wallets. currentAddresses is used when we're handing out + // P2SH addresses. They're mutually exclusive. private final EnumMap currentKeys; - - private EnumMap currentAddresses; + private final EnumMap currentAddresses; @Nullable private KeyCrypter keyCrypter; private int lookaheadSize = -1; private int lookaheadThreshold = -1; @@ -358,6 +359,22 @@ public class KeyChainGroup implements KeyBag { return null; } + public void markP2SHAddressAsUsed(Address address) { + checkState(isMarried()); + checkArgument(address.isP2SHAddress()); + RedeemData data = findRedeemDataFromScriptHash(address.getHash160()); + if (data == null) + return; // Not our P2SH address. + for (ECKey key : data.keys) { + for (DeterministicKeyChain chain : chains) { + DeterministicKey k = chain.findKeyFromPubKey(key.getPubKey()); + if (k == null) continue; + chain.markKeyAsUsed(k); + maybeMarkCurrentAddressAsUsed(address); + } + } + } + @Nullable @Override public ECKey findKeyFromPubHash(byte[] pubkeyHash) { @@ -385,12 +402,27 @@ public class KeyChainGroup implements KeyBag { } } + /** If the given P2SH address is "current", advance it to a new one. */ + private void maybeMarkCurrentAddressAsUsed(Address address) { + checkState(isMarried()); + checkArgument(address.isP2SHAddress()); + for (Map.Entry entry : currentAddresses.entrySet()) { + if (entry.getValue() != null && entry.getValue().equals(address)) { + log.info("Marking P2SH address as used: {}", address); + currentAddresses.put(entry.getKey(), freshAddress(entry.getKey())); + return; + } + } + } + /** If the given key is "current", advance the current key to a new one. */ private void maybeMarkCurrentKeyAsUsed(DeterministicKey key) { + // It's OK for currentKeys to be empty here: it means we're a married wallet and the key may be a part of a + // rotating chain. for (Map.Entry entry : currentKeys.entrySet()) { if (entry.getValue() != null && entry.getValue().equals(key)) { log.info("Marking key as used: {}", key); - currentKeys.put(entry.getKey(), null); + currentKeys.put(entry.getKey(), freshKey(entry.getKey())); return; } } diff --git a/core/src/test/java/org/bitcoinj/core/WalletTest.java b/core/src/test/java/org/bitcoinj/core/WalletTest.java index 7a81c51b..5f848f8b 100644 --- a/core/src/test/java/org/bitcoinj/core/WalletTest.java +++ b/core/src/test/java/org/bitcoinj/core/WalletTest.java @@ -23,6 +23,7 @@ import org.bitcoinj.signers.StatelessTransactionSigner; import org.bitcoinj.signers.TransactionSigner; import org.bitcoinj.store.BlockStoreException; import org.bitcoinj.store.MemoryBlockStore; +import org.bitcoinj.store.UnreadableWalletException; import org.bitcoinj.store.WalletProtobufSerializer; import org.bitcoinj.testing.*; import org.bitcoinj.utils.ExchangeRate; @@ -348,7 +349,6 @@ public class WalletTest extends TestWithWallet { assertEquals("Wrong number of ALL.3", 1, wallet.getTransactions(true).size()); assertEquals(TransactionConfidence.Source.SELF, t2.getConfidence().getSource()); assertEquals(Transaction.Purpose.USER_PAYMENT, t2.getPurpose()); - assertEquals(wallet.getChangeAddress(), t2.getOutput(1).getScriptPubKey().getToAddress(params)); // Do some basic sanity checks. basicSanityChecks(wallet, t2, destination); @@ -422,16 +422,18 @@ public class WalletTest extends TestWithWallet { } private void spendUnconfirmedChange(Wallet wallet, Transaction t2, KeyParameter aesKey) throws Exception { + if (wallet.getTransactionSigners().size() == 1) // don't bother reconfiguring the p2sh wallet + wallet = roundTrip(wallet); Coin v3 = valueOf(0, 49); assertEquals(v3, wallet.getBalance()); Wallet.SendRequest req = Wallet.SendRequest.to(new ECKey().toAddress(params), valueOf(0, 48)); req.aesKey = aesKey; - Address a = req.changeAddress = new ECKey().toAddress(params); req.ensureMinRequiredFee = false; req.shuffleOutputs = false; wallet.completeTx(req); Transaction t3 = req.tx; - assertEquals(a, t3.getOutput(1).getScriptPubKey().getToAddress(params)); + assertNotEquals(t2.getOutput(1).getScriptPubKey().getToAddress(params), + t3.getOutput(1).getScriptPubKey().getToAddress(params)); assertNotNull(t3); wallet.commitTx(t3); assertTrue(wallet.isConsistent()); @@ -2387,8 +2389,7 @@ public class WalletTest extends TestWithWallet { // We don't attempt to race an attacker against unconfirmed transactions. // Now round-trip the wallet and check the protobufs are storing the data correctly. - Protos.Wallet protos = new WalletProtobufSerializer().walletToProto(wallet); - wallet = new WalletProtobufSerializer().readWallet(params, null, protos); + wallet = roundTrip(wallet); tx = wallet.getTransaction(tx.getHash()); checkNotNull(tx); @@ -2403,6 +2404,11 @@ public class WalletTest extends TestWithWallet { assertArrayEquals(address.getHash160(), tx.getOutput(0).getScriptPubKey().getPubKeyHash()); } + private Wallet roundTrip(Wallet wallet) throws UnreadableWalletException { + Protos.Wallet protos = new WalletProtobufSerializer().walletToProto(wallet); + return new WalletProtobufSerializer().readWallet(params, null, protos); + } + @Test public void keyRotationHD() throws Exception { // Test that if we rotate an HD chain, a new one is created and all arrivals on the old keys are moved. @@ -2713,8 +2719,7 @@ public class WalletTest extends TestWithWallet { TransactionSigner signer = new NopTransactionSigner(true); wallet.addTransactionSigner(signer); assertEquals(2, wallet.getTransactionSigners().size()); - Protos.Wallet protos = new WalletProtobufSerializer().walletToProto(wallet); - wallet = new WalletProtobufSerializer().readWallet(params, null, protos); + wallet = roundTrip(wallet); assertEquals(2, wallet.getTransactionSigners().size()); assertTrue(wallet.getTransactionSigners().get(1).isReady()); }