Improve out of gas error log (#3874)

* Add additional information for out of gas error * Add guide entry for troubleshooting gas errors * Add changelog entry * Apply suggestions from code review Co-authored-by: Romain Ruetschi <romain@informal.systems> Signed-off-by: Luca Joss <43531661+ljoss17@users.noreply.github.com> --------- Signed-off-by: Luca Joss <43531661+ljoss17@users.noreply.github.com> Co-authored-by: Romain Ruetschi <romain@informal.systems>
informalsystems · Mar 6, 2024 · cac00ee · cac00ee
1 parent 0f690a0
commit cac00ee
Show file tree

Hide file tree

Showing 7 changed files with 113 additions and 28 deletions.
diff --git a/.changelog/unreleased/improvements/ibc-relayer/3530-out-of-gas-error-log.md b/.changelog/unreleased/improvements/ibc-relayer/3530-out-of-gas-error-log.md
@@ -0,0 +1,4 @@
+- Improve the log diagnostic when an out of gas error is thrown.
+  And a new entry related to gas error has been added to the Hermes
+  guide.
+  ([\#3530](https://github.com/informalsystems/hermes/issues/3530))
diff --git a/crates/relayer/src/chain/cosmos/estimate.rs b/crates/relayer/src/chain/cosmos/estimate.rs
@@ -17,13 +17,26 @@ use crate::keyring::Secp256k1KeyPair;
 use crate::telemetry;
 use crate::util::pretty::PrettyFee;
 
+pub enum EstimatedGas {
+    Simulated(u64),
+    Default(u64),
+}
+
+impl EstimatedGas {
+    pub fn get_amount(&self) -> u64 {
+        match self {
+            Self::Simulated(amount) | Self::Default(amount) => *amount,
+        }
+    }
+}
+
 pub async fn estimate_tx_fees(
     config: &TxConfig,
     key_pair: &Secp256k1KeyPair,
     account: &Account,
     tx_memo: &Memo,
     messages: &[Any],
-) -> Result<Fee, Error> {
+) -> Result<(Fee, EstimatedGas), Error> {
     let gas_config = &config.gas_config;
 
     debug!(
@@ -46,7 +59,7 @@ pub async fn estimate_tx_fees(
         signatures: signed_tx.signatures,
     };
 
-    let estimated_fee = estimate_fee_with_tx(
+    let estimated_fee_and_gas = estimate_fee_with_tx(
         gas_config,
         &config.grpc_address,
         &config.rpc_address,
@@ -56,7 +69,7 @@ pub async fn estimate_tx_fees(
     )
     .await?;
 
-    Ok(estimated_fee)
+    Ok(estimated_fee_and_gas)
 }
 
 async fn estimate_fee_with_tx(
@@ -66,7 +79,7 @@ async fn estimate_fee_with_tx(
     chain_id: &ChainId,
     tx: Tx,
     account: &Account,
-) -> Result<Fee, Error> {
+) -> Result<(Fee, EstimatedGas), Error> {
     let estimated_gas = {
         crate::time!(
             "estimate_gas_with_tx",
@@ -78,29 +91,32 @@ async fn estimate_fee_with_tx(
         estimate_gas_with_tx(gas_config, grpc_address, tx, account).await
     }?;
 
-    if estimated_gas > gas_config.max_gas {
+    let estimated_gas_amount = estimated_gas.get_amount();
+
+    if estimated_gas_amount > gas_config.max_gas {
         debug!(
-            id = %chain_id, estimated = ?estimated_gas, max = ?gas_config.max_gas,
+            id = %chain_id, estimated = ?estimated_gas_amount, max = ?gas_config.max_gas,
             "send_tx: estimated gas is higher than max gas"
         );
 
         return Err(Error::tx_simulate_gas_estimate_exceeded(
             chain_id.clone(),
-            estimated_gas,
+            estimated_gas_amount,
             gas_config.max_gas,
         ));
     }
 
-    let adjusted_fee = gas_amount_to_fee(gas_config, estimated_gas, chain_id, rpc_address).await;
+    let adjusted_fee =
+        gas_amount_to_fee(gas_config, estimated_gas_amount, chain_id, rpc_address).await;
 
     debug!(
         id = %chain_id,
         "send_tx: using {} gas, fee {}",
-        estimated_gas,
+        estimated_gas_amount,
         PrettyFee(&adjusted_fee)
     );
 
-    Ok(adjusted_fee)
+    Ok((adjusted_fee, estimated_gas))
 }
 
 /// Try to simulate the given tx in order to estimate how much gas will be needed to submit it.
@@ -116,7 +132,7 @@ async fn estimate_gas_with_tx(
     grpc_address: &Uri,
     tx: Tx,
     account: &Account,
-) -> Result<u64, Error> {
+) -> Result<EstimatedGas, Error> {
     let simulated_gas = send_tx_simulate(grpc_address, tx)
         .await
         .map(|sr| sr.gas_info);
@@ -130,7 +146,7 @@ async fn estimate_gas_with_tx(
                 gas_info.gas_used
             );
 
-            Ok(gas_info.gas_used)
+            Ok(EstimatedGas::Simulated(gas_info.gas_used))
         }
 
         Ok(None) => {
@@ -139,7 +155,7 @@ async fn estimate_gas_with_tx(
                 gas_config.default_gas
             );
 
-            Ok(gas_config.default_gas)
+            Ok(EstimatedGas::Default(gas_config.default_gas))
         }
 
         // If there is a chance that the tx will be accepted once actually submitted, we fall
@@ -158,7 +174,7 @@ async fn estimate_gas_with_tx(
                 get_error_text(&e),
             );
 
-            Ok(gas_config.default_gas)
+            Ok(EstimatedGas::Default(gas_config.default_gas))
         }
 
         Err(e) => {

diff --git a/crates/relayer/src/chain/cosmos/retry.rs b/crates/relayer/src/chain/cosmos/retry.rs
@@ -103,7 +103,7 @@ async fn do_send_tx_with_account_sequence_retry(
         // NOTE: The error code could potentially overlap between Cosmos SDK and Ibc-go channel
         // error codes. This is currently not the case of incorrect account sequence error
         //which is the Cosmos SDK code 32 and Ibc-go channel errors only go up to 25.
-        Ok(ref response) if response.code == Code::from(INCORRECT_ACCOUNT_SEQUENCE_ERR) => {
+        Ok((ref response, _)) if response.code == Code::from(INCORRECT_ACCOUNT_SEQUENCE_ERR) => {
             warn!(
                 ?response,
                 "failed to broadcast tx because of a mismatched account sequence number, \
@@ -125,7 +125,7 @@ async fn do_send_tx_with_account_sequence_retry(
 
         // Gas estimation succeeded and broadcast_tx_sync was either successful or has failed with
         // an unrecoverable error.
-        Ok(response) => {
+        Ok((response, estimated_gas)) => {
             debug!("gas estimation succeeded");
 
             // Gas estimation and broadcast_tx_sync were successful.
@@ -155,7 +155,7 @@ async fn do_send_tx_with_account_sequence_retry(
                     // Log the error.
                     error!(
                         ?response,
-                        diagnostic = ?sdk_error_from_tx_sync_error_code(code.into()),
+                        diagnostic = ?sdk_error_from_tx_sync_error_code(code.into(), estimated_gas),
                         "failed to broadcast tx with unrecoverable error"
                     );
 
@@ -195,7 +195,10 @@ async fn refresh_account_and_retry_send_tx_with_account_sequence(
     // Retry after delay
     thread::sleep(Duration::from_millis(ACCOUNT_SEQUENCE_RETRY_DELAY));
 
-    estimate_fee_and_send_tx(rpc_client, config, key_pair, account, tx_memo, messages).await
+    let (estimate_result, _) =
+        estimate_fee_and_send_tx(rpc_client, config, key_pair, account, tx_memo, messages).await?;
+
+    Ok(estimate_result)
 }
 
 /// Determine whether the given error yielded by `tx_simulate`

diff --git a/crates/relayer/src/chain/cosmos/tx.rs b/crates/relayer/src/chain/cosmos/tx.rs
@@ -17,6 +17,7 @@ use crate::event::IbcEventWithHeight;
 use crate::keyring::{Secp256k1KeyPair, SigningKeyPair};
 
 use super::batch::send_batched_messages_and_wait_commit;
+use super::estimate::EstimatedGas;
 
 pub async fn estimate_fee_and_send_tx(
     rpc_client: &HttpClient,
@@ -25,13 +26,16 @@ pub async fn estimate_fee_and_send_tx(
     account: &Account,
     tx_memo: &Memo,
     messages: &[Any],
-) -> Result<Response, Error> {
-    let fee = estimate_tx_fees(config, key_pair, account, tx_memo, messages).await?;
+) -> Result<(Response, EstimatedGas), Error> {
+    let (fee, estimated_gas) =
+        estimate_tx_fees(config, key_pair, account, tx_memo, messages).await?;
 
-    send_tx_with_fee(
+    let tx_result = send_tx_with_fee(
         rpc_client, config, key_pair, account, tx_memo, messages, &fee,
     )
-    .await
+    .await?;
+
+    Ok((tx_result, estimated_gas))
 }
 
 async fn send_tx_with_fee(
@@ -87,7 +91,7 @@ pub async fn simple_send_tx(
         .await?
         .into();
 
-    let response = estimate_fee_and_send_tx(
+    let (response, _) = estimate_fee_and_send_tx(
         rpc_client,
         config,
         key_pair,

diff --git a/crates/relayer/src/sdk_error.rs b/crates/relayer/src/sdk_error.rs
@@ -1,6 +1,8 @@
 use flex_error::define_error;
 use tendermint::abci::Code;
 
+use crate::chain::cosmos::estimate::EstimatedGas;
+
 // Provides mapping for errors returned from ibc-go and cosmos-sdk
 define_error! {
     SdkError {
@@ -24,9 +26,13 @@ define_error! {
             { code: u32 }
             | e | { format_args!("unknown TX sync response error: {}", e.code) },
 
-        OutOfGas
-            { code: u32 }
-            |_| { "the gas requirement is higher than the configured maximum gas! please check the Hermes config.toml".to_string() },
+        OutOfGasDefault
+            { code: u32, amount: u64 }
+            |e| { format_args!("due to the Tx simulation failing, the configured default gas was used. Please check the Hermes config.toml and increase the configured `default_gas`. Current value is `{}`", e.amount) },
+
+        OutOfGasSimulated
+            { code: u32, amount: u64 }
+            |e| { format_args!("the issue might have been caused by the configured max gas which binds the gas used. Please check the Hermes config.toml and increase the configured `max_gas`. Curerent value is `{}`", e.amount) },
 
         InsufficientFee
             { code: u32 }
@@ -186,12 +192,15 @@ pub fn sdk_error_from_tx_result(code: Code, codespace: &str) -> SdkError {
 /// into IBC relayer domain-type errors.
 /// See [`tendermint_rpc::endpoint::broadcast::tx_sync::Response`].
 /// Cf: <https://github.com/cosmos/cosmos-sdk/blob/v0.42.10/types/errors/errors.go>
-pub fn sdk_error_from_tx_sync_error_code(code: u32) -> SdkError {
+pub fn sdk_error_from_tx_sync_error_code(code: u32, estimated_gas: EstimatedGas) -> SdkError {
     match code {
         // The primary reason (we know of) causing broadcast_tx_sync to fail
         // is due to "out of gas" errors. These are unrecoverable at the moment
         // on Hermes side. We'll inform the user to check for misconfiguration.
-        11 => SdkError::out_of_gas(code),
+        11 => match estimated_gas {
+            EstimatedGas::Default(amount) => SdkError::out_of_gas_default(code, amount),
+            EstimatedGas::Simulated(amount) => SdkError::out_of_gas_simulated(code, amount),
+        },
         13 => SdkError::insufficient_fee(code),
         _ => SdkError::unknown_tx_sync(code),
     }

diff --git a/guide/src/SUMMARY.md b/guide/src/SUMMARY.md
@@ -53,6 +53,7 @@
         - [Cross Stack Misconfiguration](./advanced/troubleshooting/cross-comp-config.md)
         - [Genesis restart without IBC upgrade proposal](./advanced/troubleshooting/genesis-restart.md)
         - [Handling Clock Drift](./advanced/troubleshooting/clock-drift.md)
+        - [Gas Errors](./advanced/troubleshooting/gas-errors.md)
 
 - [Commands Reference](./documentation/commands/index.md)
   - [Global options and JSON output](./documentation/commands/global.md)

diff --git a/guide/src/advanced/troubleshooting/gas-errors.md b/guide/src/advanced/troubleshooting/gas-errors.md
@@ -0,0 +1,48 @@
+# Gas errors
+
+This section will expand on the out of gas error which can happen when simulating or sending Txs. The related configurations are:
+
+```toml
+default_gas = 100000
+max_gas = 4000000
+gas_multiplier = 1.1
+```
+
+Before sending a transaction, Hermes will retrieve an estimation of the gas required with the simulation capability of the chain. After retrieving the gas amount from the simulation, the `gas_multiplier` will be applied since the simulation might be slightly lower than the required amount of gas.
+Since the `max_gas` is applied after the gas_multiplier, it can happen that the value `simulated_gas * gas_multiplier > max_gas`, in which case the `max_gas` value is used.
+
+Note that if the simulation fails with a recoverable error, Hermes will use the configured `default_gas`.
+
+## Simulating Tx
+
+The first instance where an error can happen is when the tracasction simulation succeeds but the gas amount retrieved exceeds the configured `max_gas`, Hermes will throw an unrecoverable error:
+
+```
+<chain> gas estimate <simulated_gas> from simulated Tx exceeds the maximum configured <max_gas>
+```
+
+This can be fixed by increasing the configured `max_gas`.
+
+## Broadcasting Tx
+
+> __NOTE__: This issue will only arise with Cosmos chains as this is a Cosmos SDK error.
+
+The second instance when an error can happen is when sending the transaction. If the gas included for the transaction is not enough Hermes will throw an error:
+
+```
+out of gas in location: <location>; gasWanted: <max gas Hermes>, gasUsed: <gas wanted>: out of gas
+```
+
+Two cases need to be verified in order to fix this issue.
+
+### Caused by max_gas
+
+If simulated gas is close to the `max_gas` configured, after multiplying the value with the `gas_multiplier`, it can be the case that the `max_gas` is used instead. And since the simulated gas might be slightly lower than the required gas, this can cause an out of gas error.
+This can be fixed by increasing the configured `max_gas`.
+
+### Caused by default_gas
+
+When the transaction simulation fails with a recoverable error, the `default_gas` will be used. If the `default_gas` is too low an out of gas error will be thrown. This can be fixed by increasing the `default_gas`.
+But there can also be a case similar to the one explained in the previous section [Caused by max_gas](./gas-errors.md#caused-by-max_gas).
+
+If the `default_gas` is too close to the `max_gas`, the `max_gas` will be used instead of `default_gas * gas_multiplier`, causing an out of gas error. In this situation both `max_gas` and `default_gas` need to be verified, and one or both need to be increased.