-
Notifications
You must be signed in to change notification settings - Fork 600
feat(avm): calldatacopy gadget #7367
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
Changes from all commits
e55acaf
e9e257e
95e86ca
da13327
d39a1da
860a950
29a06fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| include "../main.pil"; | ||
|
|
||
| namespace slice(256); | ||
|
|
||
| pol commit clk; | ||
|
|
||
| pol commit sel_start_cd_cpy; // Selector to indicate the start of calldatacopy. Used in permutation with the main trace. | ||
| pol commit sel_cd_cpy; // Selector for any row involved in a callatacopy operation. | ||
| pol commit cnt; // Decreasing counter to track the number of memory operations. | ||
| pol commit space_id; // Copied from main trace. | ||
| pol commit addr; // Address pertaining to the memory operation. | ||
| pol commit val; // Value pertaining to the memory operation. | ||
| pol commit cd_offset; // Offset of the calldata element. It is used to get the correct value from calldata. | ||
| pol commit one_min_inv; // Helper column to assert zero/non-zero equality of cnt; | ||
|
|
||
| sel_cd_cpy * (1 - sel_cd_cpy) = 0; | ||
| // TODO: might not be required | ||
| sel_start_cd_cpy * (1 - sel_start_cd_cpy) = 0; | ||
|
|
||
| // Show that cnt != 0 <==> sel_cd_cpy == 1 | ||
| // one_min_inv == 1 - cnt^(-1) if cnt != 0 else == 0 | ||
| #[SLICE_CNT_ZERO_TEST1] | ||
| cnt * (1 - one_min_inv) - sel_cd_cpy = 0; | ||
| #[SLICE_CNT_ZERO_TEST2] | ||
| (1 - sel_cd_cpy) * one_min_inv = 0; | ||
|
|
||
| #[SLICE_CNT_DECREMENT] | ||
| sel_cd_cpy * (cnt - 1 - cnt') = 0; | ||
|
|
||
| #[ADDR_CNT_INCREMENT] | ||
| sel_cd_cpy * (addr + 1 - addr') = 0; | ||
| #[CD_OFFSET_INCREMENT] | ||
| sel_cd_cpy * (cd_offset + 1 - cd_offset') = 0; | ||
|
|
||
| #[LOOKUP_CD_VALUE] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| sel_cd_cpy {cd_offset, val} in main.sel_calldata {main.clk, main.calldata}; | ||
|
|
||
| #[PERM_CD_MEM] | ||
| sel_cd_cpy {clk, space_id, addr, val} is mem.sel_op_cd_cpy {mem.clk, mem.space_id, mem.addr, mem.val}; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| #pragma once | ||
|
|
||
| #include "barretenberg/relations/generic_lookup/generic_lookup_relation.hpp" | ||
|
|
||
| #include <cstddef> | ||
| #include <tuple> | ||
|
|
||
| namespace bb { | ||
|
|
||
| class lookup_cd_value_lookup_settings { | ||
| public: | ||
| static constexpr size_t READ_TERMS = 1; | ||
| static constexpr size_t WRITE_TERMS = 1; | ||
| static constexpr size_t READ_TERM_TYPES[READ_TERMS] = { 0 }; | ||
| static constexpr size_t WRITE_TERM_TYPES[WRITE_TERMS] = { 0 }; | ||
| static constexpr size_t LOOKUP_TUPLE_SIZE = 2; | ||
| static constexpr size_t INVERSE_EXISTS_POLYNOMIAL_DEGREE = 4; | ||
| static constexpr size_t READ_TERM_DEGREE = 0; | ||
| static constexpr size_t WRITE_TERM_DEGREE = 0; | ||
|
|
||
| template <typename AllEntities> static inline auto inverse_polynomial_is_computed_at_row(const AllEntities& in) | ||
| { | ||
| return (in.slice_sel_cd_cpy == 1 || in.main_sel_calldata == 1); | ||
| } | ||
|
|
||
| template <typename Accumulator, typename AllEntities> | ||
| static inline auto compute_inverse_exists(const AllEntities& in) | ||
| { | ||
| using View = typename Accumulator::View; | ||
| const auto is_operation = View(in.slice_sel_cd_cpy); | ||
| const auto is_table_entry = View(in.main_sel_calldata); | ||
| return (is_operation + is_table_entry - is_operation * is_table_entry); | ||
| } | ||
|
|
||
| template <typename AllEntities> static inline auto get_const_entities(const AllEntities& in) | ||
| { | ||
| return std::forward_as_tuple(in.lookup_cd_value, | ||
| in.lookup_cd_value_counts, | ||
| in.slice_sel_cd_cpy, | ||
| in.main_sel_calldata, | ||
| in.slice_cd_offset, | ||
| in.slice_val, | ||
| in.main_clk, | ||
| in.main_calldata); | ||
| } | ||
|
|
||
| template <typename AllEntities> static inline auto get_nonconst_entities(AllEntities& in) | ||
| { | ||
| return std::forward_as_tuple(in.lookup_cd_value, | ||
| in.lookup_cd_value_counts, | ||
| in.slice_sel_cd_cpy, | ||
| in.main_sel_calldata, | ||
| in.slice_cd_offset, | ||
| in.slice_val, | ||
| in.main_clk, | ||
| in.main_calldata); | ||
| } | ||
| }; | ||
|
|
||
| template <typename FF_> | ||
| class lookup_cd_value_relation : public GenericLookupRelation<lookup_cd_value_lookup_settings, FF_> { | ||
| public: | ||
| static constexpr const char* NAME = "lookup_cd_value"; | ||
| }; | ||
| template <typename FF_> using lookup_cd_value = GenericLookup<lookup_cd_value_lookup_settings, FF_>; | ||
|
|
||
| } // namespace bb |
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 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.