Skip to content

Add finalized did:ethr event-history caching - #128

Merged
moisesja merged 3 commits into
mainfrom
feat/issue-118-finalized-event-cache
Aug 3, 2026
Merged

Add finalized did:ethr event-history caching#128
moisesja merged 3 commits into
mainfrom
feat/issue-118-finalized-event-cache

Conversation

@moisesja

@moisesja moisesja commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Fixes #118

Summary

  • add an opt-in, resolver-owned cache for finalized ERC-1056 event-history prefixes
  • query the optional finalized block tag and fall back to the full walk when unsupported
  • keep changed(identity) and all post-watermark events live while eliminating repeat eth_getLogs calls for immutable history
  • revalidate cached raw logs through the existing registry, identity, block, log-index, ABI, and previousChange checks
  • bound retained/cache memory, prevent stale watermark regression, and keep existing public construction/DI behavior default-off
  • document the configuration and resolution contract

Why

did:ethr resolution previously replayed every historical change block on every request. Long-lived resolvers therefore paid one sequential eth_getLogs round trip per history block even after those blocks were finalized and immutable.

Validation

  • dotnet build netdid.sln -c Release --no-restore --tl:off --disable-build-servers -m:1 — 0 warnings, 0 errors
  • dotnet test netdid.sln -c Release --no-build — 1,653 passed, 7 environment-gated real-registry tests skipped, 0 failed
  • focused issue did:ethr: finality-aware caching of ERC-1056 event history #118 suite — 18 passed
  • all five offline samples exited successfully
  • independent adversarial review plus focused re-attack — clean verdict
  • git diff --check — clean

Compatibility and impact

Caching remains disabled by default. Existing DidEthrMethod construction and one-argument AddDidEthr overloads preserve the full uncached walk. Write/deactivate behavior and package versions are unchanged.

@moisesja moisesja self-assigned this Aug 3, 2026
@moisesja moisesja added this to net-did Aug 3, 2026
@moisesja moisesja added this to the 3.1.0 milestone Aug 3, 2026
@github-project-automation github-project-automation Bot moved this to Backlog in net-did Aug 3, 2026

moisesja commented Aug 3, 2026

Copy link
Copy Markdown
Owner Author

Reviewed the diff (DidEthrMethod.cs, CachedEthrEventHistory.cs, NetDidBuilder.cs, DefaultEthereumRpcClient.cs/IEthereumFinalityRpcClient.cs, tests, docs).

Overall: solid, well-defended change. Notes below, no blocking objections.

What's good

  • Correctly default-off (cacheFinalizedEventHistory: false unless explicitly opted in), and the cache type stays internal (only InternalsVisibleTo'd to the DI project) — an unrelated shared cache writer can't forge a structurally valid history, which is the right trust boundary for this class of feature.
  • Cache reads are not trusted data: AppendCachedHistory/SnapshotCachedBlockLogs re-run the exact same registry/identity/block/logIndex/previousChange validation as fresh RPC logs (via the shared ValidateEventBlock), and a corrupt or truncated cached prefix fails closed (IncompleteEventHistoryException/EthereumInteractionException) instead of silently returning a partial document.
  • Traced the partial-cache-hit path specifically (cache watermark older than the current finalized block, with a new event in between): CommitValidatedBlock is invoked both for freshly-walked RPC blocks and for blocks replayed out of AppendCachedHistory, so WriteCachedHistory always gets fed the full contiguous prefix down to genesis, not just the newly-fetched delta. That's what keeps SetIfNewer's full-replacement semantics (no merge) from silently truncating the cache on the next write — this is the trickiest part of the design and it holds up. Good that Issue118_ChangeAfterCachedWatermark_IsFetchedAndAppliedOnNextResolution exercises exactly this.
  • hops, byte, and event bounds are shared by ref across the RPC-walk and cache-replay code paths, so the existing DoS bounds (MaxEventChainHops, MaxCollectedEvents, MaxCollectedBytes) aren't bypassable just because a block came from cache instead of the wire.
  • Finality is treated as an optional, fail-soft capability (IEthereumFinalityRpcClientnull on unsupported/malformed responses), so chains without a finalized tag get the pre-existing full-walk behavior with no behavior change.
  • Cache read/write failures (disposed cache, throwing custom IEthrEventHistoryCache impl) are swallowed as optimization misses rather than resolution errors — appropriate given the cache is optional.
  • Docs (README/CHANGELOG) match what the code actually does, including the caveats (64 MiB aggregate bound, per-entry 5,000-event/32 MiB bounds, empty-history histories not cached).

Minor comments, non-blocking

  • The core file grew by ~570 lines and adds a second, cache-aware validation/traversal path (AppendCachedHistory/ValidateEventBlock/CommitValidatedBlock) alongside the original walk. Given this sits directly on the did:ethr authorization-history trust boundary, worth a second human pass beyond the self-review noted in the PR description, even though the automated adversarial pass and build-and-test check are green.
  • WriteCachedHistory/CommitValidatedBlock's behavior on hitting the independent raw-cache byte budget is to discard the entire accumulated prefix (cacheEligibleBlocks.Clear()) rather than caching a partial one — that's correct given the "contiguous prefix to genesis" invariant the read path relies on, just worth a one-line comment there for the next reader since it's non-obvious why a partial cache isn't kept.

Test coverage for the cache (default-off, stale-watermark rejection, corrupt-cache fail-closed, unregistered-identity non-caching, size caps, read/write failure fallback) looks appropriately adversarial. Nothing here should block merge once it's out of draft.


Generated by Claude Code

@moisesja moisesja moved this from Backlog to In review in net-did Aug 3, 2026
@moisesja
moisesja marked this pull request as ready for review August 3, 2026 12:51

moisesja commented Aug 3, 2026

Copy link
Copy Markdown
Owner Author

Revalidated head 28e1f8c. The only change since my review is the two-line comment in CommitValidatedBlock explaining why the raw-cache candidate is discarded wholesale on budget overflow (the contiguous-prefix-through-genesis invariant) — exactly the clarification requested, no behavior change. build-and-test is green on the new head. All prior review notes stand addressed; nothing blocking from my side once the PR is taken out of draft.


Generated by Claude Code

@moisesja
moisesja merged commit 0b065a1 into main Aug 3, 2026
1 check passed
@github-project-automation github-project-automation Bot moved this from In review to Done in net-did Aug 3, 2026
@moisesja
moisesja deleted the feat/issue-118-finalized-event-cache branch August 3, 2026 15:14
moisesja added a commit that referenced this pull request Aug 3, 2026
…loads (#119)

The rebase onto main brought PR #128's two-arg overloads — new RpcUrl supply
points, so they get the same archive-grade requirement note.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

did:ethr: finality-aware caching of ERC-1056 event history

1 participant