From bcf6f77340178108fdc8704cd3649e2b06e547e4 Mon Sep 17 00:00:00 2001 From: Craig Raw Date: Wed, 4 Nov 2020 12:10:40 +0200 Subject: [PATCH] improve wallet import error messaging, handle p2sh-p2wpkh payjoin --- drongo | 2 +- .../sparrow/io/CoboVaultSinglesig.java | 11 +++---- .../sparrow/io/ColdcardSinglesig.java | 11 +++---- .../sparrowwallet/sparrow/io/Electrum.java | 6 ++-- .../com/sparrowwallet/sparrow/io/Specter.java | 7 ++-- .../sparrow/payjoin/Payjoin.java | 32 +++++++++++++++---- 6 files changed, 45 insertions(+), 24 deletions(-) diff --git a/drongo b/drongo index 9c983614..3433c5f2 160000 --- a/drongo +++ b/drongo @@ -1 +1 @@ -Subproject commit 9c9836147ab77b28fed9b6bdc8eb1e14fd1e1217 +Subproject commit 3433c5f20524416f1d4225d7961f40b4300c4006 diff --git a/src/main/java/com/sparrowwallet/sparrow/io/CoboVaultSinglesig.java b/src/main/java/com/sparrowwallet/sparrow/io/CoboVaultSinglesig.java index f569c955..2f607573 100644 --- a/src/main/java/com/sparrowwallet/sparrow/io/CoboVaultSinglesig.java +++ b/src/main/java/com/sparrowwallet/sparrow/io/CoboVaultSinglesig.java @@ -6,10 +6,7 @@ import com.sparrowwallet.drongo.KeyDerivation; import com.sparrowwallet.drongo.policy.Policy; import com.sparrowwallet.drongo.policy.PolicyType; import com.sparrowwallet.drongo.protocol.ScriptType; -import com.sparrowwallet.drongo.wallet.Keystore; -import com.sparrowwallet.drongo.wallet.KeystoreSource; -import com.sparrowwallet.drongo.wallet.Wallet; -import com.sparrowwallet.drongo.wallet.WalletModel; +import com.sparrowwallet.drongo.wallet.*; import java.io.File; import java.io.InputStream; @@ -75,8 +72,10 @@ public class CoboVaultSinglesig implements KeystoreFileImport, WalletImport { wallet.getKeystores().add(keystore); wallet.setDefaultPolicy(Policy.getPolicy(PolicyType.SINGLE, ScriptType.P2WPKH, wallet.getKeystores(), null)); - if(!wallet.isValid()) { - throw new ImportException("Wallet is in an inconsistent state."); + try { + wallet.checkWallet(); + } catch(InvalidWalletException e) { + throw new ImportException("Imported Cobo Vault wallet was invalid: " + e.getMessage()); } return wallet; diff --git a/src/main/java/com/sparrowwallet/sparrow/io/ColdcardSinglesig.java b/src/main/java/com/sparrowwallet/sparrow/io/ColdcardSinglesig.java index acbdb34d..fd2fcdce 100644 --- a/src/main/java/com/sparrowwallet/sparrow/io/ColdcardSinglesig.java +++ b/src/main/java/com/sparrowwallet/sparrow/io/ColdcardSinglesig.java @@ -8,10 +8,7 @@ import com.sparrowwallet.drongo.KeyDerivation; import com.sparrowwallet.drongo.policy.Policy; import com.sparrowwallet.drongo.policy.PolicyType; import com.sparrowwallet.drongo.protocol.ScriptType; -import com.sparrowwallet.drongo.wallet.Keystore; -import com.sparrowwallet.drongo.wallet.KeystoreSource; -import com.sparrowwallet.drongo.wallet.Wallet; -import com.sparrowwallet.drongo.wallet.WalletModel; +import com.sparrowwallet.drongo.wallet.*; import java.io.File; import java.io.InputStream; @@ -101,8 +98,10 @@ public class ColdcardSinglesig implements KeystoreFileImport, WalletImport { wallet.getKeystores().add(keystore); wallet.setDefaultPolicy(Policy.getPolicy(PolicyType.SINGLE, ScriptType.P2WPKH, wallet.getKeystores(), null)); - if(!wallet.isValid()) { - throw new ImportException("Wallet is in an inconsistent state."); + try { + wallet.checkWallet(); + } catch(InvalidWalletException e) { + throw new ImportException("Imported Coldcard wallet was invalid: " + e.getMessage()); } return wallet; diff --git a/src/main/java/com/sparrowwallet/sparrow/io/Electrum.java b/src/main/java/com/sparrowwallet/sparrow/io/Electrum.java index 935f936b..4b5f92b2 100644 --- a/src/main/java/com/sparrowwallet/sparrow/io/Electrum.java +++ b/src/main/java/com/sparrowwallet/sparrow/io/Electrum.java @@ -201,8 +201,10 @@ public class Electrum implements KeystoreFileImport, WalletImport, WalletExport wallet.updateTransactions(ew.transactions); - if(!wallet.isValid()) { - throw new IllegalStateException("Electrum wallet is in an inconsistent state."); + try { + wallet.checkWallet(); + } catch(InvalidWalletException e) { + throw new IllegalStateException("Imported Electrum wallet was invalid: " + e.getMessage()); } return wallet; diff --git a/src/main/java/com/sparrowwallet/sparrow/io/Specter.java b/src/main/java/com/sparrowwallet/sparrow/io/Specter.java index c9fb4dfe..7afe04b2 100644 --- a/src/main/java/com/sparrowwallet/sparrow/io/Specter.java +++ b/src/main/java/com/sparrowwallet/sparrow/io/Specter.java @@ -3,6 +3,7 @@ package com.sparrowwallet.sparrow.io; import com.google.gson.Gson; import com.google.gson.GsonBuilder; import com.sparrowwallet.drongo.OutputDescriptor; +import com.sparrowwallet.drongo.wallet.InvalidWalletException; import com.sparrowwallet.drongo.wallet.Wallet; import com.sparrowwallet.drongo.wallet.WalletModel; @@ -57,8 +58,10 @@ public class Specter implements WalletImport, WalletExport { Wallet wallet = outputDescriptor.toWallet(); wallet.setName(specterWallet.label); - if(!wallet.isValid()) { - throw new ImportException("Specter wallet file did not contain a valid wallet"); + try { + wallet.checkWallet(); + } catch(InvalidWalletException e) { + throw new ImportException("Imported Specter wallet was invalid: " + e.getMessage()); } return wallet; diff --git a/src/main/java/com/sparrowwallet/sparrow/payjoin/Payjoin.java b/src/main/java/com/sparrowwallet/sparrow/payjoin/Payjoin.java index 8ca318d6..654640d8 100644 --- a/src/main/java/com/sparrowwallet/sparrow/payjoin/Payjoin.java +++ b/src/main/java/com/sparrowwallet/sparrow/payjoin/Payjoin.java @@ -36,6 +36,23 @@ public class Payjoin { this.payjoinURI = payjoinURI; this.wallet = wallet; this.psbt = psbt.getPublicCopy(); + + for(PSBTInput psbtInput : this.psbt.getPsbtInputs()) { + if(psbtInput.getUtxo() == null) { + throw new IllegalArgumentException("Original PSBT for payjoin transaction must have non_witness_utxo or witness_utxo fields for all inputs"); + } + if(!psbtInput.getDerivedPublicKeys().isEmpty()) { + throw new IllegalArgumentException("Original PSBT for payjoin transaction must have no derived public keys for all inputs"); + } + } + + if(!this.psbt.isFinalized()) { + throw new IllegalArgumentException("Original PSBT for payjoin transaction must be finalized"); + } + + if(!this.psbt.getExtendedPublicKeys().isEmpty()) { + throw new IllegalArgumentException("Original PSBT for payjoin transaction must have no global xpubs"); + } } public PSBT requestPayjoinPSBT(boolean allowOutputSubstitution) throws PayjoinReceiverException { @@ -57,7 +74,7 @@ public class Payjoin { long maxAdditionalFeeContribution = 0; if(changeOutputIndex > -1) { appendQuery += "&additionalfeeoutputindex=" + changeOutputIndex; - maxAdditionalFeeContribution = getAdditionalFeeContribution(psbt.getTransaction()); + maxAdditionalFeeContribution = getAdditionalFeeContribution(); appendQuery += "&maxadditionalfeecontribution=" + maxAdditionalFeeContribution; } @@ -158,8 +175,8 @@ public class Payjoin { proposedPSBTInput.setWitnessUtxo(originalPSBTInput.getWitnessUtxo()); // We fill up information we had on the signed PSBT, so we can sign it. proposedPSBTInput.getDerivedPublicKeys().putAll(originalPSBTInput.getDerivedPublicKeys()); - proposedPSBTInput.setRedeemScript(originalPSBTInput.getRedeemScript()); - proposedPSBTInput.setWitnessScript(originalPSBTInput.getWitnessScript()); + proposedPSBTInput.setRedeemScript(originalPSBTInput.getFinalScriptSig().getFirstNestedScript()); + proposedPSBTInput.setWitnessScript(originalPSBTInput.getFinalScriptWitness().getWitnessScript()); proposedPSBTInput.setSigHash(originalPSBTInput.getSigHash()); } else { // Verify the PSBT input is finalized @@ -220,7 +237,7 @@ public class Payjoin { } // Make sure the actual contribution is only paying for fee incurred by additional inputs int additionalInputsCount = proposalTx.getInputs().size() - originalTx.getInputs().size(); - if(actualContribution > getSingleInputFee(originalTx) * additionalInputsCount) { + if(actualContribution > getSingleInputFee() * additionalInputsCount) { throw new PayjoinReceiverException("The actual contribution is not only paying for additional inputs"); } } else if(allowOutputSubstitution && originalOutput.getKey().getScript().equals(payjoinURI.getAddress().getOutputScript())) { @@ -259,11 +276,12 @@ public class Payjoin { return -1; } - private long getAdditionalFeeContribution(Transaction transaction) { - return getSingleInputFee(transaction); + private long getAdditionalFeeContribution() { + return getSingleInputFee(); } - private long getSingleInputFee(Transaction transaction) { + private long getSingleInputFee() { + Transaction transaction = psbt.extractTransaction(); double feeRate = psbt.getFee().doubleValue() / transaction.getVirtualSize(); int vSize = 68;