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

Implement fee inclusive transactions #657

Merged
merged 3 commits into from
Jul 26, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
21 changes: 17 additions & 4 deletions controller/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,8 @@ where
#[derive(Clone)]
pub struct SendArgs {
pub amount: u64,
pub amount_includes_fee: bool,
pub use_max_amount: bool,
pub minimum_confirmations: u64,
pub selection_strategy: String,
pub estimate_selection_strategies: bool,
Expand Down Expand Up @@ -353,14 +355,24 @@ where
K: keychain::Keychain + 'static,
{
let mut slate = Slate::blank(2, false);
let mut amount = args.amount;
if args.use_max_amount {
controller::owner_single_use(None, keychain_mask, Some(owner_api), |api, m| {
let (_, wallet_info) =
api.retrieve_summary_info(m, true, args.minimum_confirmations)?;
amount = wallet_info.amount_currently_spendable;
Ok(())
})?;
};
controller::owner_single_use(None, keychain_mask, Some(owner_api), |api, m| {
if args.estimate_selection_strategies {
let strategies = vec!["smallest", "all"]
.into_iter()
.map(|strategy| {
let init_args = InitTxArgs {
src_acct_name: None,
amount: args.amount,
amount: amount,
amount_includes_fee: Some(args.amount_includes_fee),
minimum_confirmations: args.minimum_confirmations,
max_outputs: args.max_outputs as u32,
num_change_outputs: args.change_outputs as u32,
Expand All @@ -372,12 +384,13 @@ where
Ok((strategy, slate.amount, slate.fee_fields))
})
.collect::<Result<Vec<_>, grin_wallet_libwallet::Error>>()?;
display::estimate(args.amount, strategies, dark_scheme);
display::estimate(amount, strategies, dark_scheme);
return Ok(());
} else {
let init_args = InitTxArgs {
src_acct_name: None,
amount: args.amount,
amount: amount,
amount_includes_fee: Some(args.amount_includes_fee),
minimum_confirmations: args.minimum_confirmations,
max_outputs: args.max_outputs as u32,
num_change_outputs: args.change_outputs as u32,
Expand All @@ -394,7 +407,7 @@ where
Ok(s) => {
info!(
"Tx created: {} grin to {} (strategy '{}')",
core::amount_to_hr_string(args.amount, false),
core::amount_to_hr_string(amount, false),
args.dest,
args.selection_strategy,
);
Expand Down
41 changes: 41 additions & 0 deletions controller/tests/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,47 @@ fn basic_transaction_api(test_dir: &'static str) -> Result<(), libwallet::Error>
Ok(())
})?;

// try to send a transaction with amount inclusive of fees, but amount too
// small to cover fees. Should fail.
wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |sender_api, m| {
// note this will increment the block count as part of the transaction "Posting"
let args = InitTxArgs {
src_acct_name: None,
amount: 1,
amount_includes_fee: Some(true),
minimum_confirmations: 2,
max_outputs: 500,
num_change_outputs: 1,
selection_strategy_is_use_all: true,
..Default::default()
};
let res = sender_api.init_send_tx(m, args);
assert!(res.is_err());
Ok(())
})?;

// try to build a transaction with amount inclusive of fees. Confirm that tx
// amount + fee is equal to the originally specified amount
let amount = 60_000_000_000;
wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |sender_api, m| {
// note this will increment the block count as part of the transaction "Posting"
let args = InitTxArgs {
src_acct_name: None,
amount: amount,
amount_includes_fee: Some(true),
minimum_confirmations: 2,
max_outputs: 500,
num_change_outputs: 1,
selection_strategy_is_use_all: true,
..Default::default()
};
let slate_i = sender_api.init_send_tx(m, args)?;
assert_eq!(slate_i.state, SlateState::Standard1);
let total_spend: u64 = slate_i.amount + slate_i.fee_fields.fee();
assert_eq!(amount, total_spend);
Ok(())
})?;

// let logging finish
stopper.store(false, Ordering::Relaxed);
thread::sleep(Duration::from_millis(200));
Expand Down
4 changes: 4 additions & 0 deletions libwallet/src/api_impl/owner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,7 @@ where
&mut *w,
keychain_mask,
args.amount,
args.amount_includes_fee.unwrap_or(false),
args.minimum_confirmations,
args.max_outputs as usize,
args.num_change_outputs as usize,
Expand Down Expand Up @@ -543,6 +544,7 @@ where
&parent_key_id,
true,
use_test_rng,
args.amount_includes_fee.unwrap_or(false),
)?
};

Expand Down Expand Up @@ -696,6 +698,7 @@ where
&parent_key_id,
false,
use_test_rng,
false,
)?;

let keychain = w.keychain(keychain_mask)?;
Expand Down Expand Up @@ -823,6 +826,7 @@ where
parent_key_id.clone(),
false,
true,
false,
)?;

// Add inputs and outputs to original context
Expand Down
3 changes: 3 additions & 0 deletions libwallet/src/api_impl/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ pub struct InitTxArgs {
#[serde(with = "secp_ser::string_or_u64")]
/// The amount to send, in nanogrins. (`1 G = 1_000_000_000nG`)
pub amount: u64,
/// Does the amount include the fee, or will fees be spent in addition to the amount?
pub amount_includes_fee: Option<bool>,
#[serde(with = "secp_ser::string_or_u64")]
/// The minimum number of confirmations an output
/// should have in order to be included in the transaction.
Expand Down Expand Up @@ -105,6 +107,7 @@ impl Default for InitTxArgs {
InitTxArgs {
src_acct_name: None,
amount: 0,
amount_includes_fee: None,
minimum_confirmations: 10,
max_outputs: 500,
num_change_outputs: 1,
Expand Down
38 changes: 34 additions & 4 deletions libwallet/src/internal/selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ pub fn build_send_tx<'a, T: ?Sized, C, K>(
parent_key_id: Identifier,
use_test_nonce: bool,
is_initiator: bool,
amount_includes_fee: bool,
) -> Result<Context, Error>
where
T: WalletBackend<'a, C, K>,
Expand All @@ -61,6 +62,7 @@ where
wallet,
keychain_mask,
slate.amount,
amount_includes_fee,
current_height,
minimum_confirmations,
max_outputs,
Expand All @@ -69,6 +71,14 @@ where
&parent_key_id,
false,
)?;
if amount_includes_fee {
slate.amount = slate
.amount
.checked_sub(fee)
.ok_or(ErrorKind::GenericError(
format!("Transaction amount is too small to include fee").into(),
))?;
};

if fixed_fee.map(|f| fee != f).unwrap_or(false) {
return Err(ErrorKind::Fee("The initially selected fee is not sufficient".into()).into());
Expand Down Expand Up @@ -312,6 +322,7 @@ pub fn select_send_tx<'a, T: ?Sized, C, K, B>(
wallet: &mut T,
keychain_mask: Option<&SecretKey>,
amount: u64,
amount_includes_fee: bool,
current_height: u64,
minimum_confirmations: u64,
max_outputs: usize,
Expand All @@ -337,6 +348,7 @@ where
let (coins, _total, amount, fee) = select_coins_and_fee(
wallet,
amount,
amount_includes_fee,
current_height,
minimum_confirmations,
max_outputs,
Expand All @@ -363,6 +375,7 @@ where
pub fn select_coins_and_fee<'a, T: ?Sized, C, K>(
wallet: &mut T,
amount: u64,
amount_includes_fee: bool,
current_height: u64,
minimum_confirmations: u64,
max_outputs: usize,
Expand Down Expand Up @@ -401,7 +414,10 @@ where
// First attempt to spend without change
let mut fee = tx_fee(coins.len(), 1, 1);
let mut total: u64 = coins.iter().map(|c| c.value).sum();
let mut amount_with_fee = amount + fee;
let mut amount_with_fee = match amount_includes_fee {
true => amount,
false => amount + fee,
};

if total == 0 {
return Err(ErrorKind::NotEnoughFunds {
Expand Down Expand Up @@ -429,7 +445,10 @@ where
// We need to add a change address or amount with fee is more than total
if total != amount_with_fee {
fee = tx_fee(coins.len(), num_outputs, 1);
amount_with_fee = amount + fee;
amount_with_fee = match amount_includes_fee {
true => amount,
false => amount + fee,
};

// Here check if we have enough outputs for the amount including fee otherwise
// look for other outputs and check again
Expand Down Expand Up @@ -458,10 +477,21 @@ where
.1;
fee = tx_fee(coins.len(), num_outputs, 1);
total = coins.iter().map(|c| c.value).sum();
amount_with_fee = amount + fee;
amount_with_fee = match amount_includes_fee {
true => amount,
false => amount + fee,
};
}
}
Ok((coins, total, amount, fee))
// If original amount includes fee, the new amount should
// be reduced, to accommodate the fee.
let new_amount = match amount_includes_fee {
true => amount.checked_sub(fee).ok_or(ErrorKind::GenericError(
format!("Transaction amount is too small to include fee").into(),
))?,
false => amount,
};
Ok((coins, total, new_amount, fee))
}

/// Selects inputs and change for a transaction
Expand Down
5 changes: 5 additions & 0 deletions libwallet/src/internal/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ pub fn estimate_send_tx<'a, T: ?Sized, C, K>(
wallet: &mut T,
keychain_mask: Option<&SecretKey>,
amount: u64,
amount_includes_fee: bool,
minimum_confirmations: u64,
max_outputs: usize,
num_change_outputs: usize,
Expand Down Expand Up @@ -128,6 +129,7 @@ where
let (_coins, total, _amount, fee) = selection::select_coins_and_fee(
wallet,
amount,
amount_includes_fee,
current_height,
minimum_confirmations,
max_outputs,
Expand All @@ -151,6 +153,7 @@ pub fn add_inputs_to_slate<'a, T: ?Sized, C, K>(
parent_key_id: &Identifier,
is_initiator: bool,
use_test_rng: bool,
amount_includes_fee: bool,
) -> Result<Context, Error>
where
T: WalletBackend<'a, C, K>,
Expand Down Expand Up @@ -181,6 +184,7 @@ where
parent_key_id.clone(),
use_test_rng,
is_initiator,
amount_includes_fee,
)?;

// Generate a kernel offset and subtract from our context's secret key. Store
Expand Down Expand Up @@ -270,6 +274,7 @@ where
let (_coins, _total, _amount, fee) = selection::select_coins_and_fee(
wallet,
init_tx_args.amount,
init_tx_args.amount_includes_fee.unwrap_or(false),
current_height,
init_tx_args.minimum_confirmations,
init_tx_args.max_outputs as usize,
Expand Down
5 changes: 4 additions & 1 deletion src/bin/grin-wallet.yml
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ subcommands:
about: Builds a transaction to send coins and sends to the recipient via the Slatepack workflow
args:
- amount:
help: Number of coins to send with optional fraction, e.g. 12.423
help: Number of coins to send with optional fraction, e.g. 12.423. Keyword 'max' will send maximum amount.
index: 1
- minimum_confirmations:
help: Minimum number of confirmations required for an output to be spendable
Expand Down Expand Up @@ -181,6 +181,9 @@ subcommands:
help: Show slatepack data as QR code
short: q
long: slatepack_qr
- amount_includes_fee:
help: Transaction amount includes transaction fee. Recipient will receive (amount - fee).
long: amount_includes_fee
- unpack:
about: Unpack and display an armored Slatepack Message, decrypting if possible
args:
Expand Down
9 changes: 8 additions & 1 deletion src/cmd/wallet_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,11 @@ pub fn parse_account_args(account_args: &ArgMatches) -> Result<command::AccountA
pub fn parse_send_args(args: &ArgMatches) -> Result<command::SendArgs, ParseError> {
// amount
let amount = parse_required(args, "amount")?;
let amount = core::core::amount_from_hr_string(amount);
let (amount, spend_max) = if amount.eq_ignore_ascii_case("max") {
(Ok(0), true)
} else {
(core::core::amount_from_hr_string(amount), false)
};
let amount = match amount {
Ok(a) => a,
Err(e) => {
Expand All @@ -455,6 +459,7 @@ pub fn parse_send_args(args: &ArgMatches) -> Result<command::SendArgs, ParseErro
return Err(ParseError::ArgumentError(msg));
}
};
let amount_includes_fee = args.is_present("amount_includes_fee") || spend_max;

// minimum_confirmations
let min_c = parse_required(args, "minimum_confirmations")?;
Expand Down Expand Up @@ -524,6 +529,8 @@ pub fn parse_send_args(args: &ArgMatches) -> Result<command::SendArgs, ParseErro

Ok(command::SendArgs {
amount: amount,
amount_includes_fee: amount_includes_fee,
use_max_amount: spend_max,
minimum_confirmations: min_c,
selection_strategy: selection_strategy.to_owned(),
estimate_selection_strategies,
Expand Down
Loading