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

Implement fast summarization #271

Merged
merged 20 commits into from
Jan 16, 2024
Merged

Implement fast summarization #271

merged 20 commits into from
Jan 16, 2024

Conversation

anvacaru
Copy link
Contributor

@anvacaru anvacaru commented Jan 12, 2024

fixes: #266

Changes:

  • add a new --init-node-from flag that takes a JSON file containing the summary of a deployment process. Takes the accounts obtained after going through the summary and inserts them in the <accounts> cell of the initial node of the KCFG.

  • Extract the logic that reads the JSON summary files in foundry_summary() to its own read_summary() function.

  • Refactor DeploymentSummary class:

    • extract the logic that processes a summary entry into its own SummaryEntry class.
    • refactoring the DeploymentSummary.add_cheatcode(dict) that would process a summary entry and create the Solidity code for the cheat codes required to update the state to DeploymentSummary.extend(SummaryEntry).
  • Add summary_to_account_cells(dict) that goes through the summary JSON object and creates a list of <account> cells.

Minor changes:

  • replace occurrences of KApply(.Map) to map_empty()
  • remove unused _write_cfg function

Next steps:

  1. Apply the state diff changes over an account that has symbolic fields like {'balance': KVariable('ACCT_X_BAL'), 'nonce': KVariable('ACCT_X_NONCE') , ...} instead of {'balance': 0, 'nonce': 0, 'code': '', 'storage': {}}
  2. Insert the set of account addresses in <accessedAccounts>,<touchedAccounts> (not sure actually?)
  3. Currently, the storage updates from the summary are placed only in <storage>, apply them in <origStorage> as well.

@anvacaru anvacaru added enhancement New feature or request engagement labels Jan 12, 2024
@anvacaru anvacaru self-assigned this Jan 12, 2024
@anvacaru anvacaru requested a review from tothtamas28 January 15, 2024 13:59
Copy link
Contributor

@tothtamas28 tothtamas28 left a comment

Choose a reason for hiding this comment

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

LGTM.

It would be nice to have another reviewer look at it before merging.

@ehildenb
Copy link
Member

It would be good to test this on the Optimism codebase where the summaries are being used. @JuanCoRo can you confirm that this works for your needs?

@palinatolmach
Copy link
Collaborator

Looks good to me too! I think @anvacaru tested it on the engagement proof of interest, but I'll wait for an additional confirmation before approving and merging.

@palinatolmach palinatolmach self-requested a review January 16, 2024 06:05
@rv-jenkins rv-jenkins merged commit fff0588 into master Jan 16, 2024
@rv-jenkins rv-jenkins deleted the fast-summarization branch January 16, 2024 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable support for fast summarizations
6 participants