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

multi: Add tpl update reason to work ntfns. #1923

Merged
merged 2 commits into from
Oct 14, 2019

Conversation

davecgh
Copy link
Member

@davecgh davecgh commented Oct 13, 2019

This updates the new RPC websocket work notification that is sent when a new template is generated and a websocket client requests updated via notifywork to include the reason the template was generated.

The text-based reasons delivered via the work notification are:

  • newparent -- The template builds on a new block as compared to the previous one.
  • newvotes -- A new vote for the block the template builds on has been received.
  • newtxns -- New non-vote transactions are available and have potentially been included.

This allows pools that subscribe for work notifications to better choose when to signal workers to throw away all current work and start working on a new template immediately. In particular, the newparent and newvotes reasons should trigger workers to immediately switch. On the other hand new work as the result of newtxns is something that pools will likely want to allow workers to opportunistically switch to after completing a a work cycle.

It is also important to note that miners should roll time timestamps as they work on templates in between updates to keep the timestamps as close to accurate as possible.

This consists of two commits to make the review easier.

The first commit exports the template update reason type and associated constants in preparation to modify the template notifications to include the reason a new block template was generated.

The second commit modifies the background template generator subscription notifications, the RPC websockets notification manager, and the JSON-RPC work notification to include the reason and updates all of the intermediate plumbing code accordingly.

It also updates the JSON-RPC documentation to include the new field and enumerate the supported reasons.

@davecgh davecgh added this to the 1.5.0 milestone Oct 13, 2019
@davecgh davecgh force-pushed the mining_notifywork_tpl_update_reason branch 2 times, most recently from e5ba780 to 3f7776e Compare October 14, 2019 07:46
@davecgh
Copy link
Member Author

davecgh commented Oct 14, 2019

For reference, in addition to doing a significant amount of corner case testing with simnet before submitting this PR, I let it run over the weekend on mainnet while logging the results, . There were only two cases where the initial newparent work notification had fewer than maximum cast votes (meaning the timeout provided for votes to arrive is set reasonably) and both cases were followed up with a newvotes work notification that included all votes as soon as they arrived.

@davecgh davecgh force-pushed the mining_notifywork_tpl_update_reason branch from 3f7776e to e63d73b Compare October 14, 2019 16:51
This exports the template update reason type and associated constants in
preparation to modify the template notifications to include the reason a
new block template was generated.
This updates the new RPC websocket work notification that is sent when a
new template is generated and a websocket client requests updated via
notifywork to include the reason the template was generated.

In order to accomplish this, it modifies the background template
generator subscription notifications, the RPC websockets notification
manager, and the JSON-RPC work notification to include the reason and
updates all of the intermediate plumbing code accordingly.

It also updates the JSON-RPC documentation to include the new field and
enumerate the supported reasons.
@davecgh davecgh force-pushed the mining_notifywork_tpl_update_reason branch from e63d73b to fb9b966 Compare October 14, 2019 19:32
@davecgh davecgh merged commit fb9b966 into decred:master Oct 14, 2019
@davecgh davecgh deleted the mining_notifywork_tpl_update_reason branch October 14, 2019 19:57
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.

3 participants