Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Namada gas estimation #32

Merged
merged 5 commits into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions crates/relayer/src/chain/cosmos/gas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,16 +117,16 @@ pub fn mul_floor(a: u64, f: f64) -> BigInt {
(f * a).floor().to_integer()
}

struct AdjustGas {
gas_multiplier: f64,
max_gas: u64,
gas_amount: u64,
pub struct AdjustGas {
pub gas_multiplier: f64,
pub max_gas: u64,
pub gas_amount: u64,
}

/// Adjusts the fee based on the configured `gas_multiplier` to prevent out of gas errors.
/// The actual gas cost, when a transaction is executed, may be slightly higher than the
/// one returned by the simulation.
fn adjust_estimated_gas(
pub fn adjust_estimated_gas(
AdjustGas {
gas_multiplier,
max_gas,
Expand Down
4 changes: 4 additions & 0 deletions crates/relayer/src/chain/namada/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ define_error! {
BorshDecode
[ TraceError<std::io::Error> ]
|_| { "borsh decoding failed" },

DryRun
{ tx_result: namada_sdk::tx::data::TxResult<String> }
|e| { format!("Dry run to simulate a transaction failed: {}", e.tx_result) },
}
}

Expand Down
118 changes: 112 additions & 6 deletions crates/relayer/src/chain/namada/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::time::Instant;

use ibc_proto::google::protobuf::Any;
use itertools::Itertools;
use namada_ibc::{MsgAcknowledgement, MsgRecvPacket, MsgTimeout};
use namada_sdk::address::{Address, ImplicitAddress};
use namada_sdk::args::{self, TxBuilder};
use namada_sdk::args::{InputAmount, Tx as TxArgs, TxCustom};
Expand All @@ -19,15 +20,17 @@ use namada_sdk::ibc::core::channel::types::msgs::{
MsgTimeout as IbcMsgTimeout, ACKNOWLEDGEMENT_TYPE_URL, RECV_PACKET_TYPE_URL, TIMEOUT_TYPE_URL,
};
use namada_sdk::ibc::core::host::types::identifiers::{ChannelId, PortId};
use namada_ibc::{MsgAcknowledgement, MsgRecvPacket, MsgTimeout};
use namada_sdk::masp::{PaymentAddress, TransferTarget};
use namada_sdk::masp_primitives::transaction::Transaction as MaspTransaction;
use namada_sdk::tx::{prepare_tx, ProcessTxResponse};
use namada_sdk::{signing, tx, Namada};
use namada_token::ShieldingTransfer;
use tendermint_proto::Protobuf;
use tendermint_rpc::endpoint::broadcast::tx_sync::Response;
use tracing::{debug, debug_span, trace};

use crate::chain::cosmos::gas::{adjust_estimated_gas, AdjustGas};
use crate::chain::cosmos::types::gas::default_gas_from_config;
use crate::chain::cosmos::types::tx::{TxStatus, TxSyncResult};
use crate::chain::cosmos::wait::all_tx_results_found;
use crate::chain::endpoint::ChainEndpoint;
Expand All @@ -46,7 +49,9 @@ impl NamadaChain {

let tx_args = self.make_tx_args()?;

let relayer_addr = self.get_key()?.address;
let relayer_key = self.get_key()?;
let relayer_addr = relayer_key.address;

let rt = self.rt.clone();
rt.block_on(self.submit_reveal_aux(&tx_args, &relayer_addr))?;

Expand All @@ -57,7 +62,6 @@ impl NamadaChain {
serialized_tx: None,
owner: relayer_addr.clone(),
};

let mut txs = Vec::new();
for msg in msgs {
let (mut tx, signing_data) = rt
Expand All @@ -67,17 +71,44 @@ impl NamadaChain {
txs.push((tx, signing_data));
}
let (mut tx, signing_data) = tx::build_batch(txs).map_err(NamadaError::namada)?;
let signing_data = signing_data.first().expect("SigningData should exist");

// Estimate the fee with dry-run
let (fee_token, gas_limit, fee_amount) =
self.estimate_fee(tx.clone(), &tx_args, signing_data)?;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not all nodes should be required to enable dry running txs via their exposed RPC servers, since these may pose a DoS vector. estimate_fee should be expected to fail with a nonexistent RPC error, at which point we could fallback to another gas estimation mechanism (e.g. median values that have been benchmarked during a release).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. I'll fix it.

// Set the estimated fee
let tx_args = tx_args
.fee_token(fee_token)
.gas_limit(gas_limit.into())
.fee_amount(
fee_amount
.to_string()
.parse()
.expect("Fee should be parsable"),
);
let fee_amount = rt
.block_on(signing::validate_fee(&self.ctx, &tx_args))
.map_err(NamadaError::namada)?;
sug0 marked this conversation as resolved.
Show resolved Hide resolved
rt.block_on(prepare_tx(
&tx_args,
&mut tx,
fee_amount,
relayer_key.secret_key.to_public(),
))
.map_err(NamadaError::namada)?;

rt.block_on(self.ctx.sign(
&mut tx,
&args.tx,
signing_data.first().unwrap().clone(),
&tx_args,
signing_data.clone(),
signing::default_sign,
(),
))
.map_err(NamadaError::namada)?;

let tx_header_hash = tx.header_hash().to_string();
let response = rt
.block_on(self.ctx.submit(tx, &args.tx))
.block_on(self.ctx.submit(tx, &tx_args))
.map_err(NamadaError::namada)?;

match response {
Expand Down Expand Up @@ -284,6 +315,81 @@ impl NamadaChain {
}
}

fn estimate_fee(
&self,
mut tx: tx::Tx,
args: &TxArgs,
signing_data: &signing::SigningTxData,
) -> Result<(Address, u64, f64), Error> {
let chain_id = self.config().id.clone();

let args = args.clone().dry_run_wrapper(true);
self.rt
.block_on(self.ctx.sign(
&mut tx,
&args,
signing_data.clone(),
signing::default_sign,
(),
))
.map_err(NamadaError::namada)?;

let response = self
.rt
.block_on(self.ctx.submit(tx, &args))
.map_err(NamadaError::namada)?;
let estimated_gas = match response {
ProcessTxResponse::DryRun(result) => {
if result
.batch_results
.0
.iter()
.all(|(_, r)| matches!(&r, Ok(result) if result.is_accepted()))
{
// Convert with the decimal scale of Gas units
u64::from_str(&result.gas_used.to_string()).expect("Gas should be parsable")
} else {
return Err(Error::namada(NamadaError::dry_run(result)));
}
}
_ => unreachable!("Unexpected response"),
};
let max_gas = default_gas_from_config(self.config());
if estimated_gas > max_gas {
debug!(
id = %chain_id, estimated = ?estimated_gas, max_gas,
"send_tx: estimated gas is higher than max gas"
);

return Err(Error::tx_simulate_gas_estimate_exceeded(
chain_id,
estimated_gas,
max_gas,
));
}

let fee_token_str = self.config().gas_price.denom.clone();
let fee_token = Address::from_str(&fee_token_str)
.map_err(|_| NamadaError::address_decode(fee_token_str.clone()))?;
let gas_price = self.config().gas_price.price;
let gas_multiplier = self.config().gas_multiplier.unwrap_or_default().to_f64();

let adjusted_gas = adjust_estimated_gas(AdjustGas {
gas_multiplier,
max_gas,
gas_amount: estimated_gas,
});

debug!(
id = %chain_id,
"send_tx: using {} gas, gas_price {:?}",
estimated_gas,
gas_price,
);

Ok((fee_token, adjusted_gas, gas_price))
}

pub fn wait_for_block_commits(
&self,
tx_sync_results: &mut [TxSyncResult],
Expand Down
35 changes: 28 additions & 7 deletions e2e/namada-simple-transfers
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
# `make build` and `make build-wasm-scripts` on Namada directory in advance
# Run with `namada-simple-transfers ${namada_dir}`

set -ex
set -e

NAMADA_DIR=$1
if [ -z "${NAMADA_DIR}" ]
Expand Down Expand Up @@ -45,10 +45,34 @@ function init_relayer_balance() {
--node ${ledger_addr}
}

function wait_for_relaying() {
local chain_id=$1

for i in {1..20}
do
result=$(cargo run --bin hermes -- --config config_for_namada.toml \
query packet pending \
--chain ${chain_id} \
--channel channel-0 \
--port transfer)

echo ${result}
if [[ "${result}" =~ "=" ]];
then
echo "Waiting for packet relaying..."
sleep 5
else
echo "All packets have been relayed!"
break
fi
done
}

# ==== main ====

# Run 2 Namada chains
${HERMES_DIR}/scripts/setup-namada ${NAMADA_DIR}
sleep 5

cd ${HERMES_DIR}
ids=$(grep "id" config_for_namada.toml | awk -F"'" '/^id/ {print $2}')
Expand Down Expand Up @@ -238,8 +262,7 @@ ${NAMADAC} --base-dir ${base_dir_b} ibc-transfer \
--channel-id channel-0 \
--node ${LEDGER_ADDR_B}

# wait for relaying
sleep 15
wait_for_relaying ${chain_a}

echo "==== Balances on chain A ===="
balance=$(${NAMADAC} --base-dir ${base_dir_a} balance \
Expand Down Expand Up @@ -295,8 +318,7 @@ ${NAMADAC} --base-dir ${base_dir_a} ibc-transfer \
--gas-payer relayer \
--node ${LEDGER_ADDR_A}

# wait for relaying
sleep 40
wait_for_relaying ${chain_a}

echo "==== Balance of shielded_a on chain A ===="
${NAMADAC} --base-dir ${base_dir_a} shielded-sync \
Expand Down Expand Up @@ -373,8 +395,7 @@ ${NAMADAC} --base-dir ${base_dir_b} ibc-transfer \
--channel-id channel-0 \
--node ${LEDGER_ADDR_B}

# wait for relaying
sleep 40
wait_for_relaying ${chain_a}

echo "==== Balances of shielded_a on chain A ===="
${NAMADAC} --base-dir ${base_dir_a} shielded-sync \
Expand Down
6 changes: 3 additions & 3 deletions scripts/setup-namada
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ LEDGER_ADDR_B="http://127.0.0.1:28657"

HERMES_CONFIG_TEMPLATE="
[global]
log_level = 'info'
log_level = 'debug'

[mode]

Expand Down Expand Up @@ -67,7 +67,7 @@ event_source = { mode = 'push', url = 'ws://127.0.0.1:27657/websocket', batch_de
account_prefix = ''
key_name = 'relayer'
store_prefix = 'ibc'
gas_price = { price = 0.001, denom = '_FEE_TOKEN_A_' }
gas_price = { price = 0.000001, denom = '_FEE_TOKEN_A_' }

[[chains]]
id = '_CHAIN_ID_B_'
Expand All @@ -78,7 +78,7 @@ event_source = { mode = 'push', url = 'ws://127.0.0.1:28657/websocket', batch_de
account_prefix = ''
key_name = 'relayer'
store_prefix = 'ibc'
gas_price = { price = 0.001, denom = '_FEE_TOKEN_B_' }
gas_price = { price = 0.000001, denom = '_FEE_TOKEN_B_' }
"

function make_genesis() {
Expand Down
4 changes: 2 additions & 2 deletions scripts/setup-namada-single-node
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ LEDGER_ADDR="http://127.0.0.1:27657"

HERMES_CONFIG_TEMPLATE="
[global]
log_level = 'info'
log_level = 'debug'

[mode]

Expand Down Expand Up @@ -62,7 +62,7 @@ event_source = { mode = 'push', url = 'ws://127.0.0.1:27657/websocket', batch_de
account_prefix = ''
key_name = 'relayer'
store_prefix = 'ibc'
gas_price = { price = 0.001, denom = '_FEE_TOKEN_' }
gas_price = { price = 0.000001, denom = '_FEE_TOKEN_' }
"

function make_genesis() {
Expand Down
Loading