feat: add totalFees to global variables
#6439
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @just-mitch and the rest of your teammates on |
Base automatically changed from
05-15-feat_pay_out_arbitrary_fee_to_coinbase_on_l1
to
master
May 15, 2024 20:33
Changes to circuit sizes
🧾 Summary (100% most significant diffs)
Full diff report 👇
|
just-mitch
force-pushed
the
05-15-feat_add_totalfees_to_global_variables
branch
from
6518c54 to
7c310f1
Compare
Benchmark resultsMetrics with a significant change:
Detailed resultsAll benchmarks are run on txs on the This benchmark source data is available in JSON format on S3 here. L2 block published to L1Each column represents the number of txs on an L2 block published to L1.
L2 chain processingEach column represents the number of blocks on the L2 chain where each block has 16 txs.
Circuits statsStats on running time and I/O sizes collected for every kernel circuit run across all benchmarks.
Stats on running time collected for app circuits
Tree insertion statsThe duration to insert a fixed batch of leaves into each tree type.
MiscellaneousTransaction sizes based on how many contract classes are registered in the tx.
Transaction size based on fee payment method | Metric | | Transaction processing duration by data writes.
|
just-mitch
commented
May 16, 2024
| @@ -230,7 +230,10 @@ export const deployL1Contracts = async ( | |||
| abi: contractsToDeploy.gasToken.contractAbi, | |||
| client: walletClient, | |||
| }); | |||
| await gasToken.write.mint([rollupAddress.toString(), 100000000000000000000n], {} as any); | |||
| const receipt = await gasToken.write.mint([rollupAddress.toString(), 100000000000000000000n], {} as any); | |||
| await publicClient.waitForTransactionReceipt({ | |||
There was a problem hiding this comment.
Thank you @spalladino !
just-mitch
force-pushed
the
05-15-feat_add_totalfees_to_global_variables
branch
from
7c310f1 to
ac434c9
Compare
Docs PreviewHey there! 👋 You can check your preview at https://66466af984ebc7077ad9f228--aztec-docs-dev.netlify.app |
just-mitch
force-pushed
the
05-15-feat_add_totalfees_to_global_variables
branch
4 times, most recently
from
b1059a3 to
1bb0586
Compare
just-mitch
force-pushed
the
05-15-feat_add_totalfees_to_global_variables
branch
from
1bb0586 to
b5a573e
Compare
spalladino
reviewed
May 16, 2024
There was a problem hiding this comment.
Code looks good. However, GlobalVariables are also referenced from the CombinedConstantData that are used across the kernel circuits, and are constant across the entire block. total_fees does not seem to fit the bill in there, since it's accumulated across the block, so maybe it should be directly in the Header and not within its global_variables?
@spalladino I was kind of thinking the same thing. @LeilaWang , @LHerskind . Thoughts? |
|
Agree with Palla! |
|
Close in favor of #6522 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add
totalFeesto global variables. Update the spec.Note: we could have had the L1 rollup contract iterate over all the tx_effects and pay out that way. Doing it ourselves in the circuit feels cheaper though, and maybe safer for a world where: