master submitblock fails to detect some validation failures #5083

1. 
   luke-jr commented at 9:44 PM on October 13, 2014: member

   Part of block checking was moved out of ProcessBlock (asynchronously), so submitblock may end up returning success when the block in fact fails.

2. gmaxwell added this to the milestone 0.10.0 on Oct 13, 2014
3. 
   sipa commented at 9:53 PM on October 13, 2014: member

   It's not really moved out. The ProcessBlock logic just inserts a block after basic checks into the block database, and then calls ActivateBestChain to reorganize until a stable point is reached. Since inserting a block into the database may cause anything between 0 and infinity blocks to processed, it doesn't really make sense for it to return a specific rejection reason anymore (furthermore, we need to handle those rejection reasons internally during the reorg to find what to do next).

   This affects #1816 and #3727 too (I've mentioned this problem in #3727, but wasn't aware it had further implications).

   There are two solutions, IMHO:

   • Either have a more generic callback mechanism to be informed about block validity checks when they happen (rather than just a nodeid now), so submitblock etc can install their own handler to catch the result and report about it, in addition to ProcessBlock installing a nodeid listener that does BIP61 rejection notice or DOS banning.
   • Make submitblock call ConnectBlock directly (which should always work if its predecessor is the current tip, and otherwise submitblock should already fail anyway). The problem with this is some logic which is in AcceptBlock rather than ConnectBlock or CheckBlock, so would need to be duplicated (in particular, BIP34 version checks). Moving those should be easy, though.

   The first solution is more generic and allows for better decoupling of network handling and consensus rules, but the second is probably faster to do.

4. 
   luke-jr commented at 10:01 PM on October 13, 2014: member

   Do we really want to be putting potentially-invalid blocks in the block database in the first place?

5. 
   sipa commented at 10:04 PM on October 13, 2014: member

   I don't want to cause a reorg for every side side branch we receive just to be able to validate it, only to reorg back afterwards because the earlier branch is just as long or longer anyway.

6. 
   sipa commented at 10:05 PM on October 13, 2014: member

   Also, receiving a block can cause connection of successor blocks (orphans pre-HF, unlinked blocks post-HF).

7. 
   luke-jr commented at 10:20 PM on October 13, 2014: member

   @sipa Why would we receive a block if we aren't planning to reorg on it? O.o

   Besides, can't we just "reorg" in memory now anyway (and not affect the main tip)?

8. 
   sipa commented at 11:38 PM on October 13, 2014: member

   @luke-jr (in HF) we download any block along a chain that is a potential improvement to the one we have currently as active. But we can only reorg to it if all ancestor blocks are available, which is potentially never.

   There can still be competing chains, and you don't know which one you'll end up receiving first. Maybe there are two potentially improving branches, but the longer one has an invalid block, or the peer that has it is too slow. In that case you need to reorg to the shorter of the two anyway.

   And yes, of course we can reorg to anything in memory, but that is slow (it needs signature checks, which we'd rather delay as much as possible) and potentially requires unbounded memory.

9. luke-jr referenced this in commit 1c1ed0623b on Oct 16, 2014
10. luke-jr referenced this in commit c5437a845c on Oct 19, 2014
11. luke-jr referenced this in commit e10aa0403c on Oct 20, 2014
12. laanwj closed this on Nov 3, 2014

    ---

13. MarcoFalke locked this on Sep 8, 2021

Milestone
0.10.0