Potential Bug: `NetMessageV1::Unsubscribed` handler calls `subscribe` instead of removing the unsubscribing peer

[sanity]

Title: Potential Bug: NetMessageV1::Unsubscribed handler calls subscribe instead of removing the unsubscribing peer

Labels: bug, network, subscriptions

Body:

Description:

During a code review focused on update propagation and subscription management, identified that the handler for NetMessageV1::Unsubscribed messages appears to have unexpected behavior.

The handler is located in crates/core/src/node/mod.rs within the process_message_v1 async function:

// In crates/core/src/node/mod.rs, inside process_message_v1

            // ... other message types ...

            NetMessageV1::Unsubscribed { ref key, .. } => {
                // This is the handler logic:
                if let Err(error) = subscribe(op_manager, *key, None).await {
                    tracing::error!(%error, "Failed to subscribe to contract");
                }
                break; // Exits the retry loop for this message
            }
            _ => break, // Handles other cases
        }

Actual Behavior:

When a node receives an Unsubscribed { key, from, .. } message, indicating that the from peer no longer wishes to receive updates for key, the receiving node executes subscribe(op_manager, *key, None).await. It attempts to subscribe itself (or ensure its own subscription) to the contract that the other peer just unsubscribed from.

Crucially, it does not appear to remove the from peer from its local subscriber list for the specified key (e.g., there is no call like op_manager.ring.remove_subscriber(key, from_peer_id)).

Expected Behavior:

Receiving an Unsubscribed message from a peer should typically trigger the removal of that peer from the local node's subscriber list for the specified contract key. This ensures the subscriber list accurately reflects which peers are currently interested in updates.

Impact:

1. Inaccurate Subscriber Lists: The primary issue is that the subscriber list maintained by the Ring (specifically SeedingManager::subscribers) may become inaccurate over time. Peers that have explicitly unsubscribed might remain on the list.
2. Incorrect Update Propagation: Peers that have unsubscribed might still receive UpdateMsg::BroadcastTo messages for the contract, leading to unnecessary network traffic and processing.
3. Unnecessary subscribe Calls: The handler triggers potentially redundant subscribe operations on the receiving node, which might add unnecessary load or network requests.
4. Over-reliance on Disconnect Pruning: This leaves the location-based prune_subscriber (triggered on peer disconnect) as potentially the only mechanism removing these entries, which might be insufficient if the peer doesn't disconnect or changes location properties used for pruning.

Suggestion:

Review the logic within the NetMessageV1::Unsubscribed handler in process_message_v1. Consider changing the implementation to:

1. Remove the call to subscribe(op_manager, *key, None).await.
2. Instead, call the appropriate function on the Ring or SeedingManager to remove the from peer from the subscriber list associated with the key.
3. Verify if the current subscribe call serves any other critical, intended purpose in this specific context that needs to be preserved or handled differently.

[iduartgomez]

@sanity after reading the code, I think we are definitively missing a call to seeding_manager.prune_subscriber after receiving the unsubscribe notification, and I don't think we need to call subscribeagain, cause this is for cleaning up downstream subscribers.

However, I don't think this is the origin of the problems we are currently seeing as unsubcribe events are not even happening, and those only happen after a peer stops seeding a contract (which ain't happening right now).