docs(freeze): canceling escrows with deep frozen assets is allowed (#540)

src/ripple/app/hook/impl/applyHook.cpp

@@ -342,8 +342,7 @@ getTransactionalStakeHolders(STTx const& tx, ReadView const& rv)
         case ttOFFER_CANCEL:
         case ttTICKET_CREATE:
         case ttHOOK_SET:
-        case ttOFFER_CREATE:  // this is handled seperately
-        {
+        case ttOFFER_CREATE: {
             break;
         }
 

src/ripple/app/tx/impl/CreateOffer.cpp

@@ -1277,7 +1277,8 @@ CreateOffer::doApply()
     if (result.second)
     {
         sb.apply(ctx_.rawView());
-        addWeakTSHFromSandbox(sb);
+        if (!view().rules().enabled(featureIOUIssuerWeakTSH))
+            addWeakTSHFromBalanceChanges(sb);
     }
     else
         sbCancel.apply(ctx_.rawView());

src/ripple/app/tx/impl/Payment.cpp

@@ -425,7 +425,8 @@ Payment::doApply()
             //             on the TER. But always applying *should*
             //             be safe.
             pv.apply(ctx_.rawView());
-            addWeakTSHFromSandbox(pv);
+            if (!view().rules().enabled(featureIOUIssuerWeakTSH))
+                addWeakTSHFromBalanceChanges(pv);
         }
 
         // TODO: is this right?  If the amount is the correct amount, was

src/ripple/app/tx/impl/Transactor.cpp

@@ -1477,15 +1477,15 @@ Transactor::doHookCallback(
 }
 
 void
-Transactor::addWeakTSHFromSandbox(detail::ApplyViewBase const& pv)
+Transactor::addWeakTSHFromBalanceChanges(detail::ApplyViewBase const& pv)
 {
     // If Hooks are enabled then non-issuers who have their TL balance
-    // modified by the execution of the path have the opportunity to have their
-    // weak hooks executed.
+    // modified by the execution of the transaction have the opportunity to have
+    // their weak hooks executed.
     if (ctx_.view().rules().enabled(featureHooks))
     {
-        // anyone whose balance changed as a result of this Pathing is a weak
-        // TSH
+        // anyone whose balance changed as a result of transaction processing is
+        // a weak TSH
         auto bc = pv.balanceChanges(view());
 
         for (auto const& entry : bc)
@@ -1506,15 +1506,13 @@ Transactor::addWeakTSHFromSandbox(detail::ApplyViewBase const& pv)
 TER
 Transactor::doTSH(
     bool strong,  // only strong iff true, only weak iff false
+    std::vector<std::pair<AccountID, bool>> tsh,
     hook::HookStateMap& stateMap,
     std::vector<hook::HookResult>& results,
     std::shared_ptr<STObject const> const& provisionalMeta)
 {
     auto& view = ctx_.view();
 
-    std::vector<std::pair<AccountID, bool>> tsh =
-        hook::getTransactionalStakeHolders(ctx_.tx, view);
-
     // add the extra TSH marked out by the specific transactor (if applicable)
     if (!strong)
         for (auto& weakTsh : additionalWeakTSH_)
@@ -1772,6 +1770,9 @@ Transactor::operator()()
     // application to the ledger
     std::map<AccountID, std::set<uint256>> aawMap;
 
+    std::vector<std::pair<AccountID, bool>> tsh =
+        hook::getTransactionalStakeHolders(ctx_.tx, ctx_.view());
+
     // Pre-application (Strong TSH) Hooks are executed here
     // These TSH have the right to rollback.
     // Weak TSH and callback are executed post-application.
@@ -1800,7 +1801,7 @@ Transactor::operator()()
             // (who have the right to rollback the txn), any weak TSH will be
             // executed after doApply has been successful (callback as well)
 
-            result = doTSH(true, stateMap, hookResults, {});
+            result = doTSH(true, tsh, stateMap, hookResults, {});
         }
 
         // write state if all chains executed successfully
@@ -2054,7 +2055,23 @@ Transactor::operator()()
         hook::HookStateMap stateMap;
         std::vector<hook::HookResult> weakResults;
 
-        doTSH(false, stateMap, weakResults, proMeta);
+        if (view().rules().enabled(featureIOUIssuerWeakTSH))
+        {
+            // Regardless of the transaction type, if the result changes the
+            // trust line balance, add high and low accounts to weakTSH.
+            ApplyViewImpl& avi = dynamic_cast<ApplyViewImpl&>(ctx_.view());
+            addWeakTSHFromBalanceChanges(avi);
+        }
+
+        if (!view().rules().enabled(featureIOUIssuerWeakTSH))
+        {
+            // before amendment enabled, we need to get TSHs after txn basic
+            // processing If the object is deleted in cancen txn, it may not
+            // be possible to obtain the appropriate TSH.
+            tsh = hook::getTransactionalStakeHolders(ctx_.tx, ctx_.view());
+        }
+
+        doTSH(false, tsh, stateMap, weakResults, proMeta);
 
         // execute any hooks that nominated for 'again as weak'
         for (auto const& [accID, hookHashes] : aawMap)

src/ripple/app/tx/impl/Transactor.h

@@ -188,6 +188,7 @@ protected:
     TER
     doTSH(
         bool strong,  // only do strong TSH iff true, otheriwse only weak
+        std::vector<std::pair<AccountID, bool>> tsh,
         hook::HookStateMap& stateMap,
         std::vector<hook::HookResult>& result,
         std::shared_ptr<STObject const> const& provisionalMeta);
@@ -213,7 +214,7 @@ protected:
         std::shared_ptr<STObject const> const& provisionalMeta);
 
     void
-    addWeakTSHFromSandbox(detail::ApplyViewBase const& pv);
+    addWeakTSHFromBalanceChanges(detail::ApplyViewBase const& pv);
 
     // hooks amendment fields, these are unpopulated and unused unless
     // featureHooks is enabled

src/ripple/protocol/Feature.h

@@ -74,7 +74,7 @@ namespace detail {
 // Feature.cpp. Because it's only used to reserve storage, and determine how
 // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
 // the actual number of amendments. A LogicError on startup will verify this.
-static constexpr std::size_t numFeatures = 84;
+static constexpr std::size_t numFeatures = 85;
 
 /** Amendments that this server supports and the default voting behavior.
    Whether they are enabled depends on the Rules defined in the validated
@@ -372,6 +372,7 @@ extern uint256 const fixRewardClaimFlags;
 extern uint256 const fixProvisionalDoubleThreading;
 extern uint256 const featureClawback;
 extern uint256 const featureDeepFreeze;
+extern uint256 const featureIOUIssuerWeakTSH;
 
 }  // namespace ripple
 

src/ripple/protocol/impl/Feature.cpp

@@ -478,6 +478,7 @@ REGISTER_FIX    (fixRewardClaimFlags,           Supported::yes, VoteBehavior::De
 REGISTER_FEATURE(Clawback,                      Supported::yes, VoteBehavior::DefaultNo);
 REGISTER_FIX    (fixProvisionalDoubleThreading, Supported::yes, VoteBehavior::DefaultYes);
 REGISTER_FEATURE(DeepFreeze,                    Supported::yes, VoteBehavior::DefaultNo);
+REGISTER_FEATURE(IOUIssuerWeakTSH,              Supported::yes, VoteBehavior::DefaultNo);
 
 // The following amendments are obsolete, but must remain supported
 // because they could potentially get enabled.

src/test/app/Escrow_test.cpp

@@ -4147,7 +4147,8 @@ struct Escrow_test : public beast::unit_test::suite
             env(trust(gw, USD(10'000), bob, tfSetFreeze | tfSetDeepFreeze));
             env.close();
 
-            // bob cancel escrow fails because of deep frozen assets
+            // bob cancel escrow succeeds despite deep frozen assets (unlocking
+            // return is allowed)
             env(escrow::cancel(bob, alice, seq1),
                 fee(baseFee),
                 ter(tesSUCCESS));
@@ -4657,4 +4658,4 @@ public:
 BEAST_DEFINE_TESTSUITE(Escrow, app, ripple);
 
 }  // namespace test
-}  // namespace ripple
+}  // namespace ripple