improve implementation of adding dns payment information from psbt
What the developer wrote
Authored by Craig Raw
improve implementation of adding dns payment information from psbt
What changed, and why it matters
This commit refines how Sparrow Wallet stores DNS payment details extracted from a Bitcoin PSBT (a transaction template). The old code tried to cache DNS payment info for both regular addresses and silent payment addresses in one combined flow, using helper methods like hasAddress() and hasSilentPaymentAddress(). The new code separates the two cases and, importantly, validates that the DNS payment record actually matches the address/silent payment address before caching it. This looks like a hardening change: it reduces the chance that a malicious or malformed PSBT could trick the wallet into caching a DNS payment entry for an unrelated address.
Review the implementation of DnsPayment.bitcoinURI().getAddress() and getSilentPaymentAddress() to confirm they are robust and cannot be spoofed by a crafted PSBT. Consider whether the DNSSEC proof itself is validated before this code runs, since the check only gates on the proof being non-empty. No immediate user action is indicated beyond applying the update.
Security signals we found
Added explicit address-binding validation before caching DNS payment data
Removed silent-payment-address inference from open wallets as the primary path; now uses PSBT-provided silent payment address first
Reduced code complexity by separating regular address and silent payment address handling
Potential fix for caching DNS payment info for an address that does not actually match the PSBT output
Evidence from the diff
The change is in AppController.java’s logic for adding DNS payment information from a PSBT. Previously, for a PSBTOutput with a DNSSEC proof, it derived an address, looked for a matching silent payment address among open wallets, and cached the DNS payment if either a silent payment address or a regular address matched, using helper predicates hasSilentPaymentAddress()/hasAddress(). The new implementation: (1) handles regular addresses and silent payment addresses as separate branches; (2) uses psbtOutput.getSilentPaymentAddress() first, falling back to deriving it from open wallets; (3) validates the DNS payment by comparing address.equals(optDnsPayment.get().bitcoinURI().getAddress()) or silentPaymentAddress.equals(optDnsPayment.get().bitcoinURI().getSilentPaymentAddress()) before caching. This adds explicit address-binding validation and removes reliance on helper methods whose exact semantics are not visible in the diff.
Changed components
src/main/java/com/sparrowwallet/sparrow/AppController.javaDNS payment caching logicPSBT output parsingSilent payment address handlingInspect captured patch +25 / −26
diff --git a/src/main/java/com/sparrowwallet/sparrow/AppController.java b/src/main/java/com/sparrowwallet/sparrow/AppController.java
index dc736c7..ceb9d4a 100644
--- a/src/main/java/com/sparrowwallet/sparrow/AppController.java
+++ b/src/main/java/com/sparrowwallet/sparrow/AppController.java
@@ -1924,34 +1924,33 @@ public class AppController implements Initializable {
//Add DNS payment information if not already cached
for(PSBTOutput psbtOutput : psbt.getPsbtOutputs()) {
- if(psbtOutput.getDnssecProof() != null && !psbtOutput.getDnssecProof().isEmpty() && psbtOutput.getScript() != null) {
- Address address = psbtOutput.getScript().getToAddress();
- if(address != null) {
- Optional<SilentPaymentAddress> optSilentPaymentAddress = AppServices.get().getOpenWallets().keySet().stream()
- .map(wallet -> wallet.getSilentPaymentAddress(address)).filter(Objects::nonNull).findFirst();
- optSilentPaymentAddress.ifPresentOrElse(silentPaymentAddress -> {
- if(DnsPaymentCache.getDnsPayment(silentPaymentAddress) == null) {
- try {
- Optional<DnsPayment> optDnsPayment = psbtOutput.getDnsPayment();
- if(optDnsPayment.isPresent() && optDnsPayment.get().hasSilentPaymentAddress()) {
- DnsPaymentCache.putDnsPayment(silentPaymentAddress, optDnsPayment.get());
- }
- } catch(Exception e) {
- log.debug("Error resolving DNS payment", e);
- }
+ if(psbtOutput.getDnssecProof() != null && !psbtOutput.getDnssecProof().isEmpty()) {
+ Address address = psbtOutput.getScript() != null ? psbtOutput.getScript().getToAddress() : null;
+ if(address != null && DnsPaymentCache.getDnsPayment(address) == null) {
+ try {
+ Optional<DnsPayment> optDnsPayment = psbtOutput.getDnsPayment();
+ if(optDnsPayment.isPresent() && address.equals(optDnsPayment.get().bitcoinURI().getAddress())) {
+ DnsPaymentCache.putDnsPayment(address, optDnsPayment.get());
}
- }, () -> {
- if(DnsPaymentCache.getDnsPayment(address) == null) {
- try {
- Optional<DnsPayment> optDnsPayment = psbtOutput.getDnsPayment();
- if(optDnsPayment.isPresent() && optDnsPayment.get().hasAddress()) {
- DnsPaymentCache.putDnsPayment(address, optDnsPayment.get());
- }
- } catch(Exception e) {
- log.debug("Error resolving DNS payment", e);
- }
+ } catch(Exception e) {
+ log.debug("Error resolving DNS payment", e);
+ }
+ }
+
+ SilentPaymentAddress silentPaymentAddress = psbtOutput.getSilentPaymentAddress();
+ if(address != null && silentPaymentAddress == null) {
+ silentPaymentAddress = AppServices.get().getOpenWallets().keySet().stream()
+ .map(wallet -> wallet.getSilentPaymentAddress(address)).filter(Objects::nonNull).findFirst().orElse(null);
+ }
+ if(silentPaymentAddress != null && DnsPaymentCache.getDnsPayment(silentPaymentAddress) == null) {
+ try {
+ Optional<DnsPayment> optDnsPayment = psbtOutput.getDnsPayment();
+ if(optDnsPayment.isPresent() && silentPaymentAddress.equals(optDnsPayment.get().bitcoinURI().getSilentPaymentAddress())) {
+ DnsPaymentCache.putDnsPayment(silentPaymentAddress, optDnsPayment.get());
}
- });
+ } catch(Exception e) {
+ log.debug("Error resolving DNS payment", e);
+ }
}
}
}
Why this scored 28/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.