ilikepi63 opened PR #3792 from feature/add-fuel-amount-to-configuration-set
to main
:
Relatively new to both Rust and Wasmtime, so trying to just pick up a small first issue here.
PR to try a solution to Issue #3717
Currently just adds a "add_fuel" config option to the wasmtime API. I was taking a look and I think I might need to update this default value when the interpreter is instantiated:
pub fn new(state: InterpreterState<'a>) -> Self { Self { state, fuel: None } }
to
pub fn new(state: InterpreterState<'a>, fuel: Option<u64> = None) -> Self { Self { state, fuel: fuel } }
Don't currently have any test cases for the specific command line option just yet. Just wanted to check if I was on the right track.
CC: @fitzgen
ilikepi63 has marked PR #3792 as ready for review.
ilikepi63 updated PR #3792 from feature/add-fuel-amount-to-configuration-set
to main
.
ilikepi63 updated PR #3792 from feature/add-fuel-amount-to-configuration-set
to main
.
ilikepi63 updated PR #3792 from feature/add-fuel-amount-to-configuration-set
to main
.
a1phyr submitted PR review.
a1phyr submitted PR review.
a1phyr created PR review comment:
I don't think you want to panic here, even though this should never fail.
store.add_fuel(self.common.add_fuel)?;
ilikepi63 updated PR #3792 from feature/add-fuel-amount-to-configuration-set
to main
.
ilikepi63 updated PR #3792 from feature/add-fuel-amount-to-configuration-set
to main
.
fitzgen submitted PR review.
fitzgen submitted PR review.
fitzgen created PR review comment:
if self.common.consume_fuel { let fuel = self.common.add_fuel.expect("both add_fuel and consume_fuel must be given"); store.add_fuel(fuel)?; }
fitzgen created PR review comment:
#[structopt(long, requires = "consume_fuel")] add_fuel: Option<u64>,
fitzgen created PR review comment:
#[structopt(long, requires = "add_fuel")] consume_fuel: bool,
ilikepi63 updated PR #3792 from feature/add-fuel-amount-to-configuration-set
to main
.
ilikepi63 updated PR #3792 from feature/add-fuel-amount-to-configuration-set
to main
.
fitzgen submitted PR review.
fitzgen created PR review comment:
// If fuel has been configured, we want to add the configured // fuel amount to this store.
fitzgen created PR review comment:
// If fuel has been configured, set the `consume fuel` flag on the config.
fitzgen submitted PR review.
fitzgen updated PR #3792 from feature/add-fuel-amount-to-configuration-set
to main
.
fitzgen updated PR #3792 from feature/add-fuel-amount-to-configuration-set
to main
.
fitzgen submitted PR review.
fitzgen merged PR #3792.
Last updated: Nov 22 2024 at 16:03 UTC