feat(avm): calldatacopy gadget #7367
Conversation
jeanmon
force-pushed
the
jm/7211-calldatacopy-gadget
branch
from
bb81850
to
3d84fae
Compare
jeanmon
requested review from
IlyasRidhuan,
fcarreiro and
Maddiaa0
as code owners
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. Proof generationEach column represents the number of threads used in proof generation.
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 8 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
AVM SimulationTime to simulate various public functions in the AVM.
Public DB AccessTime to access various public DBs.
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 | | |
There was a problem hiding this comment.
LGTM modulo some naming/conceptual changes.
Otherwise, I'd really like to get my stack of PRs in first if possible (since they are all green): https://app.graphite.dev/github/pr/AztecProtocol/aztec-packages/7357/chore-avm-smaller-transcript
| struct SliceTraceEntry { | ||
| uint32_t clk = 0; | ||
| uint8_t space_id = 0; | ||
| FF addr_ff = 0; // Should normally be uint32_t but the last witness addr of a calldatacopy operation row might |
There was a problem hiding this comment.
// Should normally be uint32_t but the last witness addr of a calldatacopy operation row might
// be FF(2^32).
What does this mean? IIUC our memory goes from 0 to 2^32-1.
There was a problem hiding this comment.
The relations proving the correct calldatacopy trace requires an extra row where addr might be equal to 2^32. This happens if calldatacopy writes data at the highest memory address. In the .cpp, I have added some explanations.
| @@ -0,0 +1,39 @@ | |||
| include "../main.pil"; | |||
|
|
|||
| namespace slice(256); | |||
There was a problem hiding this comment.
I think I get what this does, my only comment is naming:
calling it slice (or better, mem_slice) makes it look like it's generic but then even the core relations like "addresses should be decreasing" and "cnt management" do use calldatacopy-specific selectors. Should this just be called the calldata gadget?
(I do understand that even if the gadget was a generic slice read/write gadget then there would be some mention of calldata, just like it happens with normal memory... we need at least some selectors for lookups/perms)
There was a problem hiding this comment.
What will happen for sure is that return opcode will be performed in the same trace. So, there will be specific selectors for calldatacopy and return opcode. Other columns like cnt, and addr, can be re-used. That is why I named it slice. I chose a short prefix, as the namespace is prefixed for any field of this trace.
| #[CD_OFFSET_INCREMENT] | ||
| sel_cd_cpy * (cd_offset + 1 - cd_offset') = 0; | ||
|
|
||
| #[LOOKUP_CD_VALUE] |
There was a problem hiding this comment.
I wonder if this one shouldn't be in main.pil? It really depends. If this is a calldata-specific gadget, then no. If this is a generic slice gadget, maybe yes? (very optional, I'm ok if things stay as they are)
There was a problem hiding this comment.
This is very calldata-specific. Calladata column being defined in the main trace, we could have defined this permutation there as well. I thought due to the specificity, we keep it here.
jeanmon
force-pushed
the
jm/7211-calldatacopy-gadget
branch
from
5161869
to
ff0b167
Compare
jeanmon
force-pushed
the
jm/7211-calldatacopy-gadget
branch
from
ff0b167
to
29a06fa
Compare
|
Obsoleted by the following PR: #7415 |
Resolves #7211