Refactor tx construction and add insufficient fee fallback (#146)

* refactor tx construction and fee fallback * log broadcast resp * bump max gas * go back to broadcasting sync * wait a bit before re-submittintg * formatting * add notional endpoint * formatting * restructure max gas * formatting * remove https from notional url * fix sender * remove notional port on url * fix terra url * set env log to debug on tests * update & test the min-tx-fee fallback impl * clippy
AbstractSDK · Jun 21, 2023 · ec584eb · ec584eb
1 parent 6cbf394
commit ec584eb
Show file tree

Hide file tree

Showing 8 changed files with 277 additions and 76 deletions.
diff --git a/cw-orch/src/daemon/error.rs b/cw-orch/src/daemon/error.rs
@@ -102,6 +102,8 @@ pub enum DaemonError {
     BuilderMissing(String),
     #[error("ibc error: {0}")]
     IbcError(String),
+    #[error("insufficient fee, check gas price: {0}")]
+    InsufficientFee(String),
 }
 
 impl DaemonError {

diff --git a/cw-orch/src/daemon/mod.rs b/cw-orch/src/daemon/mod.rs
@@ -18,12 +18,14 @@ mod tx_resp;
 // expose these as mods as they can grow
 pub mod networks;
 pub mod queriers;
+pub(crate) mod tx_builder;
 
 pub use self::{
     builder::*, chain_info::*, channel::*, core::*, error::*, state::*, sync::*, traits::*,
     tx_resp::*,
 };
 pub use sender::Wallet;
+pub use tx_builder::TxBuilder;
 
 pub(crate) mod cosmos_modules {
     pub use cosmrs::proto::{

diff --git a/cw-orch/src/daemon/networks/juno.rs b/cw-orch/src/daemon/networks/juno.rs
@@ -1,5 +1,7 @@
 use crate::daemon::chain_info::{ChainInfo, ChainKind, NetworkInfo};
 
+// https://notional.ventures/resources/endpoints#juno
+
 pub const JUNO_NETWORK: NetworkInfo = NetworkInfo {
     id: "juno",
     pub_address_prefix: "juno",
@@ -37,7 +39,10 @@ pub const JUNO_1: ChainInfo = ChainInfo {
     chain_id: "juno-1",
     gas_denom: "ujuno",
     gas_price: 0.0750,
-    grpc_urls: &["http://juno-grpc.polkachu.com:12690"],
+    grpc_urls: &[
+        "https://grpc-juno-ia.cosmosia.notional.ventures",
+        "http://juno-grpc.polkachu.com:12690",
+    ],
     network_info: JUNO_NETWORK,
     lcd_url: None,
     fcd_url: None,

diff --git a/cw-orch/src/daemon/networks/terra.rs b/cw-orch/src/daemon/networks/terra.rs
@@ -22,7 +22,7 @@ pub const PHOENIX_1: ChainInfo = ChainInfo {
     chain_id: "phoenix-1",
     gas_denom: "uluna",
     gas_price: 0.15,
-    grpc_urls: &["https://terra-grpc.polkachu.com:11790"],
+    grpc_urls: &["http://terra-grpc.polkachu.com:11790"],
     network_info: TERRA_NETWORK,
     lcd_url: None,
     fcd_url: None,

diff --git a/cw-orch/src/daemon/sender.rs b/cw-orch/src/daemon/sender.rs
@@ -4,6 +4,7 @@ use super::{
     error::DaemonError,
     queriers::{DaemonQuerier, Node},
     state::DaemonState,
+    tx_builder::TxBuilder,
     tx_resp::CosmTxResponse,
 };
 use crate::{daemon::core::parse_cw_coins, keys::private::PrivateKey};
@@ -12,18 +13,15 @@ use cosmrs::{
     crypto::secp256k1::SigningKey,
     proto::traits::Message,
     tendermint::chain::Id,
-    tx::{self, Fee, Msg, Raw, SignDoc, SignerInfo},
-    AccountId, Any, Coin,
+    tx::{self, Msg, Raw, SignDoc, SignerInfo},
+    AccountId,
 };
 use cosmwasm_std::Addr;
 use secp256k1::{All, Context, Secp256k1, Signing};
 use std::{convert::TryFrom, env, rc::Rc, str::FromStr};
 
 use tonic::transport::Channel;
 
-const GAS_LIMIT: u64 = 1_000_000;
-const GAS_BUFFER: f64 = 1.2;
-
 /// A wallet is a sender of transactions, can be safely cloned and shared within the same thread.
 pub type Wallet = Rc<Sender<All>>;
 
@@ -32,7 +30,7 @@ pub type Wallet = Rc<Sender<All>>;
 pub struct Sender<C: Signing + Context> {
     pub private_key: SigningKey,
     pub secp: Secp256k1<C>,
-    daemon_state: Rc<DaemonState>,
+    pub(crate) daemon_state: Rc<DaemonState>,
 }
 
 impl Sender<All> {
@@ -114,38 +112,17 @@ impl Sender<All> {
         self.commit_tx(vec![msg_send], Some("sending tokens")).await
     }
 
-    pub(crate) fn build_tx_body<T: Msg>(
-        &self,
-        msgs: Vec<T>,
-        memo: Option<&str>,
-        timeout: u64,
-    ) -> tx::Body {
-        let msgs = msgs
-            .into_iter()
-            .map(Msg::into_any)
-            .collect::<Result<Vec<Any>, _>>()
-            .unwrap();
-
-        tx::Body::new(msgs, memo.unwrap_or_default(), timeout as u32)
-    }
-
-    pub(crate) fn build_fee(&self, amount: impl Into<u128>, gas_limit: Option<u64>) -> Fee {
-        let fee = Coin::new(
-            amount.into(),
-            &self.daemon_state.chain_data.fees.fee_tokens[0].denom,
-        )
-        .unwrap();
-        let gas = gas_limit.unwrap_or(GAS_LIMIT);
-        Fee::from_amount_and_gas(fee, gas)
-    }
-
     pub async fn calculate_gas(
         &self,
         tx_body: &tx::Body,
         sequence: u64,
         account_number: u64,
     ) -> Result<u64, DaemonError> {
-        let fee = self.build_fee(0u8, None);
+        let fee = TxBuilder::build_fee(
+            0u8,
+            &self.daemon_state.chain_data.fees.fee_tokens[0].denom,
+            0,
+        );
 
         let auth_info =
             SignerInfo::single_direct(Some(self.private_key.public_key()), sequence).auth_info(fee);
@@ -171,41 +148,49 @@ impl Sender<All> {
     ) -> Result<CosmTxResponse, DaemonError> {
         let timeout_height = Node::new(self.channel()).block_height().await? + 10u64;
 
-        let BaseAccount {
-            account_number,
-            sequence,
-            ..
-        } = self.base_account().await?;
+        let tx_body = TxBuilder::build_body(msgs, memo, timeout_height);
 
-        let tx_body = self.build_tx_body(msgs, memo, timeout_height);
+        let mut tx_builder = TxBuilder::new(tx_body);
 
-        let sim_gas_used = self
-            .calculate_gas(&tx_body, sequence, account_number)
-            .await?;
+        // now commit and check the result, if we get an insufficient fee error, we can try again with the proposed fee
 
-        log::debug!("Simulated gas needed {:?}", sim_gas_used);
+        let tx = tx_builder.build(self).await?;
 
-        let gas_expected = sim_gas_used as f64 * GAS_BUFFER;
-        let amount_to_pay = gas_expected
-            * (self.daemon_state.chain_data.fees.fee_tokens[0].fixed_min_gas_price + 0.00001);
+        let mut tx_response = self.broadcast_tx(tx).await?;
 
-        log::debug!("Calculated gas needed: {:?}", amount_to_pay);
+        log::debug!("tx broadcast response: {:?}", tx_response);
 
-        let fee = self.build_fee(amount_to_pay as u128, Some(gas_expected as u64));
+        if has_insufficient_fee(&tx_response.raw_log) {
+            // get the suggested fee from the error message
+            let suggested_fee = parse_suggested_fee(&tx_response.raw_log);
 
-        let auth_info =
-            SignerInfo::single_direct(Some(self.private_key.public_key()), sequence).auth_info(fee);
+            let Some(new_fee) = suggested_fee else {
+                return Err(DaemonError::InsufficientFee(
+                    tx_response.raw_log,
+                ));
+            };
 
-        let sign_doc = SignDoc::new(
-            &tx_body,
-            &auth_info,
-            &Id::try_from(self.daemon_state.chain_data.chain_id.to_string())?,
-            account_number,
-        )?;
+            // update the fee and try again
+            tx_builder.fee_amount(new_fee);
+            let tx = tx_builder.build(self).await?;
 
-        let tx_raw = sign_doc.sign(&self.private_key)?;
+            tx_response = self.broadcast_tx(tx).await?;
+        }
+
+        let resp = Node::new(self.channel())
+            .find_tx(tx_response.txhash)
+            .await?;
 
-        self.broadcast(tx_raw).await
+        // if tx result != 0 then the tx failed, so we return an error
+        // if tx result == 0 then the tx succeeded, so we return the tx response
+        if resp.code == 0 {
+            Ok(resp)
+        } else {
+            Err(DaemonError::TxFailed {
+                code: resp.code,
+                reason: resp.raw_log,
+            })
+        }
     }
 
     // TODO: this does not work for injective because it's eth account
@@ -239,30 +224,73 @@ impl Sender<All> {
         Ok(acc)
     }
 
-    async fn broadcast(&self, tx: Raw) -> Result<CosmTxResponse, DaemonError> {
+    async fn broadcast_tx(
+        &self,
+        tx: Raw,
+    ) -> Result<cosmrs::proto::cosmos::base::abci::v1beta1::TxResponse, DaemonError> {
         let mut client = cosmos_modules::tx::service_client::ServiceClient::new(self.channel());
         let commit = client
             .broadcast_tx(cosmos_modules::tx::BroadcastTxRequest {
                 tx_bytes: tx.to_bytes()?,
-                mode: cosmos_modules::tx::BroadcastMode::Block.into(),
+                mode: cosmos_modules::tx::BroadcastMode::Sync.into(),
             })
             .await?;
 
-        log::debug!("{:?}", commit);
+        let commit = commit.into_inner().tx_response.unwrap();
+        Ok(commit)
+    }
+}
 
-        let resp = Node::new(self.channel())
-            .find_tx(commit.into_inner().tx_response.unwrap().txhash)
-            .await?;
+fn has_insufficient_fee(raw_log: &str) -> bool {
+    raw_log.contains("insufficient fees")
+}
 
-        // if tx result != 0 then the tx failed, so we return an error
-        // if tx result == 0 then the tx succeeded, so we return the tx response
-        if resp.code == 0 {
-            Ok(resp)
-        } else {
-            Err(DaemonError::TxFailed {
-                code: resp.code,
-                reason: resp.raw_log,
-            })
-        }
+// from logs: "insufficient fees; got: 14867ujuno
+// required: 17771ibc/C4CFF46FD6DE35CA4CF4CE031E643C8FDC9BA4B99AE598E9B0ED98FE3A2319F9,444255ujuno: insufficient fee"
+fn parse_suggested_fee(raw_log: &str) -> Option<u128> {
+    // Step 1: Split the log message into "got" and "required" parts.
+    let parts: Vec<&str> = raw_log.split("required: ").collect();
+
+    // Make sure the log message is in the expected format.
+    if parts.len() != 2 {
+        return None;
+    }
+
+    // Step 2: Split the "got" part to extract the paid fee and denomination.
+    let got_parts: Vec<&str> = parts[0].split_whitespace().collect();
+
+    // Extract the paid fee and denomination.
+    let paid_fee_with_denom = got_parts.last()?;
+    let (_, denomination) =
+        paid_fee_with_denom.split_at(paid_fee_with_denom.find(|c: char| !c.is_numeric())?);
+
+    eprintln!("denom: {}", denomination);
+
+    // Step 3: Iterate over each fee in the "required" part.
+    let required_fees: Vec<&str> = parts[1].split(denomination).collect();
+
+    eprintln!("required fees: {:?}", required_fees);
+
+    // read until the first non-numeric character backwards on the first string
+    let (_, suggested_fee) =
+        required_fees[0].split_at(required_fees[0].rfind(|c: char| !c.is_numeric())?);
+    eprintln!("suggested fee: {}", suggested_fee);
+
+    // remove the first character if parsing errors, which can be a comma
+    suggested_fee
+        .parse::<u128>()
+        .ok()
+        .or(suggested_fee[1..].parse::<u128>().ok())
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_parse_suggested_fee() {
+        let log = "insufficient fees; got: 14867ujuno required: 17771ibc/C4CFF46FD6DE35CA4CF4CE031E643C8FDC9BA4B99AE598E9B0ED98FE3A2319F9,444255ujuno: insufficient fee";
+        let fee = parse_suggested_fee(log).unwrap();
+        assert_eq!(fee, 444255);
     }
 }