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

monero-wallet-cli: added locked_sweep_all command #3629

Merged
merged 1 commit into from Jul 27, 2018

Conversation

Projects
None yet
4 participants
@jcktm
Copy link
Contributor

commented Apr 13, 2018

This implements locked_sweep_all for the simplewallet and fixes a small bug in sweep_main

@jcktm jcktm changed the title loki-wallet-cli: added locked_sweep_all command monero-wallet-cli: added locked_sweep_all command Apr 13, 2018

std::vector<uint8_t> extra;
bool payment_id_seen = false;
if (2 >= local_args.size())
if (2 <= local_args.size())

This comment has been minimized.

Copy link
@stoffu

stoffu Apr 13, 2018

Contributor

This seems wrong. The block shouldn't be executed when local_args.size()<2.

This comment has been minimized.

Copy link
@jcktm

jcktm Apr 13, 2018

Author Contributor

Exactly... this fixes that bug. Before the change the block is executed when local_args.size() < 2. After the change it isn't executed for local_args.size()<2. Perhaps if (local_args.size() >= 2) would read better?

This comment has been minimized.

Copy link
@stoffu

stoffu Apr 13, 2018

Contributor

Oops, I read it the wrong way! Yes, I'd prefer local_args.size() >= 2 much better. Thanks.

This comment has been minimized.

Copy link
@jcktm

jcktm Apr 13, 2018

Author Contributor

Fixed

@jcktm jcktm force-pushed the jcktm:monero_locked_sweep_all branch from 706b4e8 to 644274b Apr 13, 2018

@jcktm

This comment has been minimized.

Copy link
Contributor Author

commented Apr 13, 2018

This references issue #3147

@stoffu

This comment has been minimized.

Copy link
Contributor

commented Apr 13, 2018

loki-wallet-cli: added locked_sweep_all command

Please fix the commit message, too.

@jcktm jcktm force-pushed the jcktm:monero_locked_sweep_all branch from 644274b to 7f983bd Apr 13, 2018

@jcktm

This comment has been minimized.

Copy link
Contributor Author

commented Apr 13, 2018

Fixed

@stoffu

stoffu approved these changes Apr 13, 2018

@luigi1111

This comment has been minimized.

Copy link
Collaborator

commented May 31, 2018

@jcktm and please rebase. :)

@moneromooo-monero

This comment has been minimized.

Copy link
Contributor

commented Jul 20, 2018

ping, needs rebasing.

@jcktm jcktm force-pushed the jcktm:monero_locked_sweep_all branch from 7f983bd to 7ddc4bd Jul 21, 2018

@jcktm

This comment has been minimized.

Copy link
Contributor Author

commented Jul 21, 2018

Rebased

std::vector<uint8_t> extra;
bool payment_id_seen = false;
if (2 >= local_args.size())
if (local_args.size() >= 2)

This comment has been minimized.

Copy link
@moneromooo-monero

moneromooo-monero Jul 21, 2018

Contributor

Why this change ? Was it wrong in the first place ?

This comment has been minimized.

Copy link
@jcktm

jcktm Jul 22, 2018

Author Contributor

Yes

m_cmd_binder.set_handler("locked_sweep_all",
boost::bind(&simple_wallet::locked_sweep_all, this, _1),
tr("locked_sweep_all [index=<N1>[,<N2>,...]] [<priority>] [<ring_size>] <address> <lockblocks> [<payment_id>]"),
tr("Send all unlocked balance to an address and lock it for <lockblocks> (max. 1000000). If the parameter \"index<N1>[,<N2>,...]\" is specified, the wallet sweeps outputs received by those address indices. If omitted, the wallet randomly chooses an address index to be used. In any case, it tries its best not to combine outputs across multiple addresses. <priority> is the priority of the sweep. The higher the priority, the higher the transaction fee. Valid values in priority order (from lowest to highest) are: unimportant, normal, elevated, priority. If omitted, the default value (see the command \"set priority\") is used. <ring_size> is the number of inputs to include for untraceability."));

This comment has been minimized.

Copy link
@stoffu

stoffu Jul 22, 2018

Contributor

This description

In any case, it tries its best not to combine outputs across multiple addresses.

doesn't seem to apply here; with the sweeping operation, one either specifies the address indices to be swept (so that outputs belonging to different indices get mixed), or omits specifying the address index (so that outputs belonging to one address index get swept).

This comment has been minimized.

Copy link
@jcktm

jcktm Jul 23, 2018

Author Contributor

Removed

@jcktm jcktm force-pushed the jcktm:monero_locked_sweep_all branch from 7ddc4bd to ed7825d Jul 23, 2018

@stoffu

stoffu approved these changes Jul 23, 2018

@luigi1111 luigi1111 merged commit ed7825d into monero-project:master Jul 27, 2018

6 of 8 checks passed

buildbot/monero-static-ubuntu-amd64 Build done.
Details
buildbot/monero-static-win32 Build done.
Details
buildbot/monero-linux-armv7 Build done.
Details
buildbot/monero-static-osx-10.11 Build done.
Details
buildbot/monero-static-osx-10.12 Build done.
Details
buildbot/monero-static-osx-10.13 Build done.
Details
buildbot/monero-static-ubuntu-i686 Build done.
Details
buildbot/monero-static-win64 Build done.
Details

luigi1111 added a commit that referenced this pull request Jul 27, 2018

Merge pull request #3629
ed7825d monero-wallet-cli: added locked_sweep_all command (jcktm)

@stoffu stoffu referenced this pull request Dec 21, 2018

Merged

Upstream merge 3 #90

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.