Fixed comments #1

XRPLF · Nov 18, 2024 · f5efa31 · f5efa31
1 parent 306a112
commit f5efa31
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 41 deletions.
diff --git a/src/xrpld/app/paths/detail/StepChecks.h b/src/xrpld/app/paths/detail/StepChecks.h
@@ -54,6 +54,8 @@ checkFreeze(
         }
         if (view.rules().enabled(featureDeepFreeze))
         {
+            // Unline normal freeze, a deep frozen trust line acts the same
+            // regardless of which side froze it
             if (sle->isFlag(lsfHighDeepFreeze) || sle->isFlag(lsfLowDeepFreeze))
             {
                 return terNO_LINE;

diff --git a/src/xrpld/app/tx/detail/CreateOffer.cpp b/src/xrpld/app/tx/detail/CreateOffer.cpp
@@ -259,7 +259,9 @@ CreateOffer::checkAcceptAsset(
 
     if (view.rules().enabled(featureDeepFreeze))
     {
-        // An issuer can always accept its own issuance.
+        // An account can not create a trustline to itself, so no line can exist
+        // to be frozen. Additionally, an issuer can always accept its own
+        // issuance.
         if (issue.account == id)
         {
             return tesSUCCESS;

diff --git a/src/xrpld/app/tx/detail/SetTrust.cpp b/src/xrpld/app/tx/detail/SetTrust.cpp
@@ -45,6 +45,16 @@ SetTrust::preflight(PreflightContext const& ctx)
         return temINVALID_FLAG;
     }
 
+    if (!ctx.rules.enabled(featureDeepFreeze))
+    {
+        // Even though the deep freeze flags are included in the
+        // `tfTrustSetMask`, they are not valid if the amendment is not enabled.
+        if (uTxFlags & (tfSetDeepFreeze | tfClearDeepFreeze))
+        {
+            return temINVALID_FLAG;
+        }
+    }
+
     STAmount const saLimitAmount(tx.getFieldAmount(sfLimitAmount));
 
     if (!isLegalNet(saLimitAmount))
@@ -242,11 +252,8 @@ SetTrust::doApply()
     bool const bClearNoRipple = (uTxFlags & tfClearNoRipple);
     bool const bSetFreeze = (uTxFlags & tfSetFreeze);
     bool const bClearFreeze = (uTxFlags & tfClearFreeze);
-    bool const bDeepFreezeEnabled = view().rules().enabled(featureDeepFreeze);
-    bool const bSetDeepFreeze =
-        bDeepFreezeEnabled && (uTxFlags & tfSetDeepFreeze);
-    bool const bClearDeepFreeze =
-        bDeepFreezeEnabled && (uTxFlags & tfClearDeepFreeze);
+    bool const bSetDeepFreeze = (uTxFlags & tfSetDeepFreeze);
+    bool const bClearDeepFreeze = (uTxFlags & tfClearDeepFreeze);
 
     auto viewJ = ctx_.app.journal("View");
 
@@ -413,33 +420,35 @@ SetTrust::doApply()
             uFlagsOut &= ~(bHigh ? lsfHighNoRipple : lsfLowNoRipple);
         }
 
-        bool willSetFreeze = false;
         if (bSetFreeze && !bClearFreeze && !sle->isFlag(lsfNoFreeze))
         {
             uFlagsOut |= (bHigh ? lsfHighFreeze : lsfLowFreeze);
-            willSetFreeze = true;
         }
         else if (bClearFreeze && !bSetFreeze)
         {
             uFlagsOut &= ~(bHigh ? lsfHighFreeze : lsfLowFreeze);
         }
 
-        bool const alreadyFrozen =
-            bHigh ? sle->isFlag(lsfHighFreeze) : sle->isFlag(lsfLowFreeze);
-        if (bSetDeepFreeze && !bClearDeepFreeze &&
-            (willSetFreeze || alreadyFrozen))
+        if (bSetDeepFreeze && !bClearDeepFreeze && !sle->isFlag(lsfNoFreeze))
         {
-            if (ctx_.view().rules().enabled(featureDeepFreeze))
-                uFlagsOut |= (bHigh ? lsfHighDeepFreeze : lsfLowDeepFreeze);
-            else
-                return temMALFORMED;
+            uFlagsOut |= (bHigh ? lsfHighDeepFreeze : lsfLowDeepFreeze);
         }
         else if (bClearDeepFreeze && !bSetDeepFreeze)
         {
-            if (ctx_.view().rules().enabled(featureDeepFreeze))
-                uFlagsOut &= ~(bHigh ? lsfHighDeepFreeze : lsfLowDeepFreeze);
-            else
-                return temMALFORMED;
+            uFlagsOut &= ~(bHigh ? lsfHighDeepFreeze : lsfLowDeepFreeze);
+        }
+
+        // Checking freeze invariants
+        auto const frozen = uFlagsOut & (bHigh ? lsfHighFreeze : lsfLowFreeze);
+        auto const deepFrozen =
+            uFlagsOut & (bHigh ? lsfHighDeepFreeze : lsfLowDeepFreeze);
+
+        // Trying to set deep freeze on not already frozen trust line must fail
+        // This also checks that clearing normal freeze while deep frozen must
+        // not work
+        if (deepFrozen && !frozen)
+        {
+            return tecNO_PERMISSION;
         }
 
         if (QUALITY_ONE == uLowQualityOut)

diff --git a/src/xrpld/ledger/detail/View.cpp b/src/xrpld/ledger/detail/View.cpp
@@ -273,20 +273,24 @@ isDeepFrozen(
     AccountID const& issuer)
 {
     if (!view.rules().enabled(featureDeepFreeze))
+    {
         return false;
+    }
+
     if (isXRP(currency))
+    {
         return false;
-    if (issuer != account)
+    }
+
+    if (issuer == account)
     {
-        // Check if the issuer deep froze the line
-        auto sle = view.read(keylet::line(account, issuer, currency));
-        if (sle)
-        {
-            if (sle->isFlag(lsfHighDeepFreeze) || sle->isFlag(lsfLowDeepFreeze))
-                return true;
-        }
+        return false;
     }
-    return false;
+
+    // Check if the line is deep frozen
+    auto const sle = view.read(keylet::line(account, issuer, currency));
+    return sle &&
+        (sle->isFlag(lsfHighDeepFreeze) || sle->isFlag(lsfLowDeepFreeze));
 }
 
 STAmount
@@ -306,18 +310,25 @@ accountHolds(
 
     // IOU: Return balance on trust line modulo freeze
     auto const sle = view.read(keylet::line(account, issuer, currency));
-    if (!sle)
-    {
-        amount.clear(Issue{currency, issuer});
-    }
-    else if (
-        ((zeroIfFrozen == fhZERO_IF_FROZEN) &&
-         isFrozen(view, account, currency, issuer)) ||
-        isDeepFrozen(view, account, currency, issuer))
-    {
-        amount.clear(Issue{currency, issuer});
-    }
-    else
+    auto const allowBalance = [&]() {
+        if (!sle)
+        {
+            return false;
+        }
+
+        if (zeroIfFrozen == fhZERO_IF_FROZEN)
+        {
+            if (isFrozen(view, account, currency, issuer) ||
+                isDeepFrozen(view, account, currency, issuer))
+            {
+                return false;
+            }
+        }
+
+        return true;
+    }();
+
+    if (allowBalance)
     {
         amount = sle->getFieldAmount(sfBalance);
         if (account > issuer)
@@ -327,6 +338,11 @@ accountHolds(
         }
         amount.setIssuer(issuer);
     }
+    else
+    {
+        amount.clear(Issue{currency, issuer});
+    }
+
     JLOG(j.trace()) << "accountHolds:" << " account=" << to_string(account)
                     << " amount=" << amount.getFullText();