`removeDeletedTrustLines` silently skips ALL cleanup when trust line count exceeds limit

[mvadari]

Issue Description

removeDeletedTrustLines in src/libxrpl/tx/Transactor.cpp (lines 978–999) aborts all trust line cleanup when the number of collected deleted trust lines exceeds maxDeletableAMMTrustLines (512). This is a logic error: when a transaction returns tecINCOMPLETE (typically during AMM dissolution), the reapply path collects all deleted ltRIPPLE_STATE entries from the failed doApply() via ctx_.visit(), then passes them to removeDeletedTrustLines. If the total count exceeds 512, zero trust lines are cleaned up — leaving stale AMM trust line objects in the ledger.

This contrasts with the analogous removeUnfundedOffers function (lines 932–947), which processes entries up to its limit (unfundedOfferRemoveLimit) and then returns — partial cleanup rather than none.

Vulnerable Code

removeDeletedTrustLines (lines 978–999):

static void
removeDeletedTrustLines(
    ApplyView& view,
    std::vector<uint256> const& trustLines,
    beast::Journal viewJ)
{
    if (trustLines.size() > maxDeletableAMMTrustLines)  // 512
    {
        JLOG(viewJ.error()) << "removeDeletedTrustLines: deleted trustlines exceed max "
                            << trustLines.size();
        return;  // BUG: no trust lines are cleaned up at all
    }

    for (auto const& index : trustLines)
    {
        if (auto const sleState = view.peek({ltRIPPLE_STATE, index});
            !isTesSuccess(deleteAMMTrustLine(view, sleState, std::nullopt, viewJ)))
        {
            JLOG(viewJ.error()) << "removeDeletedTrustLines: failed to delete AMM trustline";
        }
    }
}

Compare with removeUnfundedOffers (lines 932–947), which handles the same pattern correctly:

static void
removeUnfundedOffers(ApplyView& view, std::vector<uint256> const& offers, beast::Journal viewJ)
{
    int removed = 0;

    for (auto const& index : offers)
    {
        if (auto const sleOffer = view.peek(keylet::offer(index)))
        {
            offerDelete(view, sleOffer, viewJ);
            if (++removed == unfundedOfferRemoveLimit)
                return;  // CORRECT: processes up to the limit, then stops
        }
    }
}

Collection Path

The trust line indices are collected during the tecINCOMPLETE reapply path in Transactor::operator() (lines 1127–1205):

1. doApply() returns tecINCOMPLETE (AMM trust line cleanup was incomplete).
2. ctx_.visit() iterates all metadata entries from the failed apply, collecting every deleted ltRIPPLE_STATE into removedTrustLines (line 1172–1176).
3. The context is reset via reset(fee) (line 1190) — discarding all changes.
4. removeDeletedTrustLines(view(), removedTrustLines, ...) is called (line 1204–1205).
5. If removedTrustLines.size() > 512, the function returns immediately — zero deletions performed.

The collection at step 2 is overly broad: it captures all deleted trust lines from the transaction, not just AMM-specific ones. A transaction that deletes many trust lines as side effects (e.g., a payment that cleans up unfunded trust lines along a path) combined with some AMM trust line deletions could exceed the 512 threshold even though the AMM-specific set is well within the limit.

Steps to Reproduce

1. Create an AMM pool with a large number of trust lines (>512 total deleted ltRIPPLE_STATE entries during dissolution).
2. Submit a transaction that triggers AMM dissolution, causing tecINCOMPLETE.
3. Observe that removeDeletedTrustLines logs the error message and returns without deleting any trust lines.
4. Verify that the stale AMM trust line objects remain in the ledger.

Expected Result

When the number of deleted trust lines exceeds maxDeletableAMMTrustLines, the function should delete up to the limit (512) rather than deleting none. This matches the behavior of removeUnfundedOffers and ensures at least partial cleanup occurs. Subsequent tecINCOMPLETE retries would then clean up the remaining trust lines incrementally.

Actual Result

When trustLines.size() > 512, the function returns immediately without performing any deletions. The ledger retains all stale AMM trust line objects. Subsequent retries may also exceed the limit (since none were cleaned up), creating a permanent failure loop where cleanup never progresses.

Environment

• File: src/libxrpl/tx/Transactor.cpp, lines 978–999
• Constant: maxDeletableAMMTrustLines = 512
• Related code: removeUnfundedOffers (lines 932–947), Transactor::operator() reapply path (lines 1126–1211)
• Version: Current develop branch

Supporting Files

Suggested Fix

Change removeDeletedTrustLines to process up to the limit, matching removeUnfundedOffers:

static void
removeDeletedTrustLines(
    ApplyView& view,
    std::vector<uint256> const& trustLines,
    beast::Journal viewJ)
{
    if (trustLines.size() > maxDeletableAMMTrustLines)
    {
        JLOG(viewJ.error()) << "removeDeletedTrustLines: deleted trustlines exceed max "
                            << trustLines.size();
        // Fall through — process up to the limit rather than aborting
    }

    int removed = 0;
    for (auto const& index : trustLines)
    {
        if (auto const sleState = view.peek({ltRIPPLE_STATE, index});
            !isTesSuccess(deleteAMMTrustLine(view, sleState, std::nullopt, viewJ)))
        {
            JLOG(viewJ.error()) << "removeDeletedTrustLines: failed to delete AMM trustline";
        }
        if (++removed == maxDeletableAMMTrustLines)
            return;
    }
}

[phamdat721101]

#6627