Skip to content

chore: Refactor epilogue to run as much code as possible before fees are computed #1698

Merged
bobbinth merged 15 commits intonextfrom
pgackst-late-compute-fee
Aug 7, 2025
Merged

chore: Refactor epilogue to run as much code as possible before fees are computed #1698
bobbinth merged 15 commits intonextfrom
pgackst-late-compute-fee

Conversation

@PhilippGackstatter
Copy link
Contributor

@PhilippGackstatter PhilippGackstatter commented Aug 6, 2025

Refactors the epilogue to run as much code as possible before fees are computed. The motivation is that every operation that runs after compute_fee has to be included in the total cycle count determined by compute_fee and so we have to calculate them. Therefore, making this as simple as possible is a high priority.

Changes

  • Removes any branches from the epilogue after compute_fee which makes it possible to compute the number of cycles that section takes.
  • Remove the new_nonce > old_nonce check, because we're already checking this in the account delta computation by asserting that if the nonce wasn't incremented, storage and vault delta must be empty. This allows us to move this check before compute_fee as well, as it no longer depends on having the FINAL_ACCOUNT_COMMITMENT.
  • Move output vault and output notes commitment before compute_fee.
  • We're now building the output vault before compute_fee and so the account vault state that is set as the initial output vault still has FEE_ASSET in it. Since we're not moving FEE_ASSET to an output note, we don't have to account for it in the output vault at all and the asset preservation check is essentially bypassed. That's a bit subtle, but I've added comments to explain this where it made sense.

Follow-up:

  • I think we should add caching to account::compute_current_storage_commitment. That way, we can pre-compute this value before compute_fee is called, and when we call it again as part of account::compute_current_commitment (to compute the final account commitment) we are guaranteed to hit the cached value because storage doesn't change as part of the fee code. That should make it possible to statically compute the number of cycles that account::compute_current_commitment will take. Wanted to double-check that this is fine to do.
  • In a follow-up PR I would move account_delta::compute_commitment before compute_fee as well, but this has additional implications mentioned in Calculate number of cycles after compute_fee #1688, so that will be part of a follow-up PR.

With that, all other code that runs after compute_fee are simple instructions, and so computing the number of cycles should be much easier than before.

part of #1688

Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you! I left a few small comments inline.

Also, I wonder how this changes our current estimate of the "cycles after fee computed" - but maybe we can check this in future PRs.

@bobbinth bobbinth merged commit dad5090 into next Aug 7, 2025
19 checks passed
@bobbinth bobbinth deleted the pgackst-late-compute-fee branch August 7, 2025 17:40
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.

2 participants