Skip to content

Commit

Permalink
Added more info to gas errors
Browse files Browse the repository at this point in the history
  • Loading branch information
batconjurer committed Nov 7, 2024
1 parent 9a06dae commit a2830ba
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 26 deletions.
40 changes: 23 additions & 17 deletions crates/gas/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ use thiserror::Error;
#[allow(missing_docs)]
#[derive(Error, Debug, Clone, PartialEq, Eq)]
pub enum Error {
#[error("Transaction gas limit exceeded")]
TransactionGasExceededError,
#[error("Transaction gas limit exceeded maximum of {0}")]
TransactionGasExceededError(u64),
#[error("Block gas limit exceeded")]
BlockGasExceeded,
#[error("Overflow during gas operations")]
Expand Down Expand Up @@ -220,6 +220,7 @@ pub type Result<T> = std::result::Result<T, Error>;
// This type should not implement the Copy trait to prevent charging gas more
// than once
#[derive(
Copy,
Clone,
Debug,
Default,
Expand Down Expand Up @@ -386,8 +387,9 @@ pub trait GasMetering {
.get_tx_consumed_gas()
.checked_add(vps_gas)
.ok_or(Error::GasOverflow)?;
if total > self.get_gas_limit() {
return Err(Error::TransactionGasExceededError);
let gas_limit = self.get_gas_limit();
if total > gas_limit {
return Err(Error::TransactionGasExceededError(gas_limit.into()));
}

Ok(())
Expand Down Expand Up @@ -432,7 +434,9 @@ impl GasMetering for TxGasMeter {
})?;

if self.transaction_gas > self.tx_gas_limit {
return Err(Error::TransactionGasExceededError);
return Err(Error::TransactionGasExceededError(
self.tx_gas_limit.into(),
));
}

Ok(())
Expand All @@ -441,15 +445,15 @@ impl GasMetering for TxGasMeter {
/// Get the entire gas used by the transaction up until this point
fn get_tx_consumed_gas(&self) -> Gas {
if !self.gas_overflow {
self.transaction_gas.clone()
self.transaction_gas
} else {
hints::cold();
u64::MAX.into()
}
}

fn get_gas_limit(&self) -> Gas {
self.tx_gas_limit.clone()
self.tx_gas_limit
}
}

Expand Down Expand Up @@ -487,7 +491,7 @@ impl TxGasMeter {
/// Get the amount of gas still available to the transaction
pub fn get_available_gas(&self) -> Gas {
self.tx_gas_limit
.checked_sub(self.transaction_gas.clone())
.checked_sub(self.transaction_gas)
.unwrap_or_default()
}
}
Expand All @@ -508,11 +512,13 @@ impl GasMetering for VpGasMeter {

let current_total = self
.initial_gas
.checked_add(self.current_gas.clone())
.checked_add(self.current_gas)
.ok_or(Error::GasOverflow)?;

if current_total > self.tx_gas_limit {
return Err(Error::TransactionGasExceededError);
return Err(Error::TransactionGasExceededError(
self.tx_gas_limit.into(),
));
}

Ok(())
Expand All @@ -521,15 +527,15 @@ impl GasMetering for VpGasMeter {
/// Get the gas consumed by the tx alone before the vps were executed
fn get_tx_consumed_gas(&self) -> Gas {
if !self.gas_overflow {
self.initial_gas.clone()
self.initial_gas
} else {
hints::cold();
u64::MAX.into()
}
}

fn get_gas_limit(&self) -> Gas {
self.tx_gas_limit.clone()
self.tx_gas_limit
}
}

Expand All @@ -538,15 +544,15 @@ impl VpGasMeter {
pub fn new_from_tx_meter(tx_gas_meter: &TxGasMeter) -> Self {
Self {
gas_overflow: false,
tx_gas_limit: tx_gas_meter.tx_gas_limit.clone(),
initial_gas: tx_gas_meter.transaction_gas.clone(),
tx_gas_limit: tx_gas_meter.tx_gas_limit,
initial_gas: tx_gas_meter.transaction_gas,
current_gas: Gas::default(),
}
}

/// Get the gas consumed by the VP alone
pub fn get_vp_consumed_gas(&self) -> Gas {
self.current_gas.clone()
self.current_gas
}
}

Expand Down Expand Up @@ -601,7 +607,7 @@ mod tests {
meter
.consume(TX_GAS_LIMIT.into())
.expect_err("unexpectedly succeeded"),
Error::TransactionGasExceededError
Error::TransactionGasExceededError(_)
);
}

Expand All @@ -624,7 +630,7 @@ mod tests {
meter
.consume((TX_GAS_LIMIT + 1).into())
.expect_err("unexpectedly succeeded"),
Error::TransactionGasExceededError
Error::TransactionGasExceededError(_)
);
}
}
15 changes: 8 additions & 7 deletions crates/node/src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ where
};

// Commit tx to the block write log even in case of subsequent errors (if
// the fee payment failed instead, than the previous two functions must
// the fee payment failed instead, then the previous two functions must
// have propagated an error)
shell_params
.state
Expand Down Expand Up @@ -901,11 +901,12 @@ where

checked!(balance - fees).map_or_else(
|_| {
Err(Error::FeeError(
Err(Error::FeeError(format!(
"Masp fee payment unshielded an insufficient \
amount"
.to_string(),
))
amount: Unshielded {balance} {}, required \
{fees}",
wrapper.fee.token
)))
},
|_| Ok(Some(valid_batched_tx_result)),
)
Expand Down Expand Up @@ -1351,7 +1352,7 @@ where
))?;
gas_meter
.borrow()
.check_vps_limit(vps_gas.clone())
.check_vps_limit(vps_gas)
.map_err(|err| Error::GasError(err.to_string()))?;

Ok((result, vps_gas))
Expand Down Expand Up @@ -1383,7 +1384,7 @@ fn merge_vp_results(
.checked_add(b_gas)
.ok_or(Error::GasError(gas::Error::GasOverflow.to_string()))?;
tx_gas_meter
.check_vps_limit(vps_gas.clone())
.check_vps_limit(vps_gas)
.map_err(|err| Error::GasError(err.to_string()))?;

Ok((
Expand Down
2 changes: 1 addition & 1 deletion crates/node/src/shell/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1492,7 +1492,7 @@ where
Ok(amount_per_gas_unit) if amount_per_gas_unit < minimum_gas_price => {
// The fees do not match the minimum required
return Err(Error::TxApply(protocol::Error::FeeError(format!(
"Fee amount {:?} do not match the minimum required amount \
"Fee amount {:?} does not match the minimum required amount \
{:?} for token {}",
wrapper.fee.amount_per_gas_unit,
minimum_gas_price,
Expand Down
2 changes: 1 addition & 1 deletion crates/tests/src/native_vp/pos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -882,7 +882,7 @@ pub mod testing {
let current_epoch = tx_host_env::with(|env| {
// Reset the gas meter on each change, so that we never run
// out in this test
let gas_limit = env.gas_meter.borrow().tx_gas_limit.clone();
let gas_limit = env.gas_meter.borrow().tx_gas_limit;
env.gas_meter = RefCell::new(TxGasMeter::new(gas_limit));
env.state.in_mem().block.epoch
});
Expand Down

0 comments on commit a2830ba

Please sign in to comment.