Skip to content

Commit

Permalink
refactor(avm): no fake rows in main trace (#7823)
Browse files Browse the repository at this point in the history
Needed for gas accounting.

There are some extra changes but it's the best I could split the PR.

PS: We do have repeated SLOADS/SSTORES which are kind of fake, but all
else should be good (modulo the stubbed gadgets that dont have a
selector).
  • Loading branch information
fcarreiro authored Aug 8, 2024
1 parent e43684c commit 5ff3554
Show file tree
Hide file tree
Showing 16 changed files with 986 additions and 1,488 deletions.
6 changes: 6 additions & 0 deletions barretenberg/cpp/format.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ if [ "$1" == "staged" ]; then
sed -i.bak 's/\r$//' $FILE && rm ${FILE}.bak
git add $FILE
done
elif [ "$1" == "changed" ]; then
echo Formatting barretenberg changed files...
for FILE in $(git diff-index --diff-filter=d --relative --name-only HEAD | grep -e '\.\(cpp\|hpp\|tcc\)$'); do
clang-format-16 -i $FILE
sed -i.bak 's/\r$//' $FILE && rm ${FILE}.bak
done
elif [ "$1" == "check" ]; then
for FILE in $(find ./src -iname *.hpp -o -iname *.cpp -o -iname *.tcc | grep -v src/msgpack-c); do
clang-format-16 --dry-run --Werror $FILE
Expand Down
9 changes: 5 additions & 4 deletions barretenberg/cpp/pil/avm/main.pil
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,11 @@ namespace main(256);
da_out_of_gas * (1 - da_out_of_gas) = 0;

// Constrain that the gas decrements correctly per instruction
// TODO: Special-case for external call.
#[L2_GAS_REMAINING_DECREMENT]
sel_gas_accounting_active * (l2_gas_remaining' - l2_gas_remaining + l2_gas_op_cost) = 0;
sel_gas_accounting_active * (1 - sel_op_external_call) * (l2_gas_remaining' - l2_gas_remaining + l2_gas_op_cost) = 0;
#[DA_GAS_REMAINING_DECREMENT]
sel_gas_accounting_active * (da_gas_remaining' - da_gas_remaining + da_gas_op_cost) = 0;
sel_gas_accounting_active * (1 - sel_op_external_call) * (da_gas_remaining' - da_gas_remaining + da_gas_op_cost) = 0;

// Constrain that the remaining gas is unchanged otherwise (multi-line operations)
#[L2_GAS_INACTIVE]
Expand Down Expand Up @@ -497,11 +498,11 @@ namespace main(256);
// Alternatively, we introduce a boolean selector for the three opcodes mentioned above.
// Note: External call gas cost is not constrained
pol commit sel_gas_accounting_active;
pol commit sel_mem_op_activate_gas; // TODO: remove this one
// TODO: remove sload and sstore from here
// This temporarily disables gas tracking for sload and sstore because our gas
// tracking doesn't work properly for instructions that span multiple rows
sel_gas_accounting_active - OPCODE_SELECTORS - SEL_ALL_CTRL_FLOW - sel_op_sload - sel_op_sstore - sel_mem_op_activate_gas = 0;
// TODO: disabling this until PR in stack.
// sel_gas_accounting_active - OPCODE_SELECTORS - SEL_ALL_CTRL_FLOW - sel_op_sload - sel_op_sstore - sel_mem_op_activate_gas = 0;

// Program counter must increment if not jumping or returning
// TODO: support for muli-rows opcode in execution trace such as
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,6 @@ AvmCircuitBuilder::ProverPolynomials AvmCircuitBuilder::compute_polynomials() co
polys.main_sel_gas_accounting_active[i] = rows[i].main_sel_gas_accounting_active;
polys.main_sel_last[i] = rows[i].main_sel_last;
polys.main_sel_mem_op_a[i] = rows[i].main_sel_mem_op_a;
polys.main_sel_mem_op_activate_gas[i] = rows[i].main_sel_mem_op_activate_gas;
polys.main_sel_mem_op_b[i] = rows[i].main_sel_mem_op_b;
polys.main_sel_mem_op_c[i] = rows[i].main_sel_mem_op_c;
polys.main_sel_mem_op_d[i] = rows[i].main_sel_mem_op_d;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ class AvmCircuitBuilder {
using Polynomial = Flavor::Polynomial;
using ProverPolynomials = Flavor::ProverPolynomials;

static constexpr size_t num_fixed_columns = 704;
static constexpr size_t num_polys = 704 + 74;
static constexpr size_t num_fixed_columns = 703;
static constexpr size_t num_polys = 703 + 74;
std::vector<Row> rows;

void set_trace(std::vector<Row>&& trace) { rows = std::move(trace); }
Expand Down
Loading

0 comments on commit 5ff3554

Please sign in to comment.