Skip to content

Commit

Permalink
fix(coinjoin): address various issues, dry run support, start, stop (#…
Browse files Browse the repository at this point in the history
…242)

* fix(coinjoin): remove duplicate get functions in SimplifiedMasternodeListEntry

* fix (coinjoin): remove ProTxToOutPoint and update CoinJoinQueue and CoinJoinBroadcastTx

* fix(coinjoin): add account index to coinjoin path

* refactor(coinjoin): manually call CoinJoinManager.start and stop

* refactor: use lambdas and add exception handling in CoinJoinExtension

* fix: improve dry run to include creating denominations

* fix(coinjoin): make dry skip key creation

* fix(coinjoin): improve stopping mixing

* fix(coinjoin): remove checks from checkAutomaticBackup

* tests(coinjoin): update coinjoin transaction type tests

* fix(coinjoin): fix NPE in CoinJoinManager.stopAsync

* tests: ignore LevelDB tests

* tests(coinjoin): add dry run check to CoinJoinSessionTest
  • Loading branch information
HashEngineering authored Jan 8, 2024
1 parent bd7d61d commit eb4e227
Show file tree
Hide file tree
Showing 24 changed files with 194 additions and 881 deletions.
56 changes: 3 additions & 53 deletions core/src/main/java/org/bitcoinj/coinjoin/CoinJoinBroadcastTx.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package org.bitcoinj.coinjoin;

import com.google.common.annotations.VisibleForTesting;
import org.bitcoinj.coinjoin.utils.ProTxToOutpoint;
import org.bitcoinj.core.Context;
import org.bitcoinj.core.MasternodeSignature;
import org.bitcoinj.core.Message;
Expand Down Expand Up @@ -47,8 +46,6 @@ public class CoinJoinBroadcastTx extends Message {
private static final Logger log = LoggerFactory.getLogger(CoinJoinQueue.class);

private Transaction tx;
@Deprecated
private TransactionOutPoint masternodeOutpoint;
private Sha256Hash proTxHash;
private MasternodeSignature signature;
private long signatureTime;
Expand All @@ -61,34 +58,6 @@ public CoinJoinBroadcastTx(NetworkParameters params, byte[] payload, int protoco
super(params, payload, 0, protocolVersion);
}

@Deprecated
public CoinJoinBroadcastTx(
NetworkParameters params,
Transaction tx,
TransactionOutPoint masternodeOutpoint,
MasternodeSignature signature,
long signatureTime
) {
super(params);
this.tx = tx;
this.masternodeOutpoint = masternodeOutpoint;
this.signature = signature;
this.signatureTime = signatureTime;
}

@Deprecated
public CoinJoinBroadcastTx(
NetworkParameters params,
Transaction tx,
TransactionOutPoint masternodeOutpoint,
long signatureTime
) {
super(params);
this.tx = tx;
this.masternodeOutpoint = masternodeOutpoint;
this.signatureTime = signatureTime;
}

public CoinJoinBroadcastTx(
NetworkParameters params,
Transaction tx,
Expand All @@ -105,12 +74,7 @@ public CoinJoinBroadcastTx(
protected void parse() throws ProtocolException {
tx = new Transaction(params, payload, cursor);
cursor += tx.getMessageSize();
if (protocolVersion >= params.getProtocolVersionNum(NetworkParameters.ProtocolVersion.COINJOIN_PROTX_HASH)) {
proTxHash = readHash();
} else {
masternodeOutpoint = new TransactionOutPoint(params, payload, cursor);
cursor += masternodeOutpoint.getMessageSize();
}
proTxHash = readHash();
signature = new MasternodeSignature(params, payload, cursor);
cursor += signature.getMessageSize();
signatureTime = readInt64();
Expand All @@ -121,11 +85,7 @@ protected void parse() throws ProtocolException {
@Override
protected void bitcoinSerializeToStream(OutputStream stream) throws IOException {
tx.bitcoinSerialize(stream);
if (protocolVersion >= params.getProtocolVersionNum(NetworkParameters.ProtocolVersion.COINJOIN_PROTX_HASH)) {
stream.write(proTxHash.getReversedBytes());
} else {
masternodeOutpoint.bitcoinSerialize(stream);
}
stream.write(proTxHash.getReversedBytes());
signature.bitcoinSerialize(stream);
Utils.int64ToByteStreamLE(signatureTime, stream);
}
Expand All @@ -134,9 +94,7 @@ public Sha256Hash getSignatureHash() {
try {
ByteArrayOutputStream bos = new UnsafeByteArrayOutputStream();
tx.bitcoinSerialize(bos);
// this still requires the masternode output
if (masternodeOutpoint == null)
masternodeOutpoint = ProTxToOutpoint.getMasternodeOutpoint(proTxHash);
bos.write(proTxHash.getReversedBytes());
Utils.int64ToByteStreamLE(signatureTime, bos);

return Sha256Hash.twiceOf(bos.toByteArray());
Expand Down Expand Up @@ -175,14 +133,6 @@ public Transaction getTx() {
return tx;
}

@Deprecated
public TransactionOutPoint getMasternodeOutpoint() {
if (masternodeOutpoint == null) {
masternodeOutpoint = ProTxToOutpoint.getMasternodeOutpoint(proTxHash);
}
return masternodeOutpoint;
}

public Sha256Hash getProTxHash() {
return proTxHash;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ public boolean isWaitingForNewBlock() {

// Make sure we have enough keys since last backup
private boolean checkAutomaticBackup() {
return CoinJoinClientOptions.isEnabled() && isMixing();
// Let the KeyChain classes handle this
return true;
}

public int cachedNumBlocks = Integer.MAX_VALUE; // used for the overview screen
Expand Down Expand Up @@ -220,7 +221,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()) {
Expand Down Expand Up @@ -288,7 +289,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);
Expand Down
70 changes: 38 additions & 32 deletions core/src/main/java/org/bitcoinj/coinjoin/CoinJoinClientSession.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -137,21 +137,18 @@ 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<CompactTallyItem>() {
@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();

for (CompactTallyItem item : vecTally) {
if (!createDenominated(balanceToDenominate, item, fCreateMixingCollaterals)) continue;
if (!createDenominated(balanceToDenominate, item, fCreateMixingCollaterals, dryRun)) continue;
return true;
}

Expand Down Expand Up @@ -190,17 +187,17 @@ 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
if (tallyItem.inputCoins.size() == 1 && CoinJoin.isDenominatedAmount(tallyItem.amount)) {
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 ************ /
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1264,7 +1263,8 @@ public boolean doAutomaticDenominating(boolean fDryRun) {
}

if (getEntriesCount() > 0) {
setStatus(PoolStatus.MIXING);
if (!fDryRun)
setStatus(PoolStatus.MIXING);
return false;
}

Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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;
}

Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit eb4e227

Please sign in to comment.