Blocks more than 12 mins into the future appear to lead to peer banning

[tromp]

Block header deserialization fails in

https://github.com/mimblewimble/grin/blob/450d235358bb14016c1c8e26d036526e6bf3af36/core/src/core/block.rs#L422-L432

if the timestamp is more than 12 mins into the future, returning a CorruptedData error. Presumably this leads to the banning of the sending peer.

But this happens almost randomly if a miner sets a timestamp nearly 12 mins into the future due to natural variation in local clock times, which would cause random peer banning left and right.

It would seem preferable to just ignore the block, rather than declare it corrupted?!

[antiochp]

Presumably this leads to the banning of the sending peer.

I may be wrong but I don't believe a CorruptedData results in peer banning. We definitely ignore the block. We may drop the peer connection. But I'm relatively certain we do not ban the peer.

These are non-bad errors (bad results in a ban) - https://github.com/mimblewimble/grin/blob/d3598e25eb46dc2ee3f03ca78a72c17323f273a5/chain/src/error.rs#L183-L196

Edit: This code is a little confusing to follow - I'm not actually sure what happens with a CorruptedData error and how/where it gets converted into an error in the block processing pipeline.

We should definitely investigate further.

[tromp]

I notice we return CorruptedData on future timestamp, which is not a bannable offense, and on invalid PoW, which does seem to be a bannable offense. Perhaps the header deserialize needs more informative error codes?

[antiochp]

Also the distinct possibility that we do not ban peers due to the timestamp but then in the process of dropping the connection (and presumably reconnecting shortly afterwards) something happens that results in one end of that peer-peer connection getting banned.

[antiochp]

From looking over this code again briefly - it does appear that bann-able offenses occur only once the block has been successfully deserialized and the block enters the processing pipeline.

Deserialization errors will only ever result in the msg and underlying peer connection getting dropped.

Which suggests the PoW verify_size() is not happening at the right level of abstraction... (assuming we only ban during pipeline). Or @tromp as you say - maybe we need more fine grained error codes during deserialization, with some being bannable offenses.

[antiochp]

There are 5 places in peers.rs where we can explicitly ban a peer during processing -

• block_received (processing a full block, after successfully deserializing it)
• compact_block_received (ditto for compact block)
• header_received (ditto for header)
• headers_received (ditto for sync headers)
• txhashset_write (when handling txhashset zip archive)

We can also explicitly ban a peer if we request headers during sync and the peer fails to provide them.

The only other place we ban peers is via the ban_peer api endpoint.