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

feat(vc): re-execution before attestation #8958

Closed
wants to merge 117 commits into from
Closed

Conversation

Maddiaa0
Copy link
Member

@Maddiaa0 Maddiaa0 commented Oct 2, 2024

Overview

Validators currently perform blind attestations to transaction updates

This pr introduces the notion of a light public processor (really light block builder, we can change the name) which

  1. Syncs to the latest
  2. Executes the transactions in a proposal on top of the head of the chain
  3. Does not run the kernel circuits

For calls which currently run lots of kernels ( nested calls ) this currently yields an extremely large performance increase.

In the current test which has an enqueued + 4 nested calls and multiple state updates block building time goes from 2.2seconds to 400 ms ( granted the archive is not calculated )

Downsides:

  • This creates a second code path other than the existing public processor
    • although: In my mind it is alot clearer what is going on in this smaller processor

part of : #8720

Issues created from this work

@Maddiaa0 Maddiaa0 marked this pull request as ready for review October 3, 2024 00:39
Copy link
Contributor

github-actions bot commented Oct 3, 2024

Changes to public function bytecode sizes

Generated at commit: 92ec4b129173685dfbd379b691ebfa90e6be9b23, compared to commit: e416dd41d688a830012dce87f7fc65dd2ed4ccf8

🧾 Summary (100% most significant diffs)

Program Bytecode size in bytes (+/-) %
Spam::public_dispatch +7,382 ❌ +161.81%

Full diff report 👇
Program Bytecode size in bytes (+/-) %
Spam::public_dispatch 11,944 (+7,382) +161.81%

@Maddiaa0 Maddiaa0 added the C-p2p Component: peer to peer label Oct 3, 2024
@Maddiaa0 Maddiaa0 added the e2e-p2p CI: Enables this CI job. label Oct 3, 2024
@@ -35,7 +35,7 @@ export class AvmExecutionEnvironment {
functionSelector,
this.contractCallDepth.add(Fr.ONE),
this.transactionFee,
this.header,
this.historicalHeader,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is supposed to be the current header and not the historical one. IIRC public will not have access to historical headers, since that would force nodes to be archivers.

Copy link
Contributor

Choose a reason for hiding this comment

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

OTOH, I'm pretty sure header is currently unused and could be removed (we only keep it for the potential future HEADERMEMBER). When I added it, it was for interoperability with ACVM: #5592

@dbanks12

Copy link
Member Author

Choose a reason for hiding this comment

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

const historicalHeader = maybeHistoricalHeader ?? merkleTree.getInitialHeader();

I updated it based on the public processor factory - it instantiates the avm executor with a variable called historical header - should this be updated instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Base automatically changed from md/e2e-p2p-snapshots to master October 3, 2024 13:08
@Maddiaa0
Copy link
Member Author

Maddiaa0 commented Nov 7, 2024

superceeded by #9768

@Maddiaa0 Maddiaa0 closed this Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-p2p Component: peer to peer e2e-p2p CI: Enables this CI job.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants