feat(core): implement slotUpdatesSubscribe WS RPC method#677
feat(core): implement slotUpdatesSubscribe WS RPC method#677MicaiahReid wants to merge 1 commit into
slotUpdatesSubscribe WS RPC method#677Conversation
Greptile SummaryThis PR implements the
Confidence Score: 4/5The implementation is functionally correct and well-tested; the findings are minor accuracy and logging issues that do not affect data integrity or correctness of the subscription lifecycle. The unsubscribe polling loop logs a spurious error on every normal client disconnect, and SlotTransactionStats always reports a hardcoded num_transaction_entries of 1. Neither causes incorrect behavior for the subscription stream itself, but clients consuming the stats field will receive an inaccurate entry count and production logs will be noisy on routine unsubscribes. The Root event also carries the current slot's wall-clock timestamp rather than the timestamp from when that older slot was actually rooted, which may mislead time-sensitive consumers. All three issues are confined to the new code paths and do not touch existing subscription logic. crates/core/src/rpc/ws.rs (polling loop log level) and crates/core/src/surfnet/svm.rs (stats accuracy and Root timestamp) Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as WS Client
participant WsRpc as SurfpoolWsRpc
participant Map as slots_updates_subscription_map
participant Task as Async Polling Task
participant Locker as SurfnetSvmLocker
participant SVM as SurfnetSvm
Client->>WsRpc: slotsUpdatesSubscribe
WsRpc->>WsRpc: assign_id → sink
WsRpc->>WsRpc: get_svm_locker()
WsRpc->>Task: spawn async task
Task->>Map: insert(sub_id, sink)
Task->>Locker: subscribe_for_slots_updates()
Locker->>SVM: with_svm_writer → push tx
SVM-->>Locker: rx
Locker-->>Task: rx
loop Every slot
SVM->>SVM: notify_slots_updates_subscribers(Frozen)
SVM->>SVM: notify_slots_updates_subscribers(CreatedBank)
SVM->>SVM: notify_slots_updates_subscribers(OptimisticConfirmation)
SVM-->>Task: "Arc<SlotUpdate> via channel"
Task->>Map: read lock → get sink
Task->>Client: sink.notify(Ok(slot_update))
end
Client->>WsRpc: slotsUpdatesUnsubscribe(sub_id)
WsRpc->>Map: write lock → remove(sub_id)
Task->>Map: read lock → get → None
Task->>Task: break loop (task exits)
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let Some(sink) = guard.get(&sub_id) else { | ||
| log::error!("Failed to get sink for subscription ID"); | ||
| break; | ||
| }; | ||
|
|
||
| if let Err(e) = sink.notify(Ok(slot_update)) { | ||
| log::error!("Failed to notify client about slots update event: {e}"); | ||
| break; | ||
| } | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| fn slots_updates_unsubscribe( |
There was a problem hiding this comment.
try_recv, slots_updates_unsubscribe can race in and remove the entry. When it does, guard.get(&sub_id) legitimately returns None — this is the expected clean-exit path, not an internal error. Logging at error level here will pollute production logs with a spurious error on every normal unsubscribe.
| self.notify_slots_updates_subscribers(SlotUpdate::Frozen { | ||
| slot, | ||
| timestamp: slots_update_ts, | ||
| stats: SlotTransactionStats { | ||
| num_transaction_entries: 1, | ||
| num_successful_transactions, | ||
| num_failed_transactions, | ||
| max_transactions_per_entry: num_transactions, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
num_transaction_entries is always hardcoded to 1, and consequently max_transactions_per_entry always equals the total transaction count. On real Solana a block can contain multiple entries (parallel execution batches). While Surfpool's single-threaded execution model means all transactions end up in one logical entry today, hardcoding it means clients that rely on the spec-correct value will silently receive inaccurate data. At minimum a named constant would make the limitation explicit and easier to fix later.
| self.notify_slots_updates_subscribers(SlotUpdate::Frozen { | |
| slot, | |
| timestamp: slots_update_ts, | |
| stats: SlotTransactionStats { | |
| num_transaction_entries: 1, | |
| num_successful_transactions, | |
| num_failed_transactions, | |
| max_transactions_per_entry: num_transactions, | |
| }, | |
| }); | |
| // Surfpool executes transactions sequentially in a single entry per block. | |
| const SURFPOOL_ENTRIES_PER_BLOCK: u64 = 1; | |
| self.notify_slots_updates_subscribers(SlotUpdate::Frozen { | |
| slot, | |
| timestamp: slots_update_ts, | |
| stats: SlotTransactionStats { | |
| num_transaction_entries: SURFPOOL_ENTRIES_PER_BLOCK, | |
| num_successful_transactions, | |
| num_failed_transactions, | |
| max_transactions_per_entry: num_transactions, | |
| }, | |
| }); |
1 participant
No description provided.