Refactorings.

Make a convenience ECKey.decrypt(KeyParameter) that doesn't require the keycrypter to be manually specified, as often (always?) the key knows it already.

Introduce a KeyBag interface that just contains findKeyBy* methods, then make Wallet implement it and change Transaction.signInputs to use it. Take out the encrypted-key specific stuff here: Transaction now requires unencrypted keys. Create a DecryptingKeyBag class that just forwards calls to Wallet and decrypts the returned keys. This decouples the signing code from Wallet a bit.

Should be all API compatible.

core/src/main/java/com/google/bitcoin/core/ECKey.java

@@ -567,7 +567,7 @@ public class ECKey implements EncryptableItem, Serializable {
         if (crypter != null) {
             if (aesKey == null)
                 throw new KeyIsEncryptedException();
-            return decrypt(crypter, aesKey).sign(input);
+            return decrypt(aesKey).sign(input);
         } else {
             // No decryption of private key required.
             if (priv == null)
@@ -966,7 +966,6 @@ public class ECKey implements EncryptableItem, Serializable {
      *
      * @param keyCrypter The keyCrypter that specifies exactly how the decrypted bytes are created.
      * @param aesKey The KeyParameter with the AES encryption key (usually constructed with keyCrypter#deriveKey and cached).
-     * @return unencryptedKey
      */
     public ECKey decrypt(KeyCrypter keyCrypter, KeyParameter aesKey) throws KeyCrypterException {
         checkNotNull(keyCrypter);
@@ -984,6 +983,20 @@ public class ECKey implements EncryptableItem, Serializable {
         return key;
     }
 
+    /**
+     * Create a decrypted private key with AES key. Note that if the AES key is wrong, this
+     * has some chance of throwing KeyCrypterException due to the corrupted padding that will result, but it can also
+     * just yield a garbage key.
+     *
+     * @param aesKey The KeyParameter with the AES encryption key (usually constructed with keyCrypter#deriveKey and cached).
+     */
+    public ECKey decrypt(KeyParameter aesKey) throws KeyCrypterException {
+        final KeyCrypter crypter = getKeyCrypter();
+        if (crypter == null)
+            throw new KeyCrypterException("No key crypter available");
+        return decrypt(crypter, aesKey);
+    }
+
     /**
      * <p>Check that it is possible to decrypt the key with the keyCrypter and that the original key is returned.</p>
      *

core/src/main/java/com/google/bitcoin/core/Transaction.java

@@ -22,7 +22,8 @@ import com.google.bitcoin.crypto.TransactionSignature;
 import com.google.bitcoin.script.Script;
 import com.google.bitcoin.script.ScriptBuilder;
 import com.google.bitcoin.script.ScriptOpCodes;
-import com.google.common.collect.ImmutableList;
+import com.google.bitcoin.wallet.DecryptingKeyBag;
+import com.google.bitcoin.wallet.KeyBag;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.primitives.Ints;
 import com.google.common.primitives.Longs;
@@ -849,7 +850,22 @@ public class Transaction extends ChildMessage implements Serializable {
      * @param wallet  A wallet is required to fetch the keys needed for signing.
      * @param aesKey The AES key to use to decrypt the key before signing. Null if no decryption is required.
      */
-    public synchronized void signInputs(SigHash hashType, Wallet wallet, @Nullable KeyParameter aesKey) throws ScriptException {
+    public void signInputs(SigHash hashType, Wallet wallet, @Nullable KeyParameter aesKey) throws ScriptException {
+        if (aesKey == null) {
+            signInputs(hashType, false, wallet);
+        } else {
+            signInputs(hashType, false, new DecryptingKeyBag(wallet, aesKey));
+        }
+    }
+
+    /**
+     * Signs as many inputs as possible using keys from the given key bag, which are expected to be usable for
+     * signing, i.e. not encrypted and not missing the private key part.
+     *
+     * @param hashType This should always be set to SigHash.ALL currently. Other types are unused.
+     * @param keyBag a provider of keys that are usable as-is for signing.
+     */
+    public synchronized void signInputs(SigHash hashType, boolean anyoneCanPay, KeyBag keyBag) throws ScriptException {
         checkState(inputs.size() > 0);
         checkState(outputs.size() > 0);
 
@@ -885,16 +901,15 @@ public class Transaction extends ChildMessage implements Serializable {
             if (input.getScriptBytes().length != 0)
                 log.warn("Re-signing an already signed transaction! Be sure this is what you want.");
             // Find the signing key we'll need to use.
-            ECKey key = input.getOutpoint().getConnectedKey(wallet);
+            ECKey key = input.getOutpoint().getConnectedKey(keyBag);
             // This assert should never fire. If it does, it means the wallet is inconsistent.
             checkNotNull(key, "Transaction exists in wallet that we cannot redeem: %s", input.getOutpoint().getHash());
             // Keep the key around for the script creation step below.
             signingKeys[i] = key;
             // The anyoneCanPay feature isn't used at the moment.
-            boolean anyoneCanPay = false;
             byte[] connectedPubKeyScript = input.getOutpoint().getConnectedPubKeyScript();
             try {
-                signatures[i] = calculateSignature(i, key, aesKey, connectedPubKeyScript, hashType, anyoneCanPay);
+                signatures[i] = calculateSignature(i, key, connectedPubKeyScript, hashType, anyoneCanPay);
             } catch (ECKey.KeyIsEncryptedException e) {
                 throw e;
             } catch (ECKey.MissingPrivateKeyException e) {
@@ -935,22 +950,21 @@ public class Transaction extends ChildMessage implements Serializable {
     /**
      * Calculates a signature that is valid for being inserted into the input at the given position. This is simply
      * a wrapper around calling {@link Transaction#hashForSignature(int, byte[], com.google.bitcoin.core.Transaction.SigHash, boolean)}
-     * followed by {@link ECKey#sign(Sha256Hash, org.spongycastle.crypto.params.KeyParameter)} and then returning
+     * followed by {@link ECKey#sign(Sha256Hash)} and then returning a new {@link TransactionSignature}. The key
-     * a new {@link TransactionSignature}.
+     * must be usable for signing as-is: if the key is encrypted it must be decrypted first external to this method.
      *
      * @param inputIndex Which input to calculate the signature for, as an index.
      * @param key The private key used to calculate the signature.
-     * @param aesKey If not null, this will be used to decrypt the key.
      * @param connectedPubKeyScript Byte-exact contents of the scriptPubKey that is being satisified.
      * @param hashType Signing mode, see the enum for documentation.
      * @param anyoneCanPay Signing mode, see the SigHash enum for documentation.
      * @return A newly calculated signature object that wraps the r, s and sighash components.
      */
-    public synchronized  TransactionSignature calculateSignature(int inputIndex, ECKey key, @Nullable KeyParameter aesKey,
+    public synchronized TransactionSignature calculateSignature(int inputIndex, ECKey key,
-                                                                 byte[] connectedPubKeyScript,
+                                                                byte[] connectedPubKeyScript,
-                                                                 SigHash hashType, boolean anyoneCanPay) {
+                                                                SigHash hashType, boolean anyoneCanPay) {
         Sha256Hash hash = hashForSignature(inputIndex, connectedPubKeyScript, hashType, anyoneCanPay);
-        return new TransactionSignature(key.sign(hash, aesKey), hashType, anyoneCanPay);
+        return new TransactionSignature(key.sign(hash), hashType, anyoneCanPay);
     }
 
     /**

core/src/main/java/com/google/bitcoin/core/TransactionOutPoint.java

@@ -17,6 +17,7 @@
 package com.google.bitcoin.core;
 
 import com.google.bitcoin.script.Script;
+import com.google.bitcoin.wallet.KeyBag;
 
 import javax.annotation.Nullable;
 import java.io.IOException;
@@ -134,19 +135,20 @@ public class TransactionOutPoint extends ChildMessage implements Serializable {
     /**
      * Returns the ECKey identified in the connected output, for either pay-to-address scripts or pay-to-key scripts.
      * If the script forms cannot be understood, throws ScriptException.
+     *
      * @return an ECKey or null if the connected key cannot be found in the wallet.
      */
     @Nullable
-    public ECKey getConnectedKey(Wallet wallet) throws ScriptException {
+    public ECKey getConnectedKey(KeyBag keyBag) throws ScriptException {
         TransactionOutput connectedOutput = getConnectedOutput();
         checkNotNull(connectedOutput, "Input is not connected so cannot retrieve key");
         Script connectedScript = connectedOutput.getScriptPubKey();
         if (connectedScript.isSentToAddress()) {
             byte[] addressBytes = connectedScript.getPubKeyHash();
-            return wallet.findKeyFromPubHash(addressBytes);
+            return keyBag.findKeyFromPubHash(addressBytes);
         } else if (connectedScript.isSentToRawPubKey()) {
             byte[] pubkeyBytes = connectedScript.getPubKey();
-            return wallet.findKeyFromPubKey(pubkeyBytes);
+            return keyBag.findKeyFromPubKey(pubkeyBytes);
         } else {
             throw new ScriptException("Could not understand form of connected output script: " + connectedScript);
         }

core/src/main/java/com/google/bitcoin/core/Wallet.java

@@ -99,7 +99,7 @@ import static com.google.common.base.Preconditions.*;
  * {@link Wallet#autosaveToFile(java.io.File, long, java.util.concurrent.TimeUnit, com.google.bitcoin.wallet.WalletFiles.Listener)}
  * for more information about this.</p>
  */
-public class Wallet extends BaseTaggableObject implements Serializable, BlockChainListener, PeerFilterProvider {
+public class Wallet extends BaseTaggableObject implements Serializable, BlockChainListener, PeerFilterProvider, KeyBag {
     private static final Logger log = LoggerFactory.getLogger(Wallet.class);
     private static final long serialVersionUID = 2L;
     private static final int MINIMUM_BLOOM_DATA_LENGTH = 8;

core/src/main/java/com/google/bitcoin/crypto/DeterministicKey.java

@@ -240,6 +240,11 @@ public class DeterministicKey extends ECKey {
         return key;
     }
 
+    @Override
+    public DeterministicKey decrypt(KeyParameter aesKey) throws KeyCrypterException {
+        return (DeterministicKey) super.decrypt(aesKey);
+    }
+
     // For when a key is encrypted, either decrypt our encrypted private key bytes, or work up the tree asking parents
     // to decrypt and re-derive.
     private BigInteger findOrDeriveEncryptedPrivateKey(KeyCrypter keyCrypter, KeyParameter aesKey) {

core/src/main/java/com/google/bitcoin/wallet/BasicKeyChain.java

@@ -424,7 +424,7 @@ public class BasicKeyChain implements EncryptableKeyChain {
                 throw new KeyCrypterException("Password/key was incorrect.");
             BasicKeyChain decrypted = new BasicKeyChain();
             for (ECKey key : hashToKeys.values()) {
-                decrypted.importKeyLocked(key.decrypt(keyCrypter, aesKey));
+                decrypted.importKeyLocked(key.decrypt(aesKey));
             }
             return decrypted;
         } finally {
@@ -467,7 +467,7 @@ public class BasicKeyChain implements EncryptableKeyChain {
             checkState(first != null, "No encrypted keys in the wallet");
 
             try {
-                ECKey rebornKey = first.decrypt(keyCrypter, aesKey);
+                ECKey rebornKey = first.decrypt(aesKey);
                 return Arrays.equals(first.getPubKey(), rebornKey.getPubKey());
             } catch (KeyCrypterException e) {
                 // The AES key supplied is incorrect.

core/src/main/java/com/google/bitcoin/wallet/DecryptingKeyBag.java

@@ -0,0 +1,55 @@
+/**
+ * Copyright 2014 The bitcoinj authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.bitcoin.wallet;
+
+import com.google.bitcoin.core.ECKey;
+import org.spongycastle.crypto.params.KeyParameter;
+
+import javax.annotation.Nullable;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+/**
+ * A DecryptingKeyBag filters a pre-existing key bag, decrypting keys as they are requested using the provided
+ * AES key.
+ */
+public class DecryptingKeyBag implements KeyBag {
+    protected final KeyBag target;
+    protected final KeyParameter aesKey;
+
+    public DecryptingKeyBag(KeyBag target, KeyParameter aesKey) {
+        this.target = checkNotNull(target);
+        this.aesKey = checkNotNull(aesKey);
+    }
+
+    @Nullable
+    private ECKey maybeDecrypt(ECKey key) {
+        return key == null ? null : key.decrypt(aesKey);
+    }
+
+    @Nullable
+    @Override
+    public ECKey findKeyFromPubHash(byte[] pubkeyHash) {
+        return maybeDecrypt(target.findKeyFromPubHash(pubkeyHash));
+    }
+
+    @Nullable
+    @Override
+    public ECKey findKeyFromPubKey(byte[] pubkey) {
+        return maybeDecrypt(target.findKeyFromPubKey(pubkey));
+    }
+}

core/src/main/java/com/google/bitcoin/wallet/DeterministicKeyChain.java

@@ -583,7 +583,7 @@ public class DeterministicKeyChain implements EncryptableKeyChain {
         checkNotNull(aesKey);
         checkState(getKeyCrypter() != null, "Key chain not encrypted");
         try {
-            return rootKey.decrypt(getKeyCrypter(), aesKey).getPubKeyPoint().equals(rootKey.getPubKeyPoint());
+            return rootKey.decrypt(aesKey).getPubKeyPoint().equals(rootKey.getPubKeyPoint());
         } catch (KeyCrypterException e) {
             return false;
         }

core/src/main/java/com/google/bitcoin/wallet/KeyBag.java

@@ -0,0 +1,40 @@
+/**
+ * Copyright 2014 The bitcoinj authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.bitcoin.wallet;
+
+import com.google.bitcoin.core.ECKey;
+
+/**
+ * A KeyBag is simply an object that can map public keys and their 160-bit hashes to ECKey objects. All
+ * {@link com.google.bitcoin.wallet.KeyChain}s are key bags.
+ */
+public interface KeyBag {
+    /**
+     * Locates a keypair from the keychain given the hash of the public key. This is needed when finding out which
+     * key we need to use to redeem a transaction output.
+     *
+     * @return ECKey object or null if no such key was found.
+     */
+    public ECKey findKeyFromPubHash(byte[] pubkeyHash);
+
+    /**
+     * Locates a keypair from the keychain given the raw public key bytes.
+     *
+     * @return ECKey or null if no such key was found.
+     */
+    public ECKey findKeyFromPubKey(byte[] pubkey);
+}

core/src/main/java/com/google/bitcoin/wallet/KeyChain.java

@@ -34,21 +34,7 @@ import java.util.concurrent.Executor;
  * restrictions is to support key chains that may be handled by external hardware or software, or which are derived
  * deterministically from a seed (and thus the notion of importing a key is meaningless).</p>
  */
-public interface KeyChain {
+public interface KeyChain extends KeyBag {
-    /**
-     * Locates a keypair from the keychain given the hash of the public key. This is needed when finding out which
-     * key we need to use to redeem a transaction output.
-     *
-     * @return ECKey object or null if no such key was found.
-     */
-    public ECKey findKeyFromPubHash(byte[] pubkeyHash);
-
-    /**
-     * Locates a keypair from the keychain given the raw public key bytes.
-     * @return ECKey or null if no such key was found.
-     */
-    public ECKey findKeyFromPubKey(byte[] pubkey);
-
     /** Returns true if the given key is in the chain. */
     public boolean hasKey(ECKey key);
 

core/src/test/java/com/google/bitcoin/core/ECKeyTest.java

@@ -32,16 +32,12 @@ import com.google.common.util.concurrent.MoreExecutors;
 import com.google.protobuf.ByteString;
 import org.bitcoinj.wallet.Protos;
 import org.bitcoinj.wallet.Protos.ScryptParameters;
-import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.spongycastle.crypto.params.ECDomainParameters;
 import org.spongycastle.crypto.params.KeyParameter;
 import org.spongycastle.util.encoders.DecoderException;
-import org.spongycastle.math.ec.ECCurve;
-import org.spongycastle.math.ec.ECPoint;
 import org.spongycastle.util.encoders.Hex;
 
 import java.io.InputStream;
@@ -273,7 +269,7 @@ public class ECKeyTest {
         assertEquals(time, encryptedKey.getCreationTimeSeconds());
         assertTrue(encryptedKey.isEncrypted());
         assertNull(encryptedKey.getSecretBytes());
-        key = encryptedKey.decrypt(keyCrypter, keyCrypter.deriveKey(PASSWORD1));
+        key = encryptedKey.decrypt(keyCrypter.deriveKey(PASSWORD1));
         assertTrue(!key.isEncrypted());
         assertArrayEquals(originalPrivateKeyBytes, key.getPrivKeyBytes());
     }
@@ -287,7 +283,7 @@ public class ECKeyTest {
         ECKey encryptedKey = ECKey.fromEncrypted(encryptedPrivateKey, keyCrypter, unencryptedKey.getPubKey());
         assertTrue(encryptedKey.isEncrypted());
         assertNull(encryptedKey.getSecretBytes());
-        ECKey rebornUnencryptedKey = encryptedKey.decrypt(keyCrypter, keyCrypter.deriveKey(PASSWORD1));
+        ECKey rebornUnencryptedKey = encryptedKey.decrypt(keyCrypter.deriveKey(PASSWORD1));
         assertTrue(!rebornUnencryptedKey.isEncrypted());
         assertArrayEquals(originalPrivateKeyBytes, rebornUnencryptedKey.getPrivKeyBytes());
     }

core/src/test/java/com/google/bitcoin/crypto/ChildKeyDerivationTest.java

@@ -133,13 +133,13 @@ public class ChildKeyDerivationTest {
 
         DeterministicKey key1 = HDKeyDerivation.createMasterPrivateKey("it was all a hoax".getBytes());
         DeterministicKey encryptedKey1 = key1.encrypt(scrypter, aesKey, null);
-        DeterministicKey decryptedKey1 = encryptedKey1.decrypt(scrypter, aesKey);
+        DeterministicKey decryptedKey1 = encryptedKey1.decrypt(aesKey);
         assertEquals(key1, decryptedKey1);
 
         DeterministicKey key2 = HDKeyDerivation.deriveChildKey(key1, ChildNumber.ZERO);
         DeterministicKey derivedKey2 = HDKeyDerivation.deriveChildKey(encryptedKey1, ChildNumber.ZERO);
         assertTrue(derivedKey2.isEncrypted());   // parent is encrypted.
-        DeterministicKey decryptedKey2 = derivedKey2.decrypt(scrypter, aesKey);
+        DeterministicKey decryptedKey2 = derivedKey2.decrypt(aesKey);
         assertFalse(decryptedKey2.isEncrypted());
         assertEquals(key2, decryptedKey2);