diff --git a/controller/src/command.rs b/controller/src/command.rs index 85fa7f3..baa5828 100644 --- a/controller/src/command.rs +++ b/controller/src/command.rs @@ -1692,12 +1692,12 @@ pub struct ContractNewArgs { pub minimum_confirmations: u64, /// Blocks until wallets should stop signing the contract pub ttl_blocks: Option, + /// Fee rate in nanogrin per unit of transaction weight + pub fee_rate: Option, /// Select and lock outputs early pub add_outputs: bool, // Future features - /// Custom fee contribution - pub fee_rate: Option, /// Save slatepack to a specific filename pub outfile: Option, } @@ -1724,6 +1724,7 @@ impl ContractNewArgs { Ok(ContractNewArgsAPI { ttl_blocks: self.ttl_blocks, setup_args: ContractSetupArgsAPI { + fee_rate: self.fee_rate, src_acct_name: match self.src_acct_name.as_ref() { Some(v) => Some(v.to_string()), None => None, @@ -1796,12 +1797,12 @@ pub struct ContractSetupArgs { pub make_outputs: Option>, /// Optional minimum number of confirmations required for an input pub minimum_confirmations: Option, + /// Fee rate in nanogrin per unit of transaction weight + pub fee_rate: Option, // Future features /// Whether we should automatically sign a receive of any value // pub auto_receive: Option, - /// Custom fee contribution - pub fee_rate: Option, /// Save slatepack to a specific filename pub outfile: Option, /// Add outputs @@ -1830,6 +1831,7 @@ impl ContractSetupArgs { fn to_api_args(&self) -> Result { let net_change = self.get_net_change()?; Ok(ContractSetupArgsAPI { + fee_rate: self.fee_rate, net_change: net_change, add_outputs: self.add_outputs, selection_args: OutputSelectionArgs { @@ -2061,4 +2063,21 @@ mod contract_tests { if message.contains("Unsupported Slatepack mode: 2") )); } + + #[test] + fn contract_sign_fee_rate() { + let args = ContractSetupArgs { + counterparty_addr: None, + receive: None, + send: None, + as_json: false, + use_inputs: None, + make_outputs: None, + minimum_confirmations: None, + fee_rate: Some(2), + outfile: None, + add_outputs: false, + }; + assert_eq!(args.to_api_args().unwrap().fee_rate, Some(2)); + } } diff --git a/controller/tests/contract_self_spend.rs b/controller/tests/contract_self_spend.rs index 74e18d4..b252073 100644 --- a/controller/tests/contract_self_spend.rs +++ b/controller/tests/contract_self_spend.rs @@ -124,6 +124,52 @@ fn contract_self_spend_tx_impl(test_dir: &'static str) -> Result<(), libwallet:: Ok(()) } +fn contract_fee_floor_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { + let (wallets, _, stopper, _) = create_wallets(vec![vec![("default", 4)]], test_dir).unwrap(); + let wallet = wallets[0].0.clone(); + let mask = wallets[0].1.as_ref(); + + wallet::controller::owner_single_use(wallet, mask, PathBuf::from(test_dir), |api, m| { + let slate = api.contract_new( + m, + &ContractNewArgsAPI { + setup_args: ContractSetupArgsAPI { + selection_args: common::contract_selection_args(), + net_change: Some(0), + num_participants: 1, + fee_rate: Some(1), + ..Default::default() + }, + ..Default::default() + }, + )?; + let (_, txs) = api.retrieve_txs(m, false, None, Some(slate.id), None)?; + let (_, outputs) = api.retrieve_outputs(m, false, false, None)?; + let err = api + .contract_sign(m, &slate, &ContractSetupArgsAPI::default()) + .unwrap_err(); + assert!(matches!( + err, + libwallet::Error::Fee(ref message) if message.starts_with("Fee Dispute Error:") + )); + let (_, current_txs) = api.retrieve_txs(m, false, None, Some(slate.id), None)?; + let (_, current_outputs) = api.retrieve_outputs(m, false, false, None)?; + assert_eq!( + serde_json::to_value(current_txs).unwrap(), + serde_json::to_value(txs).unwrap() + ); + assert_eq!( + serde_json::to_value(current_outputs).unwrap(), + serde_json::to_value(outputs).unwrap() + ); + Ok(()) + })?; + + stopper.store(false, Ordering::Relaxed); + thread::sleep(Duration::from_millis(200)); + Ok(()) +} + #[test] fn wallet_contract_self_spend_tx() -> Result<(), libwallet::Error> { let test_dir = "test_output/contract_self_spend_tx"; @@ -132,3 +178,12 @@ fn wallet_contract_self_spend_tx() -> Result<(), libwallet::Error> { clean_output_dir(test_dir); Ok(()) } + +#[test] +fn contract_fee_floor() -> Result<(), libwallet::Error> { + let test_dir = "test_output/contract_fee_floor"; + setup(test_dir); + contract_fee_floor_impl(test_dir)?; + clean_output_dir(test_dir); + Ok(()) +} diff --git a/controller/tests/contract_srs.rs b/controller/tests/contract_srs.rs index 171fa01..deafa93 100644 --- a/controller/tests/contract_srs.rs +++ b/controller/tests/contract_srs.rs @@ -70,6 +70,9 @@ fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> let recv_wallet = wallets[1].0.clone(); let recv_mask = wallets[1].1.as_ref(); let participant_fee = my_fee_contribution(1, 1, 1, 2)?.fee(); + let receiver_rate = 2 * u32::try_from(grin_core::global::get_accept_fee_base()).unwrap(); + let receiver_fee = Transaction::weight_by_iok(1, 1, 0) * u64::from(receiver_rate) + + (Transaction::weight_by_iok(0, 0, 1) * u64::from(receiver_rate)).div_ceil(2); let ttl_cutoff = bh + 20; let ttl_error = format!( "Contract TTL changed from {} to {}", @@ -111,6 +114,15 @@ fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> if msg == "Contract TTL must be at least 1 block" )); args.ttl_blocks = Some(20); + args.setup_args.fee_rate = Some(0); + let err = api.contract_new(m, &mut args).unwrap_err(); + assert!(matches!( + err, + libwallet::Error::GenericError(ref msg) + if msg == "Contract fee rate must be at least 1" + )); + args.setup_args.fee_rate = None; + assert_eq!(common::wallet_progress(api, m)?, progress); slate = api.contract_new(m, &mut args)?; let (_, txs) = api.retrieve_txs(m, false, None, Some(slate.id), None)?; assert_eq!(txs[0].ttl_cutoff_height, Some(ttl_cutoff)); @@ -253,6 +265,7 @@ fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> let args = ContractSetupArgsAPI { selection_args: common::contract_selection_args(), net_change: Some(5_000_000_000), + fee_rate: Some(receiver_rate), ..Default::default() }; let incoming = slate.clone(); @@ -325,7 +338,7 @@ fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> &slate, common::ExpectedContractSlate { amount: 5_000_000_000, - fee: 2 * participant_fee, + fee: participant_fee + receiver_fee, inputs: 1, coinbase_inputs: 1, outputs: 1, @@ -342,10 +355,10 @@ fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> |api, m| { let view = api.contract_view(m, &slate)?; assert_eq!(view.own_commitment_status, OwnCommitmentStatus::Clean); - assert_eq!(view.own_fee, Some(participant_fee)); + assert_eq!(view.own_fee, Some(receiver_fee)); assert_eq!( view.balance_change, - Some(5_000_000_000 - participant_fee as i64) + Some(5_000_000_000 - receiver_fee as i64) ); Ok(()) }, @@ -545,7 +558,7 @@ fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> &slate, common::ExpectedContractSlate { amount: 5_000_000_000, - fee: 2 * participant_fee, + fee: participant_fee + receiver_fee, inputs: 2, coinbase_inputs: 2, outputs: 2, @@ -623,11 +636,10 @@ fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> assert_eq!(tx_log.amount_debited, 0); assert_eq!(tx_log.num_inputs, 1); assert_eq!(tx_log.num_outputs, 1); - let expected_fees_paid = Some(my_fee_contribution(1, 1, 1, 2)?); - assert_eq!(tx_log.fee, expected_fees_paid); + assert_eq!(tx_log.fee.map(|fee| fee.fee()), Some(receiver_fee)); assert_eq!( wallet_info.amount_currently_spendable, - 4 * 60_000_000_000 + 5_000_000_000 - expected_fees_paid.unwrap().fee() // we expect the balance of 4 mined blocks + 5 Grin - fees paid + 4 * 60_000_000_000 + 5_000_000_000 - receiver_fee // we expect the balance of 4 mined blocks + 5 Grin - fees paid ); Ok(()) }, diff --git a/doc/contracts.md b/doc/contracts.md index 2b85ecc..8f3a864 100644 --- a/doc/contracts.md +++ b/doc/contracts.md @@ -41,6 +41,11 @@ so a late-locked contract can fail if another transaction spends the available o `contract new --ttl_blocks N` stops contract signing after `N` blocks. Finalized transactions can still be posted afterward. +`--fee_rate N` uses `N` nanogrin per weight unit for this wallet's fee contribution. Each +wallet chooses its rate when it first joins. Without it, the wallet uses `accept_fee_base` +(500000 by default). The wallet only finalizes when the combined fee meets that minimum. If it +does not, revoke the contract and create a new one. + Each side's account is fixed when it first joins the contract. Changing the active account later does not affect setup or signing; they use the account stored in the context. @@ -72,7 +77,6 @@ account selected with `--account`. the API. Only invoice proofs are supported and they require the experimental Slate V5 format. The full proposal is described in [Early Payment Proofs](https://github.com/mimblewimble/grin-rfcs/pull/70) -* Custom fee rates are not supported * There are no contract-specific history, lookup or transport commands. Payment proof memos cannot be set through the contract API or CLI. The current proof stores a memo type and 32 bytes instead of the text and hash described by the early payment proofs diff --git a/libwallet/src/contract/actions/README.md b/libwallet/src/contract/actions/README.md index 7c33acf..4497392 100644 --- a/libwallet/src/contract/actions/README.md +++ b/libwallet/src/contract/actions/README.md @@ -41,7 +41,6 @@ Separating side effects until the 'save_step' part would make these functions mu clearer to callers. - For a payjoin, `--use-inputs any` picks an input for us; naming the commitment would be clearer, for the API as much as the CLI. - - Add custom contract fee rates. - Support `--outfile` for contract slatepacks. - Raise the replacement fee when `revoke` races a transaction already in the mempool. - Merge invoice-proof retrieval with the existing payment-proof API. diff --git a/libwallet/src/contract/actions/new.rs b/libwallet/src/contract/actions/new.rs index 194cd93..37cfbfb 100644 --- a/libwallet/src/contract/actions/new.rs +++ b/libwallet/src/contract/actions/new.rs @@ -40,6 +40,7 @@ where K: Keychain, { contract::utils::verify_num_participants(setup_args.num_participants)?; + contract::utils::verify_fee_rate(setup_args.fee_rate)?; // Compute state for 'new' let (slate, context) = compute(w, keychain_mask, setup_args, ttl_blocks, slate_id)?; diff --git a/libwallet/src/contract/actions/revoke.rs b/libwallet/src/contract/actions/revoke.rs index 3c59c8d..78e2543 100644 --- a/libwallet/src/contract/actions/revoke.rs +++ b/libwallet/src/contract/actions/revoke.rs @@ -129,6 +129,7 @@ where src_acct_name: src_acct_name.clone(), net_change: Some(0), // self-spend num_participants: 1, + fee_rate: None, add_outputs: false, selection_args: OutputSelectionArgs { // Keep revoke at one confirmation @@ -149,6 +150,7 @@ where src_acct_name, net_change: None, // we already have it in the context as 0 now num_participants: 1, + fee_rate: None, add_outputs: false, selection_args: OutputSelectionArgs { minimum_confirmations: Some(1), diff --git a/libwallet/src/contract/actions/setup.rs b/libwallet/src/contract/actions/setup.rs index 085bae8..421bf03 100644 --- a/libwallet/src/contract/actions/setup.rs +++ b/libwallet/src/contract/actions/setup.rs @@ -48,6 +48,7 @@ where K: Keychain, { contract::utils::verify_num_participants(slate.num_participants)?; + contract::utils::verify_fee_rate(setup_args.fee_rate)?; if slate.num_participants != setup_args.num_participants { return Err(Error::GenericError(format!( "Inconsistent num_participants. Slate num_participants:{}, Setup num_participants: {}", diff --git a/libwallet/src/contract/actions/sign.rs b/libwallet/src/contract/actions/sign.rs index 2620d0b..c2bbc38 100644 --- a/libwallet/src/contract/actions/sign.rs +++ b/libwallet/src/contract/actions/sign.rs @@ -59,6 +59,7 @@ where K: Keychain, { contract::utils::verify_num_participants(slate.num_participants)?; + contract::utils::verify_fee_rate(setup_args.fee_rate)?; // Compute if we will add outputs at this step. Only a missing context means we // still need to add them; propagate any other error. let existing_context = match w.get_private_context(keychain_mask, slate.id.as_bytes()) { diff --git a/libwallet/src/contract/context.rs b/libwallet/src/contract/context.rs index c8e6a28..b93ed8a 100644 --- a/libwallet/src/contract/context.rs +++ b/libwallet/src/contract/context.rs @@ -80,6 +80,7 @@ where { debug!("Creating a new contract context"); contract_utils::verify_num_participants(setup_args.num_participants)?; + contract_utils::verify_fee_rate(setup_args.fee_rate)?; // sender should always refresh outputs updater::refresh_outputs(w, keychain_mask, parent_key_id, false)?; diff --git a/libwallet/src/contract/selection.rs b/libwallet/src/contract/selection.rs index ec9dbee..688fc78 100644 --- a/libwallet/src/contract/selection.rs +++ b/libwallet/src/contract/selection.rs @@ -16,7 +16,7 @@ use crate::backend::WalletBackend; use crate::contract::types::{ContractSetupArgsAPI, OutputSelectionArgs}; -use crate::contract::utils::my_fee_contribution; +use crate::contract::utils::fee_contribution; use crate::grin_core::core::amount_to_hr_string; use crate::grin_keychain::{Identifier, Keychain}; use crate::types::NodeClient; @@ -130,11 +130,12 @@ where if !is_self_spend && (pay_amount == 0 && !is_payjoin) { return Ok(( vec![], - my_fee_contribution( + fee_contribution( 0, setup_args.selection_args.num_custom_outputs() + 1, 1, setup_args.num_participants, + setup_args.fee_rate, )?, )); } @@ -179,7 +180,7 @@ where committed_fee.unwrap() } else { // We start with a fee of 1 output and a shared kernel which is minimum for both parties - my_fee_contribution(0, 1, 1, setup_args.num_participants)? + fee_contribution(0, 1, 1, setup_args.num_participants, setup_args.fee_rate)? // FeeFields::zero() }; @@ -191,11 +192,21 @@ where let must_take = out.commit.is_some() && must_use_list.contains(&&out.commit.as_ref().unwrap()[..]); // Compute the fee without this input - let fee_without = - my_fee_contribution(n_inputs, my_num_outputs, 1, setup_args.num_participants)?; + let fee_without = fee_contribution( + n_inputs, + my_num_outputs, + 1, + setup_args.num_participants, + setup_args.fee_rate, + )?; // Compute the total fee cost if we took this input - let mut fee_with = - my_fee_contribution(n_inputs + 1, my_num_outputs, 1, setup_args.num_participants)?; + let mut fee_with = fee_contribution( + n_inputs + 1, + my_num_outputs, + 1, + setup_args.num_participants, + setup_args.fee_rate, + )?; // If the current fee is lower than the committed fee (my_fee) then set it to committed fee if my_fee.fee() > fee_with.fee() { fee_with = my_fee; @@ -370,6 +381,7 @@ pub fn verify_selection_consistency( mod tests { use super::*; + use crate::contract::utils::my_fee_contribution; use crate::grin_keychain::{Identifier, IDENTIFIER_SIZE}; use crate::OutputStatus; @@ -537,6 +549,20 @@ mod tests { ); } + #[test] + fn custom_fee_rate() { + let setup_args = ContractSetupArgsAPI { + net_change: Some(-3_000_000_000), + fee_rate: Some(2), + ..Default::default() + }; + let mut inputs = _create_output_data_for(vec![4_000_000_000]); + let (_, outputs, fee) = compute(&setup_args, None, &mut inputs).unwrap(); + let expected = fee_contribution(1, 1, 1, 2, Some(2)).unwrap(); + assert_eq!(fee, expected); + assert_eq!(outputs, vec![1_000_000_000 - expected.fee()]); + } + #[test] fn sender_exact() { // net_change=-3, inputs=[3, my_fees(2, 1)], no fee committed => Ok([3, my_fees(2, 1)], fees) diff --git a/libwallet/src/contract/types.rs b/libwallet/src/contract/types.rs index 74a3777..ad7559b 100644 --- a/libwallet/src/contract/types.rs +++ b/libwallet/src/contract/types.rs @@ -127,6 +127,10 @@ pub struct ContractSetupArgsAPI { pub net_change: Option, /// The number of participants in a contract. Used for computing our kernel fee contribution pub num_participants: u8, + /// Fee rate in nanogrin per unit of transaction weight + /// `None` uses the wallet default. The rate is fixed when the local context is created + /// and may be omitted in later steps + pub fee_rate: Option, /// Should we perform an early lock of outputs pub add_outputs: bool, /// Output selection arguments @@ -141,6 +145,7 @@ impl Default for ContractSetupArgsAPI { src_acct_name: None, net_change: None, num_participants: 2, + fee_rate: None, add_outputs: false, selection_args: OutputSelectionArgs { ..Default::default() @@ -169,6 +174,7 @@ impl Default for ContractNewArgsAPI { src_acct_name: None, net_change: None, num_participants: 2, + fee_rate: None, add_outputs: false, selection_args: OutputSelectionArgs { ..Default::default() @@ -295,6 +301,25 @@ mod tests { ); } + #[test] + fn fee_rate_json() { + let mut json = serde_json::to_value(ContractSetupArgsAPI::default()).unwrap(); + json["fee_rate"] = serde_json::json!(2); + assert_eq!( + serde_json::from_value::(json.clone()) + .unwrap() + .fee_rate, + Some(2) + ); + json.as_object_mut().unwrap().remove("fee_rate"); + assert_eq!( + serde_json::from_value::(json) + .unwrap() + .fee_rate, + None + ); + } + #[test] fn own_commitment_status_json() { for (status, value) in [ diff --git a/libwallet/src/contract/utils.rs b/libwallet/src/contract/utils.rs index a089bbc..9bdfbca 100644 --- a/libwallet/src/contract/utils.rs +++ b/libwallet/src/contract/utils.rs @@ -24,7 +24,7 @@ use crate::slate::Slate; use crate::types::{Context, NodeClient, StoredProofInfo, TxLogEntryType}; use crate::util::OnionV3Address; use crate::{address, Error, OutputData, OutputStatus, TxLogEntry}; -use grin_core::core::FeeFields; +use grin_core::core::{FeeFields, Transaction}; use uuid::Uuid; /// Creates an initial TxLogEntry without input/output or kernel information @@ -345,16 +345,35 @@ pub fn my_fee_contribution( n_outputs: usize, n_kernels: usize, num_participants: u8, +) -> Result { + fee_contribution(n_inputs, n_outputs, n_kernels, num_participants, None) +} + +pub(super) fn fee_contribution( + n_inputs: usize, + n_outputs: usize, + n_kernels: usize, + num_participants: u8, + fee_rate: Option, ) -> Result { verify_num_participants(num_participants)?; + verify_fee_rate(fee_rate)?; + let fee_for = |inputs: usize, outputs: usize, kernels: usize| match fee_rate { + Some(rate) => Transaction::weight_by_iok(inputs as u64, outputs as u64, kernels as u64) + .checked_mul(u64::from(rate)) + .ok_or_else(|| Error::GenericError("Contract fee overflow".to_string())), + None => Ok(tx_fee(inputs, outputs, kernels)), + }; // Add our fee costs for our inputs and a single output - let mut fee = tx_fee(n_inputs, n_outputs, 0); + let mut fee = fee_for(n_inputs, n_outputs, 0)?; // Add out fee costs for kernel. We pay 1/num_participants of a kernel cost - let kernel_cost = tx_fee(0, 0, n_kernels); + let kernel_cost = fee_for(0, 0, n_kernels)?; // Round each participant's kernel share up, so the participants together never underpay; // the overpay is bounded to under one fee unit per participant. - let my_kernel_cost = (kernel_cost as f64 / (num_participants as f64)).ceil(); - fee += my_kernel_cost as u64; + let my_kernel_cost = kernel_cost.div_ceil(u64::from(num_participants)); + fee = fee + .checked_add(my_kernel_cost) + .ok_or_else(|| Error::GenericError("Contract fee overflow".to_string()))?; // Add my fee contribution to the slate total fee. Uses the standard FeeFields // encoding; contracts rely on every participant applying the same @@ -375,6 +394,15 @@ pub(super) fn verify_num_participants(num_participants: u8) -> Result<(), Error> Ok(()) } +pub(super) fn verify_fee_rate(fee_rate: Option) -> Result<(), Error> { + if fee_rate == Some(0) { + return Err(Error::GenericError( + "Contract fee rate must be at least 1".to_string(), + )); + } + Ok(()) +} + /// Ensure the slate still carries the stored contract deadline pub(super) fn verify_ttl(expected: Option, slate: &Slate) -> Result<(), Error> { let expected = expected.unwrap_or(0); @@ -436,6 +464,18 @@ pub fn verify_setup_args_consistency( ctx_setup_args.num_participants, cur_setup_args.num_participants ))); } + if let Some(fee_rate) = cur_setup_args.fee_rate { + if Some(fee_rate) != ctx_setup_args.fee_rate { + let setup = ctx_setup_args + .fee_rate + .map(|rate| rate.to_string()) + .unwrap_or_else(|| "default".to_string()); + return Err(Error::GenericError(format!( + "Can't change fee rate after contract setup. setup:{}, current:{}", + setup, fee_rate + ))); + } + } // add_outputs is intentionally forced true at the sign step (late lock), so it is not // part of this consistency check. parent_key_id is taken from the stored Context, so // later steps always derive under the contract's account regardless of the active one. @@ -489,12 +529,54 @@ mod tests { let half = my_fee_contribution(n_inputs, n_outputs, n_kernels, 2) .unwrap() .fee(); - let my_share = ((kernel_cost as f64) / 2.0).ceil() as u64; + let my_share = kernel_cost.div_ceil(2); assert_eq!(half, base + my_share); // ... and applying the same split, the participants together cover the kernel cost. assert!(2 * my_share >= kernel_cost); } + #[test] + fn custom_fee_rate() { + let rate = 2; + let fee = fee_contribution(1, 1, 1, 2, Some(rate)).unwrap(); + let base = Transaction::weight_by_iok(1, 1, 0) * u64::from(rate); + let kernel = Transaction::weight_by_iok(0, 0, 1) * u64::from(rate); + assert_eq!(fee.fee(), base + kernel.div_ceil(2)); + assert!(fee_contribution(1, 1, 1, 2, Some(0)).is_err()); + } + + #[test] + fn fee_rate_consistency() { + let setup = ContractSetupArgsAPI { + net_change: Some(-1), + fee_rate: Some(2), + ..Default::default() + }; + assert!(verify_setup_args_consistency(&setup, &setup).is_ok()); + assert!(verify_setup_args_consistency( + &setup, + &ContractSetupArgsAPI { + net_change: Some(-1), + ..Default::default() + } + ) + .is_ok()); + let err = verify_setup_args_consistency( + &setup, + &ContractSetupArgsAPI { + net_change: Some(-1), + fee_rate: Some(3), + ..Default::default() + }, + ) + .unwrap_err(); + assert!(matches!( + err, + Error::GenericError(ref msg) + if msg == "Can't change fee rate after contract setup. setup:2, current:3" + )); + } + #[test] fn participant_count() { assert!(verify_num_participants(1).is_ok()); diff --git a/src/bin/grin-wallet.yml b/src/bin/grin-wallet.yml index 3623814..f54cd9a 100644 --- a/src/bin/grin-wallet.yml +++ b/src/bin/grin-wallet.yml @@ -503,6 +503,10 @@ subcommands: long: min_conf default_value: "10" takes_value: true + - fee_rate: + help: Fee rate in nanogrin per unit of transaction weight (defaults to accept_fee_base) + long: fee_rate + takes_value: true - ttl_blocks: help: Positive number of blocks until wallets stop signing the contract short: b @@ -551,6 +555,10 @@ subcommands: short: c long: min_conf takes_value: true + - fee_rate: + help: Fee rate to use when first joining, in nanogrin per unit of transaction weight (defaults to accept_fee_base) + long: fee_rate + takes_value: true - no-broadcast: help: Don't broadcast the transaction if it was finalized long: no-broadcast diff --git a/src/cmd/wallet_args.rs b/src/cmd/wallet_args.rs index 6644e8c..7ae4c46 100644 --- a/src/cmd/wallet_args.rs +++ b/src/cmd/wallet_args.rs @@ -1001,6 +1001,21 @@ pub fn parse_verify_proof_args(args: &ArgMatches) -> Result Result, ParseError> { + let Some(value) = args.value_of("fee_rate") else { + return Ok(None); + }; + let rate = parse_u64(value, "fee_rate")?; + if rate == 0 { + return Err(ParseError::ArgumentError( + "Contract fee rate must be at least 1".to_string(), + )); + } + u32::try_from(rate) + .map(Some) + .map_err(|_| ParseError::ArgumentError("Contract fee rate is too large".to_string())) +} + pub fn parse_contract_new_args( args: &ArgMatches, account: &String, @@ -1032,6 +1047,7 @@ pub fn parse_contract_new_args( parse_required(args, "minimum_confirmations")?, "minimum_confirmations", )?; + let fee_rate = parse_contract_fee_rate(args)?; let ttl_blocks = match args.value_of("ttl_blocks") { Some(value) => { let blocks = parse_u64(value, "ttl_blocks")?; @@ -1099,9 +1115,9 @@ pub fn parse_contract_new_args( use_inputs: use_inputs, make_outputs: make_outputs, minimum_confirmations, + fee_rate, ttl_blocks, // Not yet wired to CLI flags: - fee_rate: None, outfile: None, }) } @@ -1132,6 +1148,7 @@ pub fn parse_contract_setup_args( Some(value) => Some(parse_u64(value, "minimum_confirmations")?), None => None, }; + let fee_rate = parse_contract_fee_rate(args)?; let no_payjoin = args.is_present("no-payjoin"); let use_inputs = match args.value_of("use-inputs") { Some(v) => { @@ -1174,8 +1191,8 @@ pub fn parse_contract_setup_args( use_inputs: use_inputs, make_outputs: make_outputs, minimum_confirmations, + fee_rate, // Not yet wired to CLI flags: - fee_rate: None, outfile: None, }) } diff --git a/tests/cmd_line_contract.rs b/tests/cmd_line_contract.rs index 586e05c..5fbcf82 100644 --- a/tests/cmd_line_contract.rs +++ b/tests/cmd_line_contract.rs @@ -26,6 +26,7 @@ use clap::App; use std::thread; use std::time::Duration; +use grin_core::core::Transaction; use grin_keychain::ExtKeychain; use grin_wallet_impls::DefaultLCProvider; @@ -102,6 +103,10 @@ fn contract_command_test_impl(test_dir: &str) -> Result<(), grin_wallet_controll let mask1 = (&mask1_i).as_ref(); let mask2 = (&mask2_i).as_ref(); let _ = test_framework::award_blocks_to_wallet(&chain, wallet1.clone(), mask1, 5, false); + let fee_rate = 2 * grin_core::global::get_accept_fee_base(); + let expected_fee = Transaction::weight_by_iok(1, 1, 0) * fee_rate + + (Transaction::weight_by_iok(0, 0, 1) * fee_rate).div_ceil(2); + let fee_rate = fee_rate.to_string(); // Wallet 1 opens a contract sending 1 grin, leaving an unencrypted slatepack because // no --encrypt-for was given @@ -115,11 +120,24 @@ fn contract_command_test_impl(test_dir: &str) -> Result<(), grin_wallet_controll "1", "--min_conf", "1", + "--fee_rate", + &fee_rate, ]; execute_command(&app, test_dir, "wallet1", &client1, arg_vec)?; // Wallet 2 views the contract before signing it let file_name = only_slatepack(test_dir, "wallet1"); + grin_wallet_controller::controller::owner_single_use( + wallet1.clone(), + mask1, + std::path::PathBuf::from(test_dir), + |api, m| { + let message = std::fs::read_to_string(&file_name)?; + let slate = api.slate_from_slatepack_message(m, message, vec![0])?; + assert_eq!(slate.fee_fields.fee(), expected_fee); + Ok(()) + }, + )?; let arg_vec = vec![ "grin-wallet", "-p", @@ -262,6 +280,8 @@ fn parses_contract_options() { "1", "--min_conf", "3", + "--fee_rate", + "2", "--ttl_blocks", "20", ]) @@ -271,6 +291,7 @@ fn parses_contract_options() { let parsed = grin_wallet::cmd::wallet_args::parse_contract_new_args(new_args, &account).unwrap(); assert_eq!(parsed.minimum_confirmations, 3); + assert_eq!(parsed.fee_rate, Some(2)); assert_eq!(parsed.ttl_blocks, Some(20)); let expected = grin_wallet_libwallet::contract::types::DEFAULT_MINIMUM_CONFIRMATIONS; @@ -283,6 +304,7 @@ fn parses_contract_options() { let parsed = grin_wallet::cmd::wallet_args::parse_contract_new_args(new_args, &account).unwrap(); assert_eq!(parsed.minimum_confirmations, expected); + assert_eq!(parsed.fee_rate, None); assert_eq!(parsed.ttl_blocks, None); let args = app .clone() @@ -300,6 +322,35 @@ fn parses_contract_options() { "Contract TTL must be at least 1 block".to_string() ) ); + for (rate, message) in [ + ("0", "Contract fee rate must be at least 1"), + ("4294967296", "Contract fee rate is too large"), + ] { + let args = app + .clone() + .get_matches_from_safe(vec![ + "grin-wallet", + "contract", + "new", + "-s", + "1", + "--fee_rate", + rate, + ]) + .unwrap(); + let new_args = args + .subcommand_matches("contract") + .unwrap() + .subcommand_matches("new") + .unwrap(); + let err = grin_wallet::cmd::wallet_args::parse_contract_new_args(new_args, &account) + .err() + .expect("invalid contract fee rate accepted"); + assert_eq!( + err, + grin_wallet::cmd::wallet_args::ParseError::ArgumentError(message.to_string()) + ); + } let args = app .clone() @@ -309,14 +360,24 @@ fn parses_contract_options() { let sign_args = contract.subcommand_matches("sign").unwrap(); let parsed = grin_wallet::cmd::wallet_args::parse_contract_setup_args(sign_args).unwrap(); assert_eq!(parsed.minimum_confirmations, None); + assert_eq!(parsed.fee_rate, None); let args = app - .get_matches_from_safe(vec!["grin-wallet", "contract", "sign", "--min_conf", "3"]) + .get_matches_from_safe(vec![ + "grin-wallet", + "contract", + "sign", + "--min_conf", + "3", + "--fee_rate", + "2", + ]) .unwrap(); let contract = args.subcommand_matches("contract").unwrap(); let sign_args = contract.subcommand_matches("sign").unwrap(); let parsed = grin_wallet::cmd::wallet_args::parse_contract_setup_args(sign_args).unwrap(); assert_eq!(parsed.minimum_confirmations, Some(3)); + assert_eq!(parsed.fee_rate, Some(2)); } #[test]