adds rpc calls for `setprivatesendrounds` and `setprivatesendamount` by PastaPastaPasta · Pull Request #2230 · dashpay/dash

PastaPastaPasta · 2018-08-17T14:53:45Z

Not tested yet. Not positive about help text, but it's probably fine.

PastaPastaPasta · 2018-08-17T14:58:53Z

src/wallet/rpcwallet.cpp

+			+ HelpExampleRpc("setprivatesendrounds", "16")
+		);
+
+	LOCK2(cs_main, pwalletMain->cs_wallet);


not sure if needed, the LOCK2 that is

gladcow · 2018-08-17T15:02:38Z

This methods parameters should be added to the https://github.com/dashpay/dash/blob/develop/src/rpc/client.cpp#L30, as far as I can see.

PastaPastaPasta · 2018-08-17T15:14:20Z

Hmm, @gladcow would that just be { "setprivatesendrounds", 1, "rounds" }, and { "setprivatesendamount", 1, "amount" }, ? I don't quite understand that list so...

gladcow · 2018-08-17T15:44:56Z

It should be smth like { "setprivatesendrounds", 0, "rounds" }, parameter index is zero-based. This list is used to convert non-string rpc parameters to their real type, both your methods use number parameters, so this parameters should be added here.

UdjinM6 · 2018-08-17T18:26:26Z

src/wallet/rpcwallet.cpp

+    if (!EnsureWalletIsAvailable(request.fHelp))
+        return NullUniValue;
+
+    if (request.fHelp || request.params.size() < 1 || request.params.size() > 1)


request.params.size() != 1

yup, that was just a copy / pasta

UdjinM6 · 2018-08-17T18:31:06Z

src/wallet/rpcwallet.cpp

+            "1. rounds         (numeric, required) The default number of rounds is " + std::to_string(DEFAULT_PRIVATESEND_ROUNDS) + 
+            " Cannot be more than " + std::to_string(MAX_PRIVATESEND_ROUNDS) + " nor less than " + std::to_string(MIN_PRIVATESEND_ROUNDS) +
+            "\nResult:\n"
+            "true|false        (boolean) Returns true if successful\n"


There is no way for this to fail, should have no Results at all imo.

I agree, was just following the setup of the rpc command above it setfee I think

UdjinM6 · 2018-08-17T18:34:09Z

src/wallet/rpcwallet.cpp

+            + HelpExampleRpc("setprivatesendrounds", "16")
+        );
+
+    LOCK2(cs_main, pwalletMain->cs_wallet);


Not needed. We should probably have some PS client specific lock but this is out of the scope of this PR.

UdjinM6 · 2018-08-17T18:34:47Z

src/wallet/rpcwallet.cpp

+    LOCK2(cs_main, pwalletMain->cs_wallet);
+
+    // Amount
+    CAmount nRounds = AmountFromValue(request.params[0]);


Mixing rounds are ints.

how would I grab that from a Value

request.params[0].get_int() ?

since it compiled, I thought there must just be a conversion form CAmount to int as CAmount is an int of some size if I remember right

Yeah, CAmount is just a int64, so if I just define it as
int nRounds = AmountFromValue(request.params[0]);
I would think that would work... Although I'm not even sure that's necessary
Thoughts?

I think you are overcomplicating things a bit here :)

privateSendClient.nPrivateSendRounds = request.params[0].get_int();

This ^^^ should do the job just fine without the need for 1) temporary variable and 2) conversions back and forth.

UdjinM6 · 2018-08-17T18:35:17Z

src/wallet/rpcwallet.cpp

+
+    privateSendClient.nPrivateSendRounds = nRounds;
+
+    return true;


Should have no result (see above) and simply call return NullUniValue; here.

UdjinM6 · 2018-08-17T18:38:58Z

src/wallet/rpcwallet.cpp

+    if (request.fHelp || request.params.size() < 1 || request.params.size() > 1)
+        throw std::runtime_error(
+            "setprivatesendamount amount\n"
+            "\nSet the goal amount for PrivateSend mixing.\n"


Pls insert in " + CURRENCY_UNIT + " after amount.

UdjinM6 · 2018-08-17T18:39:21Z

src/wallet/rpcwallet.cpp

+            "\nArguments:\n"
+            "1. amount         (numeric, required) The default amount is " + std::to_string(DEFAULT_PRIVATESEND_AMOUNT) +
+            " Cannot be more than " + std::to_string(MAX_PRIVATESEND_AMOUNT) + " nor less than " + std::to_string(MIN_PRIVATESEND_AMOUNT) +
+            "\nResult:\n"


Same as for setprivatesendrounds.

UdjinM6 · 2018-08-17T18:39:30Z

src/wallet/rpcwallet.cpp

+            + HelpExampleRpc("setprivatesendamount", "208")
+        );
+
+    LOCK2(cs_main, pwalletMain->cs_wallet);


