`balanceChanges` in legacy JSON-RPC loses `Owner` kind and reconstructs everything as `AddressOwner`

[fallintoplace]

Summary

The public legacy JSON-RPC BalanceChange type exposes owner: Owner, but the balance-change derivation path used by legacy JSON-RPC collapses coin ownership into SuiAddress and then reconstructs every derived entry as Owner::AddressOwner(...).

This means object-owned and consensus-owned coin balance changes are reported as address-owned balance changes.

Affected surfaces

This appears to affect every legacy JSON-RPC surface that derives balance changes through get_balance_changes_from_effect, including:

• executeTransactionBlock
• dryRunTransactionBlock
• getTransactionBlock
• multiGetTransactionBlocks

Why this happens

The evidence chain seems to be:

• Public legacy JSON-RPC BalanceChange carries owner: Owner in crates/sui-json-rpc-types/src/balance_changes.rs
• Shared sui-types balance-change structs carry only address: SuiAddress in crates/sui-types/src/balance_change.rs
• coins() flattens AddressOwner, ObjectOwner, and ConsensusAddressOwner into the same address-like key
• derive_detailed_balance_changes() accumulates coin and address-balance deltas by (address, coin_type)
• Legacy JSON-RPC reconstructs every result as Owner::AddressOwner(change.address) in crates/sui-json-rpc/src/balance_changes.rs

Current reconstruction:

derive_balance_changes(effects, &input_coins, &mutated_coins)
    .into_iter()
    .map(|change| BalanceChange {
        owner: Owner::AddressOwner(change.address),
        coin_type: change.coin_type,
        amount: change.amount,
    })

Minimal reproducer sketch

This is modeled on the existing tests in crates/sui-json-rpc/src/balance_changes.rs.

#[tokio::test]
async fn object_owned_coin_balance_change_keeps_owner_kind() {
    let parent = ObjectID::random();
    let gas_id = ObjectID::random();
    let old_version = SequenceNumber::from_u64(1);
    let new_version = SequenceNumber::from_u64(2);

    let input_obj = Object::new_move(
        MoveObject::new_gas_coin(old_version, gas_id, 1_000),
        Owner::ObjectOwner(parent.into()),
        TransactionDigest::random(),
    );
    let output_obj = Object::new_move(
        MoveObject::new_gas_coin(new_version, gas_id, 600),
        Owner::ObjectOwner(parent.into()),
        TransactionDigest::random(),
    );

    let mut changed_objects = BTreeMap::new();
    changed_objects.insert(
        gas_id,
        EffectsObjectChange::new(
            Some(((old_version, input_obj.digest()), Owner::ObjectOwner(parent.into()))),
            Some(&output_obj),
            false,
            false,
        ),
    );

    let effects = TransactionEffects::new_from_execution_v2(
        ExecutionStatus::Success,
        0,
        GasCostSummary::default(),
        vec![],
        std::collections::BTreeSet::new(),
        TransactionDigest::random(),
        new_version,
        changed_objects,
        Some(gas_id),
        None,
        vec![],
    );

    let mut provider = MockObjectProvider::new();
    provider.insert(input_obj);
    provider.insert(output_obj);

    let changes = get_balance_changes_from_effect(&provider, &effects, vec![], None)
        .await
        .unwrap();

    assert_eq!(changes.len(), 1);
    assert_eq!(changes[0].coin_type, GAS::type_tag());
    assert_eq!(changes[0].amount, -400);

    // Expected:
    assert_eq!(changes[0].owner, Owner::ObjectOwner(parent.into()));

    // Actual today:
    // Owner::AddressOwner(parent.into())
}

Expected JSON shape:

{
  "owner": { "ObjectOwner": "0x..." },
  "coinType": "0x2::sui::SUI",
  "amount": "-400"
}

Actual JSON shape today:

{
  "owner": { "AddressOwner": "0x..." },
  "coinType": "0x2::sui::SUI",
  "amount": "-400"
}

Worse than relabeling

This is not only owner relabeling.

A ConsensusAddressOwner { owner: a, ... } coin delta and an accumulator/address-balance delta for AddressOwner(a) are both keyed by (address, coin_type), so they can merge and even net out entirely.

That means the bug can erase distinct balance-change entries, not just mislabel them.

Suggested fix direction

Preserve Owner through the internal balance-change structs/maps for the JSON-RPC-facing derivation path, and use Owner::AddressOwner(addr) only for accumulator/address-balance events.

The indexer-alt path already follows that shape:

• accumulator events are keyed as Owner::AddressOwner(addr)
• coin deltas use object.owner().clone()

Useful regression tests

1. Object-owned coin: owner stays Owner::ObjectOwner(parent)
2. Consensus-owned coin: owner stays Owner::ConsensusAddressOwner { owner, start_version }
3. Collision regression: same a, same coin type, ConsensusAddressOwner { owner: a } coin delta -100 plus accumulator AddressOwner(a) delta +100 should produce two separate entries, not zero entries
4. JSON-RPC smoke: get_balance_changes_from_effect should not blindly emit Owner::AddressOwner for non-address-owned coin objects

Comparison point

For reference, the indexer-alt path already preserves owner kind for coin deltas in crates/sui-indexer-alt/src/handlers/tx_balance_changes.rs by using object.owner().clone() for coins and Owner::AddressOwner(addr) only for accumulator events.

[github-actions]

Thank you for opening this issue, a team member will review it shortly. Until then, please do not interact with any users that claim to be from Sui support and do not click on any links!

[terranweb3]

Thanks for the detailed report. I agree this seems like a real API correctness issue, not just owner relabeling.

Once the legacy JSON-RPC derivation path collapses Owner into SuiAddress, the original owner kind is lost, so reconstructing all entries as Owner::AddressOwner(...) can mislead clients that rely on balanceChanges.owner.

The collision/net-out case is especially important: ConsensusAddressOwner { owner: a, ... } and AddressOwner(a) should remain distinct balance-change entries, not be accumulated under the same (address, coin_type) key.

Preserving Owner through the internal balance-change map sounds like the right fix direction. Owner::AddressOwner(addr) should only be used for accumulator/address-balance events, while coin-object deltas should preserve the actual object.owner().clone().