From 7ccf954d49b0343b6da08d9066355a2df17bfe35 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Sun, 24 Dec 2023 08:21:25 -0800 Subject: [PATCH] fix: improve dry run to include creating denominations --- .../coinjoin/CoinJoinClientManager.java | 5 +- .../coinjoin/CoinJoinClientSession.java | 68 ++++++++++--------- .../coinjoin/utils/TransactionBuilder.java | 13 +++- .../utils/TransactionBuilderOutput.java | 7 +- 4 files changed, 56 insertions(+), 37 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/coinjoin/CoinJoinClientManager.java b/core/src/main/java/org/bitcoinj/coinjoin/CoinJoinClientManager.java index 8607a4b4f2..b363e5e0ac 100644 --- a/core/src/main/java/org/bitcoinj/coinjoin/CoinJoinClientManager.java +++ b/core/src/main/java/org/bitcoinj/coinjoin/CoinJoinClientManager.java @@ -220,7 +220,7 @@ public boolean doAutomaticDenominating() { return doAutomaticDenominating(false); } public boolean doAutomaticDenominating(boolean dryRun) { - if (!CoinJoinClientOptions.isEnabled() || !isMixing()) + if (!CoinJoinClientOptions.isEnabled() || (!dryRun && !isMixing())) return false; if (context.masternodeSync.hasSyncFlag(MasternodeSync.SYNC_FLAGS.SYNC_GOVERNANCE) && !mixingWallet.getContext().masternodeSync.isBlockchainSynced()) { @@ -288,7 +288,8 @@ public boolean doAutomaticDenominating(boolean dryRun) { for (CoinJoinClientSession session: deqSessions) { if (!checkAutomaticBackup()) return false; - if (waitForAnotherBlock()) { + // we may not need this + if (!dryRun && waitForAnotherBlock()) { if (Utils.currentTimeMillis() - lastTimeReportTooRecent > 15000 ) { strAutoDenomResult = "Last successful action was too recent."; log.info("DoAutomaticDenominating -- {}", strAutoDenomResult); diff --git a/core/src/main/java/org/bitcoinj/coinjoin/CoinJoinClientSession.java b/core/src/main/java/org/bitcoinj/coinjoin/CoinJoinClientSession.java index 7cfbebd28c..9ecf248291 100644 --- a/core/src/main/java/org/bitcoinj/coinjoin/CoinJoinClientSession.java +++ b/core/src/main/java/org/bitcoinj/coinjoin/CoinJoinClientSession.java @@ -122,7 +122,7 @@ public class CoinJoinClientSession extends CoinJoinBaseSession { = new CopyOnWriteArrayList<>(); /// Create denominations - private boolean createDenominated(Coin balanceToDenominate) { + private boolean createDenominated(Coin balanceToDenominate, boolean dryRun) { if (!CoinJoinClientOptions.isEnabled()) return false; @@ -137,15 +137,12 @@ private boolean createDenominated(Coin balanceToDenominate) { } // Start from the largest balances first to speed things up by creating txes with larger/largest denoms included - vecTally.sort(new Comparator() { - @Override - public int compare(CompactTallyItem o, CompactTallyItem t1) { - if (o.amount.isGreaterThan(t1.amount)) - return 1; - if (o.amount.equals(t1.amount)) - return 0; - return -1; - } + vecTally.sort((a, b) -> { + if (a.amount.isGreaterThan(b.amount)) + return 1; + if (a.amount.equals(b.amount)) + return 0; + return -1; }); boolean fCreateMixingCollaterals = !mixingWallet.hasCollateralInputs(); @@ -190,9 +187,9 @@ interface CountPossibleOutputs { int process(Coin amount); } - private boolean createDenominated(Coin balanceToDenominate, CompactTallyItem tallyItem, boolean fCreateMixingCollaterals) { + private boolean createDenominated(Coin balanceToDenominate, CompactTallyItem tallyItem, boolean fCreateMixingCollaterals, boolean dryRun) { - if (!CoinJoinClientOptions.isEnabled()) + if (!CoinJoinClientOptions.isEnabled()) return false; // denominated input is always a single one, so we can check its amount directly and return early @@ -200,7 +197,7 @@ private boolean createDenominated(Coin balanceToDenominate, CompactTallyItem tal return false; } - try (TransactionBuilder txBuilder = new TransactionBuilder(mixingWallet, tallyItem)) { + try (TransactionBuilder txBuilder = new TransactionBuilder(mixingWallet, tallyItem, dryRun)) { log.info("coinjoin: Start {}", txBuilder); // ****** Add an output for mixing collaterals ************ / @@ -367,18 +364,20 @@ public int process(Coin amount) { return false; } - StringBuilder strResult = new StringBuilder(); - if (!txBuilder.commit(strResult)) { - log.info("coinjoin: Commit failed: {}", strResult.toString()); - return false; - } + if (!dryRun) { + StringBuilder strResult = new StringBuilder(); + if (!txBuilder.commit(strResult)) { + log.info("coinjoin: Commit failed: {}", strResult); + return false; + } - // use the same nCachedLastSuccessBlock as for DS mixing to prevent race - mixingWallet.getContext().coinJoinManager.coinJoinClientManagers.get(mixingWallet.getDescription()).updatedSuccessBlock(); + // use the same nCachedLastSuccessBlock as for DS mixing to prevent race + mixingWallet.getContext().coinJoinManager.coinJoinClientManagers.get(mixingWallet.getDescription()).updatedSuccessBlock(); - log.info("coinjoin: txid: {}", strResult); + log.info("coinjoin: txid: {}", strResult); - queueTransactionListeners(txBuilder.getTransaction(), CoinJoinTransactionType.CreateDenomination); + queueTransactionListeners(txBuilder.getTransaction(), CoinJoinTransactionType.CreateDenomination); + } } return true; } @@ -444,7 +443,7 @@ private boolean makeCollateralAmounts(CompactTallyItem tallyItem, boolean fTryDe return false; } - try (TransactionBuilder txBuilder = new TransactionBuilder(wallet, tallyItem)) { + try (TransactionBuilder txBuilder = new TransactionBuilder(wallet, tallyItem, false)) { log.info("coinjoin: Start {}", txBuilder); // Skip way too tiny amounts. Smallest we want is minimum collateral amount in a one output tx @@ -1264,7 +1263,8 @@ public boolean doAutomaticDenominating(boolean fDryRun) { } if (getEntriesCount() > 0) { - setStatus(PoolStatus.MIXING); + if (!fDryRun) + setStatus(PoolStatus.MIXING); return false; } @@ -1274,7 +1274,7 @@ public boolean doAutomaticDenominating(boolean fDryRun) { return false; } try { - if (mixingWallet.getContext().masternodeListManager.getListAtChainTip().getValidMNsCount() == 0 && + if (!fDryRun && mixingWallet.getContext().masternodeListManager.getListAtChainTip().getValidMNsCount() == 0 && !mixingWallet.getContext().getParams().getId().equals(NetworkParameters.ID_REGTEST)) { strAutoDenomResult = "No Masternodes detected."; log.info("coinjoin: {}", strAutoDenomResult); @@ -1292,11 +1292,13 @@ public boolean doAutomaticDenominating(boolean fDryRun) { balanceNeedsAnonymized = CoinJoinClientOptions.getAmount().subtract(nBalanceAnonymized); if (balanceNeedsAnonymized.isLessThanOrEqualTo(Coin.ZERO)) { - log.info("coinjoin: Nothing to do"); + log.info("coinjoin: Nothing to do {}", fDryRun); // nothing to do, just keep it in idle mode //status.set(PoolStatus.FINISHED); //hasNothingToDo.set(true); - setStatus(PoolStatus.FINISHED); + if (!fDryRun) { + setStatus(PoolStatus.FINISHED); + } return false; } hasNothingToDo.set(false); @@ -1321,10 +1323,11 @@ public boolean doAutomaticDenominating(boolean fDryRun) { //queueSessionCompleteListeners(getState(), ERR_SESSION); //return false; Coin balanceLeftToMix = mixingWallet.getAnonymizableBalance(false, false); - if (!balanceLeftToMix.isGreaterThanOrEqualTo(nValueMin)) { + if (!fDryRun && !balanceLeftToMix.isGreaterThanOrEqualTo(nValueMin)) { setStatus(PoolStatus.ERR_NOT_ENOUGH_FUNDS); queueSessionCompleteListeners(getState(), ERR_SESSION); } + log.info("coinjoin: balance to low: {}", fDryRun); return false; } @@ -1375,14 +1378,17 @@ public boolean doAutomaticDenominating(boolean fDryRun) { nBalanceToDenominate.toFriendlyString() ); - if (fDryRun) return true; - // Check if we have should create more denominated inputs i.e. // there are funds to denominate and denominated balance does not exceed // max amount to mix yet. lastCreateDenominatedResult = true; if (nBalanceAnonimizableNonDenom.isGreaterThanOrEqualTo(nValueMin.add(CoinJoin.getCollateralAmount())) && nBalanceToDenominate.isGreaterThan(Coin.ZERO)) { - lastCreateDenominatedResult = createDenominated(nBalanceToDenominate); + lastCreateDenominatedResult = createDenominated(nBalanceToDenominate, fDryRun); + } + + if (fDryRun) { + log.info("coinjoin: create denominations {}, {}", lastCreateDenominatedResult, fDryRun); + return lastCreateDenominatedResult; } //check if we have the collateral sized inputs diff --git a/core/src/main/java/org/bitcoinj/coinjoin/utils/TransactionBuilder.java b/core/src/main/java/org/bitcoinj/coinjoin/utils/TransactionBuilder.java index fd9004df4e..0bdfd9198f 100644 --- a/core/src/main/java/org/bitcoinj/coinjoin/utils/TransactionBuilder.java +++ b/core/src/main/java/org/bitcoinj/coinjoin/utils/TransactionBuilder.java @@ -19,6 +19,7 @@ import org.bitcoinj.core.Coin; import org.bitcoinj.core.ECKey; import org.bitcoinj.core.InsufficientMoneyException; +import org.bitcoinj.core.KeyId; import org.bitcoinj.core.Transaction; import org.bitcoinj.core.TransactionInput; import org.bitcoinj.core.TransactionOutput; @@ -34,6 +35,7 @@ import javax.annotation.concurrent.GuardedBy; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.concurrent.locks.ReentrantLock; @@ -56,6 +58,8 @@ public class TransactionBuilder implements AutoCloseable { private int bytesOutput = 0; /// Call KeepKey for all keys in destructor if fKeepKeys is true, call ReturnKey for all key if its false. private boolean keepKeys = false; + + final private boolean dryRun; /// Protect vecOutputs private final ReentrantLock lock = Threading.lock("outputs"); /// Contains all outputs already added to the transaction @@ -65,7 +69,8 @@ public class TransactionBuilder implements AutoCloseable { private Transaction transaction; - public TransactionBuilder(WalletEx wallet, final CompactTallyItem tallyItem) { + public TransactionBuilder(WalletEx wallet, final CompactTallyItem tallyItem, boolean dryRun) { + this.dryRun = dryRun; this.wallet = wallet; dummyReserveDestination = new ReserveDestination(wallet); this.tallyItem = tallyItem; @@ -130,7 +135,7 @@ public TransactionBuilderOutput addOutput(Coin amountOutput) { if (couldAddOutput(amountOutput)) { lock.lock(); try { - vecOutputs.add(new TransactionBuilderOutput(this, wallet, amountOutput)); + vecOutputs.add(new TransactionBuilderOutput(this, wallet, amountOutput, dryRun)); return vecOutputs.get(vecOutputs.size() - 1); } finally { lock.unlock(); @@ -157,6 +162,10 @@ public boolean commit(StringBuilder strResult) { try { vecSend = Lists.newArrayListWithExpectedSize(vecOutputs.size()); for (TransactionBuilderOutput out : vecOutputs) { + // make sure the output is not invalid + if(ScriptPattern.isP2PKH(out.getScript()) && Arrays.equals(ScriptPattern.extractHashFromP2PKH(out.getScript()), new byte[20])) { + return false; + } vecSend.add(new Recipient(out.getScript(), out.getAmount(), false)); } } finally { diff --git a/core/src/main/java/org/bitcoinj/coinjoin/utils/TransactionBuilderOutput.java b/core/src/main/java/org/bitcoinj/coinjoin/utils/TransactionBuilderOutput.java index 3e88a67f46..8fcb7b2d85 100644 --- a/core/src/main/java/org/bitcoinj/coinjoin/utils/TransactionBuilderOutput.java +++ b/core/src/main/java/org/bitcoinj/coinjoin/utils/TransactionBuilderOutput.java @@ -16,6 +16,9 @@ package org.bitcoinj.coinjoin.utils; import org.bitcoinj.core.Coin; +import org.bitcoinj.core.KeyId; +import org.bitcoinj.core.NoDestination; +import org.bitcoinj.core.ScriptId; import org.bitcoinj.core.TransactionDestination; import org.bitcoinj.script.Script; import org.bitcoinj.wallet.WalletEx; @@ -30,11 +33,11 @@ public class TransactionBuilderOutput { /// ScriptPubKey of this output Script script; - public TransactionBuilderOutput(TransactionBuilder txBuilder, WalletEx wallet, Coin amount) { + public TransactionBuilderOutput(TransactionBuilder txBuilder, WalletEx wallet, Coin amount, boolean dryRun) { this.txBuilder = txBuilder; this.amount = amount; this.dest = new ReserveDestination(wallet); - TransactionDestination txdest = dest.getReservedDestination(false); + TransactionDestination txdest = dryRun ? dest.getReservedDestination(false) : KeyId.fromBytes(new byte[20]); script = txdest.getScript(); }