mirrors/bitcoinj
1
0
Fork 0
mirror of https://github.com/bitcoinj/bitcoinj.git synced 2025-01-19 05:33:44 +01:00
Code Issues Packages Projects Releases Wiki Activity

Wallet: Remove global coinSelector and allowSpendingUnconfirmedTransactions().

Browse Source 
Coin selection is a per SendRequest, per createSend() or per getBalance() call affair.
Having it wallet-global can lead to race conditions, as sometimes experienced in the
WalletTest unit tests.
This commit is contained in:
Andreas Schildbach 2019-07-14 18:29:59 +02:00
parent a8be47b2d6
commit 726c7291ac
7 changed files with 291 additions and 313 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

68
core/src/main/java/org/bitcoinj/wallet/Wallet.java
View File

@ -230,7 +230,7 @@ public class Wallet extends BaseTaggableObject
// that was created after it. Useful when you believe some keys have been compromised.
private volatile long vKeyRotationTimestamp;
protected CoinSelector coinSelector = DefaultCoinSelector.get();
protected final CoinSelector coinSelector = DefaultCoinSelector.get();
// The wallet version. This is an int that can be used to track breaking changes in the wallet format.
// You can also use it to detect wallets that come from the future (ie they contain features you
@ -3952,9 +3952,49 @@ public class Wallet extends BaseTaggableObject
*/
public Transaction createSend(Address address, Coin value)
throws InsufficientMoneyException, BadWalletEncryptionKeyException {
return createSend(address, value, false);
}
/**
* <p>Statelessly creates a transaction that sends the given value to address. The change is sent to
* {@link Wallet#currentChangeAddress()}, so you must have added at least one key.</p>
*
* <p>If you just want to send money quickly, you probably want
* {@link Wallet#sendCoins(TransactionBroadcaster, Address, Coin)} instead. That will create the sending
* transaction, commit to the wallet and broadcast it to the network all in one go. This method is lower level
* and lets you see the proposed transaction before anything is done with it.</p>
*
* <p>This is a helper method that is equivalent to using {@link SendRequest#to(Address, Coin)}
* followed by {@link Wallet#completeTx(SendRequest)} and returning the requests transaction object.
* Note that this means a fee may be automatically added if required, if you want more control over the process,
* just do those two steps yourself.</p>
*
* <p>IMPORTANT: This method does NOT update the wallet. If you call createSend again you may get two transactions
* that spend the same coins. You have to call {@link Wallet#commitTx(Transaction)} on the created transaction to
* prevent this, but that should only occur once the transaction has been accepted by the network. This implies
* you cannot have more than one outstanding sending tx at once.</p>
*
* <p>You MUST ensure that the value is not smaller than {@link TransactionOutput#getMinNonDustValue()} or the transaction
* will almost certainly be rejected by the network as dust.</p>
*
* @param address The Bitcoin address to send the money to.
* @param value How much currency to send.
* @param allowUnconfirmed Wether to allow spending unconfirmed outputs.
* @return either the created Transaction or null if there are insufficient coins.
* @throws InsufficientMoneyException if the request could not be completed due to not enough balance.
* @throws DustySendRequested if the resultant transaction would violate the dust rules.
* @throws CouldNotAdjustDownwards if emptying the wallet was requested and the output can't be shrunk for fees without violating a protocol rule.
* @throws ExceededMaxTransactionSize if the resultant transaction is too big for Bitcoin to process.
* @throws MultipleOpReturnRequested if there is more than one OP_RETURN output for the resultant transaction.
* @throws BadWalletEncryptionKeyException if the supplied {@link SendRequest#aesKey} is wrong.
*/
public Transaction createSend(Address address, Coin value, boolean allowUnconfirmed)
throws InsufficientMoneyException, BadWalletEncryptionKeyException {
SendRequest req = SendRequest.to(address, value);
if (params.getId().equals(NetworkParameters.ID_UNITTESTNET))
req.shuffleOutputs = false;
if (allowUnconfirmed)
req.allowUnconfirmed();
completeTx(req);
return req.tx;
}
@ -4471,7 +4511,7 @@ public class Wallet extends BaseTaggableObject
return candidates;
}
/** Returns the {@link CoinSelector} object which controls which outputs can be spent by this wallet. */
/** Returns the default {@link CoinSelector} object that is used by this wallet if no custom selector is specified. */
public CoinSelector getCoinSelector() {
lock.lock();
try {
@ -4481,30 +4521,6 @@ public class Wallet extends BaseTaggableObject
}
}
/**
* A coin selector is responsible for choosing which outputs to spend when creating transactions. The default
* selector implements a policy of spending transactions that appeared in the best chain and pending transactions
* that were created by this wallet, but not others. You can override the coin selector for any given send
* operation by changing {@link SendRequest#coinSelector}.
*/
public void setCoinSelector(CoinSelector coinSelector) {
lock.lock();
try {
this.coinSelector = checkNotNull(coinSelector);
} finally {
lock.unlock();
}
}
/**
* Convenience wrapper for {@code setCoinSelector(Wallet.AllowUnconfirmedCoinSelector.get())}. If this method
* is called on the wallet then transactions will be used for spending regardless of their confidence. This can
* be dangerous - only use this if you absolutely know what you're doing!
*/
public void allowSpendingUnconfirmedTransactions() {
setCoinSelector(AllowUnconfirmedCoinSelector.get());
}
/**
* Get the {@link UTXOProvider}.
* @return The UTXO provider.

5
core/src/test/java/org/bitcoinj/core/ChainSplitTest.java
View File

@ -530,10 +530,9 @@ public class ChainSplitTest {
chain.add(b1);
// Send a couple of payments one after the other (so the second depends on the change output of the first).
wallet.allowSpendingUnconfirmedTransactions();
Transaction t2 = checkNotNull(wallet.createSend(LegacyAddress.fromKey(UNITTEST, new ECKey()), CENT));
Transaction t2 = checkNotNull(wallet.createSend(LegacyAddress.fromKey(UNITTEST, new ECKey()), CENT, true));
wallet.commitTx(t2);
Transaction t3 = checkNotNull(wallet.createSend(LegacyAddress.fromKey(UNITTEST, new ECKey()), CENT));
Transaction t3 = checkNotNull(wallet.createSend(LegacyAddress.fromKey(UNITTEST, new ECKey()), CENT, true));
wallet.commitTx(t3);
chain.add(FakeTxBuilder.makeSolvedTestBlock(b1, t2, t3));

5
core/src/test/java/org/bitcoinj/core/TransactionInputTest.java
View File

@ -38,13 +38,14 @@ public class TransactionInputTest {
@Test
public void testStandardWalletDisconnect() throws Exception {
Wallet w = Wallet.createDeterministic(new Context(UNITTEST), Script.ScriptType.P2PKH);
w.setCoinSelector(new AllowUnconfirmedCoinSelector());
Address a = w.currentReceiveAddress();
Transaction tx1 = FakeTxBuilder.createFakeTxWithoutChangeAddress(UNITTEST, Coin.COIN, a);
w.receivePending(tx1, null);
Transaction tx2 = new Transaction(UNITTEST);
tx2.addOutput(Coin.valueOf(99000000), new ECKey());
w.completeTx(SendRequest.forTx(tx2));
SendRequest req = SendRequest.forTx(tx2);
req.allowUnconfirmed();
w.completeTx(req);
TransactionInput txInToDisconnect = tx2.getInput(0);

78
core/src/test/java/org/bitcoinj/wallet/WalletTest.java
View File

@ -423,7 +423,7 @@ public class WalletTest extends TestWithWallet {
Threading.waitForUserCode();
final ListenableFuture<TransactionConfidence> depthFuture = t1.getConfidence().getDepthFuture(1);
assertFalse(depthFuture.isDone());
assertEquals(ZERO, wallet.getBalance());
assertEquals(ZERO, wallet.getBalance(Wallet.BalanceType.AVAILABLE));
assertEquals(amount, wallet.getBalance(Wallet.BalanceType.ESTIMATED));
assertFalse(availFuture.isDone());
// Our estimated balance has reached the requested level.
@ -842,12 +842,11 @@ public class WalletTest extends TestWithWallet {
buf[43] = 0; // Break the signature: bitcoinj won't check in SPV mode and this is easier than other mutations.
send1 = UNITTEST.getDefaultSerializer().makeTransaction(buf);
wallet.commitTx(send2);
wallet.allowSpendingUnconfirmedTransactions();
assertEquals(value, wallet.getBalance(Wallet.BalanceType.ESTIMATED));
assertEquals(value, wallet.getBalance(BalanceType.ESTIMATED));
// Now spend the change. This transaction should die permanently when the mutant appears in the chain.
Transaction send3 = checkNotNull(wallet.createSend(OTHER_ADDRESS, value));
Transaction send3 = checkNotNull(wallet.createSend(OTHER_ADDRESS, value, true));
wallet.commitTx(send3);
assertEquals(ZERO, wallet.getBalance());
assertEquals(ZERO, wallet.getBalance(BalanceType.AVAILABLE));
final LinkedList<TransactionConfidence> dead = new LinkedList<>();
final TransactionConfidence.Listener listener = new TransactionConfidence.Listener() {
@Override
@ -862,7 +861,7 @@ public class WalletTest extends TestWithWallet {
// Double spend!
sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, send1);
// Back to having one coin.
assertEquals(value, wallet.getBalance());
assertEquals(value, wallet.getBalance(BalanceType.AVAILABLE));
assertEquals(send2.getTxId(), dead.poll().getTransactionHash());
assertEquals(send3.getTxId(), dead.poll().getTransactionHash());
}
@ -951,10 +950,6 @@ public class WalletTest extends TestWithWallet {
//
// txA1 is in conflict with txA2 and txA3. txB1 is in conflict with txB2.
CoinSelector originalCoinSelector = wallet.getCoinSelector();
try {
wallet.allowSpendingUnconfirmedTransactions();
Transaction txARoot = sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, valueOf(10, 0));
SendRequest a1Req = SendRequest.to(OTHER_ADDRESS, valueOf(1, 0));
a1Req.tx.addInput(txARoot.getOutput(0));
@ -1061,8 +1056,7 @@ public class WalletTest extends TestWithWallet {
assertDead(txD2);
// A reorg: previous block "replaced" by new block containing txA1, txB1 and txC1
FakeTxBuilder.BlockPair blockPair3 = createFakeBlock(blockStore, blockPair0.storedBlock, 2, txA1, txB1,
txC1);
FakeTxBuilder.BlockPair blockPair3 = createFakeBlock(blockStore, blockPair0.storedBlock, 2, txA1, txB1, txC1);
wallet.receiveFromBlock(txA1, blockPair3.storedBlock, AbstractBlockChain.NewBlockType.SIDE_CHAIN, 0);
wallet.receiveFromBlock(txB1, blockPair3.storedBlock, AbstractBlockChain.NewBlockType.SIDE_CHAIN, 1);
wallet.receiveFromBlock(txC1, blockPair3.storedBlock, AbstractBlockChain.NewBlockType.SIDE_CHAIN, 2);
@ -1121,10 +1115,6 @@ public class WalletTest extends TestWithWallet {
assertDead(txC2);
assertDead(txD1);
assertDead(txD2);
} finally {
wallet.setCoinSelector(originalCoinSelector);
}
}
@Test
@ -1167,13 +1157,9 @@ public class WalletTest extends TestWithWallet {
@Test
public void doubleSpendForBuildingTx() throws Exception {
CoinSelector originalCoinSelector = wallet.getCoinSelector();
try {
wallet.allowSpendingUnconfirmedTransactions();
sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, valueOf(2, 0));
Transaction send1 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 0)));
Transaction send2 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 20)));
Transaction send1 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 0), true));
Transaction send2 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 20), true));
sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, send1);
assertUnspent(send1);
@ -1181,24 +1167,16 @@ public class WalletTest extends TestWithWallet {
wallet.receivePending(send2, null);
assertUnspent(send1);
assertDead(send2);
} finally {
wallet.setCoinSelector(originalCoinSelector);
}
}
@Test
public void txSpendingDeadTx() throws Exception {
CoinSelector originalCoinSelector = wallet.getCoinSelector();
try {
wallet.allowSpendingUnconfirmedTransactions();
sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, valueOf(2, 0));
Transaction send1 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 0)));
Transaction send2 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 20)));
Transaction send1 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 0), true));
Transaction send2 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 20), true));
wallet.commitTx(send1);
assertPending(send1);
Transaction send1b = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 50)));
Transaction send1b = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 50), true));
sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, send2);
assertDead(send1);
@ -1208,10 +1186,6 @@ public class WalletTest extends TestWithWallet {
assertDead(send1);
assertUnspent(send2);
assertDead(send1b);
} finally {
wallet.setCoinSelector(originalCoinSelector);
}
}
private void assertInConflict(Transaction tx) {
@ -1241,16 +1215,13 @@ public class WalletTest extends TestWithWallet {
@Test
public void testAddTransactionsDependingOn() throws Exception {
CoinSelector originalCoinSelector = wallet.getCoinSelector();
try {
wallet.allowSpendingUnconfirmedTransactions();
sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, valueOf(2, 0));
Transaction send1 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 0)));
Transaction send2 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 20)));
Transaction send1 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 0), true));
Transaction send2 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 20), true));
wallet.commitTx(send1);
Transaction send1b = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 50)));
Transaction send1b = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 50), true));
wallet.commitTx(send1b);
Transaction send1c = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 25)));
Transaction send1c = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 25), true));
wallet.commitTx(send1c);
wallet.commitTx(send2);
Set<Transaction> txns = new HashSet<>();
@ -1260,26 +1231,20 @@ public class WalletTest extends TestWithWallet {
assertTrue(txns.contains(send1));
assertTrue(txns.contains(send1b));
assertTrue(txns.contains(send1c));
} finally {
wallet.setCoinSelector(originalCoinSelector);
}
}
@Test
public void sortTxnsByDependency() throws Exception {
CoinSelector originalCoinSelector = wallet.getCoinSelector();
try {
wallet.allowSpendingUnconfirmedTransactions();
Transaction send1 = sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, valueOf(2, 0));
Transaction send1a = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 0)));
Transaction send1a = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 0), true));
wallet.commitTx(send1a);
Transaction send1b = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 50)));
Transaction send1b = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 50), true));
wallet.commitTx(send1b);
Transaction send1c = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 25)));
Transaction send1c = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 25), true));
wallet.commitTx(send1c);
Transaction send1d = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 12)));
Transaction send1d = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 12), true));
wallet.commitTx(send1d);
Transaction send1e = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 06)));
Transaction send1e = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 06), true));
wallet.commitTx(send1e);
Transaction send2 = sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, valueOf(200, 0));
@ -1320,9 +1285,6 @@ public class WalletTest extends TestWithWallet {
assertTrue(sortedTxns.indexOf(send1d) < sortedTxns.indexOf(send1e));
assertTrue(sortedTxns.indexOf(send2a) < sortedTxns.indexOf(send2b));
assertTrue(sortedTxns.indexOf(send2b) < sortedTxns.indexOf(send2c));
} finally {
wallet.setCoinSelector(originalCoinSelector);
}
}
@Test

2
tools/src/main/java/org/bitcoinj/tools/WalletTool.java
View File

@ -602,7 +602,7 @@ public class WalletTool {
if (feePerVkb != null)
req.setFeePerVkb(feePerVkb);
if (allowUnconfirmed) {
wallet.allowSpendingUnconfirmedTransactions();
req.allowUnconfirmed();
}
if (password != null) {
req.aesKey = passwordToKey(true);

3
wallettemplate/src/main/java/wallettemplate/Main.java
View File

@ -139,9 +139,6 @@ public class Main extends Application {
bitcoin = new WalletAppKit(params, PREFERRED_OUTPUT_SCRIPT_TYPE, null, appDataDirectory, WALLET_FILE_NAME) {
@Override
protected void onSetupCompleted() {
// Don't make the user wait for confirmations for now, as the intention is they're sending it
// their own money!
bitcoin.wallet().allowSpendingUnconfirmedTransactions();
Platform.runLater(controller::onBitcoinSetup);
}
};

3
wallettemplate/src/main/java/wallettemplate/SendMoneyController.java
View File

@ -78,6 +78,9 @@ public class SendMoneyController {
else
req = SendRequest.to(destination, amount);
req.aesKey = aesKey;
// Don't make the user wait for confirmations for now, as the intention is they're sending it
// their own money!
req.allowUnconfirmed();
sendResult = Main.bitcoin.wallet().sendCoins(req);
Futures.addCallback(sendResult.broadcastComplete, new FutureCallback<Transaction>() {
@Override