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.

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

btcjson: use type params to revamp package to be more extensible and eliminate duplication #1934

Open
Roasbeef opened this issue Jan 6, 2023 · 3 comments

Comments

@Roasbeef
Copy link
Member

Roasbeef commented Jan 6, 2023

In the beginning, we would keep up with the Bitcoin Core RPC interface mostly in lock step. This meant for each change, we'd also add the message (or modify) it and try to keep up with that behavior. As the years have passed, the RPC interfaces have drifted more and more over time, and at this point we are no longer explicitly putting in the effort to add every new RPC call, sometimes due to architectural differences.

However, since we don't implement all the calls they have, this means that the rpcclient library start to lack functionality that developers need to build on top of btcd. If a new call Foo is added, then we need to update both the btcjson and the rpcclient packages for this to be available to developers.

With Go 1.18, generics or type params were used. We can use this feature to:

  • Eliminate all the duplicated code in the package
  • Create generic abstractions that allow developers to simply define the new message type they want to use seamlessly

It's worth noting that devs can already use the RawRequest method to issue any arbitrary JSON RPC requests. For this workflow, they'd define the new json struct, then pass it in as params, and then decode the opaque json blob on the other side.


Here's a sketch of what that would look like. Today we end up with a ton of duplicated code, since we implement a custom future type for each message, then also a series of methods to issue an async vs a sync code. Instead, we can rely on interfaces that look something like:

type Response struct {
	Resp json.RawMessage
	Err  error
}

type Command interface {
	Name() string
	Marshall() json.RawMessage
}

type Client interface {
	SendCmd(cmd Command) chan *Response
}

type MessageFuture[M any] interface {
	Receive() (M, error)
}

type RpcCall[Args any, Req Command, Resp any] interface {
	MapReq(arg Args) Req

	MapResp(resp *Response) MessageFuture[Resp]
}

func AsyncRequest[Arg any, Req Command, Resp any, CmdReq RpcCall[Arg, Req, Resp]](
	client Client, cmdReq RpcCall[Arg, Req, Resp], args Arg) MessageFuture[Resp] {

	jsonCmd := cmdReq.MapReq(args)

	resp := client.SendCmd(jsonCmd)

	return cmdReq.MapResp(<-resp)
}

Along the way we can do away with most instances of reflection in the package as well.

Then a new call can be added purely by the developer that needs it by implementing the RpcCall for the new command.

type UtreexoRootRpcCmd {
}

func (u *UtreexoRootReq) Name() {
    return "utreexo"
}

func (u *UtreexoRootReq) Marshall() json.RawMessage {
    // This woudl actually call jsocon.MarshalCmd
    return json.Marshall(u)
}

type UtreexoRoot struct {
    Root chainhash.Hash `json:"utreexo_roots"`
}

type UtreexoRootRpcCall struct {
}

func (u *UtreexoRootRpcCall) MapReq(_ string) *UtreexoRootRpcCmd {
    return &UtreexoRootRpcCmd{}
}
func (u *UtreexoRootRpcCall) MapResp(resp *Response) MessageFuture[UtreexoRoot] {
}
@Eoous
Copy link
Contributor

Eoous commented Jul 31, 2023

From the sketch, maybe we finally get code style like RawRequest. I think they are two different ways to register(wrap) commands, and looks that wrapping RawRequest is more clear. It depend on if user needs to customize command, right?

@Roasbeef
Copy link
Member Author

Roasbeef commented Aug 1, 2023

It depend on if user needs to customize command, right?

Do you mean if the user wants to update a struct with a breaking change vs create an entirely new command? I think in my ideal world, the only thing someone needs to do is define the new command, then the new version of the library takes care of all the other specific, which eliminates a lot of the boiler plate we have. Then at that point, having the command struct in the btcjson package is just a convenience: a project can have their own version and it all works fine (though admittedly it's nice to not have to force people to trawl throuhg bitcoind and fine the latest struct).

I think a success criteria here is that any updates to the set of JSON-RPC calls callable by the rpcclient package needs just a new PR adding the struct to the btcjson package and nothing more. Then using type params, a set of generic calls for async/sync calls are used. We'd then make btcjson an actual go module, so then if people want to go back to a version since they're using an older version of bitcoind, they can just update that dep.

@Eoous
Copy link
Contributor

Eoous commented Aug 26, 2023

An basic example: Eoous@b034c29.

For go version lower than 1.21, we can't use the expression AsyncRequest(FakeClient{}, &GetDifficultyRpcCall{}, nil) to infer parameter type.

We can define a deterministic type to call this, but this code may be slightly unclean.

func NewGetDifficultyRpcCall() RpcCall[NoParamsType, GetDifficultyRpcCmd, GetDifficultyResult] {
	return &GetDifficultyRpcCall{}
}

AsyncRequest(FakeClient{}, NewGetDifficultyRpcCall(), nil)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants