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

Default tx expiration #3123

Merged
merged 4 commits into from
Apr 27, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Set a default expiration for transactions when no value is provided.
([\#3123](https://github.com/anoma/namada/pull/3123))
38 changes: 32 additions & 6 deletions crates/apps/src/lib/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3195,6 +3195,7 @@ pub mod args {
pub const NET_ADDRESS: Arg<SocketAddr> = arg("net-address");
pub const NAMADA_START_TIME: ArgOpt<DateTimeUtc> = arg_opt("time");
pub const NO_CONVERSIONS: ArgFlag = flag("no-conversions");
pub const NO_EXPIRATION: ArgFlag = flag("no-expiration");
pub const NUT: ArgFlag = flag("nut");
pub const OUT_FILE_PATH_OPT: ArgOpt<PathBuf> = arg_opt("out-file-path");
pub const OUTPUT: ArgOpt<PathBuf> = arg_opt("output");
Expand Down Expand Up @@ -6294,12 +6295,28 @@ pub mod args {
.arg(WALLET_ALIAS_FORCE.def().help(
"Override the alias without confirmation if it already exists.",
))
.arg(EXPIRATION_OPT.def().help(
"The expiration datetime of the transaction, after which the \
tx won't be accepted anymore. All of these examples are \
equivalent:\n2012-12-12T12:12:12Z\n2012-12-12 \
12:12:12Z\n2012- 12-12T12: 12:12Z",
))
.arg(
EXPIRATION_OPT
.def()
.help(
"The expiration datetime of the transaction, after \
which the tx won't be accepted anymore. If not \
provided, a default will be set. All of these \
examples are \
equivalent:\n2012-12-12T12:12:12Z\n2012-12-12 \
12:12:12Z\n2012- 12-12T12: 12:12Z",
)
.conflicts_with_all([NO_EXPIRATION.name]),
)
.arg(
NO_EXPIRATION
.def()
.help(
"Force the construction of the transaction without an \
expiration (higly discouraged).",
grarco marked this conversation as resolved.
Show resolved Hide resolved
)
.conflicts_with_all([EXPIRATION_OPT.name]),
)
.arg(
DISPOSABLE_SIGNING_KEY
.def()
Expand Down Expand Up @@ -6382,6 +6399,15 @@ pub mod args {
let wrapper_fee_payer = FEE_PAYER_OPT.parse(matches);
let output_folder = OUTPUT_FOLDER_PATH.parse(matches);
let use_device = USE_DEVICE.parse(matches);
let no_expiration = NO_EXPIRATION.parse(matches);
let expiration = if no_expiration {
TxExpiration::NoExpiration
} else {
match expiration {
Some(exp) => TxExpiration::Custom(exp),
None => TxExpiration::Default,
}
};
Self {
dry_run,
dry_run_wrapper,
Expand Down
2 changes: 1 addition & 1 deletion crates/apps/src/lib/config/genesis/transactions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ fn get_tx_args(use_device: bool) -> TxArgs {
fee_token: genesis_fee_token_address(),
fee_unshield: None,
gas_limit: Default::default(),
expiration: None,
expiration: Default::default(),
disposable_signing_key: false,
chain_id: None,
signing_keys: vec![],
Expand Down
37 changes: 31 additions & 6 deletions crates/sdk/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1954,6 +1954,34 @@ pub struct QueryRawBytes<C: NamadaTypes = SdkTypes> {
pub query: Query<C>,
}

/// The possible values for the tx expiration
#[derive(Clone, Debug, Default)]
pub enum TxExpiration {
/// Force the tx to have no expiration
NoExpiration,
/// Request the default expiration
#[default]
Default,
/// User-provided custom expiration
Custom(DateTimeUtc),
}

impl TxExpiration {
/// Converts the expiration argument into an optional [`DateTimeUtc`]
pub fn to_datetime(&self) -> Option<DateTimeUtc> {
match self {
TxExpiration::NoExpiration => None,
// Default to 1 hour
TxExpiration::Default =>
{
#[allow(clippy::disallowed_methods)]
Some(DateTimeUtc::now() + namada_core::time::Duration::hours(1))
}
TxExpiration::Custom(exp) => Some(exp.to_owned()),
}
}
}

/// Common transaction arguments
#[derive(Clone, Debug)]
pub struct Tx<C: NamadaTypes = SdkTypes> {
Expand Down Expand Up @@ -1988,7 +2016,7 @@ pub struct Tx<C: NamadaTypes = SdkTypes> {
/// The max amount of gas used to process tx
pub gas_limit: GasLimit,
/// The optional expiration of the transaction
pub expiration: Option<DateTimeUtc>,
pub expiration: TxExpiration,
/// Generate an ephimeral signing key to be used only once to sign a
/// wrapper tx
pub disposable_signing_key: bool,
Expand Down Expand Up @@ -2106,11 +2134,8 @@ pub trait TxBuilder<C: NamadaTypes>: Sized {
self.tx(|x| Tx { gas_limit, ..x })
}
/// The optional expiration of the transaction
fn expiration(self, expiration: DateTimeUtc) -> Self {
self.tx(|x| Tx {
expiration: Some(expiration),
..x
})
fn expiration(self, expiration: TxExpiration) -> Self {
self.tx(|x| Tx { expiration, ..x })
}
/// Generate an ephimeral signing key to be used only once to sign a
/// wrapper tx
Expand Down
2 changes: 1 addition & 1 deletion crates/sdk/src/eth_bridge/bridge_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ pub async fn build_bridge_pool_tx(
.clone()
.ok_or_else(|| Error::Other("No chain id available".into()))?;

let mut tx = Tx::new(chain_id, tx_args.expiration);
let mut tx = Tx::new(chain_id, tx_args.expiration.to_datetime());
if let Some(memo) = &tx_args.memo {
tx.add_memo(memo);
}
Expand Down
4 changes: 2 additions & 2 deletions crates/sdk/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ pub trait Namada: Sized + MaybeSync + MaybeSend {
fee_token: self.native_token(),
fee_unshield: None,
gas_limit: GasLimit::from(20_000),
expiration: None,
expiration: Default::default(),
disposable_signing_key: false,
chain_id: None,
signing_keys: vec![],
Expand Down Expand Up @@ -664,7 +664,7 @@ where
fee_token: native_token,
fee_unshield: None,
gas_limit: GasLimit::from(20_000),
expiration: None,
expiration: Default::default(),
disposable_signing_key: false,
chain_id: None,
signing_keys: vec![],
Expand Down
11 changes: 10 additions & 1 deletion crates/sdk/src/masp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2007,7 +2007,16 @@ impl<U: ShieldedUtils + MaybeSend + MaybeSync> ShieldedContext<U> {
};

// Now we build up the transaction within this object
let expiration_height: u32 = match context.tx_builder().expiration {
// TODO: if the user requested the default expiration, there might be a
// small discrepancy between the datetime we calculate here and the one
// we set for the transaction. This should be small enough to not cause
// any issue, in case refactor this function to request the precise
// datetime to the caller
let expiration_height: u32 = match context
.tx_builder()
.expiration
.to_datetime()
{
Some(expiration) => {
// Try to match a DateTime expiration with a plausible
// corresponding block height
Expand Down
8 changes: 4 additions & 4 deletions crates/sdk/src/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2636,7 +2636,7 @@ pub async fn build_ibc_transfer(
};

let chain_id = args.tx.chain_id.clone().unwrap();
let mut tx = Tx::new(chain_id, args.tx.expiration);
let mut tx = Tx::new(chain_id, args.tx.expiration.to_datetime());
if let Some(memo) = &args.tx.memo {
tx.add_memo(memo);
}
Expand Down Expand Up @@ -2798,7 +2798,7 @@ where
{
let chain_id = tx_args.chain_id.clone().unwrap();

let mut tx_builder = Tx::new(chain_id, tx_args.expiration);
let mut tx_builder = Tx::new(chain_id, tx_args.expiration.to_datetime());
if let Some(memo) = &tx_args.memo {
tx_builder.add_memo(memo);
}
Expand Down Expand Up @@ -3182,7 +3182,7 @@ pub async fn build_update_account(
};

let chain_id = tx_args.chain_id.clone().unwrap();
let mut tx = Tx::new(chain_id, tx_args.expiration);
let mut tx = Tx::new(chain_id, tx_args.expiration.to_datetime());
if let Some(memo) = &tx_args.memo {
tx.add_memo(memo);
}
Expand Down Expand Up @@ -3264,7 +3264,7 @@ pub async fn build_custom(
.ok_or(Error::Other("No code path supplied".to_string()))?;
let tx_code_hash = query_wasm_code_hash_buf(context, code_path).await?;
let chain_id = tx_args.chain_id.clone().unwrap();
let mut tx = Tx::new(chain_id, tx_args.expiration);
let mut tx = Tx::new(chain_id, tx_args.expiration.to_datetime());
if let Some(memo) = &tx_args.memo {
tx.add_memo(memo);
}
Expand Down