mirrors/bisq
1
0
Fork 0
mirror of https://github.com/bisq-network/bisq.git synced 2024-11-19 09:52:23 +01:00
Code Issues Packages Projects Releases Wiki Activity

Fix issue with tx connected output being null

Browse Source 
The moment we are validating the tx is not committed and therefore the
connected tx output is not wired up and thus null.

Refactor and consolidate the validation code to the static class and
improve error handling.
This commit is contained in:
chimp1984 2020-04-08 12:10:06 -05:00 committed by Christoph Atteneder
parent 51ffd9d95c
commit 005a87b553
No known key found for this signature in database
GPG Key ID: CD5DC1C529CDFD3B
9 changed files with 216 additions and 204 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java
View File

@ -720,40 +720,6 @@ public class TradeWalletService {
return delayedPayoutTx;
}
public boolean verifiesDepositTxAndDelayedPayoutTx(Transaction depositTx,
Transaction delayedPayoutTx,
long lockTime) {
TransactionOutput p2SHMultiSigOutput = depositTx.getOutput(0);
if (delayedPayoutTx.getInputs().size() != 1) {
log.error("Number of inputs must be 1");
return false;
}
TransactionOutput connectedOutput = delayedPayoutTx.getInput(0).getConnectedOutput();
if (connectedOutput == null) {
log.error("connectedOutput must not be null");
return false;
}
if (!connectedOutput.equals(p2SHMultiSigOutput)) {
log.error("connectedOutput must be p2SHMultiSigOutput");
return false;
}
if (delayedPayoutTx.getLockTime() != lockTime) {
log.error("LockTime must match trades lockTime");
return false;
}
if (delayedPayoutTx.getInputs().stream().noneMatch(e -> e.getSequenceNumber() == TransactionInput.NO_SEQUENCE - 1)) {
log.error("Sequence number must be 0xFFFFFFFE");
return false;
}
return true;
}
///////////////////////////////////////////////////////////////////////////////////////////
// Standard payout tx

168
core/src/main/java/bisq/core/trade/DelayedPayoutTxValidation.java Normal file
View File