Same as for setprivatesendrounds.

UdjinM6 · 2018-08-17T18:39:39Z

src/wallet/rpcwallet.cpp

+    LOCK2(cs_main, pwalletMain->cs_wallet);
+
+    // Amount
+    CAmount nAmount = AmountFromValue(request.params[0]);


Same as for setprivatesendrounds.

UdjinM6 · 2018-08-17T18:39:46Z

src/wallet/rpcwallet.cpp

+
+    privateSendClient.nPrivateSendAmount = nAmount;
+
+    return true;


Same as for setprivatesendrounds.

PastaPastaPasta · 2018-08-17T20:09:07Z

@UdjinM6 All changes applied 😄

UdjinM6 · 2018-08-17T20:12:24Z

src/wallet/rpcwallet.cpp

+
+    int nRounds = request.params[0].get_int();
+
+    privateSendClient.nPrivateSendRounds = nRounds;


Why not squashing 2 lines into 1 i.e. privateSendClient.nPrivateSendRounds = request.params[0].get_int()? (same for amount)

Ah, wait a minute... you must also make sure that values actually fin into min/max.

EDIT: e.g

privateSendClient.nPrivateSendRounds = std::min(std::max(request.params[0].get_int(), MIN_PRIVATESEND_ROUNDS), MAX_PRIVATESEND_ROUNDS);

EDIT2: fixed copy/paste mistake in the code example above 🙈

Would that be best, or would it be better to return an error / return false if it doesn't fit in the range?

Hmm.. good idea 👍 But instead of returning false should probably throw an error i.e. smth like

if (nRounds < MIN_PRIVATESEND_ROUNDS || nRounds > MAX_PRIVATESEND_ROUNDS) throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid number of rounds");

PastaPastaPasta · 2018-08-20T16:13:36Z

Implemented that check + error throw @UdjinM6

UdjinM6

utACK

- Related to dashpay/dash#2230

@gladcow

…ashpay#2230) * adds rpc calls for `setprivatesendrounds` and `setprivatesendamount` * tabs -> spaces * @gladcow change request * Whops tab -> spaces * @Udjin changes, not the CAmount -> int * int stuff * Throw error when rounds / amount isn't within range

adds rpc calls for setprivatesendrounds and setprivatesendamount

8ae1928

PastaPastaPasta commented Aug 17, 2018

View reviewed changes

tabs -> spaces

abec981

Paul added 2 commits August 17, 2018 11:41

@gladcow change request

e141f7f

Whops tab -> spaces

0cc683a

UdjinM6 requested changes Aug 17, 2018

View reviewed changes

UdjinM6 added this to the 12.4 milestone Aug 17, 2018

Paul added 2 commits August 17, 2018 14:32

@Udjin changes, not the CAmount -> int

8d3e10c

int stuff

db62a34

UdjinM6 reviewed Aug 17, 2018

View reviewed changes

Throw error when rounds / amount isn't within range

9cc9719

UdjinM6 approved these changes Aug 20, 2018

View reviewed changes

UdjinM6 merged commit 98ed90c into dashpay:develop Aug 21, 2018

PastaPastaPasta deleted the rpcAdds branch September 15, 2018 02:08

thephez added a commit to thephez/dash-docs that referenced this pull request Oct 23, 2018

Content - RPC - Add PS amount and round

af714fc

- Related to dashpay/dash#2230

thephez added a commit to dash-docs/dash-docs that referenced this pull request Nov 19, 2018

Content - RPC - Add PS amount and round

4be282f

- Related to dashpay/dash#2230


		int nRounds = request.params[0].get_int();

		privateSendClient.nPrivateSendRounds = nRounds;

Conversation

PastaPastaPasta commented Aug 17, 2018

Uh oh!

PastaPastaPasta Aug 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gladcow commented Aug 17, 2018

Uh oh!

PastaPastaPasta commented Aug 17, 2018

Uh oh!

gladcow commented Aug 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta commented Aug 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

UdjinM6 Aug 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

UdjinM6 Aug 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta commented Aug 20, 2018

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

PastaPastaPasta Aug 17, 2018 •

edited

Loading

UdjinM6 Aug 17, 2018 •

edited

Loading

UdjinM6 Aug 17, 2018 •

edited

Loading