Skip to content
This repository has been archived by the owner on Jul 5, 2024. It is now read-only.

fix unconstrained is_warm in sload/sstore #1596

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion bus-mapping/src/evm/opcodes/sload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,17 @@ impl Opcode for Sload {

// First stack write
state.stack_write(&mut exec_step, stack_position, value)?;
state.push_op(
&mut exec_step,
RW::READ,
TxAccessListAccountStorageOp {
tx_id: state.tx_ctx.id(),
address: contract_addr,
key,
is_warm,
is_warm_prev: is_warm,
},
);
state.push_op_reversible(
&mut exec_step,
TxAccessListAccountStorageOp {
Expand Down Expand Up @@ -189,7 +200,7 @@ mod sload_tests {
);

let access_list_op = &builder.block.container.tx_access_list_account_storage
[step.bus_mapping_instance[7].as_usize()];
[step.bus_mapping_instance[8].as_usize()];
assert_eq!(
(access_list_op.rw(), access_list_op.op()),
(
Expand Down
17 changes: 14 additions & 3 deletions bus-mapping/src/evm/opcodes/sstore.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use super::Opcode;
use crate::{
circuit_input_builder::{CircuitInputStateRef, ExecStep},
operation::{CallContextField, StorageOp, TxAccessListAccountStorageOp, TxRefundOp},
operation::{CallContextField, StorageOp, TxAccessListAccountStorageOp, TxRefundOp, RW},
Error,
};

use eth_types::{GethExecStep, ToWord, Word};

/// Placeholder structure used to implement [`Opcode`] trait over it
Expand Down Expand Up @@ -86,6 +85,17 @@ impl Opcode for Sstore {
),
)?;

state.push_op(
&mut exec_step,
RW::READ,
TxAccessListAccountStorageOp {
tx_id: state.tx_ctx.id(),
address: contract_addr,
key,
is_warm,
is_warm_prev: is_warm,
},
);
state.push_op_reversible(
&mut exec_step,
TxAccessListAccountStorageOp {
Expand Down Expand Up @@ -250,7 +260,8 @@ mod sstore_tests {
)
)
);
let refund_op = &builder.block.container.tx_refund[step.bus_mapping_instance[9].as_usize()];
let refund_op =
&builder.block.container.tx_refund[step.bus_mapping_instance[10].as_usize()];
assert_eq!(
(refund_op.rw(), refund_op.op()),
(
Expand Down
10 changes: 8 additions & 2 deletions zkevm-circuits/src/evm_circuit/execution/sload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ impl<F: Field> ExecutionGadget<F> for SloadGadget<F> {
cb.stack_push(value.to_word());

let is_warm = cb.query_bool();
cb.account_storage_access_list_read(
tx_id.expr(),
callee_address.to_word(),
key.to_word(),
Word::from_lo_unchecked(is_warm.expr()),
);
cb.account_storage_access_list_write(
tx_id.expr(),
callee_address.to_word(),
Expand All @@ -72,7 +78,7 @@ impl<F: Field> ExecutionGadget<F> for SloadGadget<F> {

let gas_cost = SloadGasGadget::construct(cb, is_warm.expr()).expr();
let step_state_transition = StepStateTransition {
rw_counter: Delta(8.expr()),
rw_counter: Delta(9.expr()),
program_counter: Delta(1.expr()),
reversible_write_counter: Delta(1.expr()),
gas_left: Delta(-gas_cost),
Expand Down Expand Up @@ -123,7 +129,7 @@ impl<F: Field> ExecutionGadget<F> for SloadGadget<F> {
self.committed_value
.assign_u256(region, offset, committed_value)?;

let (_, is_warm) = block.get_rws(step, 7).tx_access_list_value_pair();
let (_, is_warm) = block.get_rws(step, 8).tx_access_list_value_pair();
self.is_warm
.assign(region, offset, Value::known(F::from(is_warm as u64)))?;

Expand Down
12 changes: 9 additions & 3 deletions zkevm-circuits/src/evm_circuit/execution/sstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,12 @@ impl<F: Field> ExecutionGadget<F> for SstoreGadget<F> {
);

let is_warm = cb.query_bool();
cb.account_storage_access_list_read(
tx_id.expr(),
callee_address.to_word(),
key.to_word(),
Word::from_lo_unchecked(is_warm.expr()),
);
cb.account_storage_access_list_write(
tx_id.expr(),
callee_address.to_word(),
Expand Down Expand Up @@ -129,7 +135,7 @@ impl<F: Field> ExecutionGadget<F> for SstoreGadget<F> {
);

let step_state_transition = StepStateTransition {
rw_counter: Delta(10.expr()),
rw_counter: Delta(11.expr()),
program_counter: Delta(1.expr()),
stack_pointer: Delta(2.expr()),
reversible_write_counter: Delta(3.expr()),
Expand Down Expand Up @@ -190,11 +196,11 @@ impl<F: Field> ExecutionGadget<F> for SstoreGadget<F> {
self.original_value
.assign_u256(region, offset, original_value)?;

let (_, is_warm) = block.get_rws(step, 8).tx_access_list_value_pair();
let (_, is_warm) = block.get_rws(step, 9).tx_access_list_value_pair();
self.is_warm
.assign(region, offset, Value::known(F::from(is_warm as u64)))?;

let (tx_refund, tx_refund_prev) = block.get_rws(step, 9).tx_refund_value_pair();
let (tx_refund, tx_refund_prev) = block.get_rws(step, 10).tx_refund_value_pair();
self.tx_refund_prev
.assign(region, offset, Some(tx_refund_prev.to_le_bytes()))?;

Expand Down
Loading