-
Notifications
You must be signed in to change notification settings - Fork 120
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
Fix duplicate transparent spends in getblocktemplate
RPC responses
#6195
Comments
getblocktemplate
RPC responsesgetblocktemplate
RPC responses
How often do these happen in practice? |
I've seen them happen twice today, but not at all for the last week. And we haven't really changed that code. So it could depend on network conditions (which means it could be remotely triggerable with the right transactions in the mempool). And in that case it would be a serious must-fix bug that blocks production use. But to get a better idea, we should also monitor CI for these kinds of failures, and write them down on this ticket. |
We could also do a detailed analysis of the transactions in the mempool and blocks in the chain to work out the source of the bug. They are in the logs in the ticket. |
@gustavovalverde just letting you know we're tracking main branch and PR failures in the getblocktemplate validation tests in this ticket. |
This error happened because Zebra created a block with a transaction that spent a transparent coinbase output at height 1991093. But that output was not allowed to be spent until height 1991094. This seems like either:
https://github.com/ZcashFoundation/zebra/actions/runs/4227058491/jobs/7341912371#step:8:434 |
Hey team! Please add your planning poker estimate with Zenhub @arya2 @conradoplg @dconnolly @oxarbitrage @teor2345 @upbqdn |
The duplicate spend issue just happened right after a block proposal check was interrupted by a long poll response: #6276 (comment) That transaction with the duplicate spend was on the best chain almost 100 blocks before the template height. That one should not be due to #6173. Zebra is missing a check for immature coinbase spends in the transaction verifier for mempool requests (it should be skipped for block transactions because blocks are queued so the coinbase spend could become valid if parent blocks are committed).
We could run the I think we'll have to mine testnet blocks then try spending the coinbase immediately after to test that transactions with immature transparent coinbase spends are rejected from the mempool. |
I split out the missing check for immature transparent spends into #6482 so it can be done in parallel. |
This ticket contains some errors that have been fixed. So I'm not sure what errors we still need to fix. @arya2 can you please either open a new ticket, or update the description of this ticket? |
getblocktemplate
RPC responsesgetblocktemplate
RPC responses
Motivation
If we send invalid templates to miners, they will make invalid blocks. This wastes mining power and loses the ZEC they could have mined.
These invalid templates are happening in PRs and on the
main
branch.This could be caused by #6173, but we've seen an unrelated failure in CI so there's likely another unknown cause.
Logs
Click for details...
https://github.com/ZcashFoundation/zebra/actions/runs/4385126571/jobs/7678687241#step:8:223
https://github.com/ZcashFoundation/zebra/actions/runs/4228636416/jobs/7344682318#step:8:200
Specifications
These rules are already checked for blocks in the state:
Complex Code or Requirements
This is likely to be a concurrency bug. We don't want to do expensive checks because it slows down mining in the typical case where the template is valid.
Testing
Testing is going to be a bit tricky here, any ideas?
Related Work
We tried to fix this before in:
The text was updated successfully, but these errors were encountered: