Fix RIPD-166.

[rec]

Most minimal fix to 166.

See discussion here: https://docs.google.com/a/ripple.com/document/d/1kdP4AGWaEAh8XQu8J4wagT3diwC-uSKbv3amULs3hLU/edit#

[JoelKatz]

A few issues here:

1. If verify is requested and the server is not sufficiently online, the normal error we return in this case should be returned, not BAD_SECRET.

2. If verify is requested and the master key is disabled, signing with the master key should fail verification.

3. If verify is requested and a regular key is set, signing with the regular key should pass verification.

[rec]

And David also said: You can look at Transactor::checkSig to see what that existing checks look like.

[vinniefalco]

Please add some lines to the commit message describing the bug, don't just refer to the JIRA issue. Put a colon on the first line to indicate that there's a continuation.

[rec]

1. There don't seem to be any other cases where we cause an error because we're offline. Nor is there an error case due to an account being disabled. I'm proposing adding two new RPCErrors for these cases.

2), 3): Sorry for the delay, I've been reading a bunch of code. To summarize my question:

a) Transactor::checkSig has a SerializedLedgerEntry
b) I seem to need two methods on that SLE, isFlag(lsfDisableMaster) and .getFieldAccount160 (sfRegularKey)
c) But I can't create an SLE without a Ledger, which I don't seem to have here or in the handlers...

Ideas?

[rec]

Hold off on reviewing this - Arthur and I just clarified a lot of things, and I'm going to want to make changes for his review.

Thanks!

[JoelKatz]

"There don't seem to be any other cases where we cause an error because we're offline."

Try an account_info when the server's not online and you'll get something like:

{
   "result" : {
      "error" : "noCurrent",
      "error_code" : 12,
      "error_message" : "Current ledger is unavailable.",
      "request" : {
         "account" : "rMPcaTgu5qEFKhJjXQs7iRv8dTiir35Cqz",
         "command" : "account_info"
      },
      "status" : "error"
   }
}

"But I can't create an SLE without a Ledger, which I don't seem to have here or in the handlers..."

Then how do you fill in the fee or the sequence number? I know this code does those things, and they're not possible without a ledger. Look for

    Ledger::pointer lSnapshot = mNetOps->getCurrentLedger ();
    AccountState::pointer asSrc ...

[rec]

It turns out that bOffline doesn't actually mean that the server is
"offline". :-D

See corrected change coming up...

On Thu, Apr 24, 2014 at 1:52 PM, JoelKatz notifications@github.com wrote:

"There don't seem to be any other cases where we cause an error because
we're offline."

Try an account_info when the server's not online and you'll get something
like:

{
"result" : {
"error" : "noCurrent",
"error_code" : 12,
"error_message" : "Current ledger is unavailable.",
"request" : {
"account" : "rMPcaTgu5qEFKhJjXQs7iRv8dTiir35Cqz",
"command" : "account_info"
},
"status" : "error"
}
}

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/326#issuecomment-41311154
.

 /t

http://radio.swirly.com - art music radio 24/7 366/1000

[rec]

All right!

I think I dealt with all the issues - also two tiny new changes:

1. Now the command line RPC command "submit" takes an "offline" flag.
2. I tweaked Mark SerializedObject.getFlags() to be const.

[rec]

I got comments from Arthur:

1. Check existence of regular key field before trying to get it (OOPS!)
2. Add an error rpcBAD_SECRET_MASTER because we now have two error cases.
3. Add logging.

ongoing!

[rec]

I did two out of three of these. There's still only one error case, so I didn't change the error message.

Check this out - I'm going to run the tests I have before plus:

• testing an account with a disabled master key.
• testing an account that doesn't even have a regular key (which would have failed before).

[rec]

I just did these tests and all worked fine. I believe this is ready to go.

[ahbritto]

Looks good to me.