feat(rpc): add streaming SyncAccountVaultV2 endpoint

[kkovaacs]

Summary

As proposed in issue #2356 this PR adds a PoC streaming implementation for syncing account vault changes via a new SyncAccountVaultV2 endpoint. Major changes compared to SyncAccountVault:

• Stream individual vault updates to avoid block-range pagination and response-size estimation.
• Return only the final vault value at the requested block for each key updated within the inclusive BlockRange, omitting intermediate historical values.

Changelog

[[entry]]
scope       = "rpc"
impact      = "added"
description = "Added a new SyncAccountVaultV2 public gRPC endpoint that uses response streaming instead of pagination."

[sergerad]

Hmm I don't think we can do this. This basically allows user requests to control how long a snapshot can be held for. AFAIU this could be a simple OOM DOS vector? Depending on how long streams take, how much users can control their range, and how many streams we allow to be created at once.

[sergerad]

Also the comment overstates what the view gives us: the pin is best-effort, not absolute.
PublishedGenerations::prune_tip only honors a pinned generation for
SNAPSHOT_PRUNE_LAG_CAP (= HISTORICAL_BLOCK_RETENTION = 50) blocks of chain progress
(crates/store/src/state/view/snapshot.rs:53,97) — beyond that the writer prunes anyway,
"accepting the historical-read race for that reader". A slow client on a large vault
(SEND_TIMEOUT is per-item and resets, so a stream can legally live for hours) will have
covering rows pruned between page transactions; later pages silently skip those keys and
the stream still ends OK, which the docs define as "result complete".

I don't think we need the pin at all. The result set for a fixed [from, to] is already
stable under concurrent commits (new rows fail block_num <= block_to; closing an open
row keeps valid_until > block_to); the only mid-stream hazard is pruning. Suggestion:

• don't hold the view across pages — acquire per page fetch;
• after each page's read, check the prune cutoff and terminate the stream with a
  retryable non-OK status if block_to < cutoff (check-after-read closes the race);
• this is the same predicate as the request-time pruned-horizon guard (other comment below),
  so one mechanism fixes both silent-incompleteness paths and removes the
  user-controlled snapshot lifetime entirely.

[kkovaacs]

I wasn't quite sure we can do this, but you're right: since the per-response-chunk timeout is 10s the lifetime of the response stream might actually be significantly longer than ideal.

I've hopefully fixed both: removed the pinned view and added a check so that each chunk of responses we return is still within the retention window. We now return a BlockPruned error if block_to is too old.

See 9d757fd for details.

[sergerad]

scope_range only rejects ranges beyond the tip (block_to > tip → RangeBeyondTip). I think
there's a missing check at the other end: nothing rejects a block_to older than the pruning
horizon (block_to < chain_tip − HISTORICAL_BLOCK_RETENTION).

prune_history deletes superseded rows with valid_until <= chain_tip − HISTORICAL_BLOCK_RETENTION.
This query needs exactly the covering rows (valid_until > block_to), so if block_to is below the
cutoff, a key changed in-range whose covering row was superseded before the cutoff is already
deleted — the key is silently omitted and the stream still ends OK, which the new docs define as
"result complete". The client can't distinguish "no change" from "pruned".

Note this only affects requests targeting an old block: a catch-up client requesting [C+1, tip] is
safe regardless of how old C is, since every covering row it needs has
valid_until > block_to ≥ cutoff and thus can't have been pruned.

The analogous point-read path fails loudly here (GetAccountError::BlockPruned,
crates/store/src/state/view/account/mod.rs:91); I think this endpoint needs the equivalent guard —
reject block_to < chain_tip − HISTORICAL_BLOCK_RETENTION with a BlockPruned-style error so the
client re-requests against a newer target instead of committing an incomplete delta.

[kkovaacs]

Added in 9d757fd.

[Mirko-von-Leipzig]

Looks pretty good to me.

[Mirko-von-Leipzig]

Why do we want also traverse by block number? I thought it would just be the keys themselves?

[kkovaacs]

The cursor's shaped like (block_num, vault_key) so that it matches the primary index on the table: PRIMARY KEY (account_id, block_num, vault_key). This way the cursor (plus the implicit account_id we always have in the query) expresses a single point in the primary key space.

[Mirko-von-Leipzig]

Maybe to rephrase my question a bit: are we returning every change to a key within that range, or only the final value at the end of the requested range?

[kkovaacs]

Only the final value. Quoting from the doc string of select_account_vault_updates_v2:

Requiring valid_until > block_to removes intermediate updates while retaining a historical value that was superseded after the target.

[Mirko-von-Leipzig]

Should this check come before the page load?

Ideally this would all be part of the state view that we get implicitly.

[kkovaacs]

Sure, we can move that check before the page load. The important thing is that it runs within the same transaction so that pruning cannot happen after the check.

Unfortunately StateView cannot guarantee that pruning doesn't remove historical data that's still visible relative to the block number. History pruning keys off the oldest live snapshot generation. However, to prevent extreme cases where a slow reader would stall pruning indefinitely old views are ignored once they falls more than HISTORICAL_BLOCK_RETENTION blocks behind the chain tip.

[Mirko-von-Leipzig]

However, to prevent extreme cases where a slow reader would stall pruning indefinitely old views are ignored once they falls more than HISTORICAL_BLOCK_RETENTION blocks behind the chain tip.

My perspective is that this should be a panic situation. As in, pruning should respect existing views, and if they take very long then clearly we have a major bug that requires fixing. And given that this would only affect full nodes in production, chain would continue even if this takes down the public facing RPC.

Unfortunately StateView cannot guarantee that pruning doesn't remove historical data that's still visible relative to the block number.

This should never be allowed imo - it basically removes the entire point of the view/snapshot.

[kkovaacs]

I think @sergerad might have opinions on this.

Since we're re-creating the StateView per page read I think this streaming-responses implementation is no worse than the old paged implementations?

The old implementation of select_account_vault_assets seems to have the same issue re pruning. Once pruning removes account vault data because the start of the block range goes out of the retention block range we'll return partial data. If the end of the block range goes out of the retention block range the client might miss entries completely.

[Mirko-von-Leipzig]

I may be misunderstanding. I just mean that once we have a view, it cannot be moved out from under us.

I imagined the usage/API would look something like this:

/// A read-only view of the state at a given moment in time.
///
/// SQLite, RocksDb and tips are guaranteed to be consistent wrt to each
/// other, and cannot move out from under you while this view is held.
struct View {
    db: DbTransaction,
    smt: RocksDbSnapshot,
    chain_tip: BlockNumber,
    proven_tip: BlockNumber,
}

async fn next_page(&self, state_at: BlockNumber, cursor: Key) -> Result<Page, Err> {
    let view = self.state.view().await?;

    // Does this view still support the block we need?
    if !view.contains(state_at) {
        return Err(Err::BlockPruned);
    }
    
    view.get_page(cursor, 1024).await
}

[kkovaacs]

I'm not sure we can hold a SQLite transaction for the lifetime of a stream of responses. We have a 10s send timeout for each individual value, so a rogue / slow client can control how long it would hold a transaction (and the view).