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

suggestion for introduce getblocktemplate-rpcs feature #5418

Merged
merged 2 commits into from
Oct 18, 2022

Conversation

arya2
Copy link
Contributor

@arya2 arya2 commented Oct 18, 2022

Motivation

This is a suggestion (or potential follow-up) for PR #5357 - we could add a new RPC trait with just the field that's currently being used and easily add all of the rest of the fields as they become necessary.

Review

@oxarbitrage can review & merge or close this PR.

@arya2 arya2 requested a review from oxarbitrage October 18, 2022 19:56
@arya2 arya2 requested a review from a team as a code owner October 18, 2022 19:56
@arya2 arya2 self-assigned this Oct 18, 2022
@arya2 arya2 removed the request for review from a team October 18, 2022 19:56
@github-actions github-actions bot added the C-feature Category: New features label Oct 18, 2022
@arya2 arya2 added P-Optional ✨ and removed C-feature Category: New features labels Oct 18, 2022
@github-actions github-actions bot added the C-feature Category: New features label Oct 18, 2022
@arya2 arya2 added A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG labels Oct 18, 2022
Copy link
Contributor

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

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

I think this looks good.

I am concerned on how easy it will be to pass the rest of the fields we are going to need like state or mempool. Do you think it will be better to do that now ?

@arya2
Copy link
Contributor Author

arya2 commented Oct 18, 2022

I think this looks good.

I am concerned on how easy it will be to pass the rest of the fields we are going to need like state or mempool. Do you think it will be better to do that now ?

I thought it might be difficult to add the mempool but then I tried adding everything except the queue_sender to check and it was very easy.

Copy link
Contributor

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

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

Yea, this is what i wanted initially. Thanks for making it happen. I think we should use it instead of what we have.

@oxarbitrage oxarbitrage merged commit 3891e77 into issue5305 Oct 18, 2022
@oxarbitrage oxarbitrage deleted the new-rpc-trait branch October 18, 2022 20:49
mergify bot pushed a commit that referenced this pull request Oct 19, 2022
* introduce `getblocktemplate-rpcs` feature

* reorder imports

* highlight the problem

* add docs

* add additional empty state test

* add snapshot test

* remove getblocktemplate trait

* use `cfg-if`

* leave server as it was

* add a missing space to the docs

* fix typo in test

Co-authored-by: Arya <aryasolhi@gmail.com>

* suggestion for introduce `getblocktemplate-rpcs` feature (#5418)

* adds minimal new object for the get block template methods

* updated TODO comment

Co-authored-by: Arya <aryasolhi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement C-feature Category: New features C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants