Rewrite how WalletTemplate estimates scrypt difficulty, as the old approach was horribly busted and could use tons of RAM. Backport from Lighthouse.

wallettemplate/src/main/java/wallettemplate/Main.java

@@ -96,6 +96,8 @@ public class Main extends Application {
 
         mainWindow.show();
 
+        WalletSetPasswordController.estimateKeyDerivationTimeMsec();
+
         bitcoin.addListener(new Service.Listener() {
             @Override
             public void failed(Service.State from, Throwable failure) {

wallettemplate/src/main/java/wallettemplate/WalletPasswordController.java

@@ -58,8 +58,7 @@ public class WalletPasswordController {
         checkNotNull(keyCrypter);   // We should never arrive at this GUI if the wallet isn't actually encrypted.
         KeyDerivationTasks tasks = new KeyDerivationTasks(keyCrypter, password, getTargetTime()) {
             @Override
-            protected void onFinish(KeyParameter aesKey) {
+            protected void onFinish(KeyParameter aesKey, int timeTakenMsec) {
-                super.onFinish(aesKey);
                 checkGuiThread();
                 if (Main.bitcoin.wallet().checkAESKey(aesKey)) {
                     WalletPasswordController.this.aesKey.set(aesKey);

wallettemplate/src/main/java/wallettemplate/WalletSetPasswordController.java

@@ -1,19 +1,19 @@
 package wallettemplate;
 
-import org.bitcoinj.crypto.KeyCrypterScrypt;
+import com.google.protobuf.*;
-import javafx.event.ActionEvent;
+import javafx.application.*;
-import javafx.scene.control.Button;
+import javafx.event.*;
-import javafx.scene.control.Label;
+import javafx.fxml.*;
-import javafx.scene.control.PasswordField;
+import javafx.scene.control.*;
-import javafx.scene.control.ProgressIndicator;
+import javafx.scene.layout.*;
-import javafx.scene.image.ImageView;
+import org.bitcoinj.crypto.*;
-import javafx.scene.layout.GridPane;
+import org.bitcoinj.wallet.*;
-import org.slf4j.Logger;
+import org.slf4j.*;
-import org.slf4j.LoggerFactory;
+import org.spongycastle.crypto.params.*;
-import org.spongycastle.crypto.params.KeyParameter;
+import wallettemplate.utils.*;
-import wallettemplate.utils.KeyDerivationTasks;
 
-import java.time.Duration;
+import java.time.*;
+import java.util.concurrent.*;
 
 import static wallettemplate.utils.GuiUtils.*;
 
@@ -21,19 +21,48 @@ public class WalletSetPasswordController {
     private static final Logger log = LoggerFactory.getLogger(WalletSetPasswordController.class);
     public PasswordField pass1, pass2;
 
-    public ImageView padlockImage;
     public ProgressIndicator progressMeter;
     public GridPane widgetGrid;
     public Button closeButton;
     public Label explanationLabel;
 
     public Main.OverlayUI overlayUI;
+    // These params were determined empirically on a top-range (as of 2014) MacBook Pro with native scrypt support,
+    // using the scryptenc command line tool from the original scrypt distribution, given a memory limit of 40mb.
+    public static final Protos.ScryptParameters SCRYPT_PARAMETERS = Protos.ScryptParameters.newBuilder()
+            .setP(6)
+            .setR(8)
+            .setN(32768)
+            .setSalt(ByteString.copyFrom(KeyCrypterScrypt.randomSalt()))
+            .build();
 
     public void initialize() {
-        padlockImage.setOpacity(0);
         progressMeter.setOpacity(0);
     }
 
+    public static Duration estimatedKeyDerivationTime = null;
+
+    public static CompletableFuture<Duration> estimateKeyDerivationTimeMsec() {
+        // This is run in the background after startup. If we haven't recorded it before, do a key derivation to see
+        // how long it takes. This helps us produce better progress feedback, as on Windows we don't currently have a
+        // native Scrypt impl and the Java version is ~3 times slower, plus it depends a lot on CPU speed.
+        CompletableFuture<Duration> future = new CompletableFuture<>();
+        new Thread(() -> {
+            log.info("Doing background test key derivation");
+            KeyCrypterScrypt scrypt = new KeyCrypterScrypt(SCRYPT_PARAMETERS);
+            long start = System.currentTimeMillis();
+            scrypt.deriveKey("test password");
+            long msec = System.currentTimeMillis() - start;
+            log.info("Background test key derivation took {}msec", msec);
+            Platform.runLater(() -> {
+                estimatedKeyDerivationTime = Duration.ofMillis(msec);
+                future.complete(estimatedKeyDerivationTime);
+            });
+        }).start();
+        return future;
+    }
+
+    @FXML
     public void setPasswordClicked(ActionEvent event) {
         if (!pass1.getText().equals(pass2.getText())) {
             informationalAlert("Passwords do not match", "Try re-typing your chosen passwords.");
@@ -47,24 +76,23 @@ public class WalletSetPasswordController {
         }
 
         fadeIn(progressMeter);
-        fadeIn(padlockImage);
         fadeOut(widgetGrid);
         fadeOut(explanationLabel);
         fadeOut(closeButton);
 
-        // Figure out how fast this computer can scrypt. We do it on the UI thread because the delay should be small
-        // and so we don't really care about blocking here.
-        IdealPasswordParameters params = new IdealPasswordParameters(password);
-        KeyCrypterScrypt scrypt = new KeyCrypterScrypt(params.realIterations);
-        // Write the target time to the wallet so we can make the progress bar work when entering the password.
-        WalletPasswordController.setTargetTime(params.realTargetTime);
 
-        // Deriving the actual key runs on a background thread.
+        KeyCrypterScrypt scrypt = new KeyCrypterScrypt(SCRYPT_PARAMETERS);
-        KeyDerivationTasks tasks = new KeyDerivationTasks(scrypt, password, params.realTargetTime) {
+
+        // Deriving the actual key runs on a background thread. 500msec is empirical on my laptop (actual val is more like 333 but we give padding time).
+        KeyDerivationTasks tasks = new KeyDerivationTasks(scrypt, password, estimatedKeyDerivationTime) {
             @Override
-            protected void onFinish(KeyParameter aesKey) {
+            protected void onFinish(KeyParameter aesKey, int timeTakenMsec) {
+                // Write the target time to the wallet so we can make the progress bar work when entering the password.
+                WalletPasswordController.setTargetTime(Duration.ofMillis(timeTakenMsec));
                 // The actual encryption part doesn't take very long as most private keys are derived on demand.
+                log.info("Key derived, now encrypting");
                 Main.bitcoin.wallet().encrypt(scrypt, aesKey);
+                log.info("Encryption done");
                 informationalAlert("Wallet encrypted",
                         "You can remove the password at any time from the settings screen.");
                 overlayUI.done();
@@ -77,32 +105,4 @@ public class WalletSetPasswordController {
     public void closeClicked(ActionEvent event) {
         overlayUI.done();
     }
-
-    private static class IdealPasswordParameters {
-        public final int realIterations;
-        public final Duration realTargetTime;
-
-        public IdealPasswordParameters(String password) {
-            final int targetTimeMsec = 2000;
-
-            int iterations = 16384;
-            KeyCrypterScrypt scrypt = new KeyCrypterScrypt(iterations);
-            long now = System.currentTimeMillis();
-            scrypt.deriveKey(password);
-            long time = System.currentTimeMillis() - now;
-            log.info("Initial iterations took {} msec", time);
-
-            // N can only be a power of two, so we keep shifting both iterations and doubling time taken
-            // until we are in sorta the right general area.
-            while (time < targetTimeMsec) {
-                iterations <<= 1;
-                time *= 2;
-            }
-
-            realIterations = iterations;
-            // Fudge it by +10% to ensure our progress meter is always a bit behind the real encryption. Plus
-            // without this it seems the real scrypting always takes a bit longer than we estimated for some reason.
-            realTargetTime = Duration.ofMillis((long) (time * 1.1));
-        }
-    }
 }

wallettemplate/src/main/java/wallettemplate/utils/KeyDerivationTasks.java

@@ -8,6 +8,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.spongycastle.crypto.params.KeyParameter;
 
+import javax.annotation.*;
 import java.time.Duration;
 import java.util.concurrent.TimeUnit;
 
@@ -24,39 +25,49 @@ public class KeyDerivationTasks {
 
     private final Task<Void> progressTask;
 
-    public KeyDerivationTasks(KeyCrypterScrypt scrypt, String password, Duration targetTime) {
+    private volatile int timeTakenMsec = -1;
+
+    public KeyDerivationTasks(KeyCrypterScrypt scrypt, String password, @Nullable Duration targetTime) {
         keyDerivationTask = new Task<KeyParameter>() {
             @Override
             protected KeyParameter call() throws Exception {
+                long start = System.currentTimeMillis();
                 try {
-                    return scrypt.deriveKey(password);
+                    log.info("Started key derivation");
+                    KeyParameter result = scrypt.deriveKey(password);
+                    timeTakenMsec = (int) (System.currentTimeMillis() - start);
+                    log.info("Key derivation done in {}ms", timeTakenMsec);
+                    return result;
                 } catch (Throwable e) {
-                    e.printStackTrace();
+                    log.error("Exception during key derivation", e);
                     throw e;
-                } finally {
-                    log.info("Key derivation done");
                 }
             }
         };
 
-        // And the fake progress meter ...
+        // And the fake progress meter ... if the vals were calculated correctly progress bar should reach 100%
+        // a brief moment after the keys were derived successfully.
         progressTask = new Task<Void>() {
             private KeyParameter aesKey;
 
             @Override
             protected Void call() throws Exception {
-                long startTime = System.currentTimeMillis();
+                if (targetTime != null) {
-                long curTime;
+                    long startTime = System.currentTimeMillis();
-                long targetTimeMillis = targetTime.toMillis();
+                    long curTime;
-                while ((curTime = System.currentTimeMillis()) < startTime + targetTimeMillis) {
+                    long targetTimeMillis = targetTime.toMillis();
-                    double progress = (curTime - startTime) / (double) targetTimeMillis;
+                    while ((curTime = System.currentTimeMillis()) < startTime + targetTimeMillis) {
-                    updateProgress(progress, 1.0);
+                        double progress = (curTime - startTime) / (double) targetTimeMillis;
+                        updateProgress(progress, 1.0);
 
-                    // 60fps would require 16msec sleep here.
+                        // 60fps would require 16msec sleep here.
-                    Uninterruptibles.sleepUninterruptibly(20, TimeUnit.MILLISECONDS);
+                        Uninterruptibles.sleepUninterruptibly(20, TimeUnit.MILLISECONDS);
+                    }
+                    // Wait for the encryption thread before switching back to main UI.
+                    updateProgress(1.0, 1.0);
+                } else {
+                    updateProgress(-1, -1);
                 }
-                // Wait for the encryption thread before switching back to main UI.
-                updateProgress(1.0, 1.0);
                 aesKey = keyDerivationTask.get();
                 return null;
             }
@@ -64,7 +75,7 @@ public class KeyDerivationTasks {
             @Override
             protected void succeeded() {
                 checkGuiThread();
-                onFinish(aesKey);
+                onFinish(aesKey, timeTakenMsec);
             }
         };
         progress = progressTask.progressProperty();
@@ -75,6 +86,6 @@ public class KeyDerivationTasks {
         new Thread(progressTask, "Progress ticker").start();
     }
 
-    protected void onFinish(KeyParameter aesKey) {
+    protected void onFinish(KeyParameter aesKey, int timeTakenMsec) {
     }
 }