fix(chain_manager): fix some synchronization issues

[drcpu-github]

This PR contains two distinct patches that solve some synchronization issues.

-          let protocol_version = get_protocol_version(self.current_epoch);
+          let protocol_version = get_protocol_version(Some(block.block_header.beacon.checkpoint));

Pretty self-explanatory. Don't use the current epoch for validating block transactions when synchronizing blocks that may be older than the current protocol version.

-               let block_hash = block.hash();
                let block_epoch = block.block_header.beacon.checkpoint;
+               let block_hash = block.versioned_hash(get_protocol_version(Some(block_epoch)));

Somewhat more involved. When I restart a node that is already in V1.8, I noticed it failed to synchronize. It received new blocks, but did not want to add them properly. I added some debug info and found that the block hashes it receives are non-versioned block hashes while it expects versioned ones.

I logged the InventoryAnnouncement message it receives here (for a block starting at epoch 1010):

witnet-rust/node/src/actors/session/handlers.rs

Line 770 in da223fe

session.requested_block_hashes = inv

[
    8fd76b5064d45b205dce9d8e29a388ab2af1db388c906614f213badd4c831009,
    8e70c676d64059076f9295732a94bb164589313433f07965e408116e77de8992,
    0ff6ba671f52c0994bce24d2cb7d61c545451fbcc58c4a8b4fc7ac98e9115330,
    ...
]

And the expected hashes when it receives block messages for epoch 1010:

[<] Received BLOCK #1010: 0e474e3fe73f47689c8bbb7dcd60ec50085d0c88abda5e54e34a48e7ad08cb47 message (699 bytes)
[<] Received BLOCK #1011: 8f39bb88f28eec113368c836f0ee62afd653b85008c42efa7350618e633a92eb message (699 bytes)
[<] Received BLOCK #1012: ac31e5b56174b3961f44c1228f694bd40703913aa58c56774bfdf2ea18898a85 message (699 bytes)

Note the different hashes. The reason for this is because the second hash for block 1010 is a versioned hash with protocol version V1.8 while the former is not.

This results in the following line not evaluating to true:

witnet-rust/node/src/actors/session/handlers.rs

Line 679 in da223fe

if session.requested_block_hashes.contains(&block_hash) {

After which we call AddCandidates which drops the block because of this condition:

witnet-rust/node/src/actors/chain_manager/mod.rs

Line 750 in da223fe

if !(block.block_header.beacon.checkpoint == current_epoch

However, if we use the versioned hash as proposed in the patch, you can see that the block_chain structure will insert the correctly versioned hash here:

witnet-rust/node/src/actors/chain_manager/mod.rs

Line 1322 in da223fe

self.chain_state.block_chain.insert(block_epoch, block_hash);

After which the InventoryAnnouncement which uses GetBlocksEpochRange which reads the block_chain structure should return the correct, expected versioned hash.

Just to confirm, I printed out the non-versioned and versioned hash for epoch 1010:

Consolidating block hash (old: 8fd76b5064d45b205dce9d8e29a388ab2af1db388c906614f213badd4c831009, new: 0e474e3fe73f47689c8bbb7dcd60ec50085d0c88abda5e54e34a48e7ad08cb47) for epoch 1010

As you can see, the old hash matches the hash we receive in the InventoryAnnouncement message block hash array while the new hash matches the expected versioned hash in the received block message which will eventually be processed.

I cannot fully test this works because we'd need a testnet reset since we are essentially forking the chain, but it looks reasonable.

[aesedepece]

I'm testing this in a controlled setup. We might start to plan for a testnet reset scheduled for later this week.

[drcpu-github]

And now this new and improved patch seems to actually fix the issue.

Of course, the tests will still fail due to this patch: a8872c6.