PaymentSession: proper future-style error handling for sendPayment()

This is a breaking change, but the existing code is not using futures properly and
fixing this is worth the breakage.

* Instead of returning `null`, return a failed future
* Instead of throwing errors, return failed futures
This commit is contained in:
Sean Gilligan 2022-03-25 19:03:22 +01:00 committed by Andreas Schildbach
parent 8f3ac79030
commit 091fdd9791
3 changed files with 69 additions and 30 deletions

View file

@ -21,6 +21,7 @@ import org.bitcoinj.crypto.TrustStoreLoader;
import org.bitcoinj.params.MainNetParams;
import org.bitcoinj.protocols.payments.PaymentProtocol.PkiVerificationData;
import org.bitcoinj.uri.BitcoinURI;
import org.bitcoinj.utils.FutureUtils;
import org.bitcoinj.utils.ListenableCompletableFuture;
import org.bitcoinj.utils.Threading;
import org.bitcoinj.wallet.SendRequest;
@ -304,24 +305,28 @@ public class PaymentSession {
* NOTE: This does not broadcast the transactions to the bitcoin network, it merely sends a Payment message to the
* merchant confirming the payment.
* Returns an object wrapping PaymentACK once received.
* If the PaymentRequest did not specify a payment_url, returns null and does nothing.
* If the PaymentRequest did not specify a payment_url, completes exceptionally.
* @param txns list of transactions to be included with the Payment message.
* @param refundAddr will be used by the merchant to send money back if there was a problem.
* @param memo is a message to include in the payment message sent to the merchant.
* @return a future for the PaymentACK
*/
@Nullable
public ListenableCompletableFuture<PaymentProtocol.Ack> sendPayment(List<Transaction> txns, @Nullable Address refundAddr, @Nullable String memo)
throws PaymentProtocolException, VerificationException, IOException {
Protos.Payment payment = getPayment(txns, refundAddr, memo);
public ListenableCompletableFuture<PaymentProtocol.Ack> sendPayment(List<Transaction> txns, @Nullable Address refundAddr, @Nullable String memo) {
Protos.Payment payment = null;
try {
payment = getPayment(txns, refundAddr, memo);
} catch (IOException | PaymentProtocolException.InvalidNetwork e) {
return ListenableCompletableFuture.failedFuture(e);
}
if (payment == null)
return null;
return ListenableCompletableFuture.failedFuture(new PaymentProtocolException.InvalidPaymentRequestURL("Missing Payment URL"));
if (isExpired())
throw new PaymentProtocolException.Expired("PaymentRequest is expired");
return ListenableCompletableFuture.failedFuture(new PaymentProtocolException.Expired("PaymentRequest is expired"));
URL url;
try {
url = new URL(paymentDetails.getPaymentUrl());
} catch (MalformedURLException e) {
throw new PaymentProtocolException.InvalidPaymentURL(e);
return ListenableCompletableFuture.failedFuture(new PaymentProtocolException.InvalidPaymentURL(e));
}
return sendPayment(url, payment);
}

View file

@ -37,10 +37,13 @@ import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import java.util.ArrayList;
import java.util.Date;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import static org.bitcoinj.core.Coin.COIN;
import static org.junit.Assert.*;
@ -116,7 +119,7 @@ public class PaymentSessionTest {
}
@Test
public void testExpiredPaymentRequest() throws Exception {
public void testExpiredPaymentRequest() throws PaymentProtocolException, IOException {
MockPaymentSession paymentSession = new MockPaymentSession(newExpiredPaymentRequest());
assertTrue(paymentSession.isExpired());
// Send the payment and verify that an exception is thrown.
@ -124,12 +127,19 @@ public class PaymentSessionTest {
tx.addInput(new TransactionInput(TESTNET, tx, outputToMe.getScriptBytes()));
ArrayList<Transaction> txns = new ArrayList<>();
txns.add(tx);
CompletableFuture<PaymentProtocol.Ack> ack = paymentSession.sendPayment(txns, null, null);
try {
paymentSession.sendPayment(txns, null, null);
} catch(PaymentProtocolException.Expired e) {
assertEquals(0, paymentSession.getPaymentLog().size());
assertEquals(e.getMessage(), "PaymentRequest is expired");
return;
ack.get();
} catch (ExecutionException e) {
if (e.getCause() instanceof PaymentProtocolException.Expired) {
PaymentProtocolException.Expired cause = (PaymentProtocolException.Expired) e.getCause();
assertEquals(0, paymentSession.getPaymentLog().size());
assertEquals(cause.getMessage(), "PaymentRequest is expired");
return;
}
} catch (InterruptedException e) {
// Ignore
}
fail("Expected exception due to expired PaymentRequest");
}
@ -146,7 +156,7 @@ public class PaymentSessionTest {
}
@Test(expected = PaymentProtocolException.InvalidNetwork.class)
public void testWrongNetwork() throws Exception {
public void testWrongNetwork() throws Throwable {
// Create a PaymentRequest and make sure the correct values are parsed by the PaymentSession.
MockPaymentSession paymentSession = new MockPaymentSession(newSimplePaymentRequest("main"));
assertEquals(MAINNET, paymentSession.getNetworkParameters());
@ -157,8 +167,14 @@ public class PaymentSessionTest {
ArrayList<Transaction> txns = new ArrayList<>();
txns.add(tx);
Address refundAddr = LegacyAddress.fromKey(TESTNET, serverKey);
paymentSession.sendPayment(txns, refundAddr, paymentMemo);
assertEquals(1, paymentSession.getPaymentLog().size());
try {
paymentSession.sendPayment(txns, refundAddr, paymentMemo).get();
} catch (InterruptedException e) {
fail("Incorrect exception type");
} catch (ExecutionException e) {
// We're expecting PaymentProtocolException.InvalidNetwork as the cause
throw e.getCause();
}
}
private Protos.PaymentRequest newSimplePaymentRequest(String netID) {

View file

@ -64,6 +64,7 @@ import org.bitcoinj.core.PeerGroup;
import org.bitcoinj.core.SegwitAddress;
import org.bitcoinj.core.StoredBlock;
import org.bitcoinj.core.Transaction;
import org.bitcoinj.core.TransactionBroadcast;
import org.bitcoinj.core.Utils;
import org.bitcoinj.core.VerificationException;
import org.bitcoinj.core.listeners.DownloadProgressTracker;
@ -892,22 +893,27 @@ public class WalletTool implements Callable<Integer> {
}
setup();
// No refund address specified, no user-specified memo field.
CompletableFuture<PaymentProtocol.Ack> future = session.sendPayment(ImmutableList.of(req.tx), null, null);
if (future == null) {
// No payment_url for submission so, broadcast and wait.
peerGroup.start();
peerGroup.broadcastTransaction(req.tx).future().get();
} else {
PaymentProtocol.Ack ack = future.get();
wallet.commitTx(req.tx);
System.out.println("Memo from server: " + ack.getMemo());
PaymentProtocol.Ack ack = session.sendPayment(ImmutableList.of(req.tx), null, null).get();
wallet.commitTx(req.tx);
System.out.println("Memo from server: " + ack.getMemo());
} catch (ExecutionException e) {
try {
throw e.getCause();
} catch (PaymentProtocolException.InvalidPaymentRequestURL e1) {
broadcastPayment(req);
} catch (PaymentProtocolException e1) {
System.err.println("Failed to send payment " + e.getMessage());
System.exit(1);
} catch (IOException e1) {
System.err.println("Invalid payment " + e.getMessage());
System.exit(1);
} catch (Throwable t) {
System.err.println("Unexpected error " + e.getMessage());
System.exit(1);
}
} catch (PaymentProtocolException | ExecutionException | VerificationException e) {
} catch (VerificationException e) {
System.err.println("Failed to send payment " + e.getMessage());
System.exit(1);
} catch (IOException e) {
System.err.println("Invalid payment " + e.getMessage());
System.exit(1);
} catch (InterruptedException e1) {
// Ignore.
} catch (InsufficientMoneyException e) {
@ -917,6 +923,18 @@ public class WalletTool implements Callable<Integer> {
}
}
private void broadcastPayment(SendRequest req) {
peerGroup.start();
TransactionBroadcast broadcast = peerGroup.broadcastTransaction(req.tx);
try {
// Wait for broadcast to be sent
broadcast.future().get();
} catch (InterruptedException | ExecutionException e) {
System.err.println("Failed to broadcast payment " + e.getMessage());
System.exit(1);
}
}
private void wait(WaitForEnum waitFor) throws BlockStoreException {
final CountDownLatch latch = new CountDownLatch(1);
setup();