Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow spending of unconfirmed BSQ change outputs #2482

Merged
11 changes: 11 additions & 0 deletions common/src/main/proto/pb.proto
Expand Up @@ -954,6 +954,7 @@ message PersistableEnvelope {
DaoStateStore dao_state_store = 24;
MyReputationList my_reputation_list = 25;
MyProofOfBurnList my_proof_of_burn_list = 26;
UnconfirmedBsqChangeOutputList unconfirmed_bsq_change_output_list = 27;
}
}

Expand Down Expand Up @@ -1405,6 +1406,12 @@ message BaseTxOutput {
}
}

message UnconfirmedTxOutput {
int32 index = 1;
int64 value = 2;
string tx_id = 3;
}

message RawTxOutput {
}

Expand Down Expand Up @@ -1571,6 +1578,10 @@ message MyProofOfBurnList {
repeated MyProofOfBurn my_proof_of_burn = 1;
}

message UnconfirmedBsqChangeOutputList {
repeated UnconfirmedTxOutput unconfirmed_tx_output = 1;
}

message TempProposalPayload {
Proposal proposal = 1;
bytes owner_pub_key_encoded = 2;
Expand Down
Expand Up @@ -20,9 +20,10 @@
import org.bitcoinj.core.Coin;

public interface BsqBalanceListener {
void onUpdateBalances(Coin availableBalance,
void onUpdateBalances(Coin availableConfirmedBalance,
Coin availableNonBsqBalance,
Coin unverifiedBalance,
Coin unconfirmedChangeBalance,
Coin lockedForVotingBalance,
Coin lockedInBondsBalance,
Coin unlockingBondsBalance);
Expand Down
26 changes: 22 additions & 4 deletions core/src/main/java/bisq/core/btc/wallet/BsqCoinSelector.java
Expand Up @@ -19,7 +19,9 @@

import bisq.core.dao.state.DaoStateService;
import bisq.core.dao.state.model.blockchain.TxOutputKey;
import bisq.core.dao.state.unconfirmed.UnconfirmedBsqChangeOutputListService;

import org.bitcoinj.core.Transaction;
import org.bitcoinj.core.TransactionOutput;

import javax.inject.Inject;
Expand All @@ -32,18 +34,34 @@
*/
@Slf4j
public class BsqCoinSelector extends BisqDefaultCoinSelector {
private DaoStateService daoStateService;
private final DaoStateService daoStateService;
private final UnconfirmedBsqChangeOutputListService unconfirmedBsqChangeOutputListService;

@Inject
public BsqCoinSelector(DaoStateService daoStateService) {
public BsqCoinSelector(DaoStateService daoStateService, UnconfirmedBsqChangeOutputListService unconfirmedBsqChangeOutputListService) {
super(true);
this.daoStateService = daoStateService;
this.unconfirmedBsqChangeOutputListService = unconfirmedBsqChangeOutputListService;
}

@Override
protected boolean isTxOutputSpendable(TransactionOutput output) {
// output.getParentTransaction() cannot be null as it is checked in calling method
return output.getParentTransaction() != null &&
daoStateService.isTxOutputSpendable(new TxOutputKey(output.getParentTransaction().getHashAsString(), output.getIndex()));
Transaction parentTransaction = output.getParentTransaction();
if (parentTransaction == null)
return false;

// If it is a normal confirmed BSQ output we use the default lookup at the daoState
if (daoStateService.isTxOutputSpendable(new TxOutputKey(parentTransaction.getHashAsString(), output.getIndex())))
return true;

// It might be that it is an unconfirmed change output which we allow to be used for spending without requiring a confirmation.
// We check if we have the output in the dao state, if so we have a confirmed but unspendable output (e.g. confiscated).
if (daoStateService.getTxOutput(new TxOutputKey(parentTransaction.getHashAsString(), output.getIndex())).isPresent())
return false;

// Only if its not existing yet in the dao state (unconfirmed) we use our unconfirmedBsqChangeOutputList to
// check if it is an own change output.
return unconfirmedBsqChangeOutputListService.hasTransactionOutput(output);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need to check that it's also unspent or is the unconfirmed list only for unspent txs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we remove entries for all connected outputs of any inputs we should not get that problem (e.g. user spend utxo from previously unconfirmed tx). If we would miss one it would throw an error on the BitcoinJ level if we try to use an input which is already spent. But I will have a look to add an additional check.

}
}
80 changes: 40 additions & 40 deletions core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java
Expand Up @@ -29,6 +29,8 @@
import bisq.core.dao.state.model.blockchain.Tx;
import bisq.core.dao.state.model.blockchain.TxOutput;
import bisq.core.dao.state.model.blockchain.TxOutputKey;
import bisq.core.dao.state.model.blockchain.TxType;
import bisq.core.dao.state.unconfirmed.UnconfirmedBsqChangeOutputListService;
import bisq.core.provider.fee.FeeService;
import bisq.core.user.Preferences;

Expand All @@ -47,6 +49,7 @@
import org.bitcoinj.core.TransactionOutput;
import org.bitcoinj.script.Script;
import org.bitcoinj.wallet.CoinSelection;
import org.bitcoinj.wallet.CoinSelector;
import org.bitcoinj.wallet.SendRequest;
import org.bitcoinj.wallet.Wallet;
import org.bitcoinj.wallet.listeners.AbstractWalletEventListener;
Expand Down Expand Up @@ -79,17 +82,20 @@ public class BsqWalletService extends WalletService implements DaoStateListener
private final BsqCoinSelector bsqCoinSelector;
private final NonBsqCoinSelector nonBsqCoinSelector;
private final DaoStateService daoStateService;
private final UnconfirmedBsqChangeOutputListService unconfirmedBsqChangeOutputListService;
private final ObservableList<Transaction> walletTransactions = FXCollections.observableArrayList();
private final CopyOnWriteArraySet<BsqBalanceListener> bsqBalanceListeners = new CopyOnWriteArraySet<>();

// balance of non BSQ satoshis
@Getter
private Coin availableNonBsqBalance = Coin.ZERO;
@Getter
private Coin availableBalance = Coin.ZERO;
private Coin availableConfirmedBalance = Coin.ZERO;
@Getter
private Coin unverifiedBalance = Coin.ZERO;
@Getter
private Coin unconfirmedChangeBalance = Coin.ZERO;
@Getter
private Coin lockedForVotingBalance = Coin.ZERO;
@Getter
private Coin lockupBondsBalance = Coin.ZERO;
Expand All @@ -106,6 +112,7 @@ public BsqWalletService(WalletsSetup walletsSetup,
BsqCoinSelector bsqCoinSelector,
NonBsqCoinSelector nonBsqCoinSelector,
DaoStateService daoStateService,
UnconfirmedBsqChangeOutputListService unconfirmedBsqChangeOutputListService,
Preferences preferences,
FeeService feeService) {
super(walletsSetup,
Expand All @@ -115,6 +122,7 @@ public BsqWalletService(WalletsSetup walletsSetup,
this.bsqCoinSelector = bsqCoinSelector;
this.nonBsqCoinSelector = nonBsqCoinSelector;
this.daoStateService = daoStateService;
this.unconfirmedBsqChangeOutputListService = unconfirmedBsqChangeOutputListService;

walletsSetup.addSetupCompletedHandler(() -> {
wallet = walletsSetup.getBsqWallet();
Expand All @@ -138,11 +146,13 @@ public void onCoinsSent(Wallet wallet, Transaction tx, Coin prevBalance, Coin ne
public void onReorganize(Wallet wallet) {
log.warn("onReorganize ");
updateBsqWalletTransactions();
unconfirmedBsqChangeOutputListService.onReorganize();
}

@Override
public void onTransactionConfidenceChanged(Wallet wallet, Transaction tx) {
updateBsqWalletTransactions();
unconfirmedBsqChangeOutputListService.onTransactionConfidenceChanged(tx);
}

@Override
Expand Down Expand Up @@ -260,17 +270,19 @@ private void updateBsqBalance() {
.mapToLong(TxOutput::getValue)
.sum());

availableBalance = bsqCoinSelector.select(NetworkParameters.MAX_MONEY,
availableConfirmedBalance = bsqCoinSelector.select(NetworkParameters.MAX_MONEY,
wallet.calculateAllSpendCandidates()).valueGathered;

if (availableBalance.isNegative())
availableBalance = Coin.ZERO;
if (availableConfirmedBalance.isNegative())
availableConfirmedBalance = Coin.ZERO;

unconfirmedChangeBalance = unconfirmedBsqChangeOutputListService.getBalance();

availableNonBsqBalance = nonBsqCoinSelector.select(NetworkParameters.MAX_MONEY,
wallet.calculateAllSpendCandidates()).valueGathered;

bsqBalanceListeners.forEach(e -> e.onUpdateBalances(availableBalance, availableNonBsqBalance, unverifiedBalance,
lockedForVotingBalance, lockupBondsBalance, unlockingBondsBalance));
bsqBalanceListeners.forEach(e -> e.onUpdateBalances(availableConfirmedBalance, availableNonBsqBalance, unverifiedBalance,
unconfirmedChangeBalance, lockedForVotingBalance, lockupBondsBalance, unlockingBondsBalance));
}

public void addBsqBalanceListener(BsqBalanceListener listener) {
Expand Down Expand Up @@ -444,42 +456,21 @@ public Transaction signTx(Transaction tx) throws WalletException, TransactionVer
// Commit tx
///////////////////////////////////////////////////////////////////////////////////////////

public void commitTx(Transaction tx) {
public void commitTx(Transaction tx, TxType txType) {
wallet.commitTx(tx);
//printTx("BSQ commit Tx", tx);

unconfirmedBsqChangeOutputListService.onCommitTx(tx, txType, wallet);
}


///////////////////////////////////////////////////////////////////////////////////////////
// Send BSQ with BTC fee
///////////////////////////////////////////////////////////////////////////////////////////

public Transaction getPreparedSendTx(String receiverAddress, Coin receiverAmount)
public Transaction getPreparedSendBsqTx(String receiverAddress, Coin receiverAmount)
throws AddressFormatException, InsufficientBsqException, WalletException, TransactionVerificationException {
DaoKillSwitch.assertDaoIsNotDisabled();
Transaction tx = new Transaction(params);
checkArgument(Restrictions.isAboveDust(receiverAmount),
"The amount is too low (dust limit).");
tx.addOutput(receiverAmount, Address.fromBase58(params, receiverAddress));

SendRequest sendRequest = SendRequest.forTx(tx);
sendRequest.fee = Coin.ZERO;
sendRequest.feePerKb = Coin.ZERO;
sendRequest.ensureMinRequiredFee = false;
sendRequest.aesKey = aesKey;
sendRequest.shuffleOutputs = false;
sendRequest.signInputs = false;
sendRequest.ensureMinRequiredFee = false;
sendRequest.changeAddress = getUnusedAddress();
try {
wallet.completeTx(sendRequest);
} catch (InsufficientMoneyException e) {
throw new InsufficientBsqException(e.missing);
}
checkWalletConsistency(wallet);
verifyTransaction(tx);
// printTx("prepareSendTx", tx);
return tx;
return getPreparedSendTx(receiverAddress, receiverAmount, bsqCoinSelector);
}

///////////////////////////////////////////////////////////////////////////////////////////
Expand All @@ -488,6 +479,11 @@ public Transaction getPreparedSendTx(String receiverAddress, Coin receiverAmount

public Transaction getPreparedSendBtcTx(String receiverAddress, Coin receiverAmount)
throws AddressFormatException, InsufficientBsqException, WalletException, TransactionVerificationException {
return getPreparedSendTx(receiverAddress, receiverAmount, nonBsqCoinSelector);
}

private Transaction getPreparedSendTx(String receiverAddress, Coin receiverAmount, CoinSelector coinSelector)
throws AddressFormatException, InsufficientBsqException, WalletException, TransactionVerificationException {
DaoKillSwitch.assertDaoIsNotDisabled();
Transaction tx = new Transaction(params);
checkArgument(Restrictions.isAboveDust(receiverAmount),
Expand All @@ -501,18 +497,18 @@ public Transaction getPreparedSendBtcTx(String receiverAddress, Coin receiverAmo
sendRequest.aesKey = aesKey;
sendRequest.shuffleOutputs = false;
sendRequest.signInputs = false;
sendRequest.ensureMinRequiredFee = false;
sendRequest.changeAddress = getUnusedAddress();
sendRequest.coinSelector = nonBsqCoinSelector;
sendRequest.changeAddress = getChangeAddress();
sendRequest.coinSelector = coinSelector;
try {
wallet.completeTx(sendRequest);
checkWalletConsistency(wallet);
verifyTransaction(tx);
// printTx("prepareSendTx", tx);
return tx;
} catch (InsufficientMoneyException e) {
log.error(e.toString());
throw new InsufficientBsqException(e.missing);
}
checkWalletConsistency(wallet);
verifyTransaction(tx);
// printTx("prepareSendTx", tx);
return tx;
}


Expand Down Expand Up @@ -550,7 +546,7 @@ private void addInputsAndChangeOutputForTx(Transaction tx, Coin fee, BsqCoinSele
Coin change = this.bsqCoinSelector.getChange(fee, coinSelection);
if (change.isPositive()) {
checkArgument(Restrictions.isAboveDust(change), "We must not get dust output here.");
tx.addOutput(change, getUnusedAddress());
tx.addOutput(change, getChangeAddress());
}
} catch (InsufficientMoneyException e) {
throw new InsufficientBsqException(e.missing);
Expand Down Expand Up @@ -638,6 +634,10 @@ protected Set<Address> getAllAddressesFromActiveKeys() {
collect(Collectors.toSet());
}

private Address getChangeAddress() {
return getUnusedAddress();
}

public Address getUnusedAddress() {
return wallet.getIssuedReceiveAddresses().stream()
.filter(this::isAddressUnused)
Expand Down
9 changes: 2 additions & 7 deletions core/src/main/java/bisq/core/btc/wallet/WalletService.java
Expand Up @@ -411,8 +411,7 @@ protected TransactionConfidence getMostRecentConfidence(List<TransactionConfiden
// Balance
///////////////////////////////////////////////////////////////////////////////////////////

// BalanceType.AVAILABLE
public Coin getAvailableBalance() {
public Coin getAvailableConfirmedBalance() {
return wallet != null ? wallet.getBalance(Wallet.BalanceType.AVAILABLE) : Coin.ZERO;
}

Expand Down Expand Up @@ -583,10 +582,6 @@ public DeterministicKey findKeyFromPubKey(byte[] pubKey) {
return wallet.getActiveKeyChain().findKeyFromPubKey(pubKey);
}

public Address freshReceiveAddress() {
return wallet.freshReceiveAddress();
}

public boolean isEncrypted() {
return wallet.isEncrypted();
}
Expand Down Expand Up @@ -722,7 +717,7 @@ void notifyBalanceListeners(Transaction tx) {
if (balanceListener.getAddress() != null)
balance = getBalanceForAddress(balanceListener.getAddress());
else
balance = getAvailableBalance();
balance = getAvailableConfirmedBalance();

balanceListener.onBalanceChanged(balance, tx);
}
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/java/bisq/core/btc/wallet/WalletsManager.java
Expand Up @@ -19,6 +19,7 @@

import bisq.core.btc.setup.WalletsSetup;
import bisq.core.crypto.ScryptUtil;
import bisq.core.dao.state.model.blockchain.TxType;
import bisq.core.locale.Res;

import bisq.common.handlers.ExceptionHandler;
Expand Down Expand Up @@ -146,13 +147,13 @@ public DeterministicSeed getDecryptedSeed(KeyParameter aesKey, DeterministicSeed
}

// A bsq tx has miner fees in btc included. Thus we need to handle it at both wallets.
public void publishAndCommitBsqTx(Transaction tx, TxBroadcaster.Callback callback) {
public void publishAndCommitBsqTx(Transaction tx, TxType txType, TxBroadcaster.Callback callback) {
// We need to create another instance, otherwise the tx would trigger an invalid state exception
// if it gets committed 2 times
// We clone before commit to avoid unwanted side effects
final Transaction clonedTx = btcWalletService.getClonedTransaction(tx);
btcWalletService.commitTx(clonedTx);
bsqWalletService.commitTx(tx);
bsqWalletService.commitTx(tx, txType);
bsqWalletService.broadcastTx(tx, callback);
}
}
2 changes: 2 additions & 0 deletions core/src/main/java/bisq/core/dao/DaoModule.java
Expand Up @@ -78,6 +78,7 @@
import bisq.core.dao.state.DaoStateStorageService;
import bisq.core.dao.state.GenesisTxInfo;
import bisq.core.dao.state.model.DaoState;
import bisq.core.dao.state.unconfirmed.UnconfirmedBsqChangeOutputListService;

import bisq.common.app.AppModule;

Expand Down Expand Up @@ -115,6 +116,7 @@ protected void configure() {
bind(DaoStateService.class).in(Singleton.class);
bind(DaoStateSnapshotService.class).in(Singleton.class);
bind(DaoStateStorageService.class).in(Singleton.class);
bind(UnconfirmedBsqChangeOutputListService.class).in(Singleton.class);

bind(ExportJsonFilesService.class).in(Singleton.class);

Expand Down
Expand Up @@ -32,6 +32,7 @@
import bisq.core.dao.state.model.blockchain.BaseTx;
import bisq.core.dao.state.model.blockchain.Block;
import bisq.core.dao.state.model.blockchain.Tx;
import bisq.core.dao.state.model.blockchain.TxType;
import bisq.core.dao.state.model.governance.RemoveAssetProposal;
import bisq.core.locale.CurrencyUtil;
import bisq.core.trade.statistics.TradeStatistics2;
Expand Down Expand Up @@ -338,7 +339,7 @@ public Coin getFeePerDay() {

public void publishTransaction(Transaction transaction, ResultHandler resultHandler,
ErrorMessageHandler errorMessageHandler) {
walletsManager.publishAndCommitBsqTx(transaction, new TxBroadcaster.Callback() {
walletsManager.publishAndCommitBsqTx(transaction, TxType.ASSET_LISTING_FEE, new TxBroadcaster.Callback() {
@Override
public void onSuccess(Transaction transaction) {
log.info("Asset listing fee tx has been published. TxId={}", transaction.getHashAsString());
Expand Down
Expand Up @@ -35,6 +35,7 @@
import bisq.core.dao.governance.proposal.MyProposalListService;
import bisq.core.dao.state.DaoStateListener;
import bisq.core.dao.state.DaoStateService;
import bisq.core.dao.state.model.blockchain.TxType;
import bisq.core.dao.state.model.governance.BallotList;
import bisq.core.dao.state.model.governance.CompensationProposal;
import bisq.core.dao.state.model.governance.DaoPhase;
Expand Down Expand Up @@ -323,7 +324,7 @@ public MeritList getMerits(@Nullable String blindVoteTxId) {

private void publishTx(ResultHandler resultHandler, ExceptionHandler exceptionHandler, Transaction blindVoteTx) {
log.info("blindVoteTx={}", blindVoteTx.toString());
walletsManager.publishAndCommitBsqTx(blindVoteTx, new TxBroadcaster.Callback() {
walletsManager.publishAndCommitBsqTx(blindVoteTx, TxType.BLIND_VOTE, new TxBroadcaster.Callback() {
@Override
public void onSuccess(Transaction transaction) {
log.info("BlindVote tx published. txId={}", transaction.getHashAsString());
Expand Down