From 3175501e84b2c874c0977d82d39507310ac976ed Mon Sep 17 00:00:00 2001 From: Denis Angell Date: Thu, 10 Apr 2025 00:08:44 +0200 Subject: [PATCH] fix: `fixPayChanV1` (#4717) This change introduces a new fix amendment (`fixPayChanV1`) that prevents the creation of new `PaymentChannelCreate` transaction with a `CancelAfter` time less than the current ledger time. It piggy backs off of fix1571. Once the amendment is activated, creating a new `PaymentChannel` will require that if you specify the `CancelAfter` time/value, that value must be greater than or equal to the current ledger time. Currently users can create a payment channel where the `CancelAfter` time is before the current ledger time. This results in the payment channel being immediately closed on the next PaymentChannel transaction. --- include/xrpl/protocol/detail/features.macro | 1 + src/test/app/PayChan_test.cpp | 46 +++++++++++++++++++++ src/xrpld/app/tx/detail/Escrow.cpp | 12 ------ src/xrpld/app/tx/detail/PayChan.cpp | 7 ++++ src/xrpld/ledger/View.h | 9 ++++ src/xrpld/ledger/detail/View.cpp | 6 +++ 6 files changed, 69 insertions(+), 12 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index d71aa66db..40cea0e1c 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -34,6 +34,7 @@ // If you add an amendment here, then do not forget to increment `numFeatures` // in include/xrpl/protocol/Feature.h. +XRPL_FIX (PayChanCancelAfter, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(IOURewardClaim, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (IOULockedBalanceInvariant, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (ImportIssuer, Supported::yes, VoteBehavior::DefaultYes) diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index ba528d0f1..29d0c29c2 100644 --- a/src/test/app/PayChan_test.cpp +++ b/src/test/app/PayChan_test.cpp @@ -473,6 +473,52 @@ struct PayChan_test : public beast::unit_test::suite BEAST_EXPECT(!channelExists(*env.current(), chan)); BEAST_EXPECT(env.balance(alice) == preAlice + channelFunds); } + // fixPayChanCancelAfter + // CancelAfter should be greater than close time + { + for (bool const withFixPayChan : {true, false}) + { + auto const amend = withFixPayChan + ? features + : features - fixPayChanCancelAfter; + Env env{*this, amend}; + env.fund(XRP(10000), alice, bob); + env.close(); + + auto const pk = alice.pk(); + auto const settleDelay = 100s; + auto const channelFunds = XRP(1000); + NetClock::time_point const cancelAfter = + env.current()->info().parentCloseTime - 1s; + auto const txResult = + withFixPayChan ? ter(tecEXPIRED) : ter(tesSUCCESS); + env(create( + alice, bob, channelFunds, settleDelay, pk, cancelAfter), + txResult); + } + } + // fixPayChanCancelAfter + // CancelAfter can be equal to the close time + { + for (bool const withFixPayChan : {true, false}) + { + auto const amend = withFixPayChan + ? features + : features - fixPayChanCancelAfter; + Env env{*this, amend}; + env.fund(XRP(10000), alice, bob); + env.close(); + + auto const pk = alice.pk(); + auto const settleDelay = 100s; + auto const channelFunds = XRP(1000); + NetClock::time_point const cancelAfter = + env.current()->info().parentCloseTime; + env(create( + alice, bob, channelFunds, settleDelay, pk, cancelAfter), + ter(tesSUCCESS)); + } + } } void diff --git a/src/xrpld/app/tx/detail/Escrow.cpp b/src/xrpld/app/tx/detail/Escrow.cpp index de4b30c0d..ca9b8ed1f 100644 --- a/src/xrpld/app/tx/detail/Escrow.cpp +++ b/src/xrpld/app/tx/detail/Escrow.cpp @@ -77,18 +77,6 @@ namespace ripple { //------------------------------------------------------------------------------ -/** Has the specified time passed? - - @param now the current time - @param mark the cutoff point - @return true if \a now refers to a time strictly after \a mark, else false. -*/ -static inline bool -after(NetClock::time_point now, std::uint32_t mark) -{ - return now.time_since_epoch().count() > mark; -} - TxConsequences EscrowCreate::makeTxConsequences(PreflightContext const& ctx) { diff --git a/src/xrpld/app/tx/detail/PayChan.cpp b/src/xrpld/app/tx/detail/PayChan.cpp index 3614a1407..ba54bd91d 100644 --- a/src/xrpld/app/tx/detail/PayChan.cpp +++ b/src/xrpld/app/tx/detail/PayChan.cpp @@ -338,6 +338,13 @@ PayChanCreate::doApply() if (!sle) return tefINTERNAL; + if (ctx_.view().rules().enabled(fixPayChanCancelAfter)) + { + auto const closeTime = ctx_.view().info().parentCloseTime; + if (ctx_.tx[~sfCancelAfter] && after(closeTime, ctx_.tx[sfCancelAfter])) + return tecEXPIRED; + } + auto const dst = ctx_.tx[sfDestination]; STAmount const amount{ctx_.tx[sfAmount]}; diff --git a/src/xrpld/ledger/View.h b/src/xrpld/ledger/View.h index 76c66aba4..2ce609c48 100644 --- a/src/xrpld/ledger/View.h +++ b/src/xrpld/ledger/View.h @@ -1263,6 +1263,15 @@ deleteAMMTrustLine( std::optional const& ammAccountID, beast::Journal j); +/** Has the specified time passed? + + @param now the current time + @param mark the cutoff point + @return true if \a now refers to a time strictly after \a mark, else false. +*/ +bool +after(NetClock::time_point now, std::uint32_t mark); + } // namespace ripple #endif diff --git a/src/xrpld/ledger/detail/View.cpp b/src/xrpld/ledger/detail/View.cpp index b75041e89..15e9caa4a 100644 --- a/src/xrpld/ledger/detail/View.cpp +++ b/src/xrpld/ledger/detail/View.cpp @@ -2215,4 +2215,10 @@ rippleCredit( saAmount.asset().value()); } +bool +after(NetClock::time_point now, std::uint32_t mark) +{ + return now.time_since_epoch().count() > mark; +} + } // namespace ripple