-
Notifications
You must be signed in to change notification settings - Fork 240
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
fix: set side effect hwm in child private calls #5624
fix: set side effect hwm in child private calls #5624
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
ce3b85d
to
7f4090c
Compare
1c39111
to
10c0bcd
Compare
7f4090c
to
bb0fc4f
Compare
10c0bcd
to
a5d749b
Compare
bb0fc4f
to
18350f2
Compare
a5d749b
to
31d58d1
Compare
18350f2
to
ff81957
Compare
31d58d1
to
e64797a
Compare
ff81957
to
ed2bdc1
Compare
e64797a
to
f5a40fd
Compare
ed2bdc1
to
9535bd2
Compare
f5a40fd
to
7135942
Compare
9535bd2
to
3f3c7de
Compare
7135942
to
971867b
Compare
c4ebb1f
to
c4a05e8
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. 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.
|
5b7af0e
to
ba801e0
Compare
const [kernelInputs, publicKernelOutput, publicKernelProof, newUnencryptedFunctionLogs, revertReason] = | ||
await this.processEnqueuedPublicCalls(tx, previousPublicKernelOutput, previousPublicKernelProof).catch( | ||
// the abstract phase manager throws if simulation gives error in a non-revertible phase | ||
async err => { | ||
await this.publicContractsDB.removeNewContracts(tx); |
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.
Hmm, not for this PR but maybe we need to create a seperate issue.
If App logic fails, will any contracts added here get removed?
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.
The app logic phase manager would remove all the contract from the transaction.
I just realised that this is wrong and we need to differentiate between contracts deployed in revertible vs non-revertible so that public effects are properly accounted for. I'll add a comment about this to #4712
@@ -0,0 +1,3 @@ | |||
pub fn max(a: u32, b: u32) -> u32 { |
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.
I thought I saw someone else was using a generic min and or max function. Can you check if that got merged and maybe we can use it?
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 checked and I couldn't see a "max" function in noir. I could remove this and go back to the if
statement?
ba801e0
to
4768687
Compare
This was rolled into a different PR. |
Please read contributing guidelines and remove this line.