fix(grpc): return bool if mempool is out of sync instead of returning… #6728
Conversation
Test Results (CI) 3 files 129 suites 36m 36s ⏱️ Results for commit 144637d. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
I don't like this. It super dangerous and if there are transactions flowing in the network, 9/10 I think it will provide the miner with a template it cannot use to submit a block.
If the mempool is not in sync with the node, it means it has not processed the new block yet. This means it may and probably will include transactions that are already mined in the block that it has not processed yet and thus the miner will mine a block that contains an already spent output/duplicate tx. This means the base node will reject the mined block.
We added in this error specifically because we where seeing miners submitting invalid blocks and we want the miner to request a new template and not think the template is valid.
|
I have added the check to mmproxy to treat a false in this case as an error. I have started adding it to the gpu miner, but it needs the updated field to compile properly. |
|
There are no transactions in the mempool on nextnet, yet we see this error all the time, so it leads me to believe the error is a false positive |
|
The reason we see this, is a race condition between the read write locks of the base node adding a block and miner asking for a template. If the miner's read requests come in first, it gets an error. Its its very likely that you will see this, and because we have no real transactions on nextnet, we wont see the invalid mined blocks, but we do ask for a lot of templates, so we will see this error a lot. |
|
Why dont we rather change the template request of the mempool, to release the lock and try again. That way we don't propagate the out of sync outside of the base node, |
|
The problem is that Tari Universe is using this call because it is the only one that returns the current block reward. It would be best to create a method that returns all the data for tari universe instead. However, the merge mining proxy already has been changed to treat this data the same as an error, and GPU miner can also be updated. At a later stage you can fix this properly if you wish, but I need this fix more urgently |
… an error
Removed the error "Mempool out of sync, blockchain db advanced passed current tip in mempool storage;" when getting a block template and instead return a bool that indicates such.
The clients that are worried about this can check the flag and request a new template, while clients that are not worried about this can accept the data for what it is