Skip to content

Commit

Permalink
feat: add sender balance check to begin-tx (EIP-1559) (scroll-tech#1066)
Browse files Browse the repository at this point in the history
* Add balance check for gas fee (EIP-1559).

* Fix CI with scroll feature.

* Add more comments.

* Add test case.

* Delete duplicate fixes in tx circuit.

* Replace `or::expr` with `sum::expr` to simplify for boolean expressions.

* Update and add test cases.

* Fix lint.

* Disable tracing error cases for scroll feature.
  • Loading branch information
silathdiir authored Jan 11, 2024
1 parent 8977ba3 commit 556fc28
Show file tree
Hide file tree
Showing 10 changed files with 324 additions and 31 deletions.
2 changes: 1 addition & 1 deletion bus-mapping/src/circuit_input_builder/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ impl From<&Transaction> for geth_types::Transaction {
nonce: Word::from(tx.nonce),
gas_limit: Word::from(tx.gas),
value: tx.value,
gas_price: tx.gas_price,
gas_price: Some(tx.gas_price),
call_data: tx.input.clone().into(),
v: tx.signature.v,
r: tx.signature.r,
Expand Down
16 changes: 8 additions & 8 deletions eth-types/src/geth_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ pub struct Transaction {
/// Transfered value
pub value: Word,
/// Gas Price
pub gas_price: Word,
pub gas_price: Option<Word>,
/// Gas fee cap
pub gas_fee_cap: Word,
/// Gas tip cap
Expand Down Expand Up @@ -300,9 +300,9 @@ impl From<&Transaction> for crate::Transaction {
nonce: tx.nonce,
gas: tx.gas_limit,
value: tx.value,
gas_price: Some(tx.gas_price),
max_priority_fee_per_gas: Some(tx.gas_fee_cap),
max_fee_per_gas: Some(tx.gas_tip_cap),
gas_price: tx.gas_price,
max_priority_fee_per_gas: Some(tx.gas_tip_cap),
max_fee_per_gas: Some(tx.gas_fee_cap),
input: tx.call_data.clone(),
access_list: tx.access_list.clone(),
v: tx.v.into(),
Expand All @@ -323,9 +323,9 @@ impl From<&crate::Transaction> for Transaction {
nonce: tx.nonce,
gas_limit: tx.gas,
value: tx.value,
gas_price: tx.gas_price.unwrap_or_default(),
gas_fee_cap: tx.max_priority_fee_per_gas.unwrap_or_default(),
gas_tip_cap: tx.max_fee_per_gas.unwrap_or_default(),
gas_price: tx.gas_price,
gas_tip_cap: tx.max_priority_fee_per_gas.unwrap_or_default(),
gas_fee_cap: tx.max_fee_per_gas.unwrap_or_default(),
call_data: tx.input.clone(),
access_list: tx.access_list.clone(),
v: tx.v.as_u64(),
Expand All @@ -344,7 +344,7 @@ impl From<&Transaction> for TransactionRequest {
from: Some(tx.from),
to: tx.to.map(NameOrAddress::Address),
gas: Some(tx.gas_limit),
gas_price: Some(tx.gas_price),
gas_price: tx.gas_price,
value: Some(tx.value),
data: Some(tx.call_data.clone()),
nonce: Some(tx.nonce),
Expand Down
16 changes: 10 additions & 6 deletions mock/src/transaction.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Mock Transaction definition and builder related methods.
use super::{MOCK_ACCOUNTS, MOCK_CHAIN_ID, MOCK_GASPRICE};
use super::{MOCK_ACCOUNTS, MOCK_CHAIN_ID};
use eth_types::{
geth_types::Transaction as GethTransaction, word, AccessList, Address, Bytes, Hash,
Transaction, Word, U64,
Expand Down Expand Up @@ -149,7 +149,7 @@ pub struct MockTransaction {
pub from: AddrOrWallet,
pub to: Option<AddrOrWallet>,
pub value: Word,
pub gas_price: Word,
pub gas_price: Option<Word>,
pub gas: Word,
pub input: Bytes,
pub v: Option<U64>,
Expand All @@ -174,7 +174,7 @@ impl Default for MockTransaction {
from: AddrOrWallet::random(&mut OsRng),
to: None,
value: Word::zero(),
gas_price: *MOCK_GASPRICE,
gas_price: None,
gas: Word::from(1_000_000u64),
input: Bytes::default(),
v: None,
Expand All @@ -200,7 +200,7 @@ impl From<MockTransaction> for Transaction {
from: mock.from.address(),
to: mock.to.map(|addr| addr.address()),
value: mock.value,
gas_price: Some(mock.gas_price),
gas_price: mock.gas_price,
gas: mock.gas,
input: mock.input,
v: mock.v.unwrap_or_default(),
Expand Down Expand Up @@ -274,7 +274,7 @@ impl MockTransaction {

/// Set gas_price field for the MockTransaction.
pub fn gas_price(&mut self, gas_price: Word) -> &mut Self {
self.gas_price = gas_price;
self.gas_price = Some(gas_price);
self
}

Expand Down Expand Up @@ -337,9 +337,13 @@ impl MockTransaction {
.value(self.value)
.data(self.input.clone())
.gas(self.gas)
.gas_price(self.gas_price)
.chain_id(self.chain_id);

let tx = if let Some(gas_price) = self.gas_price {
tx.gas_price(gas_price)
} else {
tx
};
let tx = if let Some(to_addr) = self.to.clone() {
tx.to(to_addr.address())
} else {
Expand Down
4 changes: 2 additions & 2 deletions testool/src/statetest/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ fn into_traceconfig(st: StateTest) -> (String, TraceConfig, StateTestResult) {
nonce: st.nonce,
value: st.value,
gas_limit: U256::from(st.gas_limit),
gas_price: st.gas_price,
gas_price: Some(st.gas_price),
gas_fee_cap: U256::zero(),
gas_tip_cap: U256::zero(),
call_data: st.data,
Expand Down Expand Up @@ -369,7 +369,7 @@ fn trace_config_to_witness_block_l1(
to: tx.to,
value: tx.value,
input: tx.call_data,
gas_price: Some(tx.gas_price),
gas_price: tx.gas_price,
access_list: tx.access_list,
nonce: tx.nonce,
gas: tx.gas_limit,
Expand Down
28 changes: 26 additions & 2 deletions zkevm-circuits/src/evm_circuit/execution/begin_tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use crate::{
util::{
and,
common_gadget::{
TransferGadgetInfo, TransferWithGasFeeGadget, TxEip2930Gadget, TxL1FeeGadget,
TxL1MsgGadget,
TransferGadgetInfo, TransferWithGasFeeGadget, TxEip1559Gadget, TxEip2930Gadget,
TxL1FeeGadget, TxL1MsgGadget,
},
constraint_builder::{
ConstrainBuilderCommon, EVMConstraintBuilder, ReversionInfo, StepStateTransition,
Expand Down Expand Up @@ -99,6 +99,7 @@ pub(crate) struct BeginTxGadget<F> {
is_coinbase_warm: Cell<F>,
tx_l1_fee: TxL1FeeGadget<F>,
tx_l1_msg: TxL1MsgGadget<F>,
tx_eip1559: TxEip1559Gadget<F>,
tx_eip2930: TxEip2930Gadget<F>,
}

Expand Down Expand Up @@ -358,6 +359,17 @@ impl<F: Field> ExecutionGadget<F> for BeginTxGadget<F> {
&mut reversion_info,
);

// Construct EIP-1559 gadget to check sender balance.
let tx_eip1559 = TxEip1559Gadget::construct(
cb,
tx_id.expr(),
tx_type.expr(),
tx_gas.expr(),
tx_l1_fee.tx_l1_fee_word(),
&tx_value,
transfer_with_gas_fee.sender_balance_prev(),
);

let caller_nonce_hash_bytes = array_init::array_init(|_| cb.query_byte());
let create = ContractCreateGadget::construct(cb);
cb.require_equal(
Expand Down Expand Up @@ -803,6 +815,7 @@ impl<F: Field> ExecutionGadget<F> for BeginTxGadget<F> {
is_coinbase_warm,
tx_l1_fee,
tx_l1_msg,
tx_eip1559,
tx_eip2930,
}
}
Expand Down Expand Up @@ -1180,6 +1193,17 @@ impl<F: Field> ExecutionGadget<F> for BeginTxGadget<F> {
tx.tx_data_gas_cost,
)?;

self.tx_eip1559.assign(
region,
offset,
tx,
tx_l1_fee,
transfer_assign_result
.sender_balance_sub_fee_pair
.unwrap()
.1,
)?;

self.tx_eip2930.assign(region, offset, tx)
}
}
Expand Down
4 changes: 2 additions & 2 deletions zkevm-circuits/src/evm_circuit/param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use halo2_proofs::{
use std::{collections::HashMap, sync::LazyLock};

// Step dimension
pub(crate) const STEP_WIDTH: usize = 140;
pub(crate) const STEP_WIDTH: usize = 141;
/// Step height
pub const MAX_STEP_HEIGHT: usize = 21;
/// The height of the state of a step, used by gates that connect two
Expand All @@ -27,7 +27,7 @@ pub(crate) const N_COPY_COLUMNS: usize = 2;
// Number of copy columns for phase2
pub(crate) const N_PHASE2_COPY_COLUMNS: usize = 1;

pub(crate) const N_BYTE_LOOKUPS: usize = 26;
pub(crate) const N_BYTE_LOOKUPS: usize = 36;

/// Amount of lookup columns in the EVM circuit dedicated to lookups.
pub(crate) const EVM_LOOKUP_COLS: usize = FIXED_TABLE_LOOKUPS
Expand Down
23 changes: 23 additions & 0 deletions zkevm-circuits/src/evm_circuit/util/common_gadget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,12 @@ use halo2_proofs::{
plonk::{Error, Expression},
};

mod tx_eip1559;
mod tx_eip2930;
mod tx_l1_fee;
mod tx_l1_msg;

pub(crate) use tx_eip1559::TxEip1559Gadget;
pub(crate) use tx_eip2930::TxEip2930Gadget;
pub(crate) use tx_l1_fee::TxL1FeeGadget;
pub(crate) use tx_l1_msg::TxL1MsgGadget;
Expand Down Expand Up @@ -591,6 +593,11 @@ impl<F: Field> TransferFromWithGasFeeGadget<F> {
value: U256,
gas_fee: U256,
) -> Result<(), Error> {
assert!(
sender_balance_sub_fee >= prev_sender_balance_sub_value,
"fee must be subtracted before value"
);

if let Either::Left(value_is_zero) = &self.value_is_zero {
value_is_zero.assign_value(region, offset, region.word_rlc(value))?;
}
Expand All @@ -609,6 +616,12 @@ impl<F: Field> TransferFromWithGasFeeGadget<F> {
sender_balance_sub_value,
)
}

/// Return sender balance before subtracting fee and value.
pub(crate) fn sender_balance_prev(&self) -> &Word<F> {
// Fee is subtracted before value.
self.sender_sub_fee.balance_prev()
}
}

impl<F: Field> TransferFromAssign<F> for TransferFromWithGasFeeGadget<F> {
Expand All @@ -626,6 +639,11 @@ impl<F: Field> TransferFromAssign<F> for TransferFromWithGasFeeGadget<F> {
} else {
(0.into(), 0.into())
};
assert!(
sender_balance_sub_fee_pair.0 >= sender_balance_sub_value_pair.1,
"fee must be subtracted before value"
);

self.assign(
region,
offset,
Expand Down Expand Up @@ -923,6 +941,11 @@ impl<F: Field> TransferWithGasFeeGadget<F> {
to,
}
}

/// Return sender balance before subtracting fee and value.
pub(crate) fn sender_balance_prev(&self) -> &Word<F> {
self.from.sender_balance_prev()
}
}

impl<F: Field> TransferGadget<F> {
Expand Down
Loading

0 comments on commit 556fc28

Please sign in to comment.