improve max cosigner ui handling in settings
What the developer wrote
Authored by Craig Raw
improve max cosigner ui handling in settings
What changed, and why it matters
This commit tightens wallet validation in Sparrow Wallet. It replaces a simple validity check with a more detailed one that reports specific problems, and it prevents users from creating or loading wallets whose number of cosigners exceeds what the chosen Bitcoin script type supports. It also fixes UI slider behavior so the maximum cosigner count stays within allowed limits when the script type changes. The changes are defensive: they catch misconfigurations earlier and give clearer error messages, which reduces the chance of accidentally using an invalid or unsupported wallet.
Treat as a hardening/bug-fix commit. Review the accompanying drongo submodule changes to confirm the exact InvalidWalletException conditions and max-cosigner values. No urgent security response is indicated, but users should upgrade to benefit from clearer validation and prevention of unsupported wallet configurations.
Security signals we found
Input validation: enforces script-type-specific maximum cosigner limits
Error handling improvement: replaces boolean validity check with exception-based checkWallet() that exposes specific failure reasons
UI state sanitization: caps multisig slider maximum when script type changes
Defensive rejection: refuses to load/replace wallets with too many cosigners for the selected script type
Evidence from the diff
The patch updates three Java files and a drongo submodule. AppController and LoadWallet now call wallet.checkWallet() and catch InvalidWalletException, surfacing the exception message in an IllegalStateException instead of a boolean isValid() check. SettingsController adds bounds checking on multisigControl’s max value against ScriptType.getMaxCosigners() during script-type changes and wallet loading, and rederiveAndReplaceWallet() now rejects output descriptors whose keystore count exceeds the script type’s maximum cosigners with an explicit error dialog. The drongo submodule bump likely carries the new checkWallet()/InvalidWalletException implementation and max-cosigner constants.
Changed components
Wallet loading flow (AppController.openWallet)Terminal wallet loading flow (LoadWallet.openWallet)Wallet settings UI (SettingsController)Drongo wallet validation library (submodule)Inspect captured patch +29 / −8
### drongo
@@ -1 +1 @@
-Subproject commit 203cfc191813a23b2b441b467375f3ce9c9272ca
+Subproject commit a47c2b3f58d7cedd504b2bd07833708866614216
### src/main/java/com/sparrowwallet/sparrow/AppController.java
@@ -1246,8 +1246,10 @@ public void openWalletFile(File file, boolean forceSameWindow) {
private void openWallet(Storage storage, WalletAndKey walletAndKey, AppController appController, boolean forceSameWindow) {
try {
storage.restorePublicKeysFromSeed(walletAndKey.getWallet(), walletAndKey.getKey());
- if(!walletAndKey.getWallet().isValid()) {
- throw new IllegalStateException("Wallet file is not valid.");
+ try {
+ walletAndKey.getWallet().checkWallet();
+ } catch(InvalidWalletException e) {
+ throw new IllegalStateException("Wallet file is not valid: " + e.getMessage());
}
AppController walletAppController = appController.addWalletTabOrWindow(storage, walletAndKey.getWallet(), forceSameWindow);
for(Map.Entry<WalletAndKey, Storage> entry : walletAndKey.getChildWallets().entrySet()) {
### src/main/java/com/sparrowwallet/sparrow/terminal/wallet/LoadWallet.java
@@ -1,10 +1,10 @@
package com.sparrowwallet.sparrow.terminal.wallet;
-import com.googlecode.lanterna.TerminalSize;
import com.googlecode.lanterna.gui2.*;
import com.googlecode.lanterna.gui2.dialogs.*;
import com.sparrowwallet.drongo.SecureString;
import com.sparrowwallet.drongo.crypto.InvalidPasswordException;
+import com.sparrowwallet.drongo.wallet.InvalidWalletException;
import com.sparrowwallet.drongo.wallet.Wallet;
import com.sparrowwallet.sparrow.EventManager;
import com.sparrowwallet.sparrow.SparrowWallet;
@@ -106,8 +106,10 @@ public void run() {
private void openWallet(Storage storage, WalletAndKey walletAndKey) {
try {
storage.restorePublicKeysFromSeed(walletAndKey.getWallet(), walletAndKey.getKey());
- if(!walletAndKey.getWallet().isValid()) {
- throw new IllegalStateException("Wallet file is not valid.");
+ try {
+ walletAndKey.getWallet().checkWallet();
+ } catch(InvalidWalletException e) {
+ throw new IllegalStateException("Wallet file is not valid: " + e.getMessage());
}
SparrowTerminal.addWallet(storage, walletAndKey.getWallet());
for(Map.Entry<WalletAndKey, Storage> entry : walletAndKey.getChildWallets().entrySet()) {
### src/main/java/com/sparrowwallet/sparrow/wallet/SettingsController.java
@@ -177,6 +177,14 @@ public ScriptType fromString(String string) {
}
walletForm.getWallet().setScriptType(newValue);
+
+ if(oldValue != null && !replacing && !reverting) {
+ if(multisigControl.getMax() > newValue.getMaxCosigners()) {
+ multisigControl.setMax(newValue.getMaxCosigners());
+ } else if(multisigControl.getMax() == multisigControl.getHighValue() && multisigControl.getMax() < newValue.getMaxCosigners()) {
+ multisigControl.setMax(multisigControl.getMax() + 1.0);
+ }
+ }
}
EventManager.get().post(new SettingsChangedEvent(walletForm.getWallet(), SettingsChangedEvent.Type.SCRIPT_TYPE));
@@ -189,7 +197,9 @@ public ScriptType fromString(String string) {
EventManager.get().post(new SettingsChangedEvent(walletForm.getWallet(), SettingsChangedEvent.Type.MUTLISIG_THRESHOLD));
});
multisigControl.highValueProperty().addListener((observable, oldValue, newValue) -> {
- if(newValue.doubleValue() == multisigControl.getMax() && newValue.doubleValue() <= 19.0) {
+ ScriptType walletScriptType = walletForm.getWallet().getScriptType();
+ int maxCosigners = walletScriptType == null ? PolicyType.MULTI_HD.getDefaultScriptType().getMaxCosigners() : walletScriptType.getMaxCosigners();
+ if(newValue.doubleValue() == multisigControl.getMax() && newValue.doubleValue() < maxCosigners) {
multisigControl.setMax(newValue.doubleValue() + 1.0);
}
});
@@ -296,7 +306,8 @@ private void setFieldsFromWallet(Wallet wallet) {
if(wallet.getPolicyType().equals(PolicyType.SINGLE_HD) || wallet.getPolicyType().equals(PolicyType.SINGLE_SP)) {
totalKeystores.setValue(1);
} else if(wallet.getPolicyType().equals(PolicyType.MULTI_HD)) {
- multisigControl.setMax(Math.max(multisigControl.getMax(), wallet.getKeystores().size()));
+ int maxCosigners = wallet.getScriptType() == null ? PolicyType.MULTI_HD.getDefaultScriptType().getMaxCosigners() : wallet.getScriptType().getMaxCosigners();
+ multisigControl.setMax(Math.max(Math.min(maxCosigners, multisigControl.getMax()), wallet.getKeystores().size()));
multisigControl.highValueProperty().set(wallet.getKeystores().size());
multisigControl.lowValueProperty().set(wallet.getDefaultPolicy().getNumSignaturesRequired());
totalKeystores.bind(multisigControl.highValueProperty());
@@ -520,6 +531,12 @@ private void rederiveAndReplaceWallet(Wallet editedWallet) {
return;
}
+ if(editedWallet.getScriptType() != null && editedWallet.getKeystores().size() > editedWallet.getScriptType().getMaxCosigners()) {
+ AppServices.showErrorDialog("Too Many Cosigners", "The provided output descriptor has " + editedWallet.getKeystores().size() + " cosigners, but " +
+ editedWallet.getScriptType().getName() + " supports a maximum of " + editedWallet.getScriptType().getMaxCosigners() + ".");
+ return;
+ }
+
if(AppServices.disallowAnyInvalidDerivationPaths(editedWallet)) {
return;
}Why this scored 27/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.