Minor error message fix #1160

petertodd commented at 8:35 PM on April 28, 2012: contributor

No description provided.

in src/bitcoinrpc.cpp:None in 8a90c2900d outdated

 998 | @@ -999,7 +999,9 @@ Value addmultisigaddress(const Array& params, bool fHelp)
 999 |          strAccount = AccountFromValue(params[2]);
1000 |  
1001 |      // Gather public keys
1002 | -    if ((nRequired < 1) || ((int)keys.size() < nRequired))
1003 | +    if (nRequired < 1)
1004 | +        throw runtime_error("must require at least one key");

sipa commented at 10:36 PM on April 28, 2012:

"must require" sounds strange

petertodd commented at 11:00 PM on April 28, 2012:

See my update below for a better version. Also a small grammer fix.

in src/bitcoinrpc.cpp:None in f3d2fd5c73 outdated

 985 | @@ -986,7 +986,7 @@ Value addmultisigaddress(const Array& params, bool fHelp)
 986 |      if (fHelp || params.size() < 2 || params.size() > 3)
 987 |      {
 988 |          string msg = "addmultisigaddress <nrequired> <'[\"key\",\"key\"]'> [account]\n"
 989 | -            "Add a nrequired-to-sign multisignature address to the wallet\"\n"
 990 | +            "Add an n-required-to-sign multisignature address to the wallet\"\n"

Diapolo commented at 10:15 PM on April 29, 2012:

Should read "Add a n-required...", as "an" goes before words that begin with vowels. Perhaps the devs intended to use the spelling "nrequired"?

petertodd commented at 10:51 PM on April 29, 2012:

Ha, yeah you're right. Elementary school was a long time ago...

The term nrequired is always capitalized as nRequired elsewhere in the code, so I think it's meant to read as two words.

sipa commented at 1:38 AM on April 30, 2012:

n is just the prefix given to variable names that hold numbers

petertodd commented at 2:06 AM on April 30, 2012:

Ahh, Hungarian.

Since it's user facing though, I'd still use the english writing convention of n-required for the rpc docs.

jgarzik commented at 10:05 PM on May 1, 2012: contributor

NAK for "Grammer", ACK for "fixed non-sensical error message"

petertodd commented at 11:02 PM on May 1, 2012: contributor

Sure, I'll delete "grammer" if that's the consensus.

sipa commented at 12:44 AM on May 2, 2012: member

Not sure whether it's intentional or not, but it's called "grammar" in English.

petertodd commented at 1:12 AM on May 2, 2012: contributor

I need to follow this up for the pull request "Me and my buddies fixed them spelling."

Diapolo commented at 12:58 PM on May 3, 2012: none

NACK, as he included an already merged commit.

sipa commented at 1:01 PM on May 3, 2012: member

Oh, thanks for noticing.

Diapolo commented at 3:22 PM on May 3, 2012: none

@retep You have to rebase to current master, so that only your commit goes into this pull-req. e.g. rebase origin upstream

Fixed non-sensical error message

Previously trying to create a multisig address that required less than
one signature would output something like the following:

"wrong number of keys(got 1, need at least 0)"

86c47a5636

petertodd commented at 4:42 AM on May 4, 2012: contributor

@Diapolo Thanks. I think I fixed it.

Sorry, this is the first time I've used github pull requests before.

Diapolo commented at 4:47 AM on May 4, 2012: none

Looks good now :) and I had a hard time learning Git, too ... don't worry.

gmaxwell referenced this in commit 5b8def797c on May 5, 2012

gmaxwell merged this on May 5, 2012

gmaxwell closed this on May 5, 2012

coblee referenced this in commit fb3a901f86 on Jul 17, 2012

lateminer referenced this in commit c191d25bf2 on Jan 22, 2019

lateminer referenced this in commit 7299031b85 on Jan 22, 2019

lateminer referenced this in commit 88ddd1a947 on Dec 25, 2019

Bushstar referenced this in commit 2e4208bb51 on Oct 21, 2020

DrahtBot locked this on Sep 8, 2021