fix: provisional PreviousTxn{Id,LedgerSeq} double threading

Builds/CMake/RippledCore.cmake

@@ -751,6 +751,7 @@ if (tests)
     src/test/app/Path_test.cpp
     src/test/app/PayChan_test.cpp
     src/test/app/PayStrand_test.cpp
+    src/test/app/PreviousTxn_test.cpp
     src/test/app/PseudoTx_test.cpp
     src/test/app/RCLCensorshipDetector_test.cpp
     src/test/app/RCLValidations_test.cpp

src/ripple/ledger/detail/ApplyStateTable.h

@@ -25,6 +25,7 @@
 #include <ripple/ledger/OpenView.h>
 #include <ripple/ledger/RawView.h>
 #include <ripple/ledger/ReadView.h>
+#include <ripple/protocol/Rules.h>
 #include <ripple/protocol/TER.h>
 #include <ripple/protocol/TxMeta.h>
 #include <memory>
@@ -53,6 +54,18 @@ class ApplyStateTable
     items_t items_;
     XRPAmount dropsDestroyed_{0};
 
+    // Track original PreviousTxnID/LgrSeq values to restore after provisional
+    // metadata. This map is populated during provisional metadata generation
+    // and consumed during final metadata generation. It is not cleared as
+    // the ApplyStateTable instance is single-use per transaction.
+    struct ThreadingState
+    {
+        uint256 prevTxnID;
+        uint32_t prevTxnLgrSeq;
+        bool hasPrevTxnID;
+    };
+    mutable std::map<key_type, ThreadingState> originalThreadingState_;
+
 public:
     ApplyStateTable() = default;
     ApplyStateTable(ApplyStateTable&&) = default;
@@ -73,7 +86,8 @@ class ApplyStateTable
         std::optional<STAmount> const& deliver,
         std::vector<STObject> const& hookExecution,
         std::vector<STObject> const& hookEmission,
-        beast::Journal j);
+        beast::Journal j,
+        bool isProvisional = false);
 
     void
     apply(
@@ -138,8 +152,11 @@ class ApplyStateTable
     }
 
 private:
