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

Don't return from long polling if the block template is an invalid block proposal #6037

Closed
teor2345 opened this issue Jan 27, 2023 · 3 comments
Labels
A-consensus Area: Consensus rule updates A-rpc Area: Remote Procedure Call interfaces C-enhancement Category: This is an improvement I-lose-funds Zebra loses user funds

Comments

@teor2345
Copy link
Contributor

teor2345 commented Jan 27, 2023

Motivation

zcashd returns an error when the block template is not a valid block (ignoring proof of work):
https://github.com/zcash/zcash/blob/9e1efad2d13dca5ee094a38e6aa25b0f2464da94/src/miner.cpp#L674-L676

Mining pools would like to start working on new blocks as soon as possible, but they are ok with some delays before seeing updated transactions.

Design

  1. If we are not long polling, return the template immediately.
  2. If we are long polling, and the tip height or hash has changed, or the maximum time has elapsed, return the template immediately.
  3. If we are long polling, and only the transactions in the mempool have changed, check the template as a proposal before returning, and continue long polling (don't return) if the new template is invalid.

Specifications

This checks all the block consensus rules:
https://zips.z.cash/protocol/protocol.pdf#blockheader
https://zips.z.cash/protocol/protocol.pdf#txnencoding

Complex Code or Requirements

We already have a function to create a block proposal and submit it to the state in our tests, so this should be simple to implement.

We should check performance long polling on large blocks before and after this change.

Testing

This change would add a runtime test for some block proposals in production code. We already do these tests in CI.

@teor2345 teor2345 added C-bug Category: This is a bug A-consensus Area: Consensus rule updates S-needs-triage Status: A bug report needs triage P-Medium ⚡ A-rpc Area: Remote Procedure Call interfaces I-lose-funds Zebra loses user funds A-concurrency Area: Async code, needs extra work to make it work properly. labels Jan 27, 2023
@mpguerra mpguerra added this to Zebra Jan 27, 2023
@github-project-automation github-project-automation bot moved this to 🆕 New in Zebra Jan 27, 2023
@teor2345
Copy link
Contributor Author

teor2345 commented Jan 27, 2023

We don't know if miners prefer performance or checking correctness here.

@teor2345 teor2345 changed the title Return a RPC error if the block template is an invalid block proposal Don't return from long polling if the block template is an invalid block proposal Jan 30, 2023
@teor2345 teor2345 added P-Low ❄️ and removed P-Optional ✨ A-concurrency Area: Async code, needs extra work to make it work properly. labels Jan 30, 2023
@teor2345 teor2345 added C-enhancement Category: This is an improvement and removed C-bug Category: This is a bug labels Jan 30, 2023
@teor2345
Copy link
Contributor Author

I split the complicated parts of this ticket into #6047.

@mpguerra
Copy link
Contributor

@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Mar 16, 2023
@mpguerra mpguerra closed this as not planned Won't fix, can't repro, duplicate, stale Aug 23, 2023
@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in Zebra Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Area: Consensus rule updates A-rpc Area: Remote Procedure Call interfaces C-enhancement Category: This is an improvement I-lose-funds Zebra loses user funds
Projects
Archived in project
Development

No branches or pull requests

2 participants