Add context pruning #112

hratoanina · 2024-03-15T23:30:41Z

Implements the pruning of stale contexts. A context is stale when it returns to its parent context; after it returns, it can never be accessed again and can be omitted from the memory of subsequent segments.

To detect when a context goes stale, we instrument SET_CONTEXT, and check if new_ctx < old_ctx. Since contexts are attributed in a strictly increasing order, this means that new_ctx is the parent context of old_ctx, and thus that old_ctx is stale.

This check is made in the CPU table. All the stale old_ctx values are then transmitted to the Memory table via a CTL.
In Memory, for each row, we check if ADDR_CONTEXT is contained in the PRUNED_CONTEXTS column via a lookup. If that's the case, it will be omitted from the CTL to MemAfter.

This PR contains #109 and #111. These are required fixes which aren't merged upstream yet. It's opened as a draft so people can look at it and run the test suite on it if they wish, but will wait for these PRs to be merged before opening it in earnest.
Edit: Done.

Closes #24

4l0n50 · 2024-03-20T16:28:50Z

and check if new_ctx < old_ctx.

It seems this happens if and only if you are in terminate_common? So maybe you can manually set pruning_flag if SET_CONTEXT expects scaled_ctx + prunning_flag on the top of the stack and we call SET_CONTEXT with pruning_flag == 1 iff we are in terminate_common. In this way you don't need to add the CTL with the arithmetic stark. Do you think this might work? or maybe I'm missing something?

hratoanina · 2024-03-20T16:37:13Z

and check if new_ctx < old_ctx.

It seems this happens if and only if you are in terminate_common? So maybe you can manually set pruning_flag if SET_CONTEXT expects scaled_ctx + prunning_flag on the top of the stack and we call SET_CONTEXT with pruning_flag == 1 iff we are in terminate_common. In this way you don't need to add the CTL with the arithmetic stark. Do you think this might work? or maybe I'm missing something?

Hmm, that's a good idea. There's another case than terminate_common (for EXTCODECOPY and EXTCODESIZE), but you can do something similar. This would require a lot of changes and I don't think the benefits would be huge, so I'll keep it as a follow-up task. Thanks!

evm_arithmetization/src/cpu/columns/general.rs

dvdplm · 2024-03-21T13:23:48Z

evm_arithmetization/src/cpu/kernel/tests/account_code.rs

+    assert_eq!(
+        interpreter.stack(),
+        vec![U256::one() << CONTEXT_SCALING_FACTOR, code.len().into()]
+    );


DQ: why do we expect this to be the content of the stack here? And why only for EXTCODESIZE but not for other tests?

That's because extcodesize (not sys_extcodesize) now returns code_size, codesize_ctx. I added the returned stack in a comment in the ASM.

evm_arithmetization/src/cpu/cpu_stark.rs

evm_arithmetization/src/generation/state.rs

dvdplm · 2024-03-21T13:35:46Z

evm_arithmetization/src/generation/state.rs

