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.

Sign up for GitHub

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

Added public_key to ValidatorList::getAvailable response #3488

Closed
wants to merge 9 commits into from
Closed

Added public_key to ValidatorList::getAvailable response #3488

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
91704a4
Added public_key to ValidatorList::getAvailable response
natenichols Jul 2, 2020
1fb4ef7
changed string literals to JSON static strings
natenichols Jul 8, 2020
52446e3
changed string literals to JSON static strings
natenichols Jul 8, 2020
1035d56
removed whitespace so clang-format will pass
natenichols Jul 8, 2020
9c7c0a5
added unit tests for ValidatorList::getAvailable
natenichols Jul 11, 2020
febeeeb
added unit tests for ValidatorList::getAvailable
natenichols Jul 11, 2020
bc43f0d
formatted tests
natenichols Jul 11, 2020
e0fdf03
changed test formatting
natenichols Jul 13, 2020
8a63ff3
ran clang-format
natenichols Jul 13, 2020
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
1 change: 1 addition & 0 deletions src/ripple/app/misc/impl/ValidatorList.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,7 @@ ValidatorList::getAvailable(boost::beast::string_view const& pubKey)

Json::Value value(Json::objectValue);

value["public_key"] = std::string{pubKey};
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change looks great, as far as it goes. However the area that this change is in has some bugs. Rather than let the bugs spread I'd like you to squash these bugs while you are in the vicinity.

Our interfaces are set up so that programmers can use literal strings to supply parameters to our interfaces. But literal strings are error prone and subject to things like typos. So we have places that we like to get common user interface strings from. This helps to assure that the always spell (and capitalize) strings in our interfaces the same way.

So, in place of "public_key" I'd like you to use jss::public_key.

That same sort of change needs to also be made for the four other near-by quoted strings. So the final result will look something like...

    value[jss::public_key] = std::string{pubKey};
    value[jss::manifest] = iter->second.rawManifest;
    value[jss::blob] = iter->second.rawBlob;
    value[jss::signature] = iter->second.rawSignature;
    value[jss::version] = iter->second.rawVersion;

Once you've done this you'll get a compile error. It turns out that a jss string (jss stands for JSON Static String) has not yet been defined for "blob". So you'll need to add that. To do so navigate to here:

https://github.com/ripple/rippled/blob/develop/src/ripple/protocol/jss.h#L146

This file contains all of the jss strings in alphabetical order. You'll add one for "blob" right above the line that says JSS(books);. The final result should look something like this:

JSS(blob);                   // out: ValidatorList
JSS(books);                  // in: Subscribe, Unsubscribe

Then, assuming everything builds and passes unit tests, I think we're good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed the strings to jss and added "blob" to jss.h. I'll talk to @ximinez about adding Validator_List unit tests for this PR.

Thanks Scott!

value["manifest"] = iter->second.rawManifest;
value["blob"] = iter->second.rawBlob;
value["signature"] = iter->second.rawSignature;
Expand Down