feat: LPT freeze

[shawnxie999]

XRPLF/rippled#5227

Introduces amendment gating in accountHolds that so that the assets in the AMM are not frozen

[shawnxie999]

@godexsoft @cindyyan317 This PR is ready for review. There is still a test CI issue around SubscribeManager that needs to be resolved, but I don’t believe it should block the review process. However, I will have limited capacity to investigate this issue further over the next two weeks.

[cindyyan317]

ammAccountHolds and accountHolds seem share some common code , can we refactor them a bit?

[shawnxie999]

i don't think we benefit much from refactoring to share common code. these two functions should be independent from each other anyways

[cindyyan317]

My main thought was that pulling out the common code could make things easier to maintain and avoid duplication. If we ever need to update that logic, we'd only have to do it in one place.

For example, the green part is the shared code, if there is bug in it, we need to change two places.
Are these two functions really independent? I saw accountHolds just does one more check comparing with ammAcountHolds. So the former can call latter like this:

ripple::STAmount
accountHolds(
    BackendInterface const& backend,
    data::AmendmentCenterInterface const& amendmentCenter,
    std::uint32_t sequence,
    ripple::AccountID const& account,
    ripple::Currency const& currency,
    ripple::AccountID const& issuer,
    bool const zeroIfFrozen,
    boost::asio::yield_context yield
)
{
    auto amountBeforeCheckingLP = ammAccountHolds(backend, sequence, account, currency, issuer, zeroIfFrozen, yield);

    if (!zeroIfFrozen || amountBeforeCheckingLP == amountBeforeCheckingLP.zeroed())
        return amountBeforeCheckingLP;

    auto const isLptFrozen = [&]() {
        if (amendmentCenter.isEnabled(yield, data::Amendments::fixFrozenLPTokenTransfer, sequence)) {
            auto const issuerBlob = backend.fetchLedgerObject(ripple::keylet::account(issuer).key, sequence, yield);

            if (!issuerBlob)
                return true;

            ripple::SLE const issuerSle{
                ripple::SerialIter{issuerBlob->data(), issuerBlob->size()}, ripple::keylet::account(issuer).key
            };

            // if the issuer is an amm account, then currency is lptoken, so we will need to check if the
            // assets in the pool are frozen as well
            if (issuerSle.isFieldPresent(ripple::sfAMMID)) {
                auto const ammKeylet = ripple::keylet::amm(issuerSle[ripple::sfAMMID]);
                auto const ammBlob = backend.fetchLedgerObject(ammKeylet.key, sequence, yield);

                if (!ammBlob)
                    return true;

                ripple::SLE const ammSle{ripple::SerialIter{ammBlob->data(), ammBlob->size()}, ammKeylet.key};

                return isLPTokenFrozen(
                    backend,
                    sequence,
                    account,
                    ammSle[ripple::sfAsset].get<ripple::Issue>(),
                    ammSle[ripple::sfAsset2].get<ripple::Issue>(),
                    yield
                );
            }
        }
        return false;
    };

    return isLptFrozen() ? amountBeforeCheckingLP.zeroed() : amountBeforeCheckingLP;
}

`

[shawnxie999]

truthfully, the code in ammAccountHolds:

    if (ripple::isXRP(currency))
        return {xrpLiquid(backend, sequence, account, yield)};

should be replaced with an assertion ASSERT(!ripple::isXRP(currency)), which is why I think these two functions are independent

[cindyyan317]

If you genuinely feel certain modifications are necessary, please go ahead and adjust the code accordingly. IMO, refactoring common logic doesn’t mean you can’t make case-specific tweaks; it just helps keep the codebase cleaner and easier to maintain.

[codecov]

Codecov Report

Attention: Patch coverage is 79.31034% with 18 lines in your changes missing coverage. Please review.

Project coverage is 71.69%. Comparing base (6ef6ca9) to head (c102d7a).
Report is 7 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/rpc/RPCHelpers.cpp	83.01%	3 Missing and 6 partials ⚠️
src/rpc/common/impl/HandlerProvider.cpp	0.00%	4 Missing ⚠️
src/app/ClioApplication.cpp	0.00%	2 Missing ⚠️
src/feed/SubscriptionManager.hpp	33.33%	2 Missing ⚠️
src/rpc/handlers/BookOffers.cpp	80.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1840      +/-   ##
===========================================
+ Coverage    71.61%   71.69%   +0.08%     
===========================================
  Files          330      333       +3     
  Lines        13413    13551     +138     
  Branches      6826     6895      +69     
===========================================
+ Hits          9606     9716     +110     
- Misses        1922     1933      +11     
- Partials      1885     1902      +17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[cindyyan317]

Please add unittests for RPCHelper in RPCHelperTests.cpp.
For example:
when fixFrozenLPTokenTransfer enable/disable
when asset is frozen and asset2 is not
when asset is not and asset2 is frozen
when sfAMMID present/not present
...