Skip to content

Commit

Permalink
fix: incorrect v3 max gas amount calculation
Browse files Browse the repository at this point in the history
When using JSON-RPC v0.7.0, the fee estimate returned contains data
gas info. Naively using `gas_consumed` as max gas amount results in
insufficient fees. The correct way to do it is to calculate a pseudo gas
amount by dividing the overall fee by the gas price.
  • Loading branch information
xJonathanLEI committed May 1, 2024
1 parent 4fa0a73 commit 85d7a21
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 10 deletions.
7 changes: 5 additions & 2 deletions starknet-accounts/src/account/declaration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,8 +402,11 @@ where
let gas = match self.gas {
Some(gas) => gas,
None => {
(((TryInto::<u64>::try_into(fee_estimate.gas_consumed)
.map_err(|_| AccountError::FeeOutOfRange)?)
(((TryInto::<u64>::try_into(
(fee_estimate.overall_fee + fee_estimate.gas_price - FieldElement::ONE)
.floor_div(fee_estimate.gas_price),
)
.map_err(|_| AccountError::FeeOutOfRange)?)
as f64)
* self.gas_estimate_multiplier) as u64
}
Expand Down
7 changes: 5 additions & 2 deletions starknet-accounts/src/account/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,11 @@ where
let gas = match self.gas {
Some(gas) => gas,
None => {
(((TryInto::<u64>::try_into(fee_estimate.gas_consumed)
.map_err(|_| AccountError::FeeOutOfRange)?)
(((TryInto::<u64>::try_into(
(fee_estimate.overall_fee + fee_estimate.gas_price - FieldElement::ONE)
.floor_div(fee_estimate.gas_price),
)
.map_err(|_| AccountError::FeeOutOfRange)?)
as f64)
* self.gas_estimate_multiplier) as u64
}
Expand Down
7 changes: 5 additions & 2 deletions starknet-accounts/src/factory/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,8 +562,11 @@ where
let gas = match self.gas {
Some(gas) => gas,
None => {
(((TryInto::<u64>::try_into(fee_estimate.gas_consumed)
.map_err(|_| AccountFactoryError::FeeOutOfRange)?)
(((TryInto::<u64>::try_into(
(fee_estimate.overall_fee + fee_estimate.gas_price - FieldElement::ONE)
.floor_div(fee_estimate.gas_price),
)
.map_err(|_| AccountFactoryError::FeeOutOfRange)?)
as f64)
* self.gas_estimate_multiplier) as u64
}
Expand Down
6 changes: 3 additions & 3 deletions starknet-accounts/tests/single_owner_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ async fn can_execute_eth_transfer_invoke_v3_inner<P: Provider + Send + Sync>(
FieldElement::ZERO,
],
}])
.gas(20000)
.gas(200000)
.gas_price(100000000000000)
.send()
.await
Expand Down Expand Up @@ -440,7 +440,7 @@ async fn can_execute_eth_transfer_invoke_v3_with_manual_gas_inner<P: Provider +
FieldElement::ZERO,
],
}])
.gas(20000)
.gas(200000)
.send()
.await
.unwrap();
Expand Down Expand Up @@ -601,7 +601,7 @@ async fn can_declare_cairo1_contract_v3_inner<P: Provider + Send + Sync>(
Arc::new(flattened_class),
FieldElement::from_hex_be(&hashes.compiled_class_hash).unwrap(),
)
.gas(20000)
.gas(200000)
.gas_price(100000000000000)
.send()
.await
Expand Down
2 changes: 1 addition & 1 deletion starknet-contract/tests/contract_deployment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ async fn can_deploy_contract_to_alpha_sepolia_with_invoke_v3() {
FieldElement::from_bytes_be(&salt_buffer).unwrap(),
true,
)
.gas(20000)
.gas(200000)
.gas_price(100000000000000)
.send()
.await;
Expand Down

0 comments on commit 85d7a21

Please sign in to comment.