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

Commit 63beeee

Browse files
authored
feat: if rws reaches limit, stop early and return err (#1669)
### Description solve #1592 ### Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ### Contents - [_item_] ### Rationale [_design decisions and extended information_] ### How Has This Been Tested? [_explanation_] <hr> ## How to fill a PR description Please give a concise description of your PR. The target readers could be future developers, reviewers, and auditors. By reading your description, they should easily understand the changes proposed in this pull request. MUST: Reference the issue to resolve ### Single responsability Is RECOMMENDED to create single responsibility commits, but not mandatory. Anyway, you MUST enumerate the changes in a unitary way, e.g. ``` This PR contains: - Cleanup of xxxx, yyyy - Changed xxxx to yyyy in order to bla bla - Added xxxx function to ... - Refactored .... ``` ### Design choices RECOMMENDED to: - What types of design choices did you face? - What decisions you have made? - Any valuable information that could help reviewers to think critically
1 parent 1d1066d commit 63beeee

34 files changed

+185
-148
lines changed

bus-mapping/src/circuit_input_builder.rs

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,21 @@ pub struct FixedCParams {
8282
pub struct DynamicCParams {}
8383

8484
/// Circuit Setup Parameters. These can be fixed/concrete or unset/dynamic.
85-
pub trait CircuitsParams: Debug + Copy {}
85+
pub trait CircuitsParams: Debug + Copy {
86+
/// Returns the max number of rws allowed
87+
fn max_rws(&self) -> Option<usize>;
88+
}
8689

87-
impl CircuitsParams for FixedCParams {}
88-
impl CircuitsParams for DynamicCParams {}
90+
impl CircuitsParams for FixedCParams {
91+
fn max_rws(&self) -> Option<usize> {
92+
Some(self.max_rws)
93+
}
94+
}
95+
impl CircuitsParams for DynamicCParams {
96+
fn max_rws(&self) -> Option<usize> {
97+
None
98+
}
99+
}
89100

90101
impl Default for FixedCParams {
91102
/// Default values for most of the unit tests of the Circuit Parameters
@@ -166,6 +177,7 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder<C> {
166177
block_ctx: &mut self.block_ctx,
167178
tx,
168179
tx_ctx,
180+
max_rws: self.circuits_params.max_rws(),
169181
}
170182
}
171183

@@ -275,11 +287,11 @@ impl CircuitInputBuilder<FixedCParams> {
275287
) -> Result<&CircuitInputBuilder<FixedCParams>, Error> {
276288
// accumulates gas across all txs in the block
277289
self.begin_handle_block(eth_block, geth_traces)?;
278-
self.set_end_block(self.circuits_params.max_rws);
290+
self.set_end_block(self.circuits_params.max_rws)?;
279291
Ok(self)
280292
}
281293

282-
fn set_end_block(&mut self, max_rws: usize) {
294+
fn set_end_block(&mut self, max_rws: usize) -> Result<(), Error> {
283295
let mut end_block_not_last = self.block.block_steps.end_block_not_last.clone();
284296
let mut end_block_last = self.block.block_steps.end_block_last.clone();
285297
end_block_not_last.rwc = self.block_ctx.rwc;
@@ -295,7 +307,7 @@ impl CircuitInputBuilder<FixedCParams> {
295307
call_id,
296308
CallContextField::TxId,
297309
Word::from(state.block.txs.len() as u64),
298-
);
310+
)?;
299311
}
300312

301313
let mut push_op = |step: &mut ExecStep, rwc: RWCounter, rw: RW, op: StartOp| {
@@ -333,6 +345,7 @@ impl CircuitInputBuilder<FixedCParams> {
333345

334346
self.block.block_steps.end_block_not_last = end_block_not_last;
335347
self.block.block_steps.end_block_last = end_block_last;
348+
Ok(())
336349
}
337350
}
338351

@@ -434,7 +447,7 @@ impl CircuitInputBuilder<DynamicCParams> {
434447
block_ctx: self.block_ctx,
435448
};
436449

437-
cib.set_end_block(c_params.max_rws);
450+
cib.set_end_block(c_params.max_rws)?;
438451
Ok(cib)
439452
}
440453
}

bus-mapping/src/circuit_input_builder/input_state_ref.rs

Lines changed: 54 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ pub struct CircuitInputStateRef<'a> {
4040
pub tx: &'a mut Transaction,
4141
/// Transaction Context
4242
pub tx_ctx: &'a mut TransactionContext,
43+
/// Max rw number limit
44+
pub max_rws: Option<usize>,
4345
}
4446

4547
impl<'a> CircuitInputStateRef<'a> {
@@ -114,7 +116,7 @@ impl<'a> CircuitInputStateRef<'a> {
114116
/// reference to the stored operation ([`OperationRef`]) inside the
115117
/// bus-mapping instance of the current [`ExecStep`]. Then increase the
116118
/// block_ctx [`RWCounter`](crate::operation::RWCounter) by one.
117-
pub fn push_op<T: Op>(&mut self, step: &mut ExecStep, rw: RW, op: T) {
119+
pub fn push_op<T: Op>(&mut self, step: &mut ExecStep, rw: RW, op: T) -> Result<(), Error> {
118120
if let OpEnum::Account(op) = op.clone().into_enum() {
119121
self.check_update_sdb_account(rw, &op)
120122
}
@@ -123,6 +125,20 @@ impl<'a> CircuitInputStateRef<'a> {
123125
.container
124126
.insert(Operation::new(self.block_ctx.rwc.inc_pre(), rw, op));
125127
step.bus_mapping_instance.push(op_ref);
128+
self.check_rw_num_limit()
129+
}
130+
131+
/// Check whether rws will overflow circuit limit.
132+
pub fn check_rw_num_limit(&self) -> Result<(), Error> {
133+
if let Some(max_rws) = self.max_rws {
134+
let rwc = self.block_ctx.rwc.0;
135+
if rwc > max_rws {
136+
log::error!("rwc > max_rws, rwc={}, max_rws={}", rwc, max_rws);
137+
return Err(Error::RwsNotEnough(max_rws, rwc));
138+
};
139+
}
140+
141+
Ok(())
126142
}
127143

128144
/// Push a read type [`CallContextOp`] into the
@@ -137,14 +153,14 @@ impl<'a> CircuitInputStateRef<'a> {
137153
call_id: usize,
138154
field: CallContextField,
139155
value: Word,
140-
) {
156+
) -> Result<(), Error> {
141157
let op = CallContextOp {
142158
call_id,
143159
field,
144160
value,
145161
};
146162

147-
self.push_op(step, RW::READ, op);
163+
self.push_op(step, RW::READ, op)
148164
}
149165

150166
/// Push a write type [`CallContextOp`] into the
@@ -159,14 +175,14 @@ impl<'a> CircuitInputStateRef<'a> {
159175
call_id: usize,
160176
field: CallContextField,
161177
value: Word,
162-
) {
178+
) -> Result<(), Error> {
163179
let op = CallContextOp {
164180
call_id,
165181
field,
166182
value,
167183
};
168184

169-
self.push_op(step, RW::WRITE, op);
185+
self.push_op(step, RW::WRITE, op)
170186
}
171187

172188
/// Push an [`Operation`](crate::operation::Operation) with reversible to be
@@ -203,7 +219,7 @@ impl<'a> CircuitInputStateRef<'a> {
203219
.push((self.tx.steps().len(), op_ref));
204220
}
205221

206-
Ok(())
222+
self.check_rw_num_limit()
207223
}
208224

209225
/// Push a read type [`MemoryOp`] into the
@@ -219,7 +235,7 @@ impl<'a> CircuitInputStateRef<'a> {
219235
value: u8,
220236
) -> Result<(), Error> {
221237
let call_id = self.call()?.call_id;
222-
self.push_op(step, RW::READ, MemoryOp::new(call_id, address, value));
238+
self.push_op(step, RW::READ, MemoryOp::new(call_id, address, value))?;
223239
Ok(())
224240
}
225241

@@ -236,7 +252,7 @@ impl<'a> CircuitInputStateRef<'a> {
236252
value: u8,
237253
) -> Result<(), Error> {
238254
let call_id = self.call()?.call_id;
239-
self.push_op(step, RW::WRITE, MemoryOp::new(call_id, address, value));
255+
self.push_op(step, RW::WRITE, MemoryOp::new(call_id, address, value))?;
240256
Ok(())
241257
}
242258

@@ -253,7 +269,7 @@ impl<'a> CircuitInputStateRef<'a> {
253269
value: Word,
254270
) -> Result<(), Error> {
255271
let call_id = self.call()?.call_id;
256-
self.push_op(step, RW::WRITE, StackOp::new(call_id, address, value));
272+
self.push_op(step, RW::WRITE, StackOp::new(call_id, address, value))?;
257273
Ok(())
258274
}
259275

@@ -270,7 +286,7 @@ impl<'a> CircuitInputStateRef<'a> {
270286
value: Word,
271287
) -> Result<(), Error> {
272288
let call_id = self.call()?.call_id;
273-
self.push_op(step, RW::READ, StackOp::new(call_id, address, value));
289+
self.push_op(step, RW::READ, StackOp::new(call_id, address, value))?;
274290
Ok(())
275291
}
276292

@@ -352,9 +368,9 @@ impl<'a> CircuitInputStateRef<'a> {
352368
address: Address,
353369
field: AccountField,
354370
value: Word,
355-
) {
371+
) -> Result<(), Error> {
356372
let op = AccountOp::new(address, field, value, value);
357-
self.push_op(step, RW::READ, op);
373+
self.push_op(step, RW::READ, op)
358374
}
359375

360376
/// Push a write type [`AccountOp`] into the
@@ -376,7 +392,7 @@ impl<'a> CircuitInputStateRef<'a> {
376392
if reversible {
377393
self.push_op_reversible(step, op)?;
378394
} else {
379-
self.push_op(step, RW::WRITE, op);
395+
self.push_op(step, RW::WRITE, op)?;
380396
}
381397
Ok(())
382398
}
@@ -400,8 +416,7 @@ impl<'a> CircuitInputStateRef<'a> {
400416
step,
401417
RW::WRITE,
402418
TxLogOp::new(tx_id, log_id, field, index, value),
403-
);
404-
Ok(())
419+
)
405420
}
406421

407422
/// Push a read type [`TxReceiptOp`] into the
@@ -425,8 +440,7 @@ impl<'a> CircuitInputStateRef<'a> {
425440
field,
426441
value,
427442
},
428-
);
429-
Ok(())
443+
)
430444
}
431445

432446
/// Push a write type [`TxReceiptOp`] into the
@@ -450,8 +464,7 @@ impl<'a> CircuitInputStateRef<'a> {
450464
field,
451465
value,
452466
},
453-
);
454-
Ok(())
467+
)
455468
}
456469

457470
/// Push a write type [`TxAccessListAccountOp`] into the
@@ -477,8 +490,7 @@ impl<'a> CircuitInputStateRef<'a> {
477490
is_warm,
478491
is_warm_prev,
479492
},
480-
);
481-
Ok(())
493+
)
482494
}
483495

484496
/// Add address to access list for the current transaction.
@@ -542,7 +554,7 @@ impl<'a> CircuitInputStateRef<'a> {
542554
value: sender_balance,
543555
value_prev: sender_balance_prev,
544556
},
545-
);
557+
)?;
546558
sender_balance_prev = sender_balance;
547559
}
548560
let sender_balance = sender_balance_prev - value;
@@ -766,29 +778,39 @@ impl<'a> CircuitInputStateRef<'a> {
766778
}
767779

768780
/// read reversion info
769-
pub(crate) fn reversion_info_read(&mut self, step: &mut ExecStep, call: &Call) {
781+
pub(crate) fn reversion_info_read(
782+
&mut self,
783+
step: &mut ExecStep,
784+
call: &Call,
785+
) -> Result<(), Error> {
770786
for (field, value) in [
771787
(
772788
CallContextField::RwCounterEndOfReversion,
773789
call.rw_counter_end_of_reversion.to_word(),
774790
),
775791
(CallContextField::IsPersistent, call.is_persistent.to_word()),
776792
] {
777-
self.call_context_read(step, call.call_id, field, value);
793+
self.call_context_read(step, call.call_id, field, value)?;
778794
}
795+
Ok(())
779796
}
780797

781798
/// write reversion info
782-
pub(crate) fn reversion_info_write(&mut self, step: &mut ExecStep, call: &Call) {
799+
pub(crate) fn reversion_info_write(
800+
&mut self,
801+
step: &mut ExecStep,
802+
call: &Call,
803+
) -> Result<(), Error> {
783804
for (field, value) in [
784805
(
785806
CallContextField::RwCounterEndOfReversion,
786807
call.rw_counter_end_of_reversion.to_word(),
787808
),
788809
(CallContextField::IsPersistent, call.is_persistent.to_word()),
789810
] {
790-
self.call_context_write(step, call.call_id, field, value);
811+
self.call_context_write(step, call.call_id, field, value)?;
791812
}
813+
Ok(())
792814
}
793815

794816
/// Check if address is a precompiled or not.
@@ -1128,7 +1150,7 @@ impl<'a> CircuitInputStateRef<'a> {
11281150
call.call_id,
11291151
CallContextField::IsSuccess,
11301152
0u64.into(),
1131-
);
1153+
)?;
11321154

11331155
// Even call.rw_counter_end_of_reversion is zero for now, it will set in
11341156
// set_value_ops_call_context_rwc_eor later
@@ -1139,7 +1161,7 @@ impl<'a> CircuitInputStateRef<'a> {
11391161
call.call_id,
11401162
CallContextField::RwCounterEndOfReversion,
11411163
call.rw_counter_end_of_reversion.into(),
1142-
);
1164+
)?;
11431165

