feat: add basic gas settings to testenv#23289
Conversation
nchamo
left a comment
nchamo
left a comment
There was a problem hiding this comment.
Good work!
Left some comments
| gas_limits: Option::none(), | ||
| teardown_gas_limits: Option::none(), | ||
| max_fees_per_gas: Option::none(), | ||
| max_priority_fees_per_gas: Option::none(), |
There was a problem hiding this comment.
I'm curious about why you went with separate config, instead of one GasSettings one and just having one setter (with_gas_settings)
There was a problem hiding this comment.
Both options are available. In some cases you may only care about one field.
| call: PrivateCall<M, N, T>, | ||
| opts: CallOptions, |
There was a problem hiding this comment.
You know I prefer to leave options at the end, but you already added options before the call in another PR. We should try to be consistent
| /// Configuration values for [`TestEnvironment::call_private_opts`] and [`TestEnvironment::call_public_opts`]. Meant | ||
| /// to be used by calling `new` and then chaining methods setting each value, e.g.: | ||
| /// ```noir | ||
| /// env.call_private_opts(CallOptions::new().with_max_fees_per_gas(GasFees::new(1, 1)), caller, |
There was a problem hiding this comment.
I don't think the order of the params is correct here
| } | ||
|
|
||
| #[test] | ||
| unconstrained fn call_public_opts_propagates_da_gas_limit() { |
There was a problem hiding this comment.
I think we are missing the equivalent to opts_with_gas_settings_sets_all_fields here
…gas-settings-in-txe
|
✅ Successfully backported to backport-to-v4-next-staging #23291. |
BEGIN_COMMIT_OVERRIDE feat(txe): add oracle versioning for test environment (AztecProtocol#23285) feat: add basic gas settings to testenv (AztecProtocol#23289) END_COMMIT_OVERRIDE
3 participants
Closes #22290.
This are the most basic settings we can provide at the moment, for
env.private_context,env.private_callandenv.public_call.public_contextis notably missing, but we don't even have TXE oracles for AVM gas at the moment, so more work is needed there to do this. Hopefully that's not as urgent as this.