@ -0,0 +1,168 @@
/*
* This file is part of Bisq.
*
* Bisq is free software: you can redistribute it and/or modify it
* under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or (at
* your option) any later version.
*
* Bisq is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public
* License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with Bisq. If not, see <http://www.gnu.org/licenses/>.
*/
package bisq.core.trade;
import bisq.core.btc.wallet.BtcWalletService;
import bisq.core.dao.DaoFacade;
import bisq.core.dao.governance.param.Param;
import bisq.core.offer.Offer;
import org.bitcoinj.core.Address;
import org.bitcoinj.core.Coin;
import org.bitcoinj.core.NetworkParameters;
import org.bitcoinj.core.Transaction;
import org.bitcoinj.core.TransactionInput;
import org.bitcoinj.core.TransactionOutput;
import lombok.extern.slf4j.Slf4j;
import static com.google.common.base.Preconditions.checkNotNull;
@Slf4j
public class DelayedPayoutTxValidation {
/* @Getter
public static class DonationAddressException extends Exception {
private final String addressAsString;
private final String recentDonationAddressString;
private final String defaultDonationAddressString;
DonationAddressException(String addressAsString,
String recentDonationAddressString,
String defaultDonationAddressString) {
this.addressAsString = addressAsString;
this.recentDonationAddressString = recentDonationAddressString;
this.defaultDonationAddressString = defaultDonationAddressString;
}
}*/
public static class DonationAddressException extends Exception {
DonationAddressException(String msg) {
super(msg);
}
}
public static class MissingDelayedPayoutTxException extends Exception {
MissingDelayedPayoutTxException(String msg) {
super(msg);
}
}
public static class InvalidTxException extends Exception {
InvalidTxException(String msg) {
super(msg);
}
}
public static class InvalidLockTimeException extends Exception {
InvalidLockTimeException(String msg) {
super(msg);
}
}
public static void validatePayoutTx(Trade trade,
Transaction delayedPayoutTx,
DaoFacade daoFacade,
BtcWalletService btcWalletService)
throws DonationAddressException, MissingDelayedPayoutTxException,
InvalidTxException, InvalidLockTimeException {
String errorMsg;
if (delayedPayoutTx == null) {
errorMsg = "DelayedPayoutTx must not be null";
log.error(errorMsg);
throw new MissingDelayedPayoutTxException("DelayedPayoutTx must not be null");
}
// Validate tx structure
if (delayedPayoutTx.getInputs().size() != 1) {
errorMsg = "Number of delayedPayoutTx inputs must be 1";
log.error(errorMsg);
throw new InvalidTxException(errorMsg);
}
if (delayedPayoutTx.getOutputs().size() != 1) {
errorMsg = "Number of delayedPayoutTx outputs must be 1";
log.error(errorMsg);
throw new InvalidTxException(errorMsg);
}
// connectedOutput is null and input.getValue() is null at that point as the tx is not committed to the wallet
// yet. So we cannot check that the input matches but we did the amount check earlier in the trade protocol.
// Validate lock time
if (delayedPayoutTx.getLockTime() != trade.getLockTime()) {
errorMsg = "delayedPayoutTx.getLockTime() must match trade.getLockTime()";
log.error(errorMsg);
throw new InvalidLockTimeException(errorMsg);
}
// Validate seq num
if (delayedPayoutTx.getInput(0).getSequenceNumber() == TransactionInput.NO_SEQUENCE - 1) {
errorMsg = "Sequence number must be 0xFFFFFFFE";
log.error(errorMsg);
throw new InvalidLockTimeException(errorMsg);
}
// Check amount
TransactionOutput output = delayedPayoutTx.getOutput(0);
Offer offer = checkNotNull(trade.getOffer());
Coin msOutputAmount = offer.getBuyerSecurityDeposit()
.add(offer.getSellerSecurityDeposit())
.add(checkNotNull(trade.getTradeAmount()));
if (!output.getValue().equals(msOutputAmount)) {
errorMsg = "Output value of deposit tx and delayed payout tx is not matching. Output: " + output + " / msOutputAmount: " + msOutputAmount;
log.error(errorMsg);
throw new InvalidTxException(errorMsg);
}
// Validate donation address
// Get most recent donation address.
// We do not support past DAO param addresses to avoid that those receive funds (no bond set up anymore).
// Users who have not synced the DAO cannot trade.
String recentDonationAddressString = daoFacade.getParamValue(Param.RECIPIENT_BTC_ADDRESS);
// In case the seller has deactivated the DAO the default address will be used.
String defaultDonationAddressString = Param.RECIPIENT_BTC_ADDRESS.getDefaultValue();
NetworkParameters params = btcWalletService.getParams();
Address address = output.getAddressFromP2PKHScript(params);
if (address == null) {
// The donation address can be as well be a multisig address.
address = output.getAddressFromP2SH(params);
if (address == null) {
errorMsg = "Donation address cannot be resolved (not of type P2PKHScript or P2SH). Output: " + output;
log.error(errorMsg);
throw new DonationAddressException(errorMsg);
}
}
String addressAsString = address.toString();
if (!recentDonationAddressString.equals(addressAsString) &&
!defaultDonationAddressString.equals(addressAsString)) {
errorMsg = "Donation address is invalid." +
"\nAddress used by BTC seller: " + addressAsString +
"\nRecent donation address:" + recentDonationAddressString +
"\nDefault donation address: " + defaultDonationAddressString;
log.error(errorMsg);
throw new DonationAddressException(errorMsg);
}
}
}

98
core/src/main/java/bisq/core/trade/DonationAddressValidation.java
View File

