-
Notifications
You must be signed in to change notification settings - Fork 19
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
Spend coinbase error #47
Comments
Re-running testnet3 synch entirely from scratch shows no such message, somewhat surprisingly. Unrelated, but a block from Sept 2018 produced the following error message but apparently was accepted anyway:
|
This is interesting and I suspect misleading. The Founders address check doesn't apply to blocks below 412300 so it is not required on the block in question. Lets increase the logging around this check and re-sync to see if we can reproduce the error. Let also check that peer and see what protocol version they are on. |
There is a bunch of corner cases during sync that Zcash has been trying to get right since last July and again in November and likely it is still not entirely correct. Getting it right can be very important. Any possibility of a node rejecting or accepting an incoming block due to an ambiguity in accepting transactions within it, due to the stateful and yet asynchronous nature of the sync, with possible confusion about the 'era', may lead to a fork, Whether accidental or malicious, it could prevent recovery from an extended chain split. Philosophically, a syncing node should not be passing any transactions, as it is clearly not equipped to validate them well against a double spend, which is the whole point, perhaps not even accept them into its mempool. Bitcoin style of P2P makes no guarantees regarding individual transaction. Whether a syncing node should be attempting to originate transactions, I'm uncertain, but would lean toward a NO, though that may impact 'user experience'. It is not ready to mine, so there may be a hint there. Whether such a node should be passing blocks is unclear, as it cannot properly validate transactions included. Disabling passing of blocks here may get in the way of P2P information propagation during the restart of the chain. Seems that passing empty blocks is fine by this logic. A frequent set of corner cases is due to the syncing node not knowing prior to getting to processing Sprout block height which 'era' it is running in right now. Since the Zcash logic hardcodes 'overwinter' and 'sapling', they will hit these issues again, with the upgrades coming after Sapling. I'm not convinced that this reasoning is correct, as there could have been a time when Sapling support was compiled in but prior to the fork, but that scenario is long past. A node compiled from any codebase from this year can assume a Sapling or later 'era' on Zcash 'mainnet' and no node with pre-Sapling behavior would successfully run on it. There may be a wrinkle with testnets used to exercise pre-fork states and fork transitions, but hopefully that can be accounted for. Having issues with how the 'synced' state is determined in IsInitialBlockDownload(). There are separate fImporting and fReindex flags in play, however the state of those is maintained. The code plays some trick with latching the 'false' return value and locking, but I see no uplock. Exactly when latchToFalse.store() at the very end of the function is being hit, after checking a whole bunch of scenarios, resulting in return(false) or even true, is unclear and I cannot tell if it is hit at all, since the function can start returning false due to one of the earlier checks always hitting. If latching is appropriate and the reentrancy concern is a real, I'd use code likely to result in just a couple of instructions, like:
Codebase has a confusing aborted attempt to pass to ContextualCheckTransaction() an optional fifth parameter (function pointer) named isInitBlockDownload(). It is initialized to IsInitialBlockDownload() in the declaration, then never used. Meanwhile IsInitialBlockDownload() is used directly in 12 places in main.cpp and 3 times in other .cpp files. This needs digesting, some parts filed with Zcash, perhaps the rest captured in a separate Issue for Zero. |
213.239.212.246:23801 appears to be the same node referenced #11 |
In reviewing debug.log ERROR messages for Zero 2.0.4 on Ubuntu 18.04, found a single instance of:
2019-04-16 11:02:13 ERROR: CheckInputs(): tried to spend coinbase at depth 719
Line 2049 in main.cpp, this may be innocuous, but if not, can pursue further. This is the testnet3, and I did not generated transactions on it.
The text was updated successfully, but these errors were encountered: