From a13602e3d25fa068b730a021aee58e2dc45d4f4b Mon Sep 17 00:00:00 2001 From: Carsten Otto Date: Sat, 14 May 2022 00:07:22 +0200 Subject: [PATCH] extend failure code handling --- .../service/LiquidityInformationUpdater.java | 19 ++--- .../LiquidityInformationUpdaterTest.java | 76 +++++++++++++++---- .../cotto/lndmanagej/model/FailureCode.java | 8 ++ .../lndmanagej/model/FailureCodeTest.java | 25 ++++++ 4 files changed, 101 insertions(+), 27 deletions(-) diff --git a/backend/src/main/java/de/cotto/lndmanagej/service/LiquidityInformationUpdater.java b/backend/src/main/java/de/cotto/lndmanagej/service/LiquidityInformationUpdater.java index 4356094b..69faa90d 100644 --- a/backend/src/main/java/de/cotto/lndmanagej/service/LiquidityInformationUpdater.java +++ b/backend/src/main/java/de/cotto/lndmanagej/service/LiquidityInformationUpdater.java @@ -9,20 +9,19 @@ import de.cotto.lndmanagej.model.HexString; import de.cotto.lndmanagej.model.PaymentAttemptHop; import de.cotto.lndmanagej.model.PaymentListener; import de.cotto.lndmanagej.model.Pubkey; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.springframework.stereotype.Service; import java.util.List; import java.util.Optional; import java.util.function.Function; +import static de.cotto.lndmanagej.model.FailureCode.TEMPORARY_CHANNEL_FAILURE; + @Service public class LiquidityInformationUpdater implements PaymentListener { private final GrpcGetInfo grpcGetInfo; private final GrpcChannelPolicy grpcChannelPolicy; private final LiquidityBoundsService liquidityBoundsService; - private final Logger logger = LoggerFactory.getLogger(getClass()); public LiquidityInformationUpdater( GrpcGetInfo grpcGetInfo, @@ -56,14 +55,12 @@ public class LiquidityInformationUpdater implements PaymentListener { @Override public void failure(List paymentAttemptHops, FailureCode failureCode, int failureSourceIndex) { removeInFlight(paymentAttemptHops); - switch (failureCode) { - case TEMPORARY_CHANNEL_FAILURE -> - markAvailableAndUnavailable(paymentAttemptHops, failureSourceIndex, PaymentAttemptHop::amount); - case UNKNOWN_NEXT_PEER, CHANNEL_DISABLED, FEE_INSUFFICIENT -> - markAvailableAndUnavailable(paymentAttemptHops, failureSourceIndex, hop -> Coins.ofSatoshis(2)); - case MPP_TIMEOUT, INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS -> - markAllAvailable(paymentAttemptHops, failureSourceIndex); - default -> logger.warn("Unknown failure code {}", failureCode); + if (TEMPORARY_CHANNEL_FAILURE.equals(failureCode)) { + markAvailableAndUnavailable(paymentAttemptHops, failureSourceIndex, PaymentAttemptHop::amount); + } else if (failureCode.isErrorFromFinalNode()) { + markAllAvailable(paymentAttemptHops, failureSourceIndex); + } else { + markAvailableAndUnavailable(paymentAttemptHops, failureSourceIndex, hop -> Coins.ofSatoshis(2)); } } diff --git a/backend/src/test/java/de/cotto/lndmanagej/service/LiquidityInformationUpdaterTest.java b/backend/src/test/java/de/cotto/lndmanagej/service/LiquidityInformationUpdaterTest.java index a195afcb..2bc58a9f 100644 --- a/backend/src/test/java/de/cotto/lndmanagej/service/LiquidityInformationUpdaterTest.java +++ b/backend/src/test/java/de/cotto/lndmanagej/service/LiquidityInformationUpdaterTest.java @@ -11,6 +11,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; @@ -22,8 +24,11 @@ import static de.cotto.lndmanagej.model.ChannelIdFixtures.CHANNEL_ID; import static de.cotto.lndmanagej.model.ChannelIdFixtures.CHANNEL_ID_2; import static de.cotto.lndmanagej.model.ChannelIdFixtures.CHANNEL_ID_3; import static de.cotto.lndmanagej.model.FailureCode.CHANNEL_DISABLED; -import static de.cotto.lndmanagej.model.FailureCode.FEE_INSUFFICIENT; +import static de.cotto.lndmanagej.model.FailureCode.FINAL_EXPIRY_TOO_SOON; +import static de.cotto.lndmanagej.model.FailureCode.FINAL_INCORRECT_CLTV_EXPIRY; +import static de.cotto.lndmanagej.model.FailureCode.FINAL_INCORRECT_HTLC_AMOUNT; import static de.cotto.lndmanagej.model.FailureCode.INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS; +import static de.cotto.lndmanagej.model.FailureCode.INCORRECT_PAYMENT_AMOUNT; import static de.cotto.lndmanagej.model.FailureCode.MPP_TIMEOUT; import static de.cotto.lndmanagej.model.FailureCode.TEMPORARY_CHANNEL_FAILURE; import static de.cotto.lndmanagej.model.FailureCode.UNKNOWN_NEXT_PEER; @@ -318,11 +323,7 @@ class LiquidityInformationUpdaterTest { @Test void after_last_hop_from_receiver() { - liquidityInformationUpdater.failure(hopsWithChannelIdsAndPubkeys, MPP_TIMEOUT, 3); - verify(liquidityBoundsService).markAsAvailable(PUBKEY, PUBKEY_2, Coins.ofSatoshis(100)); - verify(liquidityBoundsService).markAsAvailable(PUBKEY_2, PUBKEY_3, Coins.ofSatoshis(90)); - verify(liquidityBoundsService).markAsAvailable(PUBKEY_3, PUBKEY_4, Coins.ofSatoshis(80)); - verifyRemovesInFlightForAllHops(); + assertAllAvailableForFailureFromFinalNode(MPP_TIMEOUT); } @Test @@ -373,11 +374,7 @@ class LiquidityInformationUpdaterTest { @Test void after_last_hop_from_receiver() { - liquidityInformationUpdater.failure(hopsWithChannelIdsAndPubkeys, INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS, 3); - verify(liquidityBoundsService).markAsAvailable(PUBKEY, PUBKEY_2, Coins.ofSatoshis(100)); - verify(liquidityBoundsService).markAsAvailable(PUBKEY_2, PUBKEY_3, Coins.ofSatoshis(90)); - verify(liquidityBoundsService).markAsAvailable(PUBKEY_3, PUBKEY_4, Coins.ofSatoshis(80)); - verifyRemovesInFlightForAllHops(); + assertAllAvailableForFailureFromFinalNode(INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS); } @Test @@ -404,22 +401,61 @@ class LiquidityInformationUpdaterTest { } // CPD-ON - @Test - void feeInsufficient_is_treated_as_channel_failure() { - liquidityInformationUpdater.failure(hopsWithChannelIds, FEE_INSUFFICIENT, 2); + @ParameterizedTest + @EnumSource(value = FailureCode.class, names = { + "INVALID_REALM", + "EXPIRY_TOO_SOON", + "INVALID_ONION_VERSION", + "INVALID_ONION_HMAC", + "INVALID_ONION_KEY", + "AMOUNT_BELOW_MINIMUM", + "FEE_INSUFFICIENT", + "INCORRECT_CLTV_EXPIRY", + "CHANNEL_DISABLED", + "REQUIRED_NODE_FEATURE_MISSING", + "REQUIRED_CHANNEL_FEATURE_MISSING", + "UNKNOWN_NEXT_PEER", + "TEMPORARY_NODE_FAILURE", + "PERMANENT_NODE_FAILURE", + "PERMANENT_CHANNEL_FAILURE", + "EXPIRY_TOO_FAR", + "INVALID_ONION_PAYLOAD", + "UNKNOWN_FAILURE" + }) + void channel_should_be_treated_as_unavailable(FailureCode failureCode) { + liquidityInformationUpdater.failure(hopsWithChannelIds, failureCode, 2); verifyRemovesInFlightForAllHops(); verify(liquidityBoundsService).markAsAvailable(PUBKEY, PUBKEY_2, Coins.ofSatoshis(100)); verify(liquidityBoundsService).markAsAvailable(PUBKEY_2, PUBKEY_3, Coins.ofSatoshis(90)); - // see ChannelDisabled + // see ChannelDisabled test class verify(liquidityBoundsService).markAsUnavailable(PUBKEY_3, PUBKEY_4, Coins.ofSatoshis(2)); } + @Test + void incorrect_payment_amount_on_final_node_marks_everything_as_available() { + assertAllAvailableForFailureFromFinalNode(INCORRECT_PAYMENT_AMOUNT); + } + + @Test + void final_incorrect_cltv_expiry_on_final_node_marks_everything_as_available() { + assertAllAvailableForFailureFromFinalNode(FINAL_INCORRECT_CLTV_EXPIRY); + } + + @Test + void final_incorrect_htlc_amount_on_final_node_marks_everything_as_available() { + assertAllAvailableForFailureFromFinalNode(FINAL_INCORRECT_HTLC_AMOUNT); + } + + @Test + void final_expiry_too_soon_on_final_node_marks_everything_as_available() { + assertAllAvailableForFailureFromFinalNode(FINAL_EXPIRY_TOO_SOON); + } + @Test void unknown_failure_code() { liquidityInformationUpdater.failure(hopsWithChannelIdsAndPubkeys, FailureCode.UNKNOWN_FAILURE, 2); verifyRemovesInFlightForAllHops(); - verifyNoMoreInteractions(liquidityBoundsService); } @Test @@ -429,6 +465,14 @@ class LiquidityInformationUpdaterTest { verifyNoMoreInteractions(liquidityBoundsService); } + private void assertAllAvailableForFailureFromFinalNode(FailureCode failureCode) { + liquidityInformationUpdater.failure(hopsWithChannelIdsAndPubkeys, failureCode, 3); + verify(liquidityBoundsService).markAsAvailable(PUBKEY, PUBKEY_2, Coins.ofSatoshis(100)); + verify(liquidityBoundsService).markAsAvailable(PUBKEY_2, PUBKEY_3, Coins.ofSatoshis(90)); + verify(liquidityBoundsService).markAsAvailable(PUBKEY_3, PUBKEY_4, Coins.ofSatoshis(80)); + verifyRemovesInFlightForAllHops(); + } + private void verifyRemovesInFlightForAllHops() { verify(liquidityBoundsService).markAsInFlight(PUBKEY, PUBKEY_2, Coins.ofSatoshis(-100)); verify(liquidityBoundsService).markAsInFlight(PUBKEY_2, PUBKEY_3, Coins.ofSatoshis(-90)); diff --git a/model/src/main/java/de/cotto/lndmanagej/model/FailureCode.java b/model/src/main/java/de/cotto/lndmanagej/model/FailureCode.java index 4f2daa21..b2369230 100644 --- a/model/src/main/java/de/cotto/lndmanagej/model/FailureCode.java +++ b/model/src/main/java/de/cotto/lndmanagej/model/FailureCode.java @@ -45,4 +45,12 @@ public enum FailureCode { return UNKNOWN_FAILURE; }); } + + public boolean isErrorFromFinalNode() { + return this == MPP_TIMEOUT + || this == INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS + || this == FINAL_INCORRECT_CLTV_EXPIRY + || this == FINAL_INCORRECT_HTLC_AMOUNT + || this == FINAL_EXPIRY_TOO_SOON; + } } diff --git a/model/src/test/java/de/cotto/lndmanagej/model/FailureCodeTest.java b/model/src/test/java/de/cotto/lndmanagej/model/FailureCodeTest.java index 611d42a0..e86b654a 100644 --- a/model/src/test/java/de/cotto/lndmanagej/model/FailureCodeTest.java +++ b/model/src/test/java/de/cotto/lndmanagej/model/FailureCodeTest.java @@ -33,125 +33,150 @@ class FailureCodeTest { @Test void getFor_unknown() { assertThat(FailureCode.getFor(99)).isEqualTo(UNKNOWN_FAILURE); + assertThat(UNKNOWN_FAILURE.isErrorFromFinalNode()).isFalse(); } @Test void incorrectOrUnknownPaymentDetails() { assertThat(FailureCode.getFor(1)).isEqualTo(INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS); + assertThat(INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS.isErrorFromFinalNode()).isTrue(); } @Test void incorrectPaymentAmount() { assertThat(FailureCode.getFor(2)).isEqualTo(INCORRECT_PAYMENT_AMOUNT); + assertThat(INCORRECT_PAYMENT_AMOUNT.isErrorFromFinalNode()).isFalse(); } @Test void finalIncorrectCltvExpiry() { assertThat(FailureCode.getFor(3)).isEqualTo(FINAL_INCORRECT_CLTV_EXPIRY); + assertThat(FINAL_INCORRECT_CLTV_EXPIRY.isErrorFromFinalNode()).isTrue(); } @Test void finalIncorrectHtlcAmount() { assertThat(FailureCode.getFor(4)).isEqualTo(FINAL_INCORRECT_HTLC_AMOUNT); + assertThat(FINAL_INCORRECT_HTLC_AMOUNT.isErrorFromFinalNode()).isTrue(); } @Test void finalExpiryTooSoon() { assertThat(FailureCode.getFor(5)).isEqualTo(FINAL_EXPIRY_TOO_SOON); + assertThat(FINAL_EXPIRY_TOO_SOON.isErrorFromFinalNode()).isTrue(); } @Test void invalidRealm() { assertThat(FailureCode.getFor(6)).isEqualTo(INVALID_REALM); + assertThat(INVALID_REALM.isErrorFromFinalNode()).isFalse(); } @Test void expiryTooSoon() { assertThat(FailureCode.getFor(7)).isEqualTo(EXPIRY_TOO_SOON); + assertThat(EXPIRY_TOO_SOON.isErrorFromFinalNode()).isFalse(); } @Test void invalidOnionVersion() { assertThat(FailureCode.getFor(8)).isEqualTo(INVALID_ONION_VERSION); + assertThat(INVALID_ONION_VERSION.isErrorFromFinalNode()).isFalse(); } @Test void invalidOnionHmac() { assertThat(FailureCode.getFor(9)).isEqualTo(INVALID_ONION_HMAC); + assertThat(INVALID_ONION_HMAC.isErrorFromFinalNode()).isFalse(); } @Test void invalidOnionKey() { assertThat(FailureCode.getFor(10)).isEqualTo(INVALID_ONION_KEY); + assertThat(INVALID_ONION_KEY.isErrorFromFinalNode()).isFalse(); } @Test void amountBelowMinimum() { assertThat(FailureCode.getFor(11)).isEqualTo(AMOUNT_BELOW_MINIMUM); + assertThat(AMOUNT_BELOW_MINIMUM.isErrorFromFinalNode()).isFalse(); } @Test void feeInsufficient() { assertThat(FailureCode.getFor(12)).isEqualTo(FEE_INSUFFICIENT); + assertThat(FEE_INSUFFICIENT.isErrorFromFinalNode()).isFalse(); } @Test void incorrectCltvExpiry() { assertThat(FailureCode.getFor(13)).isEqualTo(INCORRECT_CLTV_EXPIRY); + assertThat(INCORRECT_CLTV_EXPIRY.isErrorFromFinalNode()).isFalse(); } @Test void channelDisabled() { assertThat(FailureCode.getFor(14)).isEqualTo(CHANNEL_DISABLED); + assertThat(CHANNEL_DISABLED.isErrorFromFinalNode()).isFalse(); } @Test void temporaryChannelFailure() { assertThat(FailureCode.getFor(15)).isEqualTo(TEMPORARY_CHANNEL_FAILURE); + assertThat(TEMPORARY_CHANNEL_FAILURE.isErrorFromFinalNode()).isFalse(); } @Test void requiredNodeFeatureMissing() { assertThat(FailureCode.getFor(16)).isEqualTo(REQUIRED_NODE_FEATURE_MISSING); + assertThat(REQUIRED_NODE_FEATURE_MISSING.isErrorFromFinalNode()).isFalse(); } @Test void requiredChannelFeatureMissing() { assertThat(FailureCode.getFor(17)).isEqualTo(REQUIRED_CHANNEL_FEATURE_MISSING); + assertThat(REQUIRED_CHANNEL_FEATURE_MISSING.isErrorFromFinalNode()).isFalse(); } @Test void unknownNextPeer() { assertThat(FailureCode.getFor(18)).isEqualTo(UNKNOWN_NEXT_PEER); + assertThat(UNKNOWN_NEXT_PEER.isErrorFromFinalNode()).isFalse(); } @Test void temporaryNodeFailure() { assertThat(FailureCode.getFor(19)).isEqualTo(TEMPORARY_NODE_FAILURE); + assertThat(TEMPORARY_NODE_FAILURE.isErrorFromFinalNode()).isFalse(); } @Test void permanentNodeFailure() { assertThat(FailureCode.getFor(20)).isEqualTo(PERMANENT_NODE_FAILURE); + assertThat(PERMANENT_NODE_FAILURE.isErrorFromFinalNode()).isFalse(); } @Test void permanentChannelFailure() { assertThat(FailureCode.getFor(21)).isEqualTo(PERMANENT_CHANNEL_FAILURE); + assertThat(PERMANENT_CHANNEL_FAILURE.isErrorFromFinalNode()).isFalse(); } @Test void expiryTooFar() { assertThat(FailureCode.getFor(22)).isEqualTo(EXPIRY_TOO_FAR); + assertThat(EXPIRY_TOO_FAR.isErrorFromFinalNode()).isFalse(); } @Test void mppTimeout() { assertThat(FailureCode.getFor(23)).isEqualTo(MPP_TIMEOUT); + assertThat(MPP_TIMEOUT.isErrorFromFinalNode()).isTrue(); } @Test void invalidOnionPayload() { assertThat(FailureCode.getFor(24)).isEqualTo(INVALID_ONION_PAYLOAD); + assertThat(INVALID_ONION_PAYLOAD.isErrorFromFinalNode()).isFalse(); } }