Skip to content

Relax invariants in recordStorageChanges for optimization.#5290

Merged
dmkozh merged 1 commit into
stellar:masterfrom
dmkozh:record_opt
May 22, 2026
Merged

Relax invariants in recordStorageChanges for optimization.#5290
dmkozh merged 1 commit into
stellar:masterfrom
dmkozh:record_opt

Conversation

@dmkozh
Copy link
Copy Markdown
Contributor

@dmkozh dmkozh commented May 22, 2026

Description

Relax invariants in recordStorageChanges for optimization.

Instead of checking that created TTL keys belong to the specific entries, we just ensure that the expected entry counts match. This should be a pretty reasonable tradeoff - it seems like it would be pretty hard to create a mismatched TTL entry (vs missing it entirely). The benefit is that we avoid building a whole separate set and making lookups based on TTL keys (a SHA-256 per key).

Checklist

  • Reviewed the contributing document
  • Rebased on top of master (no merge commits)
  • Ran clang-format v8.0.0 (via make format or the Visual Studio extension)
  • Compiles
  • Ran all tests
  • If change impacts performance, include supporting evidence per the performance document

Instead of checking that created TTL keys belong to the specific entries, we just ensure that the expected entry counts match. This should be a pretty reasonable tradeoff - it seems like it would be pretty hard to create a mismatched TTL entry (vs missing it entirely). The benefit is that we avoid building a whole separate set and making lookups based on TTL keys (a SHA-256 per key).
Copilot AI review requested due to automatic review settings May 22, 2026 19:22
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes InvokeHostFunctionOpFrame::recordStorageChanges by replacing a per-key TTL pairing invariant check (that computes TTL keys via hashing) with a cheaper count-based invariant, aiming to reduce CPU overhead in Soroban apply.

Changes:

  • Replaces the “created Soroban entry must have created TTL entry” check from per-key validation to a count equality check.
  • Removes the temporary createdKeys set and replaces it with creation counters plus a cached allowClassicCreations protocol-version check.
  • Avoids computing keySize for TTL entries when metering write bytes (consistent with TTL fees being handled separately).
Comments suppressed due to low confidence (1)

src/transactions/InvokeHostFunctionOpFrame.cpp:684

  • This comment says we "Verify that each newly created Soroban entry has a corresponding newly created TTL entry", but the code now only checks count equality, not 1:1 pairing by key. Please update the comment to match the actual invariant being enforced (count-only), or restore a key-based pairing check if that's still intended.
        // Verify that each newly created Soroban entry has a corresponding
        // newly created TTL entry (1:1 pairing guaranteed by the host).
        releaseAssertOrThrow(numCreatedSorobanEntries == numCreatedTTLEntries);

Comment thread src/transactions/InvokeHostFunctionOpFrame.cpp
@dmkozh dmkozh added this pull request to the merge queue May 22, 2026
Merged via the queue into stellar:master with commit f2d87a6 May 22, 2026
72 checks passed
@dmkozh dmkozh deleted the record_opt branch May 22, 2026 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants