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: Add getmempoolentry API #905

Merged
merged 1 commit into from
Feb 1, 2017
Merged

btcjson: Add getmempoolentry API #905

merged 1 commit into from
Feb 1, 2017

Conversation

dajohi
Copy link
Member

@dajohi dajohi commented Jan 23, 2017

Fixes #908

@FrozenPrincess
Copy link

<3

BTW the struct GetRawMempoolVerboseResult and GetMempoolEntryResult should actually be the same thing (although at the moment GetRawMempoolVerboseResult is missing fields), so it might be best to typedef them or something

@dajohi
Copy link
Member Author

dajohi commented Jan 23, 2017

Thanks for pointing that out. I'll do that after btcd gets support for the new fields in getrawmempool verbose.

@davecgh davecgh added this to the 0.13.0 milestone Jan 24, 2017
Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

This PR needs to have several things happen as detailed below. I would prefer each of these points grouped into their own commit within the PR.

  1. btcjson: Create shared mempoolEntryResult.
    Create a new shared result type for mempoolEntryResult and update the GetRawMempoolVerboseResult to use it (aka type GetRawMempoolVerboseResult mempoolEntryResult)
  2. btcjson: Add getmempoolentry
    Add the command, result, and tests (effectively what this PR already does without touching rpcserver.go)
  3. rpcserver: Implement getmempoolentry
    Add the bits needed by the RPC server to recognize the command and generate the result, update the rpcserver hlep, and update the JSON-RPC API docs

@@ -333,6 +333,19 @@ func NewGetInfoCmd() *GetInfoCmd {
return &GetInfoCmd{}
}

// GetMempoolEntryCmd defines the getmempoolentry JSON-RPC command.
type GetMempoolEntryCmd struct {
TxID string
Copy link
Member

Choose a reason for hiding this comment

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

TxID string `json:"txid"`

Copy link
Member

@jrick jrick Jan 24, 2017

Choose a reason for hiding this comment

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

doesn't need it actually. json-rpc request parameters are positional, not keyed in an object.

Copy link
Member

Choose a reason for hiding this comment

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

Ah right, I noticed it was on one of the others (GetHeadersCmd). Given it's positional though, you're right it isn't needed.

Copy link
Member

Choose a reason for hiding this comment

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

Yep.. I added it to that on accident. We can remove it but it doesn't really matter, it just gets ignored.

Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

After discussing this further in chat, we've decided that while the aforementioned things need to happen in order to ultimately properly serve the getmempoolentry RPC and provide consistency between that RPC and the existing getrawmempool RPC, the information needed to populate all of the fields is not currently available, so even though those things will need to ultimately happen, the intention of this PR is effectively just making the data structure available so btcrpcclient has access to it.

We could certainly just register it as a custom type from btcrpcclient, but ultimately we're going to want to provide all of these data and implement the getrawmempool RPC anyways, so it makes sense to go ahead and add it to btcjson now.

OK

@davecgh davecgh merged commit d9241e9 into btcsuite:master Feb 1, 2017
@dajohi dajohi deleted the getmempoolentry branch February 28, 2019 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants