Refactor Clawback plus other minor refactoring

XRPLF · Oct 10, 2024 · 9d51389 · 9d51389
1 parent eac438c
commit 9d51389
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 69 deletions.
diff --git a/include/xrpl/protocol/Rate.h b/include/xrpl/protocol/Rate.h
@@ -87,7 +87,7 @@ STAmount
 divideRound(
     STAmount const& amount,
     Rate const& rate,
-    Asset const& issue,
+    Asset const& asset,
     bool roundUp);
 
 namespace nft {

diff --git a/src/xrpld/app/tx/detail/Clawback.cpp b/src/xrpld/app/tx/detail/Clawback.cpp
@@ -36,15 +36,6 @@ template <>
 NotTEC
 preflightHelper<Issue>(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureClawback))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
-    if (ctx.tx.getFlags() & tfClawbackMask)
-        return temINVALID_FLAG;
-
     if (ctx.tx.isFieldPresent(sfMPTokenHolder))
         return temMALFORMED;
 
@@ -57,31 +48,22 @@ preflightHelper<Issue>(PreflightContext const& ctx)
     if (issuer == holder || isXRP(clawAmount) || clawAmount <= beast::zero)
         return temBAD_AMOUNT;
 
-    return preflight2(ctx);
+    return tesSUCCESS;
 }
 
 template <>
 NotTEC
 preflightHelper<MPTIssue>(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureClawback))
+    if (!ctx.rules.enabled(featureMPTokensV1))
         return temDISABLED;
 
     auto const mptHolder = ctx.tx[~sfMPTokenHolder];
     auto const clawAmount = ctx.tx[sfAmount];
 
-    if (!ctx.rules.enabled(featureMPTokensV1))
-        return temDISABLED;
-
-    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
-        return ret;
-
     if (!mptHolder)
         return temMALFORMED;
 
-    if (ctx.tx.getFlags() & tfClawbackMask)
-        return temINVALID_FLAG;
-
     // issuer is the same as holder
     if (ctx.tx[sfAccount] == *mptHolder)
         return temMALFORMED;
@@ -90,30 +72,49 @@ preflightHelper<MPTIssue>(PreflightContext const& ctx)
         clawAmount <= beast::zero)
         return temBAD_AMOUNT;
 
+    return tesSUCCESS;
+}
+
+NotTEC
+Clawback::preflight(PreflightContext const& ctx)
+{
+    if (!ctx.rules.enabled(featureClawback))
+        return temDISABLED;
+
+    if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
+        return ret;
+
+    if (ctx.tx.getFlags() & tfClawbackMask)
+        return temINVALID_FLAG;
+
+    if (auto const ret = std::visit(
+            [&]<typename T>(T const&) { return preflightHelper<T>(ctx); },
+            ctx.tx[sfAmount].asset().value());
+        !isTesSuccess(ret))
+        return ret;
+
     return preflight2(ctx);
 }
 
 template <ValidIssueType T>
 static TER
-preclaimHelper(PreclaimContext const& ctx);
+preclaimHelper(
+    PreclaimContext const& ctx,
+    SLE const& sleIssuer,
+    AccountID const& issuer,
+    AccountID const& holder,
+    STAmount const& clawAmount);
 
 template <>
 TER
-preclaimHelper<Issue>(PreclaimContext const& ctx)
+preclaimHelper<Issue>(
+    PreclaimContext const& ctx,
+    SLE const& sleIssuer,
+    AccountID const& issuer,
+    AccountID const& holder,
+    STAmount const& clawAmount)
 {
-    AccountID const issuer = ctx.tx[sfAccount];
-    STAmount const clawAmount = ctx.tx[sfAmount];
-    AccountID const& holder = clawAmount.getIssuer();
-
-    auto const sleIssuer = ctx.view.read(keylet::account(issuer));
-    auto const sleHolder = ctx.view.read(keylet::account(holder));
-    if (!sleIssuer || !sleHolder)
-        return terNO_ACCOUNT;
-
-    if (sleHolder->isFieldPresent(sfAMMID))
-        return tecAMM_ACCOUNT;
-
-    std::uint32_t const issuerFlagsIn = sleIssuer->getFieldU32(sfFlags);
+    std::uint32_t const issuerFlagsIn = sleIssuer.getFieldU32(sfFlags);
 
     // If AllowTrustLineClawback is not set or NoFreeze is set, return no
     // permission
@@ -159,20 +160,13 @@ preclaimHelper<Issue>(PreclaimContext const& ctx)
 
 template <>
 TER
-preclaimHelper<MPTIssue>(PreclaimContext const& ctx)
+preclaimHelper<MPTIssue>(
+    PreclaimContext const& ctx,
+    SLE const& sleIssuer,
+    AccountID const& issuer,
+    AccountID const& holder,
+    STAmount const& clawAmount)
 {
-    AccountID const issuer = ctx.tx[sfAccount];
-    auto const clawAmount = ctx.tx[sfAmount];
-    AccountID const& holder = ctx.tx[sfMPTokenHolder];
-
-    auto const sleIssuer = ctx.view.read(keylet::account(issuer));
-    auto const sleHolder = ctx.view.read(keylet::account(holder));
-    if (!sleIssuer || !sleHolder)
-        return terNO_ACCOUNT;
-
-    if (sleHolder->isFieldPresent(sfAMMID))
-        return tecAMM_ACCOUNT;
-
     auto const issuanceKey =
         keylet::mptIssuance(clawAmount.get<MPTIssue>().getMptID());
     auto const sleIssuance = ctx.view.read(issuanceKey);
@@ -200,6 +194,31 @@ preclaimHelper<MPTIssue>(PreclaimContext const& ctx)
     return tesSUCCESS;
 }
 
+TER
+Clawback::preclaim(PreclaimContext const& ctx)
+{
+    AccountID const issuer = ctx.tx[sfAccount];
+    auto const clawAmount = ctx.tx[sfAmount];
+    AccountID const holder = clawAmount.holds<Issue>()
+        ? clawAmount.getIssuer()
+        : ctx.tx[sfMPTokenHolder];
+
+    auto const sleIssuer = ctx.view.read(keylet::account(issuer));
+    auto const sleHolder = ctx.view.read(keylet::account(holder));
+    if (!sleIssuer || !sleHolder)
+        return terNO_ACCOUNT;
+
+    if (sleHolder->isFieldPresent(sfAMMID))
+        return tecAMM_ACCOUNT;
+
+    return std::visit(
+        [&]<typename T>(T const&) {
+            return preclaimHelper<T>(
+                ctx, *sleIssuer, issuer, holder, clawAmount);
+        },
+        ctx.tx[sfAmount].asset().value());
+}
+
 template <ValidIssueType T>
 static TER
 applyHelper(ApplyContext& ctx);
@@ -208,7 +227,7 @@ template <>
 TER
 applyHelper<Issue>(ApplyContext& ctx)
 {
-    AccountID const& issuer = ctx.tx[sfAccount];
+    AccountID const issuer = ctx.tx[sfAccount];
     STAmount clawAmount = ctx.tx[sfAmount];
     AccountID const holder = clawAmount.getIssuer();  // cannot be reference
 
@@ -239,7 +258,7 @@ template <>
 TER
 applyHelper<MPTIssue>(ApplyContext& ctx)
 {
-    AccountID const& issuer = ctx.tx[sfAccount];
+    AccountID const issuer = ctx.tx[sfAccount];
     auto clawAmount = ctx.tx[sfAmount];
     AccountID const holder = ctx.tx[sfMPTokenHolder];
 
@@ -260,22 +279,6 @@ applyHelper<MPTIssue>(ApplyContext& ctx)
         ctx.journal);
 }
 
-NotTEC
-Clawback::preflight(PreflightContext const& ctx)
-{
-    return std::visit(
-        [&]<typename T>(T const&) { return preflightHelper<T>(ctx); },
-        ctx.tx[sfAmount].asset().value());
-}
-
-TER
-Clawback::preclaim(PreclaimContext const& ctx)
-{
-    return std::visit(
-        [&]<typename T>(T const&) { return preclaimHelper<T>(ctx); },
-        ctx.tx[sfAmount].asset().value());
-}
-
 TER
 Clawback::doApply()
 {

diff --git a/src/xrpld/ledger/detail/View.cpp b/src/xrpld/ledger/detail/View.cpp
@@ -1385,8 +1385,7 @@ rippleSendMPT(
 
     // Sending 3rd party MPTs: transit.
     if (auto const sle =
-            view.read(keylet::mptIssuance(saAmount.get<MPTIssue>().getMptID()));
-        sle != nullptr)
+            view.read(keylet::mptIssuance(saAmount.get<MPTIssue>().getMptID())))
     {
         saActual = (waiveFee == WaiveTransferFee::Yes)
             ? saAmount