feat(store): cache account witnesses for tracked accounts - #2476
feat(store): cache account witnesses for tracked accounts#2476sofiazcoaga wants to merge 5 commits into
Conversation
|
@igamigo the issue mentions "we should also keep account witnesses for watched accounts, which we already request anyway", but I deliberately did not add watched accounts to account witness fetching automatically. Watched accounts are only refetched during sync when they have been modified, so for an unmodified one there is no request to reuse. Automatically tracking their account witnesses would mean an RPC call for every watched account on every sync, so a client watching N accounts for observability would pay N extra requests per sync. I believe this should be a conscious decision. Let me know your thoughts on this. |
juan518munoz
left a comment
There was a problem hiding this comment.
Looks good! Only left a comment about a possible optimization (I might have overlooked something that makes it senseless)
| // Cache MMR so pruning can reuse in-memory MMR. | ||
| self.cache_partial_mmr(partial_mmr).await?; | ||
|
|
||
| self.refresh_account_witnesses().await; |
There was a problem hiding this comment.
I wonder if we can cache the witnesses inside state_sync.sync_state and drop
refresh_account_witnesses entirely.
Doing this it means we lose the guarantee that every registered account has a fresh witness after a sync, since only the changed accounts get their witnesses updated. But we can cover the rest lazily in fetch_public_account_inputs, which already falls back to get_account there and caches the returned AccountCode, it just discards the witness it fetched. We can stamp the witness at the block the proof came back at (the _block_num we currently drop), and any further tx against the same account at the same reference block hits the cache.
This way witness cache works under these conditions:
- changed in the last sync: free hit from the overall sync.
- unchanged, first tx at this tip: one request (which we already pay today) then cached.
- unchanged, later txs at the same tip: cache because of above.
One caveat is that fetch_public_account_inputs does not validate the witness it receives
today, which is fine while it dies with the transaction, so persisting it would need a
validate_account_witness call first.
Another upside to this is that we could address the issue's "keep account witnesses for watched accounts" at no extra cost.
Problem
Every FPI transaction fetches the foreign account's AccountWitness from the node. N transactions between two syncs cost N GetAccount requests, even against the same account at the same block.
Changes
Client::track_account_witnessregisters an account whose account witness should be kept fresh. Newaccount_witnessestable, one row per account.syncrefreshes them at the new sync height, up to four requests in flight. Each witness is validated against the synced block header's account root before it is stored.AccountInputslocally. Both fall back to the existing request when there is nothing usable.The cost moves from once per transaction to once per sync.
Closes #2443