11441166
if call.is_root {
11451167
return Ok(());
@@ -1155,7 +1177,7 @@ impl<'a> CircuitInputStateRef<'a> {
11551177
call.call_id,
11561178
CallContextField::CallerId,
11571179
caller.call_id.into(),
1158-
);
1180+
)?;
11591181

11601182
let [last_callee_return_data_offset, last_callee_return_data_length] =
11611183
if exec_step.error.is_some() {
@@ -1233,7 +1255,7 @@ impl<'a> CircuitInputStateRef<'a> {
12331255
self.caller_ctx()?.reversible_write_counter.into(),
12341256
),
12351257
] {
1236-
self.call_context_read(exec_step, caller.call_id, field, value);
1258+
self.call_context_read(exec_step, caller.call_id, field, value)?;
12371259
}
12381260

12391261
// EIP-211: CREATE/CREATE2 call successful case should set RETURNDATASIZE = 0
@@ -1258,7 +1280,7 @@ impl<'a> CircuitInputStateRef<'a> {
12581280
},
12591281
),
12601282
] {
1261-
self.call_context_write(exec_step, caller.call_id, field, value);
1283+
self.call_context_write(exec_step, caller.call_id, field, value)?;
12621284
}
12631285

12641286
Ok(())
@@ -1547,7 +1569,7 @@ impl<'a> CircuitInputStateRef<'a> {
15471569
exec_step,
15481570
RW::READ,
15491571
MemoryOp::new(self.call()?.caller_id, addr.into(), byte),
1550-
);
1572+
)?;
15511573
}
15521574
byte
15531575
} else {

bus-mapping/src/error.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ pub enum Error {
4040
ExecutionError(ExecError),
4141
/// Internal Code error
4242
InternalError(&'static str),
43+
/// Rw number overflow
44+
RwsNotEnough(usize, usize),
4345
}
4446

4547
impl From<eth_types::Error> for Error {

bus-mapping/src/evm/opcodes/address.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ impl Opcode for Address {
2626
state.call()?.call_id,
2727
CallContextField::CalleeAddress,
2828
address,
29-
);
29+
)?;
3030

3131
// Write the address to stack.
3232
state.stack_write(

0 commit comments

Comments
 (0)