← Watch feed
improve sp related output descriptor and psbt behaviour
Public commit record
50/100 · Thin
What the developer wrote
Authored by Craig Raw
improve sp related output descriptor and psbt behaviour
✓ Specific, descriptive subject✓ Names a concrete action or component! No meaningful explanatory body
What changed, and why it matters
This commit improves how Sparrow Wallet handles a newer Bitcoin address type called 'Silent Payments' (SP). It adds null-safety checks so the app doesn't crash when an extended public key is missing, supports exporting and importing labels tied to Silent Payment scan keys, and updates UI labels. Most changes are defensive or feature-related rather than fixes for active attacks.
Recommended action
Review the new Silent Payments descriptor parsing and label import paths for malformed input handling; ensure the 'spscan' label ref is validated before being used for keystore matching; verify that hardware wallet importer restrictions do not inadvertently allow incompatible policy types.
Security signals we found
01
Null-pointer dereference prevention added in BaseController, WalletLabels, KeystoreController, and SettingsController
02
New Silent Payments descriptor export format with additional arguments
03
New 'spscan' label type in WalletLabels import/export
04
PolicyType enum values renamed from SINGLE/MULTI to SINGLE_HD/MULTI_HD, with new SINGLE_SP branch handling
05
Objects.equals() used to safely compare nullable extended public keys
Evidence from the diff
The patch extends Silent Payments support across descriptor export, PSBT/QR handling, wallet label import/export, and keystore UI controllers. Key changes include: (1) null-guarding extended public keys before calling toString(), (2) writing Silent Payment scan addresses into output descriptors when no xpub exists, (3) adding a two-argument SP descriptor variant to exports, (4) adding an ‘spscan’ label type for BIP-329 style wallet labels, (5) replacing a direct xpub equality check with Objects.equals() to avoid NPE, and (6) restricting certain hardware/card importers to HD policy types. The diff is additive and defensive; no obvious vulnerability is introduced, but the new code paths increase the attack surface for descriptor parsing and label import.
Changed components
BaseControllerQRScanDialogTransactionDiagramDescriptor exportWalletLabels import/exportHwAirgappedControllerKeystoreControllerSettingsControllerSilent Payments (BIP-352) integrationInspect captured patch +46 / −14
diff --git a/src/main/java/com/sparrowwallet/sparrow/BaseController.java b/src/main/java/com/sparrowwallet/sparrow/BaseController.java
index 29e33e0..5e57259 100644
--- a/src/main/java/com/sparrowwallet/sparrow/BaseController.java
+++ b/src/main/java/com/sparrowwallet/sparrow/BaseController.java
@@ -75,7 +75,11 @@ public abstract class BaseController {
builder.append(keystore.getKeyDerivation().getMasterFingerprint());
builder.append(KeyDerivation.writePath(KeyDerivation.parsePath(keystore.getKeyDerivation().getDerivationPath())).substring(1));
builder.append("]");
- builder.append(keystore.getExtendedPublicKey().toString());
+ if(keystore.getExtendedPublicKey() != null) {
+ builder.append(keystore.getExtendedPublicKey().toString());
+ } else if(keystore.getSilentPaymentScanAddress() != null) {
+ builder.append(keystore.getSilentPaymentScanAddress().toKeyString());
+ }
return builder.toString();
}
diff --git a/src/main/java/com/sparrowwallet/sparrow/control/QRScanDialog.java b/src/main/java/com/sparrowwallet/sparrow/control/QRScanDialog.java
index 9e384f8..1198b47 100644
--- a/src/main/java/com/sparrowwallet/sparrow/control/QRScanDialog.java
+++ b/src/main/java/com/sparrowwallet/sparrow/control/QRScanDialog.java
@@ -691,7 +691,7 @@ public class QRScanDialog extends Dialog<QRScanDialog.Result> {
KeyDerivation keyDerivation = getKeyDerivation(urhdKey.getOrigin());
if(urhdKey.getChainCode() == null) {
ECKey ecKey = getKey(urhdKey);
- source = source.replaceAll("@" + i, OutputDescriptor.writeKey(ecKey, keyDerivation, true));
+ source = source.replaceAll("@" + i, OutputDescriptor.writeKey(ecKey, keyDerivation, true, true));
} else {
ExtendedKey extendedKey = getExtendedKey(urhdKey);
source = source.replaceAll("@" + i, OutputDescriptor.writeKey(extendedKey, keyDerivation, null, true, true));
diff --git a/src/main/java/com/sparrowwallet/sparrow/control/TransactionDiagram.java b/src/main/java/com/sparrowwallet/sparrow/control/TransactionDiagram.java
index af8a9a8..3bfe6d5 100644
--- a/src/main/java/com/sparrowwallet/sparrow/control/TransactionDiagram.java
+++ b/src/main/java/com/sparrowwallet/sparrow/control/TransactionDiagram.java
@@ -851,8 +851,7 @@ public class TransactionDiagram extends GridPane {
actionBox.setAlignment(Pos.CENTER_LEFT);
SilentPayment silentPayment = spChangeOutput.getSilentPayment();
SilentPaymentAddress spAddress = silentPayment.getSilentPaymentAddress();
- String changeDesc = spAddress.toString().substring(0, 8) + "...";
- Label changeLabel = new Label(changeDesc, getChangeGlyph());
+ Label changeLabel = new Label("Change", getChangeGlyph());
changeLabel.getStyleClass().addAll("output-label", "change-label");
changeLabel.setSkin(new AddressLabelSkin(changeLabel));
Tooltip changeTooltip = new Tooltip("Change of " + getCoinValue(silentPayment.getAmount()) + "\n" + spAddress);
diff --git a/src/main/java/com/sparrowwallet/sparrow/io/Descriptor.java b/src/main/java/com/sparrowwallet/sparrow/io/Descriptor.java
index 7ba96de..551e60c 100644
--- a/src/main/java/com/sparrowwallet/sparrow/io/Descriptor.java
+++ b/src/main/java/com/sparrowwallet/sparrow/io/Descriptor.java
@@ -31,8 +31,19 @@ public class Descriptor implements WalletImport, WalletExport {
BufferedWriter bufferedWriter = new BufferedWriter(new OutputStreamWriter(outputStream));
if(wallet.getPolicyType() == PolicyType.SINGLE_SP) {
OutputDescriptor outputDescriptor = OutputDescriptor.getOutputDescriptor(wallet);
+
+ bufferedWriter.write("# Single argument descriptor:");
+ bufferedWriter.newLine();
+
bufferedWriter.write(outputDescriptor.toString(true));
bufferedWriter.newLine();
+ bufferedWriter.newLine();
+
+ bufferedWriter.write("# Two argument descriptor:");
+ bufferedWriter.newLine();
+
+ bufferedWriter.write(outputDescriptor.toString(true, true, true, true));
+ bufferedWriter.newLine();
} else {
bufferedWriter.write("# Receive and change descriptor:");
bufferedWriter.newLine();
diff --git a/src/main/java/com/sparrowwallet/sparrow/io/WalletLabels.java b/src/main/java/com/sparrowwallet/sparrow/io/WalletLabels.java
index 1cb8050..6f89b21 100644
--- a/src/main/java/com/sparrowwallet/sparrow/io/WalletLabels.java
+++ b/src/main/java/com/sparrowwallet/sparrow/io/WalletLabels.java
@@ -6,6 +6,7 @@ import com.sparrowwallet.drongo.KeyDerivation;
import com.sparrowwallet.drongo.KeyPurpose;
import com.sparrowwallet.drongo.OutputDescriptor;
import com.sparrowwallet.drongo.Utils;
+import com.sparrowwallet.drongo.policy.PolicyType;
import com.sparrowwallet.drongo.protocol.*;
import com.sparrowwallet.drongo.wallet.*;
import com.sparrowwallet.sparrow.AppServices;
@@ -68,7 +69,11 @@ public class WalletLabels implements WalletImport, WalletExport {
for(Keystore keystore : exportWallet.getKeystores()) {
if(keystore.getLabel() != null && !keystore.getLabel().isEmpty()) {
- labels.add(new Label(Type.xpub, keystore.getExtendedPublicKey().toString(), keystore.getLabel(), null, null));
+ if(exportWallet.getPolicyType() == PolicyType.SINGLE_SP && keystore.getSilentPaymentScanAddress() != null) {
+ labels.add(new Label(Type.spscan, keystore.getSilentPaymentScanAddress().toKeyString(), keystore.getLabel(), null, null));
+ } else if(keystore.getExtendedPublicKey() != null) {
+ labels.add(new Label(Type.xpub, keystore.getExtendedPublicKey().toString(), keystore.getLabel(), null, null));
+ }
}
}
@@ -220,7 +225,17 @@ public class WalletLabels implements WalletImport, WalletExport {
if(label.type == Type.xpub) {
for(Keystore keystore : wallet.getKeystores()) {
- if(keystore.getExtendedPublicKey().toString().equals(label.ref)) {
+ if(keystore.getExtendedPublicKey() != null && keystore.getExtendedPublicKey().toString().equals(label.ref)) {
+ keystore.setLabel(label.label);
+ List<Keystore> changedKeystores = changedWalletKeystores.computeIfAbsent(wallet, w -> new ArrayList<>());
+ changedKeystores.add(keystore);
+ }
+ }
+ }
+
+ if(label.type == Type.spscan) {
+ for(Keystore keystore : wallet.getKeystores()) {
+ if(keystore.getSilentPaymentScanAddress() != null && keystore.getSilentPaymentScanAddress().toKeyString().equals(label.ref)) {
keystore.setLabel(label.label);
List<Keystore> changedKeystores = changedWalletKeystores.computeIfAbsent(wallet, w -> new ArrayList<>());
changedKeystores.add(keystore);
@@ -432,7 +447,7 @@ public class WalletLabels implements WalletImport, WalletExport {
}
private enum Type {
- tx, addr, pubkey, input, output, xpub
+ tx, addr, pubkey, input, output, xpub, spscan
}
private static class Label {
diff --git a/src/main/java/com/sparrowwallet/sparrow/keystoreimport/HwAirgappedController.java b/src/main/java/com/sparrowwallet/sparrow/keystoreimport/HwAirgappedController.java
index dfa69c4..114c6fb 100644
--- a/src/main/java/com/sparrowwallet/sparrow/keystoreimport/HwAirgappedController.java
+++ b/src/main/java/com/sparrowwallet/sparrow/keystoreimport/HwAirgappedController.java
@@ -26,9 +26,9 @@ public class HwAirgappedController extends KeystoreImportDetailController {
public void initializeView() {
List<KeystoreFileImport> fileImporters = Collections.emptyList();
- if(getMasterController().getWallet().getPolicyType().equals(PolicyType.SINGLE)) {
+ if(getMasterController().getWallet().getPolicyType().equals(PolicyType.SINGLE_HD)) {
fileImporters = List.of(new ColdcardSinglesig(), new CoboVaultSinglesig(), new Jade(), new KeystoneSinglesig(), new PassportSinglesig(), new SeedSigner(), new GordianSeedTool(), new SpecterDIY(), new Krux(), new AirGapVault(), new KeycardShellSinglesig());
- } else if(getMasterController().getWallet().getPolicyType().equals(PolicyType.MULTI)) {
+ } else if(getMasterController().getWallet().getPolicyType().equals(PolicyType.MULTI_HD)) {
fileImporters = List.of(new Bip129(), new ColdcardMultisig(), new CoboVaultMultisig(), new JadeMultisig(), new KeystoneMultisig(), new PassportMultisig(), new SeedSigner(), new GordianSeedTool(), new SpecterDIY(), new Krux(), new KeycardShellMultisig());
}
@@ -41,7 +41,10 @@ public class HwAirgappedController extends KeystoreImportDetailController {
}
}
- List<KeystoreCardImport> cardImporters = List.of(new Tapsigner(), new Satochip(), new Satschip(), new Keycard());
+ List<KeystoreCardImport> cardImporters = Collections.emptyList();
+ if(getMasterController().getWallet().getPolicyType().equals(PolicyType.SINGLE_HD) || getMasterController().getWallet().getPolicyType().equals(PolicyType.MULTI_HD)) {
+ cardImporters = List.of(new Tapsigner(), new Satochip(), new Satschip(), new Keycard());
+ }
for(KeystoreCardImport importer : cardImporters) {
if(!importer.isDeprecated() || Config.get().isShowDeprecatedImportExport()) {
CardImportPane importPane = new CardImportPane(getMasterController().getWallet(), importer, getMasterController().getDefaultDerivation(), getMasterController().getRequiredDerivation());
diff --git a/src/main/java/com/sparrowwallet/sparrow/wallet/KeystoreController.java b/src/main/java/com/sparrowwallet/sparrow/wallet/KeystoreController.java
index 5ac22c1..61eb295 100644
--- a/src/main/java/com/sparrowwallet/sparrow/wallet/KeystoreController.java
+++ b/src/main/java/com/sparrowwallet/sparrow/wallet/KeystoreController.java
@@ -760,8 +760,8 @@ public class KeystoreController extends WalletFormController implements Initiali
public void keystoreLabelsChanged(KeystoreLabelsChangedEvent event) {
if(event.getWalletId().equals(walletForm.getWalletId())) {
for(Keystore changedKeystore : event.getChangedKeystores()) {
- if(xpub.getText().trim().equals(changedKeystore.getExtendedPublicKey().toString()) && !label.getText().equals(changedKeystore.getLabel())
- || spScan.getText().trim().equals(changedKeystore.getSilentPaymentScanAddress().toKeyString()) && !label.getText().equals(changedKeystore.getLabel())) {
+ if((changedKeystore.getExtendedPublicKey() != null && xpub.getText().trim().equals(changedKeystore.getExtendedPublicKey().toString()) && !label.getText().equals(changedKeystore.getLabel()))
+ || (changedKeystore.getSilentPaymentScanAddress() != null && spScan.getText().trim().equals(changedKeystore.getSilentPaymentScanAddress().toKeyString()) && !label.getText().equals(changedKeystore.getLabel()))) {
label.textProperty().removeListener(labelChangeListener);
label.setText(changedKeystore.getLabel());
keystore.setLabel(changedKeystore.getLabel());
diff --git a/src/main/java/com/sparrowwallet/sparrow/wallet/SettingsController.java b/src/main/java/com/sparrowwallet/sparrow/wallet/SettingsController.java
index d3f4ba3..d52759c 100644
--- a/src/main/java/com/sparrowwallet/sparrow/wallet/SettingsController.java
+++ b/src/main/java/com/sparrowwallet/sparrow/wallet/SettingsController.java
@@ -924,7 +924,7 @@ public class SettingsController extends WalletFormController implements Initiali
List<Keystore> importedKeystores = event.getImportedWallet().getKeystores();
List<Keystore> nonWatchKeystores = walletForm.getWallet().getKeystores().stream().filter(k -> k.isValid() && k.getSource() != KeystoreSource.SW_WATCH).collect(Collectors.toList());
for(Keystore nonWatchKeystore : nonWatchKeystores) {
- Optional<Keystore> optReplacedKeystore = importedKeystores.stream().filter(k -> nonWatchKeystore.getExtendedPublicKey().equals(k.getExtendedPublicKey())).findFirst();
+ Optional<Keystore> optReplacedKeystore = importedKeystores.stream().filter(k -> Objects.equals(nonWatchKeystore.getExtendedPublicKey(), k.getExtendedPublicKey())).findFirst();
if(optReplacedKeystore.isPresent()) {
int index = importedKeystores.indexOf(optReplacedKeystore.get());
importedKeystores.remove(index);
Risk score
Our methodology →Why this scored 26/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.