diff --git a/core/src/main/java/org/bitcoinj/wallet/SendRequest.java b/core/src/main/java/org/bitcoinj/wallet/SendRequest.java index ef0046320..9d17fafd1 100644 --- a/core/src/main/java/org/bitcoinj/wallet/SendRequest.java +++ b/core/src/main/java/org/bitcoinj/wallet/SendRequest.java @@ -146,6 +146,12 @@ public class SendRequest { */ public String memo = null; + /** + * If false (default value), tx fee is paid by the sender If true, tx fee is paid by the recipient/s. If there is + * more than one recipient, the tx fee is split equally between them regardless of output value and size. + */ + public boolean recipientsPayFees = false; + // Tracks if this has been passed to wallet.completeTx already: just a safety check. boolean completed; @@ -262,6 +268,7 @@ public class SendRequest { helper.add("aesKey", aesKey != null ? "set" : null); // careful to not leak the key helper.add("coinSelector", coinSelector); helper.add("shuffleOutputs", shuffleOutputs); + helper.add("recipientsPayFees", recipientsPayFees); return helper.toString(); } } \ No newline at end of file diff --git a/core/src/main/java/org/bitcoinj/wallet/Wallet.java b/core/src/main/java/org/bitcoinj/wallet/Wallet.java index 74b84e16e..33a19a3b4 100644 --- a/core/src/main/java/org/bitcoinj/wallet/Wallet.java +++ b/core/src/main/java/org/bitcoinj/wallet/Wallet.java @@ -3964,11 +3964,13 @@ public class Wallet extends BaseTaggableObject CoinSelection bestCoinSelection; TransactionOutput bestChangeOutput = null; + List updatedOutputValues = null; if (!req.emptyWallet) { // This can throw InsufficientMoneyException. FeeCalculation feeCalculation = calculateFee(req, value, originalInputs, req.ensureMinRequiredFee, candidates); bestCoinSelection = feeCalculation.bestCoinSelection; bestChangeOutput = feeCalculation.bestChangeOutput; + updatedOutputValues = feeCalculation.updatedOutputValues; } else { // We're being asked to empty the wallet. What this means is ensuring "tx" has only a single output // of the total value we can currently spend as determined by the selector, and then subtracting the fee. @@ -3989,6 +3991,12 @@ public class Wallet extends BaseTaggableObject throw new CouldNotAdjustDownwards(); } + if (updatedOutputValues != null) { + for (int i = 0; i < updatedOutputValues.size(); i++) { + req.tx.getOutput(i).setValue(updatedOutputValues.get(i)); + } + } + if (bestChangeOutput != null) { req.tx.addOutput(bestChangeOutput); log.info(" with {} change", bestChangeOutput.getValue().toFriendlyString()); @@ -4807,8 +4815,12 @@ public class Wallet extends BaseTaggableObject /******************************************************************************************************************/ private static class FeeCalculation { + // Selected UTXOs to spend public CoinSelection bestCoinSelection; + // Change output (may be null if no change) public TransactionOutput bestChangeOutput; + // List of output values adjusted downwards when recipients pay fees (may be null if no adjustment needed). + public List updatedOutputValues; } //region Fee calculation code @@ -4816,183 +4828,113 @@ public class Wallet extends BaseTaggableObject public FeeCalculation calculateFee(SendRequest req, Coin value, List originalInputs, boolean needAtLeastReferenceFee, List candidates) throws InsufficientMoneyException { checkState(lock.isHeldByCurrentThread()); - // There are 3 possibilities for what adding change might do: - // 1) No effect - // 2) Causes increase in fee (change < 0.01 COINS) - // 3) Causes the transaction to have a dust output or change < fee increase (ie change will be thrown away) - // If we get either of the last 2, we keep note of what the inputs looked like at the time and try to - // add inputs as we go up the list (keeping track of minimum inputs for each category). At the end, we pick - // the best input set as the one which generates the lowest total fee. - Coin additionalValueForNextCategory = null; - CoinSelection selection3 = null; - CoinSelection selection2 = null; - TransactionOutput selection2Change = null; - CoinSelection selection1 = null; - TransactionOutput selection1Change = null; - // We keep track of the last size of the transaction we calculated. - int lastCalculatedSize = 0; - Coin valueNeeded, valueMissing = null; + FeeCalculation result; + Coin fee = Coin.ZERO; while (true) { - resetTxInputs(req, originalInputs); + result = new FeeCalculation(); + Transaction tx = new Transaction(params); + addSuppliedInputs(tx, req.tx.getInputs()); - Coin fees = req.feePerKb.multiply(lastCalculatedSize).divide(1000); - if (needAtLeastReferenceFee && fees.compareTo(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE) < 0) - fees = Transaction.REFERENCE_DEFAULT_MIN_TX_FEE; - - valueNeeded = value.add(fees); - if (additionalValueForNextCategory != null) - valueNeeded = valueNeeded.add(additionalValueForNextCategory); - Coin additionalValueSelected = additionalValueForNextCategory; - - // Of the coins we could spend, pick some that we actually will spend. + Coin valueNeeded = value; + if (!req.recipientsPayFees) { + valueNeeded = valueNeeded.add(fee); + } + if (req.recipientsPayFees) { + result.updatedOutputValues = new ArrayList(); + } + for (int i = 0; i < req.tx.getOutputs().size(); i++) { + TransactionOutput output = new TransactionOutput(params, tx, + req.tx.getOutputs().get(i).bitcoinSerialize(), 0); + if (req.recipientsPayFees) { + // Subtract fee equally from each selected recipient + output.setValue(output.getValue().subtract(fee.divide(req.tx.getOutputs().size()))); + // first receiver pays the remainder not divisible by output count + if (i == 0) { + output.setValue( + output.getValue().subtract(fee.divideAndRemainder(req.tx.getOutputs().size())[1])); // Subtract fee equally from each selected recipient + } + result.updatedOutputValues.add(output.getValue()); + if (output.getMinNonDustValue().isGreaterThan(output.getValue())) { + throw new CouldNotAdjustDownwards(); + } + } + tx.addOutput(output); + } CoinSelector selector = req.coinSelector == null ? coinSelector : req.coinSelector; // selector is allowed to modify candidates list. CoinSelection selection = selector.select(valueNeeded, new LinkedList<>(candidates)); + result.bestCoinSelection = selection; // Can we afford this? if (selection.valueGathered.compareTo(valueNeeded) < 0) { - valueMissing = valueNeeded.subtract(selection.valueGathered); - break; + Coin valueMissing = valueNeeded.subtract(selection.valueGathered); + throw new InsufficientMoneyException(valueMissing); } - checkState(selection.gathered.size() > 0 || originalInputs.size() > 0); - - // We keep track of an upper bound on transaction size to calculate fees that need to be added. - // Note that the difference between the upper bound and lower bound is usually small enough that it - // will be very rare that we pay a fee we do not need to. - // - // We can't be sure a selection is valid until we check fee per kb at the end, so we just store - // them here temporarily. - boolean eitherCategory2Or3 = false; - boolean isCategory3 = false; - Coin change = selection.valueGathered.subtract(valueNeeded); - if (additionalValueSelected != null) - change = change.add(additionalValueSelected); - - // If change is < 0.01 BTC, we will need to have at least minfee to be accepted by the network - if (req.ensureMinRequiredFee && !change.equals(Coin.ZERO) && - change.compareTo(Coin.CENT) < 0 && fees.compareTo(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE) < 0) { - // This solution may fit into category 2, but it may also be category 3, we'll check that later - eitherCategory2Or3 = true; - additionalValueForNextCategory = Coin.CENT; - // If the change is smaller than the fee we want to add, this will be negative - change = change.subtract(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.subtract(fees)); - } - - int size = 0; - TransactionOutput changeOutput = null; - if (change.signum() > 0) { + if (change.isGreaterThan(Coin.ZERO)) { // The value of the inputs is greater than what we want to send. Just like in real life then, // we need to take back some coins ... this is called "change". Add another output that sends the change // back to us. The address comes either from the request or currentChangeAddress() as a default. Address changeAddress = req.changeAddress; if (changeAddress == null) changeAddress = currentChangeAddress(); - changeOutput = new TransactionOutput(params, req.tx, change, changeAddress); - // If the change output would result in this transaction being rejected as dust, just drop the change and make it a fee - if (req.ensureMinRequiredFee && changeOutput.isDust()) { - // This solution definitely fits in category 3 - isCategory3 = true; - additionalValueForNextCategory = Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.add( - changeOutput.getMinNonDustValue().add(Coin.SATOSHI)); - } else { - size += changeOutput.unsafeBitcoinSerialize().length + VarInt.sizeOf(req.tx.getOutputs().size()) - VarInt.sizeOf(req.tx.getOutputs().size() - 1); - // This solution is either category 1 or 2 - if (!eitherCategory2Or3) // must be category 1 - additionalValueForNextCategory = null; + TransactionOutput changeOutput = new TransactionOutput(params, tx, change, changeAddress); + if (req.recipientsPayFees && changeOutput.isDust()) { + // We do not move dust-change to fees, because the sender would end up paying more than requested. + // This would be against the purpose of the all-inclusive feature. + // So instead we raise the change and deduct from the first recipient. + Coin missingToNotBeDust = changeOutput.getMinNonDustValue().subtract(changeOutput.getValue()); + changeOutput.setValue(changeOutput.getValue().add(missingToNotBeDust)); + TransactionOutput firstOutput = tx.getOutputs().get(0); + firstOutput.setValue(firstOutput.getValue().subtract(missingToNotBeDust)); + result.updatedOutputValues.set(0, firstOutput.getValue()); + if (firstOutput.isDust()) { + throw new CouldNotAdjustDownwards(); + } } - } else { - if (eitherCategory2Or3) { - // This solution definitely fits in category 3 (we threw away change because it was smaller than MIN_TX_FEE) - isCategory3 = true; - additionalValueForNextCategory = Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.add(Coin.SATOSHI); + if (changeOutput.isDust()) { + // Never create dust outputs; if we would, just + // add the dust to the fee. + // Oscar comment: This seems like a way to make the condition below "if + // (!fee.isLessThan(feeNeeded))" to become true. + // This is a non-easy to understand way to do that. + // Maybe there are other effects I am missing + fee = fee.add(changeOutput.getValue()); + } else { + tx.addOutput(changeOutput); + result.bestChangeOutput = changeOutput; } } - // Now add unsigned inputs for the selected coins. - for (TransactionOutput output : selection.gathered) { - TransactionInput input = req.tx.addInput(output); + for (TransactionOutput selectedOutput : selection.gathered) { + TransactionInput input = tx.addInput(selectedOutput); // If the scriptBytes don't default to none, our size calculations will be thrown off. checkState(input.getScriptBytes().length == 0); } - // Estimate transaction size and loop again if we need more fee per kb. The serialized tx doesn't - // include things we haven't added yet like input signatures/scripts or the change output. - size += req.tx.unsafeBitcoinSerialize().length; + int size = tx.unsafeBitcoinSerialize().length; size += estimateBytesForSigning(selection); - if (size > lastCalculatedSize && req.feePerKb.signum() > 0) { - lastCalculatedSize = size; - // We need more fees anyway, just try again with the same additional value - additionalValueForNextCategory = additionalValueSelected; - continue; + + Coin feePerKb = req.feePerKb; + if (needAtLeastReferenceFee && feePerKb.compareTo(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE) < 0) { + feePerKb = Transaction.REFERENCE_DEFAULT_MIN_TX_FEE; + } + Coin feeNeeded = feePerKb.multiply(size).divide(1000); + + if (!fee.isLessThan(feeNeeded)) { + // Done, enough fee included. + break; } - if (isCategory3) { - if (selection3 == null) - selection3 = selection; - } else if (eitherCategory2Or3) { - // If we are in selection2, we will require at least CENT additional. If we do that, there is no way - // we can end up back here because CENT additional will always get us to 1 - checkState(selection2 == null); - checkState(additionalValueForNextCategory.equals(Coin.CENT)); - selection2 = selection; - selection2Change = checkNotNull(changeOutput); // If we get no change in category 2, we are actually in category 3 - } else { - // Once we get a category 1 (change kept), we should break out of the loop because we can't do better - checkState(selection1 == null); - checkState(additionalValueForNextCategory == null); - selection1 = selection; - selection1Change = changeOutput; - } - - if (additionalValueForNextCategory != null) { - if (additionalValueSelected != null) - checkState(additionalValueForNextCategory.compareTo(additionalValueSelected) > 0); - continue; - } - break; - } - - resetTxInputs(req, originalInputs); - - if (selection3 == null && selection2 == null && selection1 == null) { - checkNotNull(valueMissing); - log.warn("Insufficient value in wallet for send: needed {} more", valueMissing.toFriendlyString()); - throw new InsufficientMoneyException(valueMissing); - } - - Coin lowestFee = null; - FeeCalculation result = new FeeCalculation(); - if (selection1 != null) { - if (selection1Change != null) - lowestFee = selection1.valueGathered.subtract(selection1Change.getValue()); - else - lowestFee = selection1.valueGathered; - result.bestCoinSelection = selection1; - result.bestChangeOutput = selection1Change; - } - - if (selection2 != null) { - Coin fee = selection2.valueGathered.subtract(checkNotNull(selection2Change).getValue()); - if (lowestFee == null || fee.compareTo(lowestFee) < 0) { - lowestFee = fee; - result.bestCoinSelection = selection2; - result.bestChangeOutput = selection2Change; - } - } - - if (selection3 != null) { - if (lowestFee == null || selection3.valueGathered.compareTo(lowestFee) < 0) { - result.bestCoinSelection = selection3; - result.bestChangeOutput = null; - } + // Include more fee and try again. + fee = feeNeeded; } return result; + } - private void resetTxInputs(SendRequest req, List originalInputs) { - req.tx.clearInputs(); + private void addSuppliedInputs(Transaction tx, List originalInputs) { for (TransactionInput input : originalInputs) - req.tx.addInput(input); + tx.addInput(new TransactionInput(params, tx, input.bitcoinSerialize())); } private int estimateBytesForSigning(CoinSelection selection) { diff --git a/core/src/test/java/org/bitcoinj/protocols/channels/PaymentChannelStateTest.java b/core/src/test/java/org/bitcoinj/protocols/channels/PaymentChannelStateTest.java index 98e9be8eb..92221a197 100644 --- a/core/src/test/java/org/bitcoinj/protocols/channels/PaymentChannelStateTest.java +++ b/core/src/test/java/org/bitcoinj/protocols/channels/PaymentChannelStateTest.java @@ -729,14 +729,18 @@ public class PaymentChannelStateTest extends TestWithWallet { assertEquals(PaymentChannelClientState.State.NEW, clientState.getState()); // We'll have to pay REFERENCE_DEFAULT_MIN_TX_FEE twice (multisig+refund), and we'll end up getting back nearly nothing... clientState.initiate(); - assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.multiply(2), clientState.getRefundTxFees()); + // Hardcoded tx length because actual length may vary depending on actual signature length + // The value is close to clientState.getContractInternal().unsafeBitcoinSerialize().length; + int contractSize = versionSelector == PaymentChannelClient.VersionSelector.VERSION_1 ? 273 : 225; + Coin expectedFees = Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.multiply(contractSize).divide(1000).add(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE); + assertEquals(expectedFees, clientState.getRefundTxFees()); assertEquals(getInitialClientState(), clientState.getState()); // Now actually use a more useful CENT clientState = makeClientState(wallet, myKey, ECKey.fromPublicOnly(serverKey.getPubKey()), CENT, EXPIRE_TIME); assertEquals(PaymentChannelClientState.State.NEW, clientState.getState()); clientState.initiate(); - assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.multiply(2), clientState.getRefundTxFees()); + assertEquals(expectedFees, clientState.getRefundTxFees()); assertEquals(getInitialClientState(), clientState.getState()); if (useRefunds()) { @@ -870,10 +874,12 @@ public class PaymentChannelStateTest extends TestWithWallet { pair.future.set(pair.tx); assertEquals(PaymentChannelServerState.State.READY, serverState.getState()); + int expectedSize = versionSelector == PaymentChannelClient.VersionSelector.VERSION_1 ? 271 : 355; + Coin expectedFee = Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.multiply(expectedSize).divide(1000); // Both client and server are now in the ready state, split the channel in half - byte[] signature = clientState.incrementPaymentBy(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.subtract(Coin.SATOSHI), null) + byte[] signature = clientState.incrementPaymentBy(expectedFee.subtract(Coin.SATOSHI), null) .signature.encodeToBitcoin(); - Coin totalRefund = CENT.subtract(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.subtract(SATOSHI)); + Coin totalRefund = CENT.subtract(expectedFee.subtract(SATOSHI)); serverState.incrementPayment(totalRefund, signature); // We need to pay MIN_TX_FEE, but we only have MIN_NONDUST_OUTPUT @@ -881,6 +887,7 @@ public class PaymentChannelStateTest extends TestWithWallet { serverState.close(); fail(); } catch (InsufficientMoneyException e) { + assertTrue(e.getMessage().contains("Insufficient money, missing ")); } // Now give the server enough coins to pay the fee @@ -894,8 +901,8 @@ public class PaymentChannelStateTest extends TestWithWallet { assertTrue(e.getMessage().contains("more in fees")); } - signature = clientState.incrementPaymentBy(SATOSHI, null).signature.encodeToBitcoin(); - totalRefund = totalRefund.subtract(SATOSHI); + signature = clientState.incrementPaymentBy(SATOSHI.multiply(20), null).signature.encodeToBitcoin(); + totalRefund = totalRefund.subtract(SATOSHI.multiply(20)); serverState.incrementPayment(totalRefund, signature); // And settle the channel. diff --git a/core/src/test/java/org/bitcoinj/wallet/WalletTest.java b/core/src/test/java/org/bitcoinj/wallet/WalletTest.java index 844732375..818e4f189 100644 --- a/core/src/test/java/org/bitcoinj/wallet/WalletTest.java +++ b/core/src/test/java/org/bitcoinj/wallet/WalletTest.java @@ -59,6 +59,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; import com.google.common.util.concurrent.ListenableFuture; import com.google.protobuf.ByteString; + import org.bitcoinj.wallet.Protos.Wallet.EncryptionType; import org.junit.After; import org.junit.Before; @@ -79,6 +80,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import junit.framework.Assert; import static org.bitcoinj.core.Coin.*; import static org.bitcoinj.core.Utils.HEX; import static org.bitcoinj.testing.FakeTxBuilder.*; @@ -2298,82 +2300,6 @@ public class WalletTest extends TestWithWallet { assertEquals(ZERO, wallet.getBalance()); } - @Test - public void basicFeeSolverTests() throws Exception { - sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, Coin.COIN); - - // Simple test to make sure if we have an ouput < 0.01 we get a fee - SendRequest request1 = SendRequest.to(OTHER_ADDRESS, CENT.subtract(SATOSHI)); - request1.ensureMinRequiredFee = true; - wallet.completeTx(request1); - Transaction spend1 = request1.tx; - assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, request1.tx.getFee()); - assertEquals(2, spend1.getOutputs().size()); - - // ...but not more fee than what we request - SendRequest request3 = SendRequest.to(OTHER_ADDRESS, CENT.subtract(SATOSHI)); - request3.feePerKb = Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.add(SATOSHI); - request3.ensureMinRequiredFee = true; - wallet.completeTx(request3); - assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, request3.tx.getFee()); - assertEquals(2, request3.tx.getOutputs().size()); - - // ...unless we need it - SendRequest request4 = SendRequest.to(OTHER_ADDRESS, CENT.subtract(SATOSHI)); - request4.feePerKb = Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.subtract(SATOSHI); - request4.ensureMinRequiredFee = true; - wallet.completeTx(request4); - assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, request4.tx.getFee()); - assertEquals(2, request4.tx.getOutputs().size()); - - // If we would have a change output < 0.01, it should add the fee - SendRequest request5 = SendRequest.to(OTHER_ADDRESS, Coin.COIN.subtract(CENT.subtract(SATOSHI))); - request5.ensureMinRequiredFee = true; - wallet.completeTx(request5); - assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, request5.tx.getFee()); - assertEquals(2, request5.tx.getOutputs().size()); - - // If change is 0.1-satoshi and we already have a 0.1-satoshi output, fee should be reference fee - SendRequest request7 = SendRequest.to(OTHER_ADDRESS, Coin.COIN.subtract(CENT.subtract(SATOSHI.multiply(2)).multiply(2))); - request7.ensureMinRequiredFee = true; - request7.tx.addOutput(CENT.subtract(SATOSHI), OTHER_ADDRESS); - wallet.completeTx(request7); - assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, request7.tx.getFee()); - assertEquals(3, request7.tx.getOutputs().size()); - - // If we would have a change output == REFERENCE_DEFAULT_MIN_TX_FEE that would cause a fee, throw it away and make it fee - SendRequest request8 = SendRequest.to(OTHER_ADDRESS, COIN.subtract(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE)); - request8.ensureMinRequiredFee = true; - wallet.completeTx(request8); - assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, request8.tx.getFee()); - assertEquals(1, request8.tx.getOutputs().size()); - - // ...in fact, also add fee if we would get back less than MIN_NONDUST_OUTPUT - SendRequest request9 = SendRequest.to(OTHER_ADDRESS, COIN.subtract( - Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.add(Transaction.MIN_NONDUST_OUTPUT).subtract(SATOSHI))); - request9.ensureMinRequiredFee = true; - wallet.completeTx(request9); - assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.add(Transaction.MIN_NONDUST_OUTPUT).subtract(SATOSHI), request9.tx.getFee()); - assertEquals(1, request9.tx.getOutputs().size()); - - // ...but if we get back any more than that, we should get a refund (but still pay fee) - SendRequest request10 = SendRequest.to(OTHER_ADDRESS, COIN.subtract( - Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.add(Transaction.MIN_NONDUST_OUTPUT))); - request10.ensureMinRequiredFee = true; - wallet.completeTx(request10); - assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, request10.tx.getFee()); - assertEquals(2, request10.tx.getOutputs().size()); - - // ...of course fee should be min(request.fee, MIN_TX_FEE) so we should get MIN_TX_FEE.add(SATOSHI) here - SendRequest request11 = SendRequest.to(OTHER_ADDRESS, COIN.subtract( - Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.add(Transaction.MIN_NONDUST_OUTPUT).add(SATOSHI.multiply(2)))); - request11.feePerKb = Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.add(SATOSHI); - request11.ensureMinRequiredFee = true; - wallet.completeTx(request11); - assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, request11.tx.getFee()); - assertEquals(2, request11.tx.getOutputs().size()); - } - @Test public void coinSelection_coinTimesDepth() throws Exception { Transaction txCent = sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, CENT); @@ -2442,7 +2368,6 @@ public class WalletTest extends TestWithWallet { wallet.completeTx(request15); assertEquals(Coin.valueOf(121300), request15.tx.getFee()); Transaction spend15 = request15.tx; - // If a transaction is over 1kb, 2 satoshis should be added. assertEquals(31, spend15.getOutputs().size()); // We optimize for priority, so the output selected should be the largest one assertEquals(1, spend15.getInputs().size()); @@ -2457,7 +2382,8 @@ public class WalletTest extends TestWithWallet { assertTrue(request16.tx.unsafeBitcoinSerialize().length > 1000); wallet.completeTx(request16); // Just the reference fee should be added if feePerKb == 0 - assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, request16.tx.getFee()); + // Hardcoded tx length because actual length may vary depending on actual signature length + assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.multiply(1213).divide(1000), request16.tx.getFee()); Transaction spend16 = request16.tx; assertEquals(31, spend16.getOutputs().size()); // We optimize for priority, so the output selected should be the largest one @@ -2559,17 +2485,19 @@ public class WalletTest extends TestWithWallet { assertEquals(COIN, request20.tx.getInput(0).getValue()); assertEquals(CENT, request20.tx.getInput(1).getValue()); - // Same as request 19, but make the change 0 (so it doesnt force fee) and make us require min fee as a - // result of an output < CENT. + // Same as request 19, but make the change 0 (so it doesnt force fee) and make us require min fee SendRequest request21 = SendRequest.to(OTHER_ADDRESS, CENT); request21.feePerKb = ZERO; request21.ensureMinRequiredFee = true; for (int i = 0; i < 99; i++) request21.tx.addOutput(CENT, OTHER_ADDRESS); - request21.tx.addOutput(CENT.subtract(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE), OTHER_ADDRESS); + //request21.tx.addOutput(CENT.subtract(Coin.valueOf(18880-10)), OTHER_ADDRESS); //fails because tx size is calculated with a change output + request21.tx.addOutput(CENT.subtract(Coin.valueOf(18880)), OTHER_ADDRESS); //3739 bytes, fee 5048 sat/kb + //request21.tx.addOutput(CENT.subtract(Coin.valueOf(500000)), OTHER_ADDRESS); //3774 bytes, fee 5003 sat/kb // If we send without a feePerKb, we should still require REFERENCE_DEFAULT_MIN_TX_FEE because we have an output < 0.01 wallet.completeTx(request21); - assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, request21.tx.getFee()); + // Hardcoded tx length because actual length may vary depending on actual signature length + assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.multiply(3776).divide(1000), request21.tx.getFee()); assertEquals(2, request21.tx.getInputs().size()); assertEquals(COIN, request21.tx.getInput(0).getValue()); assertEquals(CENT, request21.tx.getInput(1).getValue()); @@ -2604,21 +2532,21 @@ public class WalletTest extends TestWithWallet { sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, spendTx5); assertEquals(COIN, wallet.getBalance()); - // Ensure change is discarded if it results in a fee larger than the chain (same as 8 and 9 but with feePerKb) + // Ensure change is discarded if it is dust SendRequest request26 = SendRequest.to(OTHER_ADDRESS, CENT); for (int i = 0; i < 98; i++) request26.tx.addOutput(CENT, OTHER_ADDRESS); - request26.tx.addOutput(CENT.subtract( - Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.add(Transaction.MIN_NONDUST_OUTPUT).subtract(SATOSHI)), + // Hardcoded tx length because actual length may vary depending on actual signature length + Coin fee = Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.multiply(3560).divide(1000); + Coin dustMinusOne = Transaction.MIN_NONDUST_OUTPUT.subtract(SATOSHI); + request26.tx.addOutput(CENT.subtract(fee.add(dustMinusOne)), OTHER_ADDRESS); assertTrue(request26.tx.unsafeBitcoinSerialize().length > 1000); request26.feePerKb = SATOSHI; request26.ensureMinRequiredFee = true; wallet.completeTx(request26); - assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.add(Transaction.MIN_NONDUST_OUTPUT).subtract(SATOSHI), - request26.tx.getFee()); + assertEquals(fee.add(dustMinusOne), request26.tx.getFee()); Transaction spend26 = request26.tx; - // If a transaction is over 1kb, the set fee should be added assertEquals(100, spend26.getOutputs().size()); // We optimize for priority, so the output selected should be the largest one assertEquals(1, spend26.getInputs().size()); @@ -2626,108 +2554,124 @@ public class WalletTest extends TestWithWallet { } @Test - @Ignore("disabled for now as this test is not maintainable") - public void basicCategoryStepTest() throws Exception { - // Creates spends that step through the possible fee solver categories + public void recipientPaysFees() throws Exception { + sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, COIN); - // Generate a ton of small outputs - StoredBlock block = new StoredBlock(makeSolvedTestBlock(blockStore, OTHER_ADDRESS), BigInteger.ONE, 1); - int i = 0; - Coin tenThousand = Coin.valueOf(10000); - while (i <= 100) { - Transaction tx = createFakeTxWithChangeAddress(PARAMS, tenThousand, myAddress, OTHER_ADDRESS); - tx.getInput(0).setSequenceNumber(i++); // Keep every transaction unique - wallet.receiveFromBlock(tx, block, AbstractBlockChain.NewBlockType.BEST_CHAIN, i); - } - Coin balance = wallet.getBalance(); + // Simplest recipientPaysFees use case + Coin valueToSend = CENT.divide(2); + SendRequest request = SendRequest.to(OTHER_ADDRESS, valueToSend); + request.feePerKb = Transaction.DEFAULT_TX_FEE; + request.ensureMinRequiredFee = true; + request.recipientsPayFees = true; + request.shuffleOutputs = false; + wallet.completeTx(request); + // Hardcoded tx length because actual length may vary depending on actual signature length + Coin fee = request.feePerKb.multiply(227).divide(1000); + assertEquals(fee, request.tx.getFee()); + Transaction spend = request.tx; + assertEquals(2, spend.getOutputs().size()); + assertEquals(valueToSend.subtract(fee), spend.getOutput(0).getValue()); + assertEquals(COIN.subtract(valueToSend), spend.getOutput(1).getValue()); + assertEquals(1, spend.getInputs().size()); + assertEquals(COIN, spend.getInput(0).getValue()); - // Create a spend that will throw away change (category 3 type 2 in which the change causes fee which is worth more than change) - SendRequest request1 = SendRequest.to(OTHER_ADDRESS, balance.subtract(SATOSHI)); - request1.ensureMinRequiredFee = true; - wallet.completeTx(request1); - assertEquals(SATOSHI, request1.tx.getFee()); - assertEquals(request1.tx.getInputs().size(), i); // We should have spent all inputs - - // Give us one more input... - Transaction tx1 = createFakeTxWithChangeAddress(PARAMS, tenThousand, myAddress, OTHER_ADDRESS); - tx1.getInput(0).setSequenceNumber(i++); // Keep every transaction unique - wallet.receiveFromBlock(tx1, block, AbstractBlockChain.NewBlockType.BEST_CHAIN, i); - - // ... and create a spend that will throw away change (category 3 type 1 in which the change causes dust output) - SendRequest request2 = SendRequest.to(OTHER_ADDRESS, balance.subtract(SATOSHI)); + // Fee is split between the 2 outputs + SendRequest request2 = SendRequest.to(OTHER_ADDRESS, valueToSend); + request2.tx.addOutput(valueToSend, OTHER_ADDRESS); + request2.feePerKb = Transaction.DEFAULT_TX_FEE; request2.ensureMinRequiredFee = true; + request2.recipientsPayFees = true; + request2.shuffleOutputs = false; wallet.completeTx(request2); - assertEquals(SATOSHI, request2.tx.getFee()); - assertEquals(request2.tx.getInputs().size(), i - 1); // We should have spent all inputs - 1 + // Hardcoded tx length because actual length may vary depending on actual signature length + Coin fee2 = request2.feePerKb.multiply(261).divide(1000); + assertEquals(fee2, request2.tx.getFee()); + Transaction spend2 = request2.tx; + assertEquals(3, spend2.getOutputs().size()); + assertEquals(valueToSend.subtract(fee2.divide(2)), spend2.getOutput(0).getValue()); + assertEquals(valueToSend.subtract(fee2.divide(2)), spend2.getOutput(1).getValue()); + assertEquals(COIN.subtract(valueToSend.multiply(2)), spend2.getOutput(2).getValue()); + assertEquals(1, spend2.getInputs().size()); + assertEquals(COIN, spend2.getInput(0).getValue()); - // Give us one more input... - Transaction tx2 = createFakeTxWithChangeAddress(PARAMS, tenThousand, myAddress, OTHER_ADDRESS); - tx2.getInput(0).setSequenceNumber(i++); // Keep every transaction unique - wallet.receiveFromBlock(tx2, block, AbstractBlockChain.NewBlockType.BEST_CHAIN, i); - - // ... and create a spend that will throw away change (category 3 type 1 in which the change causes dust output) - // but that also could have been category 2 if it wanted - SendRequest request3 = SendRequest.to(OTHER_ADDRESS, CENT.add(tenThousand).subtract(SATOSHI)); + // Fee is split between the 3 outputs. Division has a remainder which is taken from the first output + SendRequest request3 = SendRequest.to(OTHER_ADDRESS, valueToSend); + request3.tx.addOutput(valueToSend, OTHER_ADDRESS); + request3.tx.addOutput(valueToSend, OTHER_ADDRESS); + request3.feePerKb = Transaction.DEFAULT_TX_FEE; request3.ensureMinRequiredFee = true; + request3.recipientsPayFees = true; + request3.shuffleOutputs = false; wallet.completeTx(request3); - assertEquals(SATOSHI, request3.tx.getFee()); - assertEquals(request3.tx.getInputs().size(), i - 2); // We should have spent all inputs - 2 + // Hardcoded tx length because actual length may vary depending on actual signature length + Coin fee3 = request3.feePerKb.multiply(295).divide(1000); + assertEquals(fee3, request3.tx.getFee()); + Transaction spend3 = request3.tx; + assertEquals(4, spend3.getOutputs().size()); + // 1st output pays the fee division remainder + assertEquals(valueToSend.subtract(fee3.divideAndRemainder(3)[0]).subtract(fee3.divideAndRemainder(3)[1]), + spend3.getOutput(0).getValue()); + assertEquals(valueToSend.subtract(fee3.divide(3)), spend3.getOutput(1).getValue()); + assertEquals(valueToSend.subtract(fee3.divide(3)), spend3.getOutput(2).getValue()); + assertEquals(COIN.subtract(valueToSend.multiply(3)), spend3.getOutput(3).getValue()); + assertEquals(1, spend3.getInputs().size()); + assertEquals(COIN, spend3.getInput(0).getValue()); - // - SendRequest request4 = SendRequest.to(OTHER_ADDRESS, balance.subtract(SATOSHI)); - request4.feePerKb = Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.divide(request3.tx.unsafeBitcoinSerialize().length); + // Output when subtracted fee is dust + // Hardcoded tx length because actual length may vary depending on actual signature length + Coin fee4 = Transaction.DEFAULT_TX_FEE.multiply(227).divide(1000); + valueToSend = fee4.add(Transaction.MIN_NONDUST_OUTPUT).subtract(SATOSHI); + SendRequest request4 = SendRequest.to(OTHER_ADDRESS, valueToSend); + request4.feePerKb = Transaction.DEFAULT_TX_FEE; request4.ensureMinRequiredFee = true; - wallet.completeTx(request4); - assertEquals(SATOSHI, request4.tx.getFee()); - assertEquals(request4.tx.getInputs().size(), i - 2); // We should have spent all inputs - 2 - - // Give us a few more inputs... - while (wallet.getBalance().compareTo(CENT.multiply(2)) < 0) { - Transaction tx3 = createFakeTxWithChangeAddress(PARAMS, tenThousand, myAddress, OTHER_ADDRESS); - tx3.getInput(0).setSequenceNumber(i++); // Keep every transaction unique - wallet.receiveFromBlock(tx3, block, AbstractBlockChain.NewBlockType.BEST_CHAIN, i); + request4.recipientsPayFees = true; + request4.shuffleOutputs = false; + try { + wallet.completeTx(request4); + fail("Expected CouldNotAdjustDownwards exception"); + } catch (Wallet.CouldNotAdjustDownwards e) { } - // ...that is just slightly less than is needed for category 1 - SendRequest request5 = SendRequest.to(OTHER_ADDRESS, CENT.add(tenThousand).subtract(SATOSHI)); + // Change is dust, so it is incremented to min non dust value. First output value is reduced to compensate. + // Hardcoded tx length because actual length may vary depending on actual signature length + Coin fee5 = Transaction.DEFAULT_TX_FEE.multiply(261).divide(1000); + valueToSend = COIN.divide(2).subtract(Coin.MICROCOIN); + SendRequest request5 = SendRequest.to(OTHER_ADDRESS, valueToSend); + request5.tx.addOutput(valueToSend, OTHER_ADDRESS); + request5.feePerKb = Transaction.DEFAULT_TX_FEE; request5.ensureMinRequiredFee = true; + request5.recipientsPayFees = true; + request5.shuffleOutputs = false; wallet.completeTx(request5); - assertEquals(SATOSHI, request5.tx.getFee()); - assertEquals(1, request5.tx.getOutputs().size()); // We should have no change output + assertEquals(fee5, request5.tx.getFee()); + Transaction spend5 = request5.tx; + assertEquals(3, spend5.getOutputs().size()); + Coin valueSubtractedFromFirstOutput = Transaction.MIN_NONDUST_OUTPUT + .subtract(COIN.subtract(valueToSend.multiply(2))); + assertEquals(valueToSend.subtract(fee5.divide(2)).subtract(valueSubtractedFromFirstOutput), + spend5.getOutput(0).getValue()); + assertEquals(valueToSend.subtract(fee5.divide(2)), spend5.getOutput(1).getValue()); + assertEquals(Transaction.MIN_NONDUST_OUTPUT, spend5.getOutput(2).getValue()); + assertEquals(1, spend5.getInputs().size()); + assertEquals(COIN, spend5.getInput(0).getValue()); - // Give us one more input... - Transaction tx4 = createFakeTxWithChangeAddress(PARAMS, tenThousand, myAddress, OTHER_ADDRESS); - tx4.getInput(0).setSequenceNumber(i); // Keep every transaction unique - wallet.receiveFromBlock(tx4, block, AbstractBlockChain.NewBlockType.BEST_CHAIN, i); - - // ... that puts us in category 1 (no fee!) - SendRequest request6 = SendRequest.to(OTHER_ADDRESS, CENT.add(tenThousand).subtract(SATOSHI)); + // Change is dust, so it is incremented to min non dust value. First output value is about to be reduced to + // compensate, but after subtracting some satoshis, first output is dust. + // Hardcoded tx length because actual length may vary depending on actual signature length + Coin fee6 = Transaction.DEFAULT_TX_FEE.multiply(261).divide(1000); + Coin valueToSend1 = fee6.divide(2).add(Transaction.MIN_NONDUST_OUTPUT).add(Coin.MICROCOIN); + Coin valueToSend2 = COIN.subtract(valueToSend1).subtract(Coin.MICROCOIN.multiply(2)); + SendRequest request6 = SendRequest.to(OTHER_ADDRESS, valueToSend1); + request6.tx.addOutput(valueToSend2, OTHER_ADDRESS); + request6.feePerKb = Transaction.DEFAULT_TX_FEE; request6.ensureMinRequiredFee = true; - wallet.completeTx(request6); - assertEquals(ZERO, request6.tx.getFee()); - assertEquals(2, request6.tx.getOutputs().size()); // We should have a change output - } - - @Test - public void testCategory2WithChange() throws Exception { - // Specifically target case 2 with significant change - - // Generate a ton of small outputs - StoredBlock block = new StoredBlock(makeSolvedTestBlock(blockStore, OTHER_ADDRESS), BigInteger.ONE, 1); - int i = 0; - while (i <= CENT.divide(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.multiply(10))) { - Transaction tx = createFakeTxWithChangeAddress(PARAMS, Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.multiply(10), myAddress, OTHER_ADDRESS); - tx.getInput(0).setSequenceNumber(i++); // Keep every transaction unique - wallet.receiveFromBlock(tx, block, AbstractBlockChain.NewBlockType.BEST_CHAIN, i); + request6.recipientsPayFees = true; + request6.shuffleOutputs = false; + try { + wallet.completeTx(request6); + fail("Expected CouldNotAdjustDownwards exception"); + } catch (Wallet.CouldNotAdjustDownwards e) { } - - // The selector will choose 2 with MIN_TX_FEE fee - SendRequest request1 = SendRequest.to(OTHER_ADDRESS, CENT.add(SATOSHI)); - request1.ensureMinRequiredFee = true; - wallet.completeTx(request1); - assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, request1.tx.getFee()); - assertEquals(request1.tx.getInputs().size(), i); // We should have spent all inputs - assertEquals(2, request1.tx.getOutputs().size()); // and gotten change back } @Test