From 1f850b600207b03108a74a0acb96542196b6172c Mon Sep 17 00:00:00 2001 From: sqrrm Date: Mon, 3 Sep 2018 17:25:42 +0200 Subject: [PATCH 1/6] Parse opreturn before other outputs Since the effects of opreturn act on the BSQ outputs it makes more sense to parse the opreturn before parsing the remaining outputs. Put the lock time in the LOCKUP txOutput. It makes more logical sense to keep it there. --- .../bisq/core/dao/node/parser/TxOutputParser.java | 8 +++++--- src/main/java/bisq/core/dao/node/parser/TxParser.java | 11 +++++++++-- src/main/java/bisq/core/dao/state/blockchain/Tx.java | 6 ++---- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java b/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java index 418b0d10..e9f1561b 100644 --- a/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java +++ b/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java @@ -47,6 +47,7 @@ public class TxOutputParser { @Getter @Setter private long availableInputValue = 0; + private int lockTime; @Setter private int unlockBlockHeight; @Setter @@ -81,8 +82,9 @@ public void processGenesisTxOutput(TempTx genesisTx) { } } - void processOpReturnCandidate(TempTxOutput txOutput) { + boolean processOpReturnCandidate(TempTxOutput txOutput) { optionalOpReturnTypeCandidate = OpReturnParser.getOptionalOpReturnTypeCandidate(txOutput); + return optionalOpReturnTypeCandidate.isPresent(); } /** @@ -164,6 +166,7 @@ private void handleBsqOutput(TempTxOutput txOutput, int index, long txOutputValu optionalVoteRevealUnlockStakeOutput = Optional.of(txOutput); } else if (isFirstOutput && opReturnTypeCandidate == OpReturnType.LOCKUP) { bsqOutput = TxOutputType.LOCKUP; + txOutput.setLockTime(lockTime); optionalLockupOutput = Optional.of(txOutput); } else { bsqOutput = TxOutputType.BSQ_OUTPUT; @@ -199,8 +202,7 @@ private void handleOpReturnOutput(TempTxOutput tempTxOutput, boolean isLastOutpu .ifPresent(verifiedOpReturnType -> { byte[] opReturnData = tempTxOutput.getOpReturnData(); checkNotNull(opReturnData, "opReturnData must not be null"); - int lockTime = BondingConsensus.getLockTime(opReturnData); - tempTxOutput.setLockTime(lockTime); + lockTime = BondingConsensus.getLockTime(opReturnData); }); } diff --git a/src/main/java/bisq/core/dao/node/parser/TxParser.java b/src/main/java/bisq/core/dao/node/parser/TxParser.java index d1c3fb1b..75ce8e1b 100644 --- a/src/main/java/bisq/core/dao/node/parser/TxParser.java +++ b/src/main/java/bisq/core/dao/node/parser/TxParser.java @@ -115,11 +115,15 @@ public Optional findTx(RawTx rawTx, String genesisTxId, int genesisBlockHeig // We keep the temporary opReturn type in the parsingModel object. checkArgument(!outputs.isEmpty(), "outputs must not be empty"); int lastIndex = outputs.size() - 1; - txOutputParser.processOpReturnCandidate(outputs.get(lastIndex)); + int lastNonOpReturnIndex = lastIndex; + if (txOutputParser.processOpReturnCandidate(outputs.get(lastIndex))) { + txOutputParser.processTxOutput(true, outputs.get(lastIndex), lastIndex); + lastNonOpReturnIndex -= 1; + } // We use order of output index. An output is a BSQ utxo as long there is enough input value // We iterate all outputs including the opReturn to do a full validation including the BSQ fee - for (int index = 0; index < outputs.size(); index++) { + for (int index = 0; index <= lastNonOpReturnIndex; index++) { boolean isLastOutput = index == lastIndex; txOutputParser.processTxOutput(isLastOutput, outputs.get(index), @@ -154,6 +158,9 @@ public Optional findTx(RawTx rawTx, String genesisTxId, int genesisBlockHeig DevEnv.logErrorAndThrowIfDevMode(msg); } } else { + // TODO(SQ): The transaction has already been parsed here and the individual txouputs are considered + // spendable or otherwise correct. Perhaps this check should be done earlier. + // We don't consider a tx with multiple OpReturn outputs valid. tempTx.setTxType(TxType.INVALID); String msg = "Invalid tx. We have multiple opReturn outputs. tx=" + tempTx; diff --git a/src/main/java/bisq/core/dao/state/blockchain/Tx.java b/src/main/java/bisq/core/dao/state/blockchain/Tx.java index 75c369c5..3331a42c 100644 --- a/src/main/java/bisq/core/dao/state/blockchain/Tx.java +++ b/src/main/java/bisq/core/dao/state/blockchain/Tx.java @@ -138,14 +138,12 @@ public TxOutput getLastTxOutput() { /** - * OpReturn output might contain the lockTime in case of a LockTx. It has to be the last output. - * We store technically the lockTime there as is is stored in the OpReturn data but conceptually we want to provide - * it from the transaction. + * The locktime is stored in the LOCKUP txOutput, which is the first txOutput. * * @return */ public int getLockTime() { - return getLastTxOutput().getLockTime(); + return txOutputs.get(0).getLockTime(); } @Override From 31b96c36218e531e791f0bedf69f49e4678bdd6d Mon Sep 17 00:00:00 2001 From: sqrrm Date: Tue, 4 Sep 2018 12:55:53 +0200 Subject: [PATCH 2/6] Rename processOpReturnCandidate -> isOpReturnCandidate --- src/main/java/bisq/core/dao/node/parser/TxOutputParser.java | 2 +- src/main/java/bisq/core/dao/node/parser/TxParser.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java b/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java index e9f1561b..3d6a933e 100644 --- a/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java +++ b/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java @@ -82,7 +82,7 @@ public void processGenesisTxOutput(TempTx genesisTx) { } } - boolean processOpReturnCandidate(TempTxOutput txOutput) { + boolean isOpReturnCandidate(TempTxOutput txOutput) { optionalOpReturnTypeCandidate = OpReturnParser.getOptionalOpReturnTypeCandidate(txOutput); return optionalOpReturnTypeCandidate.isPresent(); } diff --git a/src/main/java/bisq/core/dao/node/parser/TxParser.java b/src/main/java/bisq/core/dao/node/parser/TxParser.java index 75ce8e1b..b48a02a5 100644 --- a/src/main/java/bisq/core/dao/node/parser/TxParser.java +++ b/src/main/java/bisq/core/dao/node/parser/TxParser.java @@ -116,7 +116,7 @@ public Optional findTx(RawTx rawTx, String genesisTxId, int genesisBlockHeig checkArgument(!outputs.isEmpty(), "outputs must not be empty"); int lastIndex = outputs.size() - 1; int lastNonOpReturnIndex = lastIndex; - if (txOutputParser.processOpReturnCandidate(outputs.get(lastIndex))) { + if (txOutputParser.isOpReturnCandidate(outputs.get(lastIndex))) { txOutputParser.processTxOutput(true, outputs.get(lastIndex), lastIndex); lastNonOpReturnIndex -= 1; } From b2e1da3653716609db516651b98f7d82c4eb8ee8 Mon Sep 17 00:00:00 2001 From: sqrrm Date: Wed, 5 Sep 2018 16:07:18 +0200 Subject: [PATCH 3/6] Extract opreturn post processing functions --- .../bisq/core/dao/node/parser/TxParser.java | 105 +++++++++++------- 1 file changed, 62 insertions(+), 43 deletions(-) diff --git a/src/main/java/bisq/core/dao/node/parser/TxParser.java b/src/main/java/bisq/core/dao/node/parser/TxParser.java index b48a02a5..0110480a 100644 --- a/src/main/java/bisq/core/dao/node/parser/TxParser.java +++ b/src/main/java/bisq/core/dao/node/parser/TxParser.java @@ -193,55 +193,16 @@ private void processOpReturnType(int blockHeight, TempTx tempTx) { boolean isFeeAndPhaseValid; switch (verifiedOpReturnType) { case PROPOSAL: - isFeeAndPhaseValid = isFeeAndPhaseValid(blockHeight, bsqFee, DaoPhase.Phase.PROPOSAL, Param.PROPOSAL_FEE); - if (!isFeeAndPhaseValid) { - tempTx.setTxType(TxType.INVALID); - } + processProposal(blockHeight, tempTx, bsqFee); break; case COMPENSATION_REQUEST: - isFeeAndPhaseValid = isFeeAndPhaseValid(blockHeight, bsqFee, DaoPhase.Phase.PROPOSAL, Param.PROPOSAL_FEE); - Optional optionalIssuanceCandidate = txOutputParser.getOptionalIssuanceCandidate(); - if (isFeeAndPhaseValid) { - if (optionalIssuanceCandidate.isPresent()) { - // Now after we have validated the opReturn data we will apply the TxOutputType - optionalIssuanceCandidate.get().setTxOutputType(TxOutputType.ISSUANCE_CANDIDATE_OUTPUT); - } else { - log.warn("It can be that we have a opReturn which is correct from its structure but the whole tx " + - "in not valid as the issuanceCandidate in not there. " + - "As the BSQ fee is set it must be either a buggy tx or an manually crafted invalid tx."); - } - } else { - tempTx.setTxType(TxType.INVALID); - optionalIssuanceCandidate.ifPresent(tempTxOutput -> tempTxOutput.setTxOutputType(TxOutputType.BTC_OUTPUT)); - // Empty Optional case is a possible valid case where a random tx matches our opReturn rules but it is not a - // valid BSQ tx. - } + processCompensationRequest(blockHeight, tempTx, bsqFee); break; case BLIND_VOTE: - isFeeAndPhaseValid = isFeeAndPhaseValid(blockHeight, bsqFee, DaoPhase.Phase.BLIND_VOTE, Param.BLIND_VOTE_FEE); - if (!isFeeAndPhaseValid) { - tempTx.setTxType(TxType.INVALID); - Optional optionalBlindVoteLockStakeOutput = txOutputParser.getOptionalBlindVoteLockStakeOutput(); - optionalBlindVoteLockStakeOutput.ifPresent(tempTxOutput -> tempTxOutput.setTxOutputType(TxOutputType.BTC_OUTPUT)); - // Empty Optional case is a possible valid case where a random tx matches our opReturn rules but it is not a - // valid BSQ tx. - } + processBlindVote(blockHeight, tempTx, bsqFee); break; case VOTE_REVEAL: - boolean isPhaseValid = isPhaseValid(blockHeight, DaoPhase.Phase.VOTE_REVEAL); - boolean isVoteRevealInputInValid = txInputParser.getVoteRevealInputState() != TxInputParser.VoteRevealInputState.VALID; - if (!isPhaseValid) { - tempTx.setTxType(TxType.INVALID); - } - if (!isPhaseValid || isVoteRevealInputInValid) { - Optional optionalVoteRevealUnlockStakeOutput = txOutputParser - .getOptionalVoteRevealUnlockStakeOutput(); - optionalVoteRevealUnlockStakeOutput.ifPresent( - tempTxOutput -> tempTxOutput - .setTxOutputType(TxOutputType.BTC_OUTPUT)); - // Empty Optional case is a possible valid case where a random tx matches our opReturn rules but it is not a - // valid BSQ tx. - } + processVoteReveal(blockHeight, tempTx); break; case LOCKUP: // do nothing @@ -268,6 +229,64 @@ private void processOpReturnType(int blockHeight, TempTx tempTx) { } } + private void processVoteReveal(int blockHeight, TempTx tempTx) { + boolean isPhaseValid = isPhaseValid(blockHeight, DaoPhase.Phase.VOTE_REVEAL); + boolean isVoteRevealInputInValid = txInputParser.getVoteRevealInputState() != TxInputParser.VoteRevealInputState.VALID; + if (!isPhaseValid) { + tempTx.setTxType(TxType.INVALID); + } + if (!isPhaseValid || isVoteRevealInputInValid) { + Optional optionalVoteRevealUnlockStakeOutput = txOutputParser + .getOptionalVoteRevealUnlockStakeOutput(); + optionalVoteRevealUnlockStakeOutput.ifPresent( + tempTxOutput -> tempTxOutput + .setTxOutputType(TxOutputType.BTC_OUTPUT)); + // Empty Optional case is a possible valid case where a random tx matches our opReturn rules but it is not a + // valid BSQ tx. + } + } + + private void processBlindVote(int blockHeight, TempTx tempTx, long bsqFee) { + boolean isFeeAndPhaseValid; + isFeeAndPhaseValid = isFeeAndPhaseValid(blockHeight, bsqFee, DaoPhase.Phase.BLIND_VOTE, Param.BLIND_VOTE_FEE); + if (!isFeeAndPhaseValid) { + tempTx.setTxType(TxType.INVALID); + Optional optionalBlindVoteLockStakeOutput = txOutputParser.getOptionalBlindVoteLockStakeOutput(); + optionalBlindVoteLockStakeOutput.ifPresent(tempTxOutput -> tempTxOutput.setTxOutputType(TxOutputType.BTC_OUTPUT)); + // Empty Optional case is a possible valid case where a random tx matches our opReturn rules but it is not a + // valid BSQ tx. + } + } + + private void processCompensationRequest(int blockHeight, TempTx tempTx, long bsqFee) { + boolean isFeeAndPhaseValid; + isFeeAndPhaseValid = isFeeAndPhaseValid(blockHeight, bsqFee, DaoPhase.Phase.PROPOSAL, Param.PROPOSAL_FEE); + Optional optionalIssuanceCandidate = txOutputParser.getOptionalIssuanceCandidate(); + if (isFeeAndPhaseValid) { + if (optionalIssuanceCandidate.isPresent()) { + // Now after we have validated the opReturn data we will apply the TxOutputType + optionalIssuanceCandidate.get().setTxOutputType(TxOutputType.ISSUANCE_CANDIDATE_OUTPUT); + } else { + log.warn("It can be that we have a opReturn which is correct from its structure but the whole tx " + + "in not valid as the issuanceCandidate in not there. " + + "As the BSQ fee is set it must be either a buggy tx or an manually crafted invalid tx."); + } + } else { + tempTx.setTxType(TxType.INVALID); + optionalIssuanceCandidate.ifPresent(tempTxOutput -> tempTxOutput.setTxOutputType(TxOutputType.BTC_OUTPUT)); + // Empty Optional case is a possible valid case where a random tx matches our opReturn rules but it is not a + // valid BSQ tx. + } + } + + private void processProposal(int blockHeight, TempTx tempTx, long bsqFee) { + boolean isFeeAndPhaseValid; + isFeeAndPhaseValid = isFeeAndPhaseValid(blockHeight, bsqFee, DaoPhase.Phase.PROPOSAL, Param.PROPOSAL_FEE); + if (!isFeeAndPhaseValid) { + tempTx.setTxType(TxType.INVALID); + } + } + /** * Whether the BSQ fee and phase is valid for a transaction. * From 8fddd5b629a91bdfa004505c0e73486a22c904bf Mon Sep 17 00:00:00 2001 From: sqrrm Date: Wed, 5 Sep 2018 16:34:59 +0200 Subject: [PATCH 4/6] Process opReturn txOutput before remaining outputs --- .../core/dao/node/parser/TxOutputParser.java | 30 ++++++++++--------- .../bisq/core/dao/node/parser/TxParser.java | 27 ++++------------- 2 files changed, 21 insertions(+), 36 deletions(-) diff --git a/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java b/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java index 3d6a933e..d7a62273 100644 --- a/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java +++ b/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java @@ -59,8 +59,6 @@ public class TxOutputParser { @Getter private boolean bsqOutputFound; @Getter - private Optional optionalOpReturnTypeCandidate = Optional.empty(); - @Getter private Optional optionalVerifiedOpReturnType = Optional.empty(); @Getter private Optional optionalIssuanceCandidate = Optional.empty(); @@ -82,9 +80,17 @@ public void processGenesisTxOutput(TempTx genesisTx) { } } - boolean isOpReturnCandidate(TempTxOutput txOutput) { - optionalOpReturnTypeCandidate = OpReturnParser.getOptionalOpReturnTypeCandidate(txOutput); - return optionalOpReturnTypeCandidate.isPresent(); + boolean isOpReturnOutput(TempTxOutput txOutput) { + return txOutput.getOpReturnData() != null; + } + + void processOpReturnOutput(boolean isLastOutput, TempTxOutput tempTxOutput) { + byte[] opReturnData = tempTxOutput.getOpReturnData(); + if (opReturnData != null) { + handleOpReturnOutput(tempTxOutput, isLastOutput); + } else { + log.error("This should be an opReturn output"); + } } /** @@ -110,14 +116,10 @@ void processTxOutput(boolean isLastOutput, TempTxOutput tempTxOutput, int index) handleBtcOutput(tempTxOutput, index); } } else { - handleOpReturnOutput(tempTxOutput, isLastOutput); + log.error("This should not be an opReturn output"); } } - boolean isOpReturnOutput(TempTxOutput txOutput) { - return txOutput.getOpReturnData() != null; - } - /** * Whether a transaction is a valid unlock bond transaction or not. * @@ -154,8 +156,8 @@ private void handleBsqOutput(TempTxOutput txOutput, int index, long txOutputValu boolean isFirstOutput = index == 0; OpReturnType opReturnTypeCandidate = null; - if (optionalOpReturnTypeCandidate.isPresent()) - opReturnTypeCandidate = optionalOpReturnTypeCandidate.get(); + if (optionalVerifiedOpReturnType.isPresent()) + opReturnTypeCandidate = optionalVerifiedOpReturnType.get(); TxOutputType bsqOutput; if (isFirstOutput && opReturnTypeCandidate == OpReturnType.BLIND_VOTE) { @@ -182,8 +184,8 @@ private void handleBtcOutput(TempTxOutput txOutput, int index) { // candidate to the parsingModel and we don't apply the TxOutputType as we do that later as the OpReturn check. if (availableInputValue > 0 && index == 1 && - optionalOpReturnTypeCandidate.isPresent() && - optionalOpReturnTypeCandidate.get() == OpReturnType.COMPENSATION_REQUEST) { + optionalVerifiedOpReturnType.isPresent() && + optionalVerifiedOpReturnType.get() == OpReturnType.COMPENSATION_REQUEST) { // We don't set the txOutputType yet as we have not fully validated the tx but put the candidate // into our local optionalIssuanceCandidate. diff --git a/src/main/java/bisq/core/dao/node/parser/TxParser.java b/src/main/java/bisq/core/dao/node/parser/TxParser.java index 0110480a..7d94509b 100644 --- a/src/main/java/bisq/core/dao/node/parser/TxParser.java +++ b/src/main/java/bisq/core/dao/node/parser/TxParser.java @@ -116,8 +116,9 @@ public Optional findTx(RawTx rawTx, String genesisTxId, int genesisBlockHeig checkArgument(!outputs.isEmpty(), "outputs must not be empty"); int lastIndex = outputs.size() - 1; int lastNonOpReturnIndex = lastIndex; - if (txOutputParser.isOpReturnCandidate(outputs.get(lastIndex))) { - txOutputParser.processTxOutput(true, outputs.get(lastIndex), lastIndex); + if (txOutputParser.isOpReturnOutput(outputs.get(lastIndex))){ + // TODO(SQ): perhaps the check for isLastOutput could be skipped + txOutputParser.processOpReturnOutput(true, outputs.get(lastIndex)); lastNonOpReturnIndex -= 1; } @@ -145,7 +146,7 @@ public Optional findTx(RawTx rawTx, String genesisTxId, int genesisBlockHeig // use RawTx? TxType txType = TxParser.getBisqTxType( tempTx, - txOutputParser.getOptionalOpReturnTypeCandidate().isPresent(), + txOutputParser.getOptionalVerifiedOpReturnType().isPresent(), remainingInputValue, getOptionalOpReturnType() ); @@ -395,25 +396,7 @@ private static TxType getTxTypeForOpReturn(TempTx tx, OpReturnType opReturnType) */ private Optional getOptionalOpReturnType() { if (txOutputParser.isBsqOutputFound()) { - // We want to be sure that the initial assumption of the opReturn type was matching the result after full - // validation. - Optional optionalOpReturnTypeCandidate = txOutputParser.getOptionalOpReturnTypeCandidate(); - if (optionalOpReturnTypeCandidate.isPresent()) { - OpReturnType opReturnTypeCandidate = optionalOpReturnTypeCandidate.get(); - Optional optionalVerifiedOpReturnType = txOutputParser.getOptionalVerifiedOpReturnType(); - if (optionalVerifiedOpReturnType.isPresent()) { - OpReturnType verifiedOpReturnType = optionalVerifiedOpReturnType.get(); - if (opReturnTypeCandidate == verifiedOpReturnType) { - return optionalVerifiedOpReturnType; - } - } - } - - String msg = "We got a different opReturn type after validation as we expected initially. " + - "optionalOpReturnTypeCandidate=" + optionalOpReturnTypeCandidate + - ", optionalVerifiedOpReturnType=" + txOutputParser.getOptionalVerifiedOpReturnType(); - log.error(msg); - + return txOutputParser.getOptionalVerifiedOpReturnType(); } else { String msg = "We got a tx without any valid BSQ output but with burned BSQ. " + "Burned fee=" + remainingInputValue / 100D + " BSQ."; From 6a6f84d0f159b52b8b692b6b22e2bcf2ceaa490f Mon Sep 17 00:00:00 2001 From: sqrrm Date: Wed, 5 Sep 2018 16:51:12 +0200 Subject: [PATCH 5/6] Check for multiple opReturns before processing txOutputs --- .../core/dao/node/parser/TxOutputParser.java | 6 ++- .../bisq/core/dao/node/parser/TxParser.java | 39 ++++++++++--------- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java b/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java index d7a62273..b4930e17 100644 --- a/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java +++ b/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java @@ -24,6 +24,7 @@ import bisq.core.dao.state.blockchain.TempTxOutput; import bisq.core.dao.state.blockchain.TxOutput; import bisq.core.dao.state.blockchain.TxOutputType; +import bisq.core.dao.state.blockchain.TxType; import com.google.common.annotations.VisibleForTesting; @@ -105,7 +106,10 @@ void processTxOutput(boolean isLastOutput, TempTxOutput tempTxOutput, int index) byte[] opReturnData = tempTxOutput.getOpReturnData(); if (opReturnData == null) { long txOutputValue = tempTxOutput.getValue(); - if (isUnlockBondTx(tempTxOutput.getValue(), index)) { + if (tempTx.getTxType() == TxType.INVALID) { + // Set all non opReturn outputs to BTC_OUTPUT if the tx is invalid + tempTxOutput.setTxOutputType(TxOutputType.BTC_OUTPUT); + } else if (isUnlockBondTx(tempTxOutput.getValue(), index)) { // We need to handle UNLOCK transactions separately as they don't follow the pattern on spending BSQ // The LOCKUP BSQ is burnt unless the output exactly matches the input, that would cause the // output to not be BSQ output at all diff --git a/src/main/java/bisq/core/dao/node/parser/TxParser.java b/src/main/java/bisq/core/dao/node/parser/TxParser.java index 7d94509b..5389bd0a 100644 --- a/src/main/java/bisq/core/dao/node/parser/TxParser.java +++ b/src/main/java/bisq/core/dao/node/parser/TxParser.java @@ -101,6 +101,15 @@ public Optional findTx(RawTx rawTx, String genesisTxId, int genesisBlockHeig long accumulatedInputValue = txInputParser.getAccumulatedInputValue(); txOutputParser.setAvailableInputValue(accumulatedInputValue); + + // We don't allow multiple opReturn outputs (they are non-standard but to be safe lets check it) + long numOpReturnOutputs = tempTx.getTempTxOutputs().stream().filter(txOutputParser::isOpReturnOutput).count(); + if (numOpReturnOutputs > 1) { + tempTx.setTxType(TxType.INVALID); + String msg = "Invalid tx. We have multiple opReturn outputs. tx=" + tempTx; + log.warn(msg); + } + txOutputParser.setUnlockBlockHeight(txInputParser.getUnlockBlockHeight()); txOutputParser.setOptionalSpentLockupTxOutput(txInputParser.getOptionalSpentLockupTxOutput()); txOutputParser.setTempTx(tempTx); //TODO remove @@ -116,7 +125,7 @@ public Optional findTx(RawTx rawTx, String genesisTxId, int genesisBlockHeig checkArgument(!outputs.isEmpty(), "outputs must not be empty"); int lastIndex = outputs.size() - 1; int lastNonOpReturnIndex = lastIndex; - if (txOutputParser.isOpReturnOutput(outputs.get(lastIndex))){ + if (txOutputParser.isOpReturnOutput(outputs.get(lastIndex))) { // TODO(SQ): perhaps the check for isLastOutput could be skipped txOutputParser.processOpReturnOutput(true, outputs.get(lastIndex)); lastNonOpReturnIndex -= 1; @@ -134,11 +143,15 @@ public Optional findTx(RawTx rawTx, String genesisTxId, int genesisBlockHeig remainingInputValue = txOutputParser.getAvailableInputValue(); + // TODO(SQ): If the tx is set to INVALID in this check the txOutputs stay valid processOpReturnType(blockHeight, tempTx); - // We don't allow multiple opReturn outputs (they are non-standard but to be safe lets check it) - long numOpReturnOutputs = tempTx.getTempTxOutputs().stream().filter(txOutputParser::isOpReturnOutput).count(); - if (numOpReturnOutputs <= 1) { + // TODO(SQ): Should the destroyed BSQ from an INVALID tx be considered as burnt fee? + if (remainingInputValue > 0) + tempTx.setBurntFee(remainingInputValue); + + // Process the type of transaction if not already determined to be INVALID + if (tempTx.getTxType() != TxType.INVALID) { boolean isAnyTxOutputTypeUndefined = tempTx.getTempTxOutputs().stream() .anyMatch(txOutput -> TxOutputType.UNDEFINED == txOutput.getTxOutputType()); if (!isAnyTxOutputTypeUndefined) { @@ -151,21 +164,11 @@ public Optional findTx(RawTx rawTx, String genesisTxId, int genesisBlockHeig getOptionalOpReturnType() ); tempTx.setTxType(txType); - if (remainingInputValue > 0) - tempTx.setBurntFee(remainingInputValue); } else { tempTx.setTxType(TxType.INVALID); String msg = "We have undefined txOutput types which must not happen. tx=" + tempTx; DevEnv.logErrorAndThrowIfDevMode(msg); } - } else { - // TODO(SQ): The transaction has already been parsed here and the individual txouputs are considered - // spendable or otherwise correct. Perhaps this check should be done earlier. - - // We don't consider a tx with multiple OpReturn outputs valid. - tempTx.setTxType(TxType.INVALID); - String msg = "Invalid tx. We have multiple opReturn outputs. tx=" + tempTx; - log.warn(msg); } } @@ -260,8 +263,8 @@ private void processBlindVote(int blockHeight, TempTx tempTx, long bsqFee) { } private void processCompensationRequest(int blockHeight, TempTx tempTx, long bsqFee) { - boolean isFeeAndPhaseValid; - isFeeAndPhaseValid = isFeeAndPhaseValid(blockHeight, bsqFee, DaoPhase.Phase.PROPOSAL, Param.PROPOSAL_FEE); + boolean isFeeAndPhaseValid = + isFeeAndPhaseValid(blockHeight, bsqFee, DaoPhase.Phase.PROPOSAL, Param.PROPOSAL_FEE); Optional optionalIssuanceCandidate = txOutputParser.getOptionalIssuanceCandidate(); if (isFeeAndPhaseValid) { if (optionalIssuanceCandidate.isPresent()) { @@ -281,8 +284,8 @@ private void processCompensationRequest(int blockHeight, TempTx tempTx, long bsq } private void processProposal(int blockHeight, TempTx tempTx, long bsqFee) { - boolean isFeeAndPhaseValid; - isFeeAndPhaseValid = isFeeAndPhaseValid(blockHeight, bsqFee, DaoPhase.Phase.PROPOSAL, Param.PROPOSAL_FEE); + boolean isFeeAndPhaseValid = + isFeeAndPhaseValid(blockHeight, bsqFee, DaoPhase.Phase.PROPOSAL, Param.PROPOSAL_FEE); if (!isFeeAndPhaseValid) { tempTx.setTxType(TxType.INVALID); } From 28a5c515359996bb4a4ef97d619a11c8a4964f77 Mon Sep 17 00:00:00 2001 From: sqrrm Date: Wed, 5 Sep 2018 17:31:16 +0200 Subject: [PATCH 6/6] Wait to commit BSQ txOutputs to bsqStateService until after parsing If a transaction is considered invalid it should consider all the txOutputs to be invalid. To avoid counting a txOutput as valid it's better to commit to stateservice only after checking the validity of the transaction. --- .../core/dao/node/parser/TxOutputParser.java | 26 ++++++++++++++++--- .../bisq/core/dao/node/parser/TxParser.java | 9 +++++-- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java b/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java index b4930e17..cbac28a0 100644 --- a/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java +++ b/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java @@ -28,6 +28,8 @@ import com.google.common.annotations.VisibleForTesting; +import java.util.ArrayList; +import java.util.List; import java.util.Optional; import lombok.Getter; @@ -49,6 +51,7 @@ public class TxOutputParser { @Setter private long availableInputValue = 0; private int lockTime; + private List bsqOutputs = new ArrayList<>(); @Setter private int unlockBlockHeight; @Setter @@ -74,11 +77,28 @@ public class TxOutputParser { this.bsqStateService = bsqStateService; } + public void commitTxOutputs() { + bsqOutputs.forEach(bsqOutput -> { + bsqStateService.addUnspentTxOutput(TxOutput.fromTempOutput(bsqOutput)); + }); + } + + /** + * This sets all outputs to BTC_OUTPUT and doesn't add any txOutputs to the bsqStateService + */ + public void commitTxOutputsForInvalidTx() { + bsqOutputs.forEach(bsqOutput -> { + bsqOutput.setTxOutputType(TxOutputType.BTC_OUTPUT); + }); + + } + public void processGenesisTxOutput(TempTx genesisTx) { for (int i = 0; i < genesisTx.getTempTxOutputs().size(); ++i) { TempTxOutput tempTxOutput = genesisTx.getTempTxOutputs().get(i); - bsqStateService.addUnspentTxOutput(TxOutput.fromTempOutput(tempTxOutput)); + bsqOutputs.add(tempTxOutput); } + commitTxOutputs(); } boolean isOpReturnOutput(TempTxOutput txOutput) { @@ -144,7 +164,7 @@ private void handleUnlockBondTx(TempTxOutput txOutput) { availableInputValue -= optionalSpentLockupTxOutput.get().getValue(); txOutput.setTxOutputType(TxOutputType.UNLOCK); - bsqStateService.addUnspentTxOutput(TxOutput.fromTempOutput(txOutput)); + bsqOutputs.add(txOutput); //TODO move up to TxParser // We should add unlockBlockHeight to TempTxOutput and remove unlockBlockHeight from tempTx @@ -178,7 +198,7 @@ private void handleBsqOutput(TempTxOutput txOutput, int index, long txOutputValu bsqOutput = TxOutputType.BSQ_OUTPUT; } txOutput.setTxOutputType(bsqOutput); - bsqStateService.addUnspentTxOutput(TxOutput.fromTempOutput(txOutput)); + bsqOutputs.add(txOutput); bsqOutputFound = true; } diff --git a/src/main/java/bisq/core/dao/node/parser/TxParser.java b/src/main/java/bisq/core/dao/node/parser/TxParser.java index 5389bd0a..482eba39 100644 --- a/src/main/java/bisq/core/dao/node/parser/TxParser.java +++ b/src/main/java/bisq/core/dao/node/parser/TxParser.java @@ -66,7 +66,7 @@ public TxParser(PeriodService periodService, } // Apply state changes to tx, inputs and outputs - // return true if any input contained BSQ + // return Tx if any input contained BSQ // Any tx with BSQ input is a BSQ tx (except genesis tx but that is not handled in // that class). // There might be txs without any valid BSQ txOutput but we still keep track of it, @@ -143,7 +143,6 @@ public Optional findTx(RawTx rawTx, String genesisTxId, int genesisBlockHeig remainingInputValue = txOutputParser.getAvailableInputValue(); - // TODO(SQ): If the tx is set to INVALID in this check the txOutputs stay valid processOpReturnType(blockHeight, tempTx); // TODO(SQ): Should the destroyed BSQ from an INVALID tx be considered as burnt fee? @@ -170,6 +169,12 @@ public Optional findTx(RawTx rawTx, String genesisTxId, int genesisBlockHeig DevEnv.logErrorAndThrowIfDevMode(msg); } } + + if (tempTx.getTxType() != TxType.INVALID){ + txOutputParser.commitTxOutputs(); + } else { + txOutputParser.commitTxOutputsForInvalidTx(); + } } // TODO || parsingModel.getBurntBondValue() > 0; should not be necessary