+                        }
+                        Some(mem)
+                    } else {
+                        None


For my education, what is the case when get_full_memory is None? What does that mean that GenerationState use the default impl and returns None?

When you're generating the proof (with the full trace), you don't need this memory. The Memory trace will be generated from the list of MemOps instead.

evm_arithmetization/src/generation/state.rs

evm_arithmetization/src/memory/memory_stark.rs

LindaGuiga

Looks great, thank you!

evm_arithmetization/src/cpu/kernel/asm/memory/syscalls.asm

evm_arithmetization/src/memory/memory_stark.rs

4l0n50 · 2024-03-25T14:45:01Z

evm_arithmetization/src/cpu/kernel/asm/memory/syscalls.asm

@@ -112,6 +112,15 @@ calldataload_large_offset:
 codecopy_within_bounds:
    // stack: total_size, segment, src_ctx, kexit_info, dest_offset, offset, size
    POP
+    // stack: segment, src_ctx, kexit_info, dest_offset, offset, size


Why do you need to add these changes?

codecopy_within_bounds calls codecopy_after instead of wcopy_after. I agree that's more or less code duplication, but I didn't see an easy way to do it more cleanly.

My problem is that I can't see where are you changing the context here.

From my understanding, the problem with codecopy etc. is that they do not change contexts, but write code in another context. And the idea here is to still be able to prune that extra context when needed by setting the context in codecopy_after.

Ahhh, and why src_ctx will be never accessed again?

In the case of extcodecopy and extcodesize, we generate src_ctx with next_context_id which increments @GLOBAL_METADATA_LARGEST_CONTEXT, meaning that any other created context will have a different id.

Ahhh ok, I think see now. It's because you just need the new_ctx for writing the code there (and check the hash etc) but after you copy the code you can forget about it?

4l0n50

LGTM modulo some nits.

evm_arithmetization/src/cpu/kernel/asm/memory/syscalls.asm

evm_arithmetization/src/memory/memory_stark.rs

evm_arithmetization/src/memory/columns.rs

evm_arithmetization/src/cpu/columns/general.rs

4l0n50 · 2024-03-29T08:48:03Z

evm_arithmetization/src/cpu/kernel/asm/memory/syscalls.asm

@@ -112,6 +112,15 @@ calldataload_large_offset:
 codecopy_within_bounds:
    // stack: total_size, segment, src_ctx, kexit_info, dest_offset, offset, size
    POP
+    // stack: segment, src_ctx, kexit_info, dest_offset, offset, size


My problem is that I can't see where are you changing the context here.

Nashtare

Overall LGTM

evm_arithmetization/src/generation/state.rs

evm_arithmetization/src/memory/memory_stark.rs

sonarcloud · 2024-04-02T14:43:35Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

github-actions bot added the crate: evm_arithmetization Anything related to the evm_arithmetization crate. label Mar 15, 2024

vgnom added this to the zk-continuations - Q1 2024 milestone Mar 18, 2024

Implement context pruning

02ba230

hratoanina force-pushed the context_pruning_rebased branch from 125a7cc to 02ba230 Compare March 19, 2024 00:56

Fix clippy

d7fe960

hratoanina marked this pull request as ready for review March 19, 2024 01:35

hratoanina requested review from wborgeaud, muursh and Nashtare as code owners March 19, 2024 01:35

dvdplm reviewed Mar 21, 2024

View reviewed changes

hratoanina added 3 commits March 22, 2024 17:30

Apply comments

13eeeb8

Remove dummy CTL table

6c01ea3

Merge branch 'feat/continuations' into context_pruning_rebased

fea4185

LindaGuiga approved these changes Mar 25, 2024

View reviewed changes

evm_arithmetization/src/cpu/kernel/asm/memory/syscalls.asm Outdated Show resolved Hide resolved

evm_arithmetization/src/memory/memory_stark.rs Outdated Show resolved Hide resolved

4l0n50 reviewed Mar 25, 2024

View reviewed changes

Apply comments

3c5f3df

4l0n50 reviewed Mar 29, 2024

View reviewed changes

Apply comments

f5a5769

Nashtare approved these changes Apr 2, 2024

View reviewed changes

evm_arithmetization/src/generation/state.rs Outdated Show resolved Hide resolved

evm_arithmetization/src/memory/memory_stark.rs Outdated Show resolved Hide resolved

Nashtare assigned hratoanina Apr 2, 2024

4l0n50 approved these changes Apr 2, 2024

View reviewed changes

Apply comments

91c02d6

hratoanina merged commit 867a172 into feat/continuations Apr 2, 2024
7 checks passed

hratoanina deleted the context_pruning_rebased branch April 2, 2024 15:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add context pruning #112

Add context pruning #112

hratoanina commented Mar 15, 2024 •

edited

Loading

4l0n50 commented Mar 20, 2024 •

edited

Loading

hratoanina commented Mar 20, 2024

dvdplm Mar 21, 2024

hratoanina Mar 22, 2024

dvdplm Mar 21, 2024

hratoanina Mar 22, 2024

LindaGuiga left a comment

4l0n50 Mar 25, 2024 •

edited

Loading

hratoanina Mar 25, 2024

4l0n50 Mar 29, 2024

LindaGuiga Mar 29, 2024

4l0n50 Mar 29, 2024

hratoanina Apr 1, 2024

4l0n50 Apr 2, 2024

4l0n50 left a comment

4l0n50 Mar 29, 2024

Nashtare left a comment

sonarcloud bot commented Apr 2, 2024

Add context pruning #112

Add context pruning #112

Conversation

hratoanina commented Mar 15, 2024 • edited Loading

4l0n50 commented Mar 20, 2024 • edited Loading

hratoanina commented Mar 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LindaGuiga left a comment

Choose a reason for hiding this comment

4l0n50 Mar 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

4l0n50 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nashtare left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Apr 2, 2024

Quality Gate passed

hratoanina commented Mar 15, 2024 •

edited

Loading

4l0n50 commented Mar 20, 2024 •

edited

Loading

4l0n50 Mar 25, 2024 •

edited

Loading