-
Notifications
You must be signed in to change notification settings - Fork 290
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: add gas costs to TxEffect #5337
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @just-mitch and the rest of your teammates on Graphite |
c8511f9
to
9e1d351
Compare
5c95101
to
606fdd5
Compare
f4530d4
to
e91c5a1
Compare
f33cdd4
to
f1663fc
Compare
4302fd3
to
c820cf8
Compare
e9a42b3
to
8b29014
Compare
8b29014
to
cdd0d2b
Compare
Changes to circuit sizes
🧾 Summary (100% most significant diffs)
Full diff report 👇
|
e093115
to
5caf400
Compare
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
4f20cc7
to
a9d6d84
Compare
@@ -515,6 +517,15 @@ export async function executeBaseRollupCircuit( | |||
): Promise<[BaseOrMergeRollupPublicInputs, Proof]> { | |||
logger?.(`Running base rollup for ${tx.hash}`); | |||
const rollupOutput = await simulator.baseRollupCircuit(inputs); | |||
const txEffect = toTxEffect(tx); | |||
const txEffectHash = txEffect.hash(); | |||
if (!rollupOutput.txsEffectsHash.toBuffer().equals(txEffectHash)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just flagging: I believe at present this would kill the whole proving ticket, which is probably not the behavior we want when one TX is invalid.
This addition at least enables a "fail fast" in that the ticket gets killed as soon as it knows that a single transaction would have invalidated the block, instead of waiting until the after the root rollup runs.
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. Values are compared against data from master at commit 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 circuit run across all benchmarks.
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
Transaction processing duration by data writes.
|
61fb9d3
to
d71f161
Compare
d71f161
to
95438ff
Compare
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/safe-buffer@5.2.1, npm/safer-buffer@2.1.2, npm/semver@7.5.4, npm/serialize-javascript@6.0.1, npm/shell-quote@1.8.1, npm/signal-exit@3.0.7, npm/space-separated-tokens@1.1.5, npm/string-width@5.1.2, npm/strip-ansi@6.0.1, npm/style-to-object@0.3.0, npm/svgo@2.8.0, npm/tapable@2.2.1, npm/terser-webpack-plugin@5.3.9, npm/terser@5.21.0, npm/tslib@2.6.2, npm/typescript@4.9.5, npm/unified@9.2.2, npm/unist-builder@2.0.3, npm/unist-util-visit@2.0.3, npm/unpipe@1.0.0, npm/util-deprecate@1.0.2, npm/vfile-location@3.2.0, npm/webpack-sources@3.2.3, npm/xtend@4.0.2, npm/yaml@1.10.2 |
95438ff
to
7f428b3
Compare
Built to the description provided in the yellowpaper changes.
TLDR: Adds
da_gas_used
throughout the kernel circuits, and includes it in theTxEffect
that gets published, as well as theTxReceipt
. See the token contract e2e test for end flow.Immediate future work is to update/accumulate the gas during public kernel execution. See #5507
Fix #4101
Fix #5306