Fixes #430 (#443)

* Fixes #430 * Supersedes PR #440 (now closed). * Fixes #430: Clarify and enhance the InterledgerErrorCodes that the SimpleStreamSender will fail-fast on and stop the stream. These include any F or R-family error, except F08 and F99. Any T-family error will NOT abort the sendMoney operation. * Fixes Invalid Denomination: If a receiver doesn't send back `ConnectionAssetDetails` frames to the sender during preflight, then the denomination will be absent. In this case, the FixedReceiverAmountPaymentTracker doesn't handle this condition properly. * Enhance unit & integration tests to cover these scenarios. * Add Javadoc in various places and cleanup formatting Signed-off-by: David Fuelling <[email protected]>
hyperledger-archives · Apr 1, 2020 · d3d4aa7 · d3d4aa7
1 parent 77f405c
commit d3d4aa7
Show file tree

Hide file tree

Showing 27 changed files with 1,129 additions and 465 deletions.
diff --git a/examples-parent/send-money/src/main/java/org/interledger/examples/SendMoneyExample.java b/examples-parent/send-money/src/main/java/org/interledger/examples/SendMoneyExample.java
@@ -16,7 +16,6 @@
 import org.interledger.stream.Denominations;
 import org.interledger.stream.SendMoneyRequest;
 import org.interledger.stream.SendMoneyResult;
-import org.interledger.stream.SenderAmountMode;
 import org.interledger.stream.sender.FixedSenderAmountPaymentTracker;
 import org.interledger.stream.sender.SimpleStreamSender;
 
