GET with subscribe=true doesn't create network subscriptions for locally cached contracts

[sanity]

GET with subscribe=true doesn't create network subscriptions for locally cached contracts

Problem Description

When a client performs a GET operation with subscribe: true on a contract that is already cached locally, only a local subscription listener is registered but no network subscription is created with the gateway. This causes the client to miss UPDATE notifications from other peers.

Code Analysis

1. Local Cache Hit Path

In crates/core/src/client_events/mod.rs, when handling ContractRequest::Get with subscribe: true:

When contract is found locally (lines 519-571):

if (!return_contract_code && state.is_some())
    || (return_contract_code && state.is_some() && contract.is_some())
{
    if let Some(state) = state {
        // Handle subscription for locally found contracts
        if subscribe {
            if let Some(subscription_listener) = subscription_listener {
                tracing::debug!(%client_id, %key, "Subscribing to locally found contract");
                let register_listener = op_manager
                    .notify_contract_handler(
                        ContractHandlerEvent::RegisterSubscriberListener {
                            key,
                            client_id,
                            summary: None,
                            subscriber_listener: subscription_listener,
                        },
                    )
                    .await
                    // ...
            }
        }
        
        return Ok(Some(Either::Left(QueryResult::GetResult {
            key,
            state,
            contract,
        })));
    }
}

This path only registers a local listener and returns immediately without creating any network subscription.

2. Cache Miss Path (Contract NOT found locally)

When contract is not cached (lines 573-598):

else {
    // Initialize a get op.
    tracing::debug!(
        this_peer = %peer_id,
        "Contract not found, starting get op",
    );

    let op = get::start_op(key, return_contract_code, subscribe);
    // ... continues with GET operation
}

This initiates a GET operation that MAY create a network subscription, but with an important limitation...

3. GET Messages Don't Forward Subscribe Flag

In operations/get.rs, the GET message types don't include a subscribe field:

pub(crate) enum GetMsg {
    RequestGet {
        id: Transaction,
        target: PeerKeyLocation,
        key: ContractKey,
        fetch_contract: bool,
        skip_list: HashSet<PeerId>,
        // Note: NO subscribe field
    },
    SeekNode {
        id: Transaction,
        key: ContractKey,
        fetch_contract: bool,
        target: PeerKeyLocation,
        sender: PeerKeyLocation,
        htl: usize,
        skip_list: HashSet<PeerId>,
        // Note: NO subscribe field
    },
    // ...
}

The subscribe flag is only stored in local state (GetState::AwaitingResponse { subscribe, ... }) and is NOT communicated to other peers.

4. Subscription Logic at Receiving Peers

When a peer receives a contract via GET and caches it (lines 915-932):

let is_subscribed_contract = op_manager.ring.is_seeding_contract(&key);

// Start subscription if not already seeding
if !is_subscribed_contract {
    tracing::debug!(tx = %id, %key, peer = %op_manager.ring.connection_manager.get_peer_key().unwrap(), "Contract not cached @ peer, caching");
    op_manager.ring.seed_contract(key);
    let mut new_skip_list = skip_list.clone();
    new_skip_list.insert(sender.peer.clone());

    super::start_subscription_request(
        op_manager,
        key,
        false,
        new_skip_list,
    )
    .await;
}

Intermediate peers only create subscriptions if they're not already seeding the contract, regardless of the original requester's subscribe intent.

Comparison with PUT behavior

PUT operations handle subscriptions differently in operations/put.rs:491:

if subscribe && is_seeding_contract {
    tracing::debug!(
        tx = %id,
        %key,
        peer = %op_manager.ring.connection_manager.get_peer_key().unwrap(),
        "Starting subscription request"
    );
    super::start_subscription_request(
        op_manager,
        key,
        false,
        HashSet::new(),
    )
    .await;
}

PUT creates network subscriptions even when the contract is already seeded, ensuring the subscription intent is honored.

Test Case Demonstrating the Issue

Using the gateway test framework with River chat:

1. User1 creates room (PUT with subscribe=true)
2. User2 joins room (GET with subscribe=true)
3. User2 sends message (UPDATE)
4. User1 never receives User2's message

Evidence from logs:

Gateway shows only one subscription (from User1):

20:23:20.991 - Peer successfully subscribed to contract, tx: 01K1RSMSM8AGZEJ0NKVAQK6G03, key: ATnn213q2PvKx3CSm2MQhqhvPHNHJbSzwvXtS11RzZzT, subscriber: v6MWKgqJG3pVZkfz

User2's GET completes instantly (served from local cache):

20:23:21.012 - sending request request=ContractOp(Get { key: ContractKey { instance: ContractInstanceId("ATnn213q2PvKx3CSm2MQhqhvPHNHJbSzwvXtS11RzZzT"), code: None }, return_contract_code: false, subscribe: true })
20:23:21.013 - Successfully retrieved room state

User2's UPDATE goes to gateway but isn't forwarded to User1:

20:23:26.094 - UPDATE RequestUpdate: forwarding update from v6MWKgqJG3pVZkfz to v6MZagaafJbiyF1z

Summary of Issues

1. Local cache hits bypass network subscription: GET with subscribe=true on cached contracts only creates local listeners
2. Subscribe flag not forwarded: The GetMsg types don't include subscribe field, so intermediate peers don't know about subscription intent
3. Inconsistent behavior: PUT operations honor subscribe=true even for cached contracts, but GET operations don't

Potential Solutions

1. Consistent subscription behavior: When subscribe: true is specified, always create a network subscription regardless of cache status (similar to PUT behavior)

2. Forward subscribe flag: Include subscribe field in GetMsg variants so intermediate peers can honor the subscription intent

3. Minimal fix: In the local cache hit path, after registering the local listener, also call start_subscription_request to ensure network subscription

The current optimization (serving cached data without network round-trip) improves performance but breaks the subscription contract when subscribe: true is explicitly requested.

[gogo2464]

Is it a good first issue? I may try sunday.

[gogo2464]

edit: sorry it has already started #1739

[sanity]

Per @netsirius, subscribed:true is not yet implemented for GET.