[PermissionedDEX] `book_changes` RPC merges domain and open book volumes into same tally entry

[q73zhao]

Summary

The computeBookChanges function in src/xrpld/rpc/BookChanges.h builds its tally map key from the currency pair string only (e.g. "XRP_drops|USD/gateway"), without incorporating the domain ID. When both a domain offer and an open offer for the same currency pair are crossed in the same ledger, their volumes are incorrectly merged into a single entry and the domain field is overwritten by whichever offer is processed last.

Severity

Low. This is an RPC output issue only — no on-chain or ledger state impact. Downstream consumers of book_changes (analytics, compliance tooling, market data feeds) will receive incorrect volume data when domain and open book trades occur for the same currency pair in the same ledger. The domain field can flip between present and absent nondeterministically based on transaction processing order.

Root Cause

In src/xrpld/rpc/BookChanges.h:122-157, the tally key is constructed from the currency pair but does not incorporate the domain ID:

std::stringstream ss;
if (noswap)
    ss << g << "|" << p;
else
    ss << p << "|" << g;

std::optional<uint256> domain = finalFields[~sfDomainID];

std::string key{ss.str()};  // <-- no domain in key

When two offers with different domain contexts (one with sfDomainID, one without) share the same currency pair, they produce identical keys. The merge logic sums their volumes and unconditionally overwrites the domain field:

auto& entry = tally[key];
std::get<0>(entry) += first;   // side A vol merged
std::get<1>(entry) += second;  // side B vol merged
// ...
std::get<6>(entry) = domain;   // domain OVERWRITTEN

Expected Fix

Include the domain ID in the tally key so domain and open book trades are kept separate:

std::optional<uint256> domain = finalFields[~sfDomainID];

if (domain)
    ss << "|" << to_string(*domain);

std::string key{ss.str()};

Reproduction Unit Test

The following test can be added to src/test/rpc/BookChanges_test.cpp and confirms the bug (the BEAST_EXPECT(changes.size() == 2u) assertion fails — 1 merged entry is returned instead of 2):

void
testDomainAndOpenOfferMerge()
{
    testcase("Domain and Open Offer Merge Bug");
    using namespace jtx;

    FeatureBitset const all{
        jtx::testable_amendments() | featurePermissionedDomains |
        featureCredentials | featurePermissionedDEX};

    Env env(*this, all);
    PermissionedDEX permDex(env);
    auto const& [gw, domainOwner, alice, bob, carol, USD, domainID,
                  credType] = permDex;

    // Create non-domain accounts for the open offer
    Account const eve("eve");
    Account const frank("frank");
    env.fund(XRP(100000), eve, frank);
    env.close();
    env.trust(USD(1000), eve);
    env.trust(USD(1000), frank);
    env.close();
    env(pay(gw, eve, USD(100)));
    env.close();

    // Place a domain offer: alice wants XRP, pays USD
    env(offer(alice, XRP(10), USD(10)), domain(domainID));
    env.close();

    // Place an open (non-domain) offer: eve wants XRP, pays USD
    env(offer(eve, XRP(10), USD(10)));
    env.close();

    // Cross BOTH offers in the same ledger close.
    // Domain crossing: bob pays carol USD via XRP path on the domain.
    env(pay(bob, carol, USD(5)),
        path(~USD),
        sendmax(XRP(5)),
        domain(domainID));

    // Open crossing: frank places a crossing offer against eve's.
    env(offer(frank, USD(5), XRP(5)));

    env.close();

    auto const ledgerIndex = env.closed()->seq();

    auto wsc = makeWSClient(env.app().config());

    Json::Value jvParams;
    jvParams[jss::ledger_index] = ledgerIndex;

    auto jv = wsc->invoke("book_changes", jvParams);
    auto jrr = jv[jss::result];

    auto const& changes = jrr[jss::changes];

    // BUG: Returns 1 merged entry instead of 2 separate entries.
    // Domain trade (5 XRP / 5 USD) and open trade (5 XRP / 5 USD)
    // are merged into one entry showing 10 XRP / 10 USD.
    BEAST_EXPECT(changes.size() == 2u);

    if (changes.size() == 2u)
    {
        bool foundDomain = false;
        bool foundOpen = false;
        for (unsigned i = 0; i < changes.size(); ++i)
        {
            if (changes[i].isMember(jss::domain))
                foundDomain = true;
            else
                foundOpen = true;
        }
        BEAST_EXPECT(foundDomain);
        BEAST_EXPECT(foundOpen);
    }
}

Run with: .build/xrpld --unittest=xrpl.rpc.BookChanges