feat: do not estimate fee when gas is specified (#583)

Similar to setting gas limit on Ethereum txs, setting L1 gas on a
v3 Starknet tx signals that the caller understands the tx's impact.
Previously, the library would incorrectly always perform a full fee
estimation as long as any of `gas` and `gas_price` is not specified.

starknet-accounts/src/account/declaration.rs

@@ -373,7 +373,29 @@ where
         // Resolves fee settings
         let (gas, gas_price) = match (self.gas, self.gas_price) {
             (Some(gas), Some(gas_price)) => (gas, gas_price),
-            // We have to perform fee estimation as long as it's not fully specified
+            (Some(gas), _) => {
+                // When `gas` is specified, we only need the L1 gas price in FRI. By specifying a
+                // a `gas` value, the user might be trying to avoid a full fee estimation (e.g.
+                // flaky dependencies), so it's in appropriate to call `estimate_fee` here.
+
+                // This is the lightest-weight block we can get
+                let block_l1_gas_price = self
+                    .account
+                    .provider()
+                    .get_block_with_tx_hashes(self.account.block_id())
+                    .await
+                    .map_err(AccountError::Provider)?
+                    .l1_gas_price()
+                    .price_in_fri;
+
+                let gas_price = (((TryInto::<u64>::try_into(block_l1_gas_price)
+                    .map_err(|_| AccountError::FeeOutOfRange)?)
+                    as f64)
+                    * self.gas_price_estimate_multiplier) as u128;
+
+                (gas, gas_price)
+            }
+            // We have to perform fee estimation as long as gas is not specified
             _ => {
                 let fee_estimate = self.estimate_fee_with_nonce(nonce).await?;
 

starknet-accounts/src/account/execution.rs

@@ -353,7 +353,29 @@ where
         // Resolves fee settings
         let (gas, gas_price) = match (self.gas, self.gas_price) {
             (Some(gas), Some(gas_price)) => (gas, gas_price),
-            // We have to perform fee estimation as long as it's not fully specified
+            (Some(gas), _) => {
+                // When `gas` is specified, we only need the L1 gas price in FRI. By specifying a
+                // a `gas` value, the user might be trying to avoid a full fee estimation (e.g.
+                // flaky dependencies), so it's in appropriate to call `estimate_fee` here.
+
+                // This is the lightest-weight block we can get
+                let block_l1_gas_price = self
+                    .account
+                    .provider()
+                    .get_block_with_tx_hashes(self.account.block_id())
+                    .await
+                    .map_err(AccountError::Provider)?
+                    .l1_gas_price()
+                    .price_in_fri;
+
+                let gas_price = (((TryInto::<u64>::try_into(block_l1_gas_price)
+                    .map_err(|_| AccountError::FeeOutOfRange)?)
+                    as f64)
+                    * self.gas_price_estimate_multiplier) as u128;
+
+                (gas, gas_price)
+            }
+            // We have to perform fee estimation as long as gas is not specified
             _ => {
                 let fee_estimate = self.estimate_fee_with_nonce(nonce).await?;
 

starknet-accounts/src/factory/mod.rs

@@ -526,7 +526,29 @@ where
         // Resolves fee settings
         let (gas, gas_price) = match (self.gas, self.gas_price) {
             (Some(gas), Some(gas_price)) => (gas, gas_price),
-            // We have to perform fee estimation as long as it's not fully specified
+            (Some(gas), _) => {
+                // When `gas` is specified, we only need the L1 gas price in FRI. By specifying a
+                // a `gas` value, the user might be trying to avoid a full fee estimation (e.g.
+                // flaky dependencies), so it's in appropriate to call `estimate_fee` here.
+
+                // This is the lightest-weight block we can get
+                let block_l1_gas_price = self
+                    .factory
+                    .provider()
+                    .get_block_with_tx_hashes(self.factory.block_id())
+                    .await
+                    .map_err(AccountFactoryError::Provider)?
+                    .l1_gas_price()
+                    .price_in_fri;
+
+                let gas_price = (((TryInto::<u64>::try_into(block_l1_gas_price)
+                    .map_err(|_| AccountFactoryError::FeeOutOfRange)?)
+                    as f64)
+                    * self.gas_price_estimate_multiplier) as u128;
+
+                (gas, gas_price)
+            }
+            // We have to perform fee estimation as long as gas is not specified
             _ => {
                 let fee_estimate = self.estimate_fee_with_nonce(nonce).await?;
 

starknet-accounts/tests/single_owner_account.rs

@@ -32,7 +32,7 @@ fn create_sequencer_client() -> SequencerGatewayProvider {
 
 fn create_jsonrpc_client() -> JsonRpcClient<HttpTransport> {
     let rpc_url = std::env::var("STARKNET_RPC")
-        .unwrap_or("https://pathfinder.rpc.sepolia.starknet.rs/rpc/v0_7".into());
+        .unwrap_or("https://juno.rpc.sepolia.starknet.rs/rpc/v0_7".into());
     JsonRpcClient::new(HttpTransport::new(url::Url::parse(&rpc_url).unwrap()))
 }
 
@@ -123,6 +123,15 @@ async fn can_execute_eth_transfer_invoke_v3_with_jsonrpc() {
     .await
 }
 
+#[tokio::test]
+async fn can_execute_eth_transfer_invoke_v3_with_manual_gas_with_jsonrpc() {
+    can_execute_eth_transfer_invoke_v3_with_manual_gas_inner(
+        create_jsonrpc_client(),
+        "0x04a3189bdbc8716f416f7d54d9bf0d0f55ffb454bb89c547118d023a652277dd",
+    )
+    .await
+}
+
 #[tokio::test]
 #[ignore = "endpoint deprecated since Starknet v0.12.3"]
 async fn can_declare_cairo1_contract_v2_with_sequencer() {
@@ -388,7 +397,7 @@ async fn can_execute_eth_transfer_invoke_v3_inner<P: Provider + Send + Sync>(
                 FieldElement::ZERO,
             ],
         }])
-        .gas(8000)
+        .gas(20000)
         .gas_price(100000000000000)
         .send()
         .await
@@ -397,6 +406,48 @@ async fn can_execute_eth_transfer_invoke_v3_inner<P: Provider + Send + Sync>(
     assert!(result.transaction_hash > FieldElement::ZERO);
 }
 
+async fn can_execute_eth_transfer_invoke_v3_with_manual_gas_inner<P: Provider + Send + Sync>(
+    provider: P,
+    address: &str,
+) {
+    // This test tx reverts, as the account does not have sufficient ETH balance. However, the point
+    // is to test that a fee estimation is _not_ performed when `gas` is specified. A fee estimation
+    // performed on this call would have thrown.
+
+    let signer = LocalWallet::from(SigningKey::from_secret_scalar(
+        FieldElement::from_hex_be(
+            "00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+        )
+        .unwrap(),
+    ));
+    let address = FieldElement::from_hex_be(address).unwrap();
+    let eth_token_address = FieldElement::from_hex_be(
+        "049d36570d4e46f48e99674bd3fcc84644ddd6b96f7c741b1562b82f9e004dc7",
+    )
+    .unwrap();
+
+    let mut account =
+        SingleOwnerAccount::new(provider, signer, address, CHAIN_ID, ExecutionEncoding::New);
+    account.set_block_id(BlockId::Tag(BlockTag::Pending));
+
+    let result = account
+        .execute_v3(vec![Call {
+            to: eth_token_address,
+            selector: get_selector_from_name("transfer").unwrap(),
+            calldata: vec![
+                FieldElement::from_hex_be("0x1234").unwrap(),
+                FieldElement::from_dec_str("10000000000000000000").unwrap(),
+                FieldElement::ZERO,
+            ],
+        }])
+        .gas(20000)
+        .send()
+        .await
+        .unwrap();
+
+    assert!(result.transaction_hash > FieldElement::ZERO);
+}
+
 async fn can_declare_cairo1_contract_v2_inner<P: Provider + Send + Sync>(
     provider: P,
     address: &str,
@@ -550,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(8000)
+        .gas(20000)
         .gas_price(100000000000000)
         .send()
         .await

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(8000)
+        .gas(20000)
         .gas_price(100000000000000)
         .send()
         .await;

starknet-core/src/types/mod.rs

@@ -340,6 +340,13 @@ impl MaybePendingBlockWithTxHashes {
             MaybePendingBlockWithTxHashes::PendingBlock(block) => &block.transactions,
         }
     }
+
+    pub fn l1_gas_price(&self) -> &ResourcePrice {
+        match self {
+            MaybePendingBlockWithTxHashes::Block(block) => &block.l1_gas_price,
+            MaybePendingBlockWithTxHashes::PendingBlock(block) => &block.l1_gas_price,
+        }
+    }
 }
 
 impl MaybePendingBlockWithTxs {
@@ -349,6 +356,13 @@ impl MaybePendingBlockWithTxs {
             MaybePendingBlockWithTxs::PendingBlock(block) => &block.transactions,
         }
     }
+
+    pub fn l1_gas_price(&self) -> &ResourcePrice {
+        match self {
+            MaybePendingBlockWithTxs::Block(block) => &block.l1_gas_price,
+            MaybePendingBlockWithTxs::PendingBlock(block) => &block.l1_gas_price,
+        }
+    }
 }
 
 impl MaybePendingBlockWithReceipts {
@@ -358,6 +372,13 @@ impl MaybePendingBlockWithReceipts {
             MaybePendingBlockWithReceipts::PendingBlock(block) => &block.transactions,
         }
     }
+
+    pub fn l1_gas_price(&self) -> &ResourcePrice {
+        match self {
+            MaybePendingBlockWithReceipts::Block(block) => &block.l1_gas_price,
+            MaybePendingBlockWithReceipts::PendingBlock(block) => &block.l1_gas_price,
+        }
+    }
 }
 
 impl TransactionStatus {