Skip to content

feat: Use PartialAccount in TransactionInputs#1840

Merged
bobbinth merged 13 commits intonextfrom
pgackst-transaction-inputs-partial-account
Sep 3, 2025
Merged

feat: Use PartialAccount in TransactionInputs#1840
bobbinth merged 13 commits intonextfrom
pgackst-transaction-inputs-partial-account

Conversation

@PhilippGackstatter
Copy link
Copy Markdown
Contributor

@PhilippGackstatter PhilippGackstatter commented Sep 2, 2025

Use PartialAccount in TransactionInputs as preparation for lazy account loading.

Changes:

  • The above change has ripple effects, namely that ExecutedTransaction no longer contains the full Account state, and this creates issues for ProvenTransaction (see below).
  • Avoids cloning input notes in LocalTransactionProver by returning the input notes when destructuring the prover host into its parts.
  • Test Code
    • Remove From<ExecutedTransaction> for TxContextInput, which we can no longer support due to the above change.
    • I added the full account state to TransactionContext, for convenience in tests (which was previously taken from TransactionInputs).

Notes:

  • PartialAccount should eventually implement SequentialCommit, but doing that is out of scope since it requires a few changes. We can update this in one go together with Account::commitment.

Since we no longer have the full account state after executing a transaction, the main question here is how we want new public accounts to be represented in ProvenTransaction:

  1. Contain the full Account state directly like now.
  2. Contain an AccountDelta that, when applied to an "empty account", results in the full Account state.

The second option could be nicer as we reduce the number of variants of AccountUpdateDetails to two and it could look like this:

pub enum AccountUpdateDetails {
    /// The Account is private (no on-chain state change).
    Private,

    /// The delta for public accounts.
    Public(AccountDelta),
}

Moreover, it would mean:

  • that account delta commitments become more meaningful for new accounts, since they'd actually include all "changes" to the account. If every transaction can be represented as an account delta, that should allow for more uniformity when handling SigningInputs::TransactionSummary, for example.
  • A TxAccountUpdate contains the account delta commitment which can be used to check that the delta contained in AccountUpdateDetails::Delta matches that commitment. However, we can't do that for AccountUpdateDetails::New. Removing New would allow validating the delta of a new account as well.

We can't easily do the account delta option yet, due to missing account code tracking and other things as mentioned here. So, for now, option 1 is implemented in this PR, but I would consider it a temporary solution.

Part of #401

This comment was marked as resolved.

Copy link
Copy Markdown
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 comments inline - but they are mostly naming and code organization related.

Copy link
Copy Markdown
Collaborator

@mmagician mmagician left a comment

Choose a reason for hiding this comment

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

LGTM ✅

Copy link
Copy Markdown
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!

@bobbinth bobbinth merged commit 269472f into next Sep 3, 2025
18 checks passed
@bobbinth bobbinth deleted the pgackst-transaction-inputs-partial-account branch September 3, 2025 21:05
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.

4 participants