Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memoize transaction related hashes #16056

Draft
wants to merge 3 commits into
base: compatible
Choose a base branch
from

Conversation

georgeee
Copy link
Member

Some hashes that are computed during staged ledger application are dependent only on a transaction itself. These hashes could be precomputed before staged ledger application, to minimize time of block creation.

Main change revolves around introducing and using the following two types:

type zkapp_precomputed_t =
    { all_account_updates :
        ( Account_update.t
        , Zkapp_command.Digest.Account_update.t
        , Zkapp_command.Digest.Forest.t )
        Zkapp_command.Call_forest.t
    ; init_account_update_result : get_next_account_update_result
    ; tx_commitment_on_start : Inputs.Transaction_commitment.t
    ; full_commitment_on_start : Inputs.Transaction_commitment.t
    ; lookup_actions_hash : Actions.t -> Field.t option
    }

type precomputed_t =
    { memo_hash : Random_oracle.Digest.t
    ; derived_token_ids : Token_id.t Account_id.Map.t
    ; zkapp_precomputed : zkapp_precomputed_t option
    }

Type precomputed_t contains data that is dependent only on a transaction (and not on the ledger to which it is to be applied). It contains some hashes either in direct form (as memo_hash) or indirect form (as all_account_updates, for computation of which some hashing is used deep inside the function call).

TODO this PR lacks a change to block producer to keep precomputed_t in transaction pool and use for block creation.

As measured by mina ledger test apply benchmark, this PR improves performance of staged ledger diff application by some 27% (measured together with #15784, #15980, #16055).

Explain how you tested your changes:

  • Ran a benchmark
  • Ran CI/nightly tests

Checklist:

  • Dependency versions are unchanged
    • Notify Velocity team if dependencies must change in CI
  • Modified the current draft of release notes with details on what is completed or incomplete within this project
  • Document code purpose, how to use it
    • Mention expected invariants, implicit constraints
  • Tests were added for the new behavior
    • Document test purpose, significance of failures
    • Test names should reflect their purpose
  • All tests pass (CI will check this if you didn't)
  • Serialized types are in stable-versioned modules
  • Does this close issues?

Context: when staged ledger diff application is performed, some hash
operations that are invoked do not depend on an underlying ledger, but
only on the transaction.

Problem: when staged ledger diff application is executed within context
of block creation, these hashing routines unnecessarily slow down the
block creation.

Solution: allow to precompute certain values for a transaction and be
supplied for staged ledger diff application.

P.S. this commit is first in series, and it doesn't use precomputed data
everywhere it could be used. Neither it updates block creation and tx
pool.
Use derived token ids in masks: store them in masks and use instead of
re-deriving them during update operations.
@georgeee georgeee self-assigned this Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant