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

Add ability to compare selection strategies #2516

Merged
merged 1 commit into from
Feb 12, 2019
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
22 changes: 19 additions & 3 deletions src/bin/cmd/wallet_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,9 @@ pub fn parse_send_args(args: &ArgMatches) -> Result<command::SendArgs, ParseErro
// selection_strategy
let selection_strategy = parse_required(args, "selection_strategy")?;

// estimate_selection_strategies
let estimate_selection_strategies = args.is_present("estimate_selection_strategies");

// method
let method = parse_required(args, "method")?;

Expand All @@ -360,10 +363,18 @@ pub fn parse_send_args(args: &ArgMatches) -> Result<command::SendArgs, ParseErro
None => "default",
}
} else {
parse_required(args, "dest")?
if !estimate_selection_strategies {
parse_required(args, "dest")?
} else {
""
}
}
};
if method == "http" && !dest.starts_with("http://") && !dest.starts_with("https://") {
if !estimate_selection_strategies
&& method == "http"
&& !dest.starts_with("http://")
&& !dest.starts_with("https://")
{
let msg = format!(
"HTTP Destination should start with http://: or https://: {}",
dest,
Expand All @@ -386,6 +397,7 @@ pub fn parse_send_args(args: &ArgMatches) -> Result<command::SendArgs, ParseErro
message: message,
minimum_confirmations: min_c,
selection_strategy: selection_strategy.to_owned(),
estimate_selection_strategies,
method: method.to_owned(),
dest: dest.to_owned(),
change_outputs: change_outputs,
Expand Down Expand Up @@ -562,7 +574,11 @@ pub fn wallet_command(
}
("send", Some(args)) => {
let a = arg_parse!(parse_send_args(&args));
command::send(inst_wallet(), a)
command::send(
inst_wallet(),
a,
wallet_config.dark_background_color_scheme.unwrap_or(true),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it unrelated change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is needed, because send command would able to emit a table with information, which uses colors.

)
}
("receive", Some(args)) => {
let a = arg_parse!(parse_receive_args(&args));
Expand Down
4 changes: 4 additions & 0 deletions src/bin/grin.yml
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,10 @@ subcommands:
- smallest
default_value: all
takes_value: true
- estimate_selection_strategies:
help: Estimates all possible Coin/Output selection strategies.
short: e
long: estimate-selection
- change_outputs:
help: Number of change outputs to generate (mainly for testing)
short: o
Expand Down
132 changes: 77 additions & 55 deletions wallet/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ pub struct SendArgs {
pub message: Option<String>,
pub minimum_confirmations: u64,
pub selection_strategy: String,
pub estimate_selection_strategies: bool,
pub method: String,
pub dest: String,
pub change_outputs: usize,
Expand All @@ -210,68 +211,89 @@ pub struct SendArgs {
pub fn send(
wallet: Arc<Mutex<WalletInst<impl NodeClient + 'static, keychain::ExtKeychain>>>,
args: SendArgs,
dark_scheme: bool,
) -> Result<(), Error> {
controller::owner_single_use(wallet.clone(), |api| {
let result = api.initiate_tx(
None,
args.amount,
args.minimum_confirmations,
args.max_outputs,
args.change_outputs,
args.selection_strategy == "all",
args.message.clone(),
);
let (mut slate, lock_fn) = match result {
Ok(s) => {
info!(
"Tx created: {} grin to {} (strategy '{}')",
core::amount_to_hr_string(args.amount, false),
args.dest,
args.selection_strategy,
);
s
}
Err(e) => {
info!("Tx not created: {}", e);
return Err(e);
}
};
let adapter = match args.method.as_str() {
"http" => HTTPWalletCommAdapter::new(),
"file" => FileWalletCommAdapter::new(),
"keybase" => KeybaseWalletCommAdapter::new(),
"self" => NullWalletCommAdapter::new(),
_ => NullWalletCommAdapter::new(),
};
if adapter.supports_sync() {
slate = adapter.send_tx_sync(&args.dest, &slate)?;
api.tx_lock_outputs(&slate, lock_fn)?;
if args.method == "self" {
controller::foreign_single_use(wallet, |api| {
api.receive_tx(&mut slate, Some(&args.dest), None)?;
Ok(())
})?;
}
if let Err(e) = api.verify_slate_messages(&slate) {
error!("Error validating participant messages: {}", e);
return Err(e);
}
api.finalize_tx(&mut slate)?;
if args.estimate_selection_strategies {
let strategies = vec!["smallest", "all"]
.into_iter()
.map(|strategy| {
let (total, fee) = api
.estimate_initiate_tx(
None,
args.amount,
args.minimum_confirmations,
args.max_outputs,
args.change_outputs,
strategy == "all",
)
.unwrap();
(strategy, total, fee)
})
.collect();
display::estimate(args.amount, strategies, dark_scheme);
} else {
adapter.send_tx_async(&args.dest, &slate)?;
api.tx_lock_outputs(&slate, lock_fn)?;
}
if adapter.supports_sync() {
let result = api.post_tx(&slate.tx, args.fluff);
match result {
Ok(_) => {
info!("Tx sent ok",);
return Ok(());
let result = api.initiate_tx(
None,
args.amount,
args.minimum_confirmations,
args.max_outputs,
args.change_outputs,
args.selection_strategy == "all",
args.message.clone(),
);
let (mut slate, lock_fn) = match result {
Ok(s) => {
info!(
"Tx created: {} grin to {} (strategy '{}')",
core::amount_to_hr_string(args.amount, false),
args.dest,
args.selection_strategy,
);
s
}
Err(e) => {
error!("Tx sent fail: {}", e);
info!("Tx not created: {}", e);
return Err(e);
}
};
let adapter = match args.method.as_str() {
"http" => HTTPWalletCommAdapter::new(),
"file" => FileWalletCommAdapter::new(),
"keybase" => KeybaseWalletCommAdapter::new(),
"self" => NullWalletCommAdapter::new(),
_ => NullWalletCommAdapter::new(),
};
if adapter.supports_sync() {
slate = adapter.send_tx_sync(&args.dest, &slate)?;
api.tx_lock_outputs(&slate, lock_fn)?;
if args.method == "self" {
controller::foreign_single_use(wallet, |api| {
api.receive_tx(&mut slate, Some(&args.dest), None)?;
Ok(())
})?;
}
if let Err(e) = api.verify_slate_messages(&slate) {
error!("Error validating participant messages: {}", e);
return Err(e);
}
api.finalize_tx(&mut slate)?;
} else {
adapter.send_tx_async(&args.dest, &slate)?;
api.tx_lock_outputs(&slate, lock_fn)?;
}
if adapter.supports_sync() {
let result = api.post_tx(&slate.tx, args.fluff);
match result {
Ok(_) => {
info!("Tx sent ok",);
return Ok(());
}
Err(e) => {
error!("Tx sent fail: {}", e);
return Err(e);
}
}
}
}
Ok(())
Expand Down
43 changes: 43 additions & 0 deletions wallet/src/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,49 @@ pub fn info(
);
}
}

/// Display summary info in a pretty way
pub fn estimate(
amount: u64,
strategies: Vec<(
&str, // strategy
u64, // total amount to be locked
u64, // fee
)>,
dark_background_color_scheme: bool,
) {
println!(
"\nEstimation for sending {}:\n",
amount_to_hr_string(amount, false)
);

let mut table = table!();

table.set_titles(row![
bMG->"Selection strategy",
bMG->"Fee",
bMG->"Will be locked",
]);

for (strategy, total, fee) in strategies {
if dark_background_color_scheme {
table.add_row(row![
bFC->strategy,
FR->amount_to_hr_string(fee, false),
FY->amount_to_hr_string(total, false),
]);
} else {
table.add_row(row![
bFD->strategy,
FR->amount_to_hr_string(fee, false),
FY->amount_to_hr_string(total, false),
]);
}
}
table.printstd();
println!();
}

/// Display list of wallet accounts in a pretty way
pub fn accounts(acct_mappings: Vec<AcctPathMapping>) {
println!("\n____ Wallet Accounts ____\n",);
Expand Down
68 changes: 68 additions & 0 deletions wallet/src/libwallet/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,74 @@ where
Ok((slate, lock_fn))
}

/// Estimates the amount to be locked and fee for the transaction without creating one
///
/// # Arguments
/// * `src_acct_name` - The human readable account name from which to draw outputs
/// for the transaction, overriding whatever the active account is as set via the
/// [`set_active_account`](struct.APIOwner.html#method.set_active_account) method.
/// If None, the transaction will use the active account.
/// * `amount` - The amount to send, in nanogrins. (`1 G = 1_000_000_000nG`)
/// * `minimum_confirmations` - The minimum number of confirmations an output
/// should have in order to be included in the transaction.
/// * `max_outputs` - By default, the wallet selects as many inputs as possible in a
/// transaction, to reduce the Output set and the fees. The wallet will attempt to spend
/// include up to `max_outputs` in a transaction, however if this is not enough to cover
/// the whole amount, the wallet will include more outputs. This parameter should be considered
/// a soft limit.
/// * `num_change_outputs` - The target number of change outputs to create in the transaction.
/// The actual number created will be `num_change_outputs` + whatever remainder is needed.
/// * `selection_strategy_is_use_all` - If `true`, attempt to use up as many outputs as
/// possible to create the transaction, up the 'soft limit' of `max_outputs`. This helps
/// to reduce the size of the UTXO set and the amount of data stored in the wallet, and
/// minimizes fees. This will generally result in many inputs and a large change output(s),
/// usually much larger than the amount being sent. If `false`, the transaction will include
/// as many outputs as are needed to meet the amount, (and no more) starting with the smallest
/// value outputs.
///
/// # Returns
/// * a result containing:
/// * (total, fee) - A tuple:
/// * Total amount to be locked.
/// * Transaction fee
pub fn estimate_initiate_tx(
&mut self,
src_acct_name: Option<&str>,
amount: u64,
minimum_confirmations: u64,
max_outputs: usize,
num_change_outputs: usize,
selection_strategy_is_use_all: bool,
) -> Result<
(
u64, // total
u64, // fee
),
Error,
> {
let mut w = self.wallet.lock();
w.open_with_credentials()?;
let parent_key_id = match src_acct_name {
Some(d) => {
let pm = w.get_acct_path(d.to_owned())?;
match pm {
Some(p) => p.path,
None => w.parent_key_id(),
}
}
None => w.parent_key_id(),
};
tx::estimate_send_tx(
&mut *w,
amount,
minimum_confirmations,
max_outputs,
num_change_outputs,
selection_strategy_is_use_all,
&parent_key_id,
)
}

/// Lock outputs associated with a given slate/transaction
pub fn tx_lock_outputs(
&mut self,
Expand Down
Loading