refactor: Improve payment channel closing and returned error codes

This commit is contained in:
Sergey Kuznetsov
2026-05-27 14:42:48 +01:00
committed by Ayaz Salikhov
parent 2728e11809
commit e29dc474b3
5 changed files with 109 additions and 24 deletions

View File

@@ -5,8 +5,21 @@
#include <xrpl/protocol/TER.h>
#include <xrpl/protocol/UintTypes.h>
#include <cstdint>
#include <memory>
#include <optional>
namespace xrpl {
/** Close a payment channel and return its remaining funds to the channel owner.
*
* @param slep The SLE for the PayChannel object to close.
* @param view The apply view in which ledger state modifications are made.
* @param key The ledger key identifying the PayChannel entry.
* @param j Journal used for fatal-level diagnostic messages.
* @return tesSUCCESS on success; tefBAD_LEDGER if a directory removal
* fails; tefINTERNAL if the source account SLE cannot be found.
*/
TER
closeChannel(
std::shared_ptr<SLE> const& slep,
@@ -14,4 +27,27 @@ closeChannel(
uint256 const& key,
beast::Journal j);
/** Add two uint32_t values with saturation at UINT32_MAX.
*
* @param rules The current ledger rules used to check amendment status.
* @param lhs Left-hand operand.
* @param rhs Right-hand operand.
* @return @p lhs + @p rhs, saturated at UINT32_MAX when the amendment
* is active.
*/
uint32_t
saturatingAdd(Rules const& rules, uint32_t const lhs, uint32_t const rhs);
/** Determine whether a payment channel time field represents an expired time.
*
* @param view The apply view providing the parent close time and rules.
* @param timeField The optional expiry timestamp (seconds since the XRP
* Ledger epoch). If empty, the function returns false.
* @return @c true if @p timeField is set and the indicated time is
* in the past relative to the view's parent close time;
* @c false otherwise.
*/
bool
isChannelExpired(ApplyView const& view, std::optional<std::uint32_t> timeField);
} // namespace xrpl

View File

@@ -5,14 +5,20 @@
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/beast/utility/instrumentation.h>
#include <xrpl/ledger/ApplyView.h>
#include <xrpl/ledger/View.h>
#include <xrpl/ledger/helpers/AccountRootHelpers.h>
#include <xrpl/protocol/AccountID.h>
#include <xrpl/protocol/Feature.h>
#include <xrpl/protocol/Indexes.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STLedgerEntry.h>
#include <xrpl/protocol/TER.h>
#include <algorithm>
#include <cstdint>
#include <limits>
#include <memory>
#include <optional>
namespace xrpl {
@@ -65,4 +71,28 @@ closeChannel(
return tesSUCCESS;
}
uint32_t
saturatingAdd(Rules const& rules, uint32_t const lhs, uint32_t const rhs)
{
if (rules.enabled(fixCleanup3_2_0))
{
static constexpr auto kUint32Max =
static_cast<uint64_t>(std::numeric_limits<uint32_t>::max());
uint64_t const saturatedResult = std::min(uint64_t{lhs} + rhs, kUint32Max);
return static_cast<uint32_t>(saturatedResult);
}
return lhs + rhs;
}
bool
isChannelExpired(ApplyView const& view, std::optional<uint32_t> timeField)
{
if (!timeField)
return false;
if (view.rules().enabled(fixCleanup3_2_0))
return after(view.header().parentCloseTime, *timeField);
return view.header().parentCloseTime.time_since_epoch().count() >= *timeField;
}
} // namespace xrpl

View File

@@ -43,6 +43,9 @@ PaymentChannelClaim::getFlagsMask(PreflightContext const&)
NotTEC
PaymentChannelClaim::preflight(PreflightContext const& ctx)
{
if (ctx.rules.enabled(fixCleanup3_2_0) && ctx.tx[sfChannel] == beast::kZero)
return temMALFORMED;
auto const bal = ctx.tx[~sfBalance];
if (bal && (!isXRP(*bal) || *bal <= beast::kZero))
return temBAD_AMOUNT;
@@ -117,12 +120,10 @@ PaymentChannelClaim::doApply()
AccountID const txAccount = ctx_.tx[sfAccount];
auto const curExpiration = (*slep)[~sfExpiration];
if (isChannelExpired(ctx_.view(), (*slep)[~sfCancelAfter]) ||
isChannelExpired(ctx_.view(), curExpiration))
{
auto const cancelAfter = (*slep)[~sfCancelAfter];
auto const closeTime = ctx_.view().header().parentCloseTime.time_since_epoch().count();
if ((cancelAfter && closeTime >= *cancelAfter) ||
(curExpiration && closeTime >= *curExpiration))
return closeChannel(slep, ctx_.view(), k.key, ctx_.registry.get().getJournal("View"));
return closeChannel(slep, ctx_.view(), k.key, ctx_.registry.get().getJournal("View"));
}
if (txAccount != src && txAccount != dst)
@@ -135,13 +136,19 @@ PaymentChannelClaim::doApply()
auto const reqBalance = ctx_.tx[sfBalance].xrp();
if (txAccount == dst && !ctx_.tx[~sfSignature])
return temBAD_SIGNATURE;
{
return ctx_.view().rules().enabled(fixCleanup3_2_0) ? TER{tecNO_PERMISSION}
: TER{temBAD_SIGNATURE};
}
if (ctx_.tx[~sfSignature])
{
PublicKey const pk((*slep)[sfPublicKey]);
if (ctx_.tx[sfPublicKey] != pk)
return temBAD_SIGNER;
{
return ctx_.view().rules().enabled(fixCleanup3_2_0) ? TER{tecNO_PERMISSION}
: TER{temBAD_SIGNER};
}
}
if (reqBalance > chanFunds)
@@ -185,9 +192,10 @@ PaymentChannelClaim::doApply()
if (dst == txAccount || (*slep)[sfBalance] == (*slep)[sfAmount])
return closeChannel(slep, ctx_.view(), k.key, ctx_.registry.get().getJournal("View"));
auto const settleExpiration =
ctx_.view().header().parentCloseTime.time_since_epoch().count() +
(*slep)[sfSettleDelay];
auto const settleExpiration = saturatingAdd(
ctx_.view().rules(),
ctx_.view().header().parentCloseTime.time_since_epoch().count(),
(*slep)[sfSettleDelay]);
if (!curExpiration || *curExpiration > settleExpiration)
{

View File

@@ -6,6 +6,7 @@
#include <xrpl/ledger/ReadView.h>
#include <xrpl/ledger/helpers/PaymentChannelHelpers.h>
#include <xrpl/protocol/AccountID.h>
#include <xrpl/protocol/Feature.h>
#include <xrpl/protocol/Indexes.h>
#include <xrpl/protocol/Keylet.h>
#include <xrpl/protocol/LedgerFormats.h>
@@ -31,6 +32,9 @@ PaymentChannelFund::makeTxConsequences(PreflightContext const& ctx)
NotTEC
PaymentChannelFund::preflight(PreflightContext const& ctx)
{
if (ctx.rules.enabled(fixCleanup3_2_0) && ctx.tx[sfChannel] == beast::kZero)
return temMALFORMED;
if (!isXRP(ctx.tx[sfAmount]) || (ctx.tx[sfAmount] <= beast::kZero))
return temBAD_AMOUNT;
@@ -47,13 +51,12 @@ PaymentChannelFund::doApply()
AccountID const src = (*slep)[sfAccount];
auto const txAccount = ctx_.tx[sfAccount];
auto const expiration = (*slep)[~sfExpiration];
auto const curExpiration = (*slep)[~sfExpiration];
if (isChannelExpired(ctx_.view(), (*slep)[~sfCancelAfter]) ||
isChannelExpired(ctx_.view(), curExpiration))
{
auto const cancelAfter = (*slep)[~sfCancelAfter];
auto const closeTime = ctx_.view().header().parentCloseTime.time_since_epoch().count();
if ((cancelAfter && closeTime >= *cancelAfter) || (expiration && closeTime >= *expiration))
return closeChannel(slep, ctx_.view(), k.key, ctx_.registry.get().getJournal("View"));
return closeChannel(slep, ctx_.view(), k.key, ctx_.registry.get().getJournal("View"));
}
if (src != txAccount)
@@ -62,16 +65,21 @@ PaymentChannelFund::doApply()
return tecNO_PERMISSION;
}
if (auto extend = ctx_.tx[~sfExpiration])
if (auto newExpiration = ctx_.tx[~sfExpiration])
{
auto minExpiration = ctx_.view().header().parentCloseTime.time_since_epoch().count() +
(*slep)[sfSettleDelay];
if (expiration && *expiration < minExpiration)
minExpiration = *expiration;
auto minExpiration = saturatingAdd(
ctx_.view().rules(),
ctx_.view().header().parentCloseTime.time_since_epoch().count(),
(*slep)[sfSettleDelay]);
if (curExpiration && *curExpiration < minExpiration)
minExpiration = *curExpiration;
if (*extend < minExpiration)
return temBAD_EXPIRATION;
(*slep)[~sfExpiration] = *extend;
if (*newExpiration < minExpiration)
{
return ctx_.view().rules().enabled(fixCleanup3_2_0) ? TER{tecNO_PERMISSION}
: TER{temBAD_EXPIRATION};
}
(*slep)[~sfExpiration] = *newExpiration;
ctx_.view().update(slep);
}

View File

@@ -1992,7 +1992,10 @@ public:
run() override
{
using namespace test::jtx;
FeatureBitset const all{testableAmendments()};
// fixCleanup3_2_0 changes payment-channel error codes (tem* -> tec*)
// and channel-closing semantics. This suite asserts the
// pre-amendment behavior, so run it with the amendment disabled.
FeatureBitset const all{testableAmendments() - fixCleanup3_2_0};
testWithFeats(all);
testDepositAuthCreds();
testMetaAndOwnership(all - fixIncludeKeyletFields);