-    static void
-    threadItem(TxMeta& meta, std::shared_ptr<SLE> const& to);
+    void
+    threadItem(
+        TxMeta& meta,
+        std::shared_ptr<SLE> const& to,
+        Rules const& rules);
 
     std::shared_ptr<SLE>
     getForMod(

src/ripple/ledger/impl/ApplyStateTable.cpp

@@ -118,7 +118,8 @@ ApplyStateTable::generateTxMeta(
     std::optional<STAmount> const& deliver,
     std::vector<STObject> const& hookExecution,
     std::vector<STObject> const& hookEmission,
-    beast::Journal j)
+    beast::Journal j,
+    bool isProvisional)
 {
     TxMeta meta(tx.getTransactionID(), to.seq());
     if (deliver)
@@ -196,7 +197,7 @@ ApplyStateTable::generateTxMeta(
 
             if (curNode->isThreadedType())  // thread transaction to node
                                             // item modified
-                threadItem(meta, curNode);
+                threadItem(meta, curNode, to.rules());
 
             STObject prevs(sfPreviousFields);
             for (auto const& obj : *origNode)
@@ -229,7 +230,7 @@ ApplyStateTable::generateTxMeta(
             threadOwners(to, meta, curNode, newMod, j);
 
             if (curNode->isThreadedType())  // always thread to self
-                threadItem(meta, curNode);
+                threadItem(meta, curNode, to.rules());
 
             STObject news(sfNewFields);
             for (auto const& obj : *curNode)
@@ -254,6 +255,34 @@ ApplyStateTable::generateTxMeta(
         }
     }
 
+    // After provisional metadata generation, restore the original PreviousTxnID
+    // values to prevent contamination of the "before" state used for final
+    // metadata comparison. This ensures PreviousTxnID appears correctly in
+    // ModifiedNode metadata.
+    if (isProvisional && to.rules().enabled(fixProvisionalDoubleThreading))
+    {
+        for (auto const& [key, state] : originalThreadingState_)
+        {
+            auto iter = items_.find(key);
+            if (iter != items_.end())
+            {
+                auto sle = iter->second.second;
+                if (state.hasPrevTxnID)
+                {
+                    sle->setFieldH256(sfPreviousTxnID, state.prevTxnID);
+                    sle->setFieldU32(sfPreviousTxnLgrSeq, state.prevTxnLgrSeq);
+                    // Restored to original PreviousTxnID
+                }
+                else
+                {
+                    sle->makeFieldAbsent(sfPreviousTxnID);
+                    sle->makeFieldAbsent(sfPreviousTxnLgrSeq);
+                    // Restored to no PreviousTxnID
+                }
+            }
+        }
+    }
+
     return {meta, newMod};
 }
 
@@ -287,6 +316,8 @@ ApplyStateTable::apply(
         // VFALCO For diagnostics do we want to show
         //        metadata even when the base view is open?
         JLOG(j.trace()) << "metadata " << meta.getJson(JsonOptions::none);
+
+        // Metadata has been generated
     }
     to.rawTxInsert(tx.getTransactionID(), sTx, sMeta);
     apply(to);
@@ -549,9 +580,51 @@ ApplyStateTable::destroyXRP(XRPAmount const& fee)
 //------------------------------------------------------------------------------
 
 // Insert this transaction to the SLE's threading list
+//
+// This method is called during metadata generation to update the
+// PreviousTxnID/PreviousTxnLgrSeq fields on SLEs. However, it's called
+// twice for each transaction:
+// 1. During provisional metadata generation (for hooks to see)
+// 2. During final metadata generation (for the actual ledger)
+//
+// The fixProvisionalDoubleThreading amendment fixes a bug where the
+// provisional threading would contaminate the "original" state used
+// for metadata comparison, causing PreviousTxnID to be missing from
+// the final metadata.
+//
+// The fix works by:
+// - Saving the original PreviousTxnID state for an SLE the first time it's
+//   threaded during the provisional pass.
+// - Restoring the original state for all affected SLEs in a single batch
+//   after the entire provisional metadata generation is complete.
+//
+// This batch-restore is critical because threadItem() can be called on the
+// same SLE multiple times within one metadata pass. Restoring immediately
+// would be incorrect. This approach ensures the final metadata comparison
+// starts from the correct, uncontaminated "before" state.
 void
-ApplyStateTable::threadItem(TxMeta& meta, std::shared_ptr<SLE> const& sle)
+ApplyStateTable::threadItem(
+    TxMeta& meta,
+    std::shared_ptr<SLE> const& sle,
+    const Rules& rules)
 {
+    if (rules.enabled(fixProvisionalDoubleThreading))
+    {
+        auto const key = sle->key();
+        if (originalThreadingState_.find(key) == originalThreadingState_.end())
+        {
+            // First time (provisional metadata) - save the original state
+            ThreadingState state;
+            state.hasPrevTxnID = sle->isFieldPresent(sfPreviousTxnID);
+            if (state.hasPrevTxnID)
+            {
+                state.prevTxnID = sle->getFieldH256(sfPreviousTxnID);
+                state.prevTxnLgrSeq = sle->getFieldU32(sfPreviousTxnLgrSeq);
+            }
+            originalThreadingState_[key] = state;
+        }
+    }
+
     key_type prevTxID;
     LedgerIndex prevLgrID;
 
@@ -567,6 +640,11 @@ ApplyStateTable::threadItem(TxMeta& meta, std::shared_ptr<SLE> const& sle)
             assert(node.getFieldIndex(sfPreviousTxnLgrSeq) == -1);
             node.setFieldH256(sfPreviousTxnID, prevTxID);
             node.setFieldU32(sfPreviousTxnLgrSeq, prevLgrID);
+            // Added PreviousTxnID to metadata
+        }
+        else
+        {
+            // PreviousTxnID already present in metadata
         }
 
         assert(node.getFieldH256(sfPreviousTxnID) == prevTxID);
@@ -640,7 +718,7 @@ ApplyStateTable::threadTx(
         JLOG(j.warn()) << "Threading to non-existent account: " << toBase58(to);
         return;
     }
-    threadItem(meta, sle);
+    threadItem(meta, sle, base.rules());
 }
 
 void

src/ripple/ledger/impl/ApplyViewImpl.cpp

@@ -41,8 +41,13 @@ ApplyViewImpl::generateProvisionalMeta(
     beast::Journal j)
 {
     auto [meta, _] = items_.generateTxMeta(
-        to, tx, deliver_, hookExecution_, hookEmission_, j);
-
+        to,
+        tx,
+        deliver_,
+        hookExecution_,
+        hookEmission_,
+        j,
+        true);  // isProvisional = true
     return meta;
 }
 

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 = 81;
+static constexpr std::size_t numFeatures = 82;
 
 /** Amendments that this server supports and the default voting behavior.
    Whether they are enabled depends on the Rules defined in the validated
@@ -369,6 +369,7 @@ extern uint256 const fixXahauV3;
 extern uint256 const fix20250131;
 extern uint256 const featureHookCanEmit;
 extern uint256 const fixRewardClaimFlags;
+extern uint256 const fixProvisionalDoubleThreading;
 
 }  // namespace ripple
 

src/ripple/protocol/impl/Feature.cpp

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