Skip to content

Commit

Permalink
evm: Fix divide by zero error in eth_estimateGas (#2697)
Browse files Browse the repository at this point in the history
* Fix divide by zero error

* Fix RPC errors

* Fix tests

* Do not mutate CallRequest

* Rename DivideByZero

* Rename guess_tx_type

* Return tx_type itself

---------

Co-authored-by: jouzo <[email protected]>
  • Loading branch information
shohamc1 and Jouzo authored Nov 20, 2023
1 parent bd24e47 commit c10105e
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 36 deletions.
91 changes: 58 additions & 33 deletions lib/ain-grpc/src/call_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,45 @@ pub struct CallRequest {
pub transaction_type: Option<U256>,
}

enum TxType {
Legacy,
EIP2930,
EIP1559,
}

impl TryFrom<U256> for TxType {
type Error = Error;

fn try_from(v: U256) -> Result<Self, Self::Error> {
match v {
v if v == U256::zero() => Ok(Self::Legacy),
v if v == U256::one() => Ok(Self::EIP2930),
v if v == U256::from(2) => Ok(Self::EIP1559),
_ => Err(RPCError::InvalidTransactionType.into()),
}
}
}

fn guess_tx_type(req: &CallRequest) -> Result<TxType, Error> {
if let Some(tx_type) = req.transaction_type {
return TxType::try_from(tx_type);
}

if req.gas_price.is_some()
&& (req.max_fee_per_gas.is_some() || req.max_priority_fee_per_gas.is_some())
{
return Err(RPCError::InvalidGasPrice.into());
}

if req.max_fee_per_gas.is_some() && req.max_priority_fee_per_gas.is_some() {
Ok(TxType::EIP1559)
} else if req.access_list.is_some() {
Ok(TxType::EIP2930)
} else {
Ok(TxType::Legacy)
}
}

impl CallRequest {
pub fn get_effective_gas_price(&self, block_base_fee: U256) -> Result<U256, Error> {
if self.gas_price.is_some()
Expand All @@ -46,41 +85,27 @@ impl CallRequest {
return Err(RPCError::InvalidGasPrice.into());
}

match self.transaction_type {
// Legacy
Some(tx_type) if tx_type == U256::zero() => {
if let Some(gas_price) = self.gas_price {
return Ok(gas_price);
} else {
return Ok(block_base_fee);
match guess_tx_type(self)? {
TxType::Legacy | TxType::EIP2930 => match self.gas_price {
Some(gas_price) => {
if gas_price == U256::zero() {
Ok(block_base_fee)
} else {
Ok(gas_price)
}
}
}
// EIP2930
Some(tx_type) if tx_type == U256::one() => {
if let Some(gas_price) = self.gas_price {
return Ok(gas_price);
} else {
return Ok(block_base_fee);
None => Ok(block_base_fee),
},
TxType::EIP1559 => match self.max_fee_per_gas {
Some(max_fee_per_gas) => {
if max_fee_per_gas == U256::zero() {
Ok(block_base_fee)
} else {
Ok(max_fee_per_gas)
}
}
}
// EIP1559
Some(tx_type) if tx_type == U256::from(2) => {
if let Some(max_fee_per_gas) = self.max_fee_per_gas {
return Ok(max_fee_per_gas);
} else {
return Ok(block_base_fee);
}
}
None => (),
_ => return Err(RPCError::InvalidTransactionType.into()),
}

if let Some(gas_price) = self.gas_price {
Ok(gas_price)
} else if let Some(gas_price) = self.max_fee_per_gas {
Ok(gas_price)
} else {
Ok(block_base_fee)
None => Ok(block_base_fee),
},
}
}

Expand Down
4 changes: 4 additions & 0 deletions lib/ain-grpc/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ pub enum RPCError {
TxExecutionFailed,
TxNotFound(H256),
ValueOverflow,
ValueUnderflow,
DivideError,
}

impl From<RPCError> for Error {
Expand Down Expand Up @@ -76,6 +78,8 @@ impl From<RPCError> for Error {
hash
)),
RPCError::ValueOverflow => to_custom_err("value overflow"),
RPCError::ValueUnderflow => to_custom_err("value underflow"),
RPCError::DivideError => to_custom_err("divide error"),
}
}
}
Expand Down
8 changes: 5 additions & 3 deletions lib/ain-grpc/src/rpc/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ impl MetachainRPCModule {
impl MetachainRPCServer for MetachainRPCModule {
fn call(&self, call: CallRequest, block_number: Option<BlockNumber>) -> RpcResult<Bytes> {
debug!(target:"rpc", "Call, input {:#?}", call);

let caller = call.from.unwrap_or_default();
let byte_data = call.get_data()?;
let data = byte_data.0.as_slice();
Expand Down Expand Up @@ -764,6 +765,7 @@ impl MetachainRPCServer for MetachainRPCModule {
block_number: Option<BlockNumber>,
) -> RpcResult<U256> {
debug!(target:"rpc", "Estimate gas, input {:#?}", call);

let caller = call.from.unwrap_or_default();
let byte_data = call.get_data()?;
let data = byte_data.0.as_slice();
Expand Down Expand Up @@ -799,12 +801,12 @@ impl MetachainRPCServer for MetachainRPCModule {
if balance < value {
return Err(RPCError::InsufficientFunds.into());
}
available = balance.checked_sub(value).ok_or(RPCError::ValueOverflow)?;
available = balance.checked_sub(value).ok_or(RPCError::ValueUnderflow)?;
}

let allowance = available
.checked_div(fee_cap)
.ok_or(RPCError::ValueOverflow)?;
.ok_or(RPCError::DivideError)?;
debug!(target:"rpc", "[estimate_gas] allowance: {:#?}", allowance);

if let Ok(allowance) = u64::try_from(allowance) {
Expand Down Expand Up @@ -846,7 +848,7 @@ impl MetachainRPCServer for MetachainRPCModule {

while lo + 1 < hi {
let sum = hi.checked_add(lo).ok_or(RPCError::ValueOverflow)?;
let mid = sum.checked_div(2u64).ok_or(RPCError::ValueOverflow)?;
let mid = sum.checked_div(2u64).ok_or(RPCError::DivideError)?;

let (failed, ..) = executable(mid)?;
if failed {
Expand Down

0 comments on commit c10105e

Please sign in to comment.