@@ -72,7 +71,6 @@ public static void main(String[] args) throws ExecutionException, InterruptedExc
     SendMoneyResult result = simpleStreamSender.sendMoney(
         SendMoneyRequest.builder()
             .sourceAddress(SENDER_ADDRESS)
-            .senderAmountMode(SenderAmountMode.SENDER_AMOUNT)
             .amount(UnsignedLong.valueOf(100000))
             .denomination(Denominations.XRP)
             .destinationAddress(connectionDetails.destinationAddress())

diff --git a/...nt/link-ilp-over-http/src/main/java/org/interledger/link/http/IlpOverHttpLinkFactory.java b/...nt/link-ilp-over-http/src/main/java/org/interledger/link/http/IlpOverHttpLinkFactory.java
@@ -90,10 +90,6 @@ public Link<?> constructLink(
         decryptor.decrypt(outgoingLinkSettings.simpleAuthSettings().get().authToken().getBytes())
       ));
     } else {
-      // TODO: For now, we assume the bytes are a String that conform to the Crypt CLI. However, this should be made
-      // type-safe and more generic if possible. E.g., CryptoCLI formate vs Protobuf. Or, standardize on a single
-      // type-safe format?
-
       // NOTE: This supplier will always create a copy of the decrypted bytes so that the consumer of each call can
       // safely wipe the bytes from memory without affecting other callers.
       final SharedSecretBytesSupplier sharedSecretSupplier = () -> decryptor

diff --git a/...t/stream-client/src/main/java/org/interledger/stream/sender/AimdCongestionController.java b/...t/stream-client/src/main/java/org/interledger/stream/sender/AimdCongestionController.java
@@ -187,7 +187,7 @@ public void reject(final UnsignedLong prepareAmount, final InterledgerRejectPack
       default: {
         // No special treatment for unhandled errors, but warn just in case we start to see a lot of them.
         // Actual packet data is logged by the StreamSender, so no need to log packet details here.
-        logger.warn("For Congestion control purposes, ignoring unhandled packet rejection ({}: {}).",
+        logger.debug("For Congestion control purposes, ignoring unhandled packet rejection ({}: {}).",
             rejectPacket.getCode().getCode(), rejectPacket.getCode().getName()
         );
       }

diff --git a/...client/src/main/java/org/interledger/stream/sender/FixedReceiverAmountPaymentTracker.java b/...client/src/main/java/org/interledger/stream/sender/FixedReceiverAmountPaymentTracker.java
@@ -13,7 +13,6 @@
 import com.google.common.primitives.UnsignedLong;
 
 import java.util.Objects;
-import java.util.Optional;
 import java.util.concurrent.atomic.AtomicReference;
 
 /**
@@ -90,9 +89,7 @@ public UnsignedLong getDeliveredAmountInReceiverUnits() {
 
   @Override
   public PrepareAmounts getSendPacketAmounts(
-      final UnsignedLong congestionLimit,
-      final Denomination senderDenomination,
-      final Optional<Denomination> receiverDenomination
+      final UnsignedLong congestionLimit, final Denomination senderDenomination, final Denomination receiverDenomination
   ) {
     Objects.requireNonNull(congestionLimit);
     Objects.requireNonNull(senderDenomination);
@@ -101,12 +98,14 @@ public PrepareAmounts getSendPacketAmounts(
     if (congestionLimit.equals(UnsignedLong.ZERO) || amountLeftToDeliver.get().equals(UnsignedLong.ZERO)) {
       return PrepareAmounts.builder().amountToSend(UnsignedLong.ZERO).minimumAmountToAccept(UnsignedLong.ZERO).build();
     }
-    UnsignedLong amountToSendInSenderUnits =
-        rateCalculator.calculateAmountToSend(amountLeftToDeliver.get(), senderDenomination, receiverDenomination.get());
-    final UnsignedLong packetAmountToSend = StreamUtils.max(StreamUtils.min(amountToSendInSenderUnits, congestionLimit),
-        UnsignedLong.ONE);
-    UnsignedLong minAmountToAcceptInReceiverUnits =
+
+    final UnsignedLong amountToSendInSenderUnits = rateCalculator
+        .calculateAmountToSend(amountLeftToDeliver.get(), senderDenomination, receiverDenomination);
+    final UnsignedLong packetAmountToSend = StreamUtils
+        .max(StreamUtils.min(amountToSendInSenderUnits, congestionLimit), UnsignedLong.ONE);
+    final UnsignedLong minAmountToAcceptInReceiverUnits =
         rateCalculator.calculateMinAmountToAccept(packetAmountToSend, senderDenomination, receiverDenomination);
+
     return PrepareAmounts.builder()
         .minimumAmountToAccept(minAmountToAcceptInReceiverUnits)
         .amountToSend(packetAmountToSend)

diff --git a/...m-client/src/main/java/org/interledger/stream/sender/FixedSenderAmountPaymentTracker.java b/...m-client/src/main/java/org/interledger/stream/sender/FixedSenderAmountPaymentTracker.java
@@ -13,7 +13,6 @@
 import com.google.common.primitives.UnsignedLong;
 
 import java.util.Objects;
-import java.util.Optional;
 import java.util.concurrent.atomic.AtomicReference;
 
 /**
@@ -82,19 +81,13 @@ public UnsignedLong getDeliveredAmountInReceiverUnits() {
   }
 
   @Override
-  public PrepareAmounts getSendPacketAmounts(
-      final UnsignedLong congestionLimit,
-      final Denomination senderDenomination,
-      final Optional<Denomination> receiverDenomination
-  ) {
+  public PrepareAmounts getSendPacketAmounts(UnsignedLong congestionLimit, Denomination senderDenomination) {
     Objects.requireNonNull(congestionLimit);
     Objects.requireNonNull(senderDenomination);
-    Objects.requireNonNull(receiverDenomination);
 
     final UnsignedLong packetAmountToSend = StreamUtils.min(amountLeftToSend.get(), congestionLimit);
     return PrepareAmounts.builder()
-        .minimumAmountToAccept(
-            rateCalculator.calculateMinAmountToAccept(packetAmountToSend, senderDenomination, receiverDenomination))
+        .minimumAmountToAccept(rateCalculator.calculateMinAmountToAccept(packetAmountToSend, senderDenomination))
         .amountToSend(packetAmountToSend)
         .build();
   }