Page MenuHomePhabricator

Added RPCCommand and associated classes
AbandonedPublic

Authored by jasonbcox on Jul 26 2018, 00:02.

Details

Reviewers
deadalnix
schancel
Group Reviewers
Restricted Project
Summary

These classes begin to build the foundation for the new RPC system. The server will register RPCCommands, which are made up of a name and parameter. Each implementation of RPCCommand must specify an Execute() function. RPCParameter implementations must specify a Validate() function, which can be set to simply return true; if no validation is required.

Definitions for RPCServer and associated RPC command registration to follow in successive diffs.

Test Plan

make check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
newrpc
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 3042
Build 4176: Bitcoin ABC Buildbot (legacy)
Build 4175: arc lint + arc unit

Event Timeline

src/rpc/command.h
24 ↗(On Diff #4431)

Private.

deadalnix requested changes to this revision.Jul 26 2018, 00:12
This revision now requires changes to proceed.Jul 26 2018, 00:12
src/rpc/command.h
27 ↗(On Diff #4431)

What's this for? Can you give me an example?

src/rpc/command.h
27 ↗(On Diff #4431)

Pseudo-code example:
RPCCommand("getblocktemplate", {GBTMode, GBTCapabilities})
Such that GBTMode and GBTCapabilities are instantiations of base classes for those particular parameters, similar to (again, pseudo-code):

class GBTMode : public RPCParameter {

GBTMode() : RPCParameter("mode", "some description of mode")
...
Validate() {
    // some validation for valid values of 'mode'
}
};

The basic idea is RPCCommands will be instantiated in such a way that validation is self-enforced when they are called. This also allows us to generate help messages for every command.

Fixed RPCCommand private members

src/rpc/command.h
27 ↗(On Diff #4431)

It seems like a template system could ensure type safety better than this can.

src/rpc/command.h
39 ↗(On Diff #4440)

You may want a virtual Validate method with a default implementation here that validates all the params before moving on?

After a discussion offline, I'm going to experiment with a solution of templated-RPCParameter class + RPCCommandFactory in order to solve the issue of parsing parameters in a strongly typed manner.

  • Using templates to ensure strongly typed params
  • Introduced RPCCommandFactory to build RPCCommand instances, which will contain params with their values set after validation

Working example implementation for getexcessiveblock:

class GetExcessiveBlockCommand : public RPCCommand {
private:
    const Config &config;

public:
    GetExcessiveBlockCommand(const RPCCommandName &name,
                             std::vector<std::unique_ptr<RPCParameterBase>> params,
                             const Config &configIn)
                         : RPCCommand(name, params), config(configIn) {}

    // No params supported
    void SaveParameterValue(RPCParameterBase &param) {}

    UniValue Execute(const JSONRPCRequest &jsonRequest) const {
        if (jsonRequest.fHelp || jsonRequest.params.size() != 0) {
            throw std::runtime_error(
                "getexcessiveblock\n"
                "\nReturn the excessive block size."
                "\nResult\n"
                "  excessiveBlockSize (integer) block size in bytes\n"
                "\nExamples:\n" +
                HelpExampleCli(GetName().GetNameStr(), "") +
                HelpExampleRpc(GetName().GetNameStr(), ""));
        }

        UniValue ret(UniValue::VOBJ);
        ret.push_back(Pair("excessiveBlockSize", config.GetMaxBlockSize()));
        return ret;
    }   
};

class GetExcessiveBlockFactory : public RPCCommandFactory {
private:
    const Config &config;

public:
    GetExcessiveBlockFactory(const Config &configIn) : config(configIn) {}

    RPCCommandName GetName() const {
        return RPCCommandName("getexcessiveblock");
    }   

    std::vector<std::unique_ptr<RPCParameterBase>> MakeParameters() const {
        return {}; 
    }   

    std::unique_ptr<RPCCommand> NewCommand(const JSONRPCRequest &jsonRequest) const {
        return std::unique_ptr<GetExcessiveBlockCommand>(new GetExcessiveBlockCommand(GetName(), MakeParameters(), config));
    }   
};

Regarding the example, weren't we going to move the Help messages into a special method that executes if validation fails? I thought you didn't want them inline with the normal handling?

schancel requested changes to this revision.Aug 9 2018, 15:12
This revision now requires changes to proceed.Aug 9 2018, 15:12

Regarding the example, weren't we going to move the Help messages into a special method that executes if validation fails? I thought you didn't want them inline with the normal handling?

Yes, this is absolutely true. This was more of a copy-paste just to show that the original implementation works with the new system. Sorry for the confusion.

Talked more offline and determined that RPCCommand needs to be templatized as well.