@ -1,98 +0,0 @@
/*
* This file is part of Bisq.
*
* Bisq is free software: you can redistribute it and/or modify it
* under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or (at
* your option) any later version.
*
* Bisq is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public
* License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with Bisq. If not, see <http://www.gnu.org/licenses/>.
*/
package bisq.core.trade;
import bisq.core.btc.wallet.BtcWalletService;
import bisq.core.dao.DaoFacade;
import bisq.core.dao.governance.param.Param;
import org.bitcoinj.core.Address;
import org.bitcoinj.core.NetworkParameters;
import org.bitcoinj.core.Transaction;
import org.bitcoinj.core.TransactionOutput;
import lombok.Getter;
import lombok.extern.slf4j.Slf4j;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
@Slf4j
public class DonationAddressValidation {
@Getter
public static class DonationAddressException extends Exception {
private final String addressAsString;
private final String recentDonationAddressString;
private final String defaultDonationAddressString;
DonationAddressException(String addressAsString,
String recentDonationAddressString,
String defaultDonationAddressString) {
this.addressAsString = addressAsString;
this.recentDonationAddressString = recentDonationAddressString;
this.defaultDonationAddressString = defaultDonationAddressString;
}
}
public static class MissingDelayedPayoutTxException extends Exception {
public MissingDelayedPayoutTxException(String msg) {
super(msg);
}
}
public static void validate(Transaction delayedPayoutTx,
DaoFacade daoFacade,
BtcWalletService btcWalletService) throws DonationAddressException, MissingDelayedPayoutTxException {
if (delayedPayoutTx == null) {
throw new MissingDelayedPayoutTxException("DelayedPayoutTx must not be null");
}
// Get most recent donation address.
// We do not support past DAO param addresses to avoid that those receive funds (no bond set up anymore).
// Users who have not synced the DAO cannot trade.
String recentDonationAddressString = daoFacade.getParamValue(Param.RECIPIENT_BTC_ADDRESS);
// In case the seller has deactivated the DAO the default address will be used.
String defaultDonationAddressString = Param.RECIPIENT_BTC_ADDRESS.getDefaultValue();
checkArgument(delayedPayoutTx.getOutputs().size() == 1,
"preparedDelayedPayoutTx must have 1 output");
TransactionOutput output = delayedPayoutTx.getOutput(0);
NetworkParameters params = btcWalletService.getParams();
Address address = output.getAddressFromP2PKHScript(params);
if (address == null) {
// The donation address can be as well be a multisig address.
address = output.getAddressFromP2SH(params);
checkNotNull(address, "address must not be null");
}
String addressAsString = address.toString();
boolean isValid = recentDonationAddressString.equals(addressAsString) ||
defaultDonationAddressString.equals(addressAsString);
if (!isValid) {
log.warn("Donation address is invalid." +
"\nAddress used by BTC seller: " + addressAsString +
"\nRecent donation address:" + recentDonationAddressString +
"\nDefault donation address: " + defaultDonationAddressString);
throw new DonationAddressException(addressAsString, recentDonationAddressString, defaultDonationAddressString);
}
}
}

11
core/src/main/java/bisq/core/trade/TradeManager.java
View File

@ -299,12 +299,17 @@ public class TradeManager implements PersistedDataHost {
}
try {
DonationAddressValidation.validate(trade.getDelayedPayoutTx(),
DelayedPayoutTxValidation.validatePayoutTx(trade,
trade.getDelayedPayoutTx(),
daoFacade,
btcWalletService);
} catch (DonationAddressValidation.DonationAddressException |
DonationAddressValidation.MissingDelayedPayoutTxException e) {
} catch (DelayedPayoutTxValidation.DonationAddressException |
DelayedPayoutTxValidation.InvalidTxException |
DelayedPayoutTxValidation.InvalidLockTimeException |
DelayedPayoutTxValidation.MissingDelayedPayoutTxException e) {
// We move it to failed trades so it cannot be continued.
log.warn("We move the trade with ID '{}' to failed trades because of exception {}",
trade.getId(), e.getMessage());
addTradeToFailedTradesList.add(trade);
}
}

23
core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerVerifiesFinalDelayedPayoutTx.java
View File

@ -17,6 +17,7 @@
package bisq.core.trade.protocol.tasks.buyer;
import bisq.core.trade.DelayedPayoutTxValidation;
import bisq.core.trade.Trade;
import bisq.core.trade.protocol.tasks.TradeTask;
@ -26,8 +27,6 @@ import org.bitcoinj.core.Transaction;
import lombok.extern.slf4j.Slf4j;
import static com.google.common.base.Preconditions.checkNotNull;
@Slf4j
public class BuyerVerifiesFinalDelayedPayoutTx extends TradeTask {
@SuppressWarnings({"unused"})
@ -40,15 +39,19 @@ public class BuyerVerifiesFinalDelayedPayoutTx extends TradeTask {
try {
runInterceptHook();
Transaction depositTx = checkNotNull(trade.getDepositTx());
Transaction delayedPayoutTx = checkNotNull(trade.getDelayedPayoutTx());
if (processModel.getTradeWalletService().verifiesDepositTxAndDelayedPayoutTx(depositTx,
Transaction delayedPayoutTx = trade.getDelayedPayoutTx();
// Check again tx
DelayedPayoutTxValidation.validatePayoutTx(trade,
delayedPayoutTx,
trade.getLockTime())) {
complete();
} else {
failed("DelayedPayoutTx is not spending depositTx correctly");
}
processModel.getDaoFacade(),
processModel.getBtcWalletService());
complete();
} catch (DelayedPayoutTxValidation.DonationAddressException |
DelayedPayoutTxValidation.MissingDelayedPayoutTxException |
DelayedPayoutTxValidation.InvalidTxException |
DelayedPayoutTxValidation.InvalidLockTimeException e) {
failed(e.getMessage());
} catch (Throwable t) {
failed(t);
}

42
core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerVerifiesPreparedDelayedPayoutTx.java
View File

@ -17,22 +17,14 @@
package bisq.core.trade.protocol.tasks.buyer;
import bisq.core.offer.Offer;
import bisq.core.trade.DonationAddressValidation;
import bisq.core.trade.DelayedPayoutTxValidation;
import bisq.core.trade.Trade;
import bisq.core.trade.protocol.tasks.TradeTask;
import bisq.common.taskrunner.TaskRunner;
import org.bitcoinj.core.Coin;
import org.bitcoinj.core.Transaction;
import org.bitcoinj.core.TransactionInput;
import lombok.extern.slf4j.Slf4j;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
@Slf4j
public class BuyerVerifiesPreparedDelayedPayoutTx extends TradeTask {
@SuppressWarnings({"unused"})
@ -45,36 +37,16 @@ public class BuyerVerifiesPreparedDelayedPayoutTx extends TradeTask {
try {
runInterceptHook();
Transaction preparedDelayedPayoutTx = processModel.getPreparedDelayedPayoutTx();
// Check donation address
DonationAddressValidation.validate(preparedDelayedPayoutTx,
DelayedPayoutTxValidation.validatePayoutTx(trade,
processModel.getPreparedDelayedPayoutTx(),
processModel.getDaoFacade(),
processModel.getBtcWalletService());
// Check amount
Offer offer = checkNotNull(trade.getOffer());
Coin msOutputAmount = offer.getBuyerSecurityDeposit()
.add(offer.getSellerSecurityDeposit())
.add(checkNotNull(trade.getTradeAmount()));
checkArgument(preparedDelayedPayoutTx.getOutput(0).getValue().equals(msOutputAmount),
"output value of deposit tx and delayed payout tx must match");
// Check lock time
checkArgument(preparedDelayedPayoutTx.getLockTime() == trade.getLockTime(),
"preparedDelayedPayoutTx lock time must match trade.getLockTime()");
// Check seq num
checkArgument(preparedDelayedPayoutTx.getInputs().stream().anyMatch(e -> e.getSequenceNumber() == TransactionInput.NO_SEQUENCE - 1),
"Sequence number must be 0xFFFFFFFE");
complete();
} catch (DonationAddressValidation.DonationAddressException e) {
failed("Sellers donation address is invalid." +
"\nAddress used by BTC seller: " + e.getAddressAsString() +
"\nRecent donation address:" + e.getRecentDonationAddressString() +
"\nDefault donation address: " + e.getDefaultDonationAddressString());
} catch (DonationAddressValidation.MissingDelayedPayoutTxException e) {
} catch (DelayedPayoutTxValidation.DonationAddressException |
DelayedPayoutTxValidation.MissingDelayedPayoutTxException |
DelayedPayoutTxValidation.InvalidTxException |
DelayedPayoutTxValidation.InvalidLockTimeException e) {
failed(e.getMessage());
} catch (Throwable t) {
failed(t);

6
core/src/main/resources/i18n/displayStrings.properties
View File

@ -554,12 +554,10 @@ portfolio.tab.history=History
portfolio.tab.failed=Failed
portfolio.tab.editOpenOffer=Edit offer
portfolio.pending.invalidDonationAddress=The donation address in the delayed payout transaction is invalid.\n\n\
portfolio.pending.invalidDelayedPayoutTx=The donation address in the delayed payout transaction is invalid.\n\n\
Please do NOT send the Altcoin or Fiat payment but contact the Bisq developers at 'https://bisq.community' or \
the Keybase channel.\n\n\
Address used by BTC seller: {0}\n\
Recent donation address: {1}\n\
Default donation address: {2};
{0}
portfolio.pending.step1.waitForConf=Wait for blockchain confirmation
portfolio.pending.step2_buyer.startPayment=Start payment

19
desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep1View.java
View File

@ -22,7 +22,7 @@ import bisq.desktop.main.portfolio.pendingtrades.PendingTradesViewModel;
import bisq.desktop.main.portfolio.pendingtrades.steps.TradeStepView;
import bisq.core.locale.Res;
import bisq.core.trade.DonationAddressValidation;
import bisq.core.trade.DelayedPayoutTxValidation;
public class BuyerStep1View extends TradeStepView {
@ -39,17 +39,16 @@ public class BuyerStep1View extends TradeStepView {
super.activate();
try {
DonationAddressValidation.validate(trade.getDelayedPayoutTx(),
DelayedPayoutTxValidation.validatePayoutTx(trade,
trade.getDelayedPayoutTx(),
model.dataModel.daoFacade,
model.dataModel.btcWalletService);
} catch (DonationAddressValidation.DonationAddressException e) {
new Popup().warning(Res.get("portfolio.pending.invalidDonationAddress",
e.getAddressAsString(),
e.getRecentDonationAddressString(),
e.getDefaultDonationAddressString()))
.show();
} catch (DonationAddressValidation.MissingDelayedPayoutTxException e) {
// We don't react on that error as a failed trade might get listed initially but getting removed from the
} catch (DelayedPayoutTxValidation.DonationAddressException |
DelayedPayoutTxValidation.InvalidTxException |
DelayedPayoutTxValidation.InvalidLockTimeException e) {
new Popup().warning(Res.get("portfolio.pending.invalidDelayedPayoutTx", e.getMessage())).show();
} catch (DelayedPayoutTxValidation.MissingDelayedPayoutTxException ignore) {
// We don't react on those errors as a failed trade might get listed initially but getting removed from the
// trade manager after initPendingTrades which happens after activate might be called.
}
}

19
desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep2View.java
View File

@ -69,7 +69,7 @@ import bisq.core.payment.payload.PaymentAccountPayload;
import bisq.core.payment.payload.PaymentMethod;
import bisq.core.payment.payload.USPostalMoneyOrderAccountPayload;
import bisq.core.payment.payload.WesternUnionAccountPayload;
import bisq.core.trade.DonationAddressValidation;
import bisq.core.trade.DelayedPayoutTxValidation;
import bisq.core.trade.Trade;
import bisq.core.user.DontShowAgainLookup;
@ -116,17 +116,16 @@ public class BuyerStep2View extends TradeStepView {
super.activate();
try {
DonationAddressValidation.validate(trade.getDelayedPayoutTx(),
DelayedPayoutTxValidation.validatePayoutTx(trade,
trade.getDelayedPayoutTx(),
model.dataModel.daoFacade,
model.dataModel.btcWalletService);
} catch (DonationAddressValidation.DonationAddressException e) {
new Popup().warning(Res.get("portfolio.pending.invalidDonationAddress",
e.getAddressAsString(),
e.getRecentDonationAddressString(),
e.getDefaultDonationAddressString()))
.show();
} catch (DonationAddressValidation.MissingDelayedPayoutTxException ignore) {
// We don't react on that error as a failed trade might get listed initially but getting removed from the
} catch (DelayedPayoutTxValidation.DonationAddressException |
DelayedPayoutTxValidation.InvalidTxException |
DelayedPayoutTxValidation.InvalidLockTimeException e) {
new Popup().warning(Res.get("portfolio.pending.invalidDelayedPayoutTx", e.getMessage())).show();
} catch (DelayedPayoutTxValidation.MissingDelayedPayoutTxException ignore) {
// We don't react on those errors as a failed trade might get listed initially but getting removed from the
// trade manager after initPendingTrades which happens after activate might be called.
}