From 85d7a2176510b707fa68015ec63ec3c26b2dfb10 Mon Sep 17 00:00:00 2001
From: Jonathan LEI <me@xjonathan.dev>
Date: Wed, 1 May 2024 13:40:54 +0800
Subject: [PATCH] fix: incorrect v3 max gas amount calculation

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.
---
 starknet-accounts/src/account/declaration.rs    | 7 +++++--
 starknet-accounts/src/account/execution.rs      | 7 +++++--
 starknet-accounts/src/factory/mod.rs            | 7 +++++--
 starknet-accounts/tests/single_owner_account.rs | 6 +++---
 starknet-contract/tests/contract_deployment.rs  | 2 +-
 5 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/starknet-accounts/src/account/declaration.rs b/starknet-accounts/src/account/declaration.rs
index 20f831e2..f3535ae2 100644
--- a/starknet-accounts/src/account/declaration.rs
+++ b/starknet-accounts/src/account/declaration.rs
@@ -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
                     }
diff --git a/starknet-accounts/src/account/execution.rs b/starknet-accounts/src/account/execution.rs
index 2afd9ab3..d16ec031 100644
--- a/starknet-accounts/src/account/execution.rs
+++ b/starknet-accounts/src/account/execution.rs
@@ -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
                     }
diff --git a/starknet-accounts/src/factory/mod.rs b/starknet-accounts/src/factory/mod.rs
index e9fd13ab..33488367 100644
--- a/starknet-accounts/src/factory/mod.rs
+++ b/starknet-accounts/src/factory/mod.rs
@@ -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
                     }
diff --git a/starknet-accounts/tests/single_owner_account.rs b/starknet-accounts/tests/single_owner_account.rs
index 47382e4b..2de0ad85 100644
--- a/starknet-accounts/tests/single_owner_account.rs
+++ b/starknet-accounts/tests/single_owner_account.rs
@@ -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
@@ -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();
@@ -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
diff --git a/starknet-contract/tests/contract_deployment.rs b/starknet-contract/tests/contract_deployment.rs
index a6a51d53..b4bc6b0c 100644
--- a/starknet-contract/tests/contract_deployment.rs
+++ b/starknet-contract/tests/contract_deployment.rs
@@ -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;