test: add extra stats to cross_shard_tx

[pmnoxx]

The usefulness and flakiness of cross_shard_tx depends heavily on the block production time.

This change adds printing block stats every minute, we will print the following:

• ratio of number of blocks to longest chain
• maximum divergence between recently produced blocks

In addition we will fail cross_shard_tx test if the ratio is too high or to low, depending on test parameters.

#3014

[gitpod-io]

[codecov]

Codecov Report

Merging #3057 into master will decrease coverage by 0.13%.
The diff coverage is 27.41%.

@@            Coverage Diff             @@
##           master    #3057      +/-   ##
==========================================
- Coverage   87.39%   87.26%   -0.14%     
==========================================
  Files         212      212              
  Lines       41322    41377      +55     
==========================================
- Hits        36115    36108       -7     
- Misses       5207     5269      +62     

Impacted Files	Coverage Δ
chain/client/src/test_utils.rs	81.32% <19.64%> (-10.20%)	⬇️
chain/client/tests/bug_repros.rs	94.70% <100.00%> (ø)
chain/client/tests/chunks_management.rs	97.42% <100.00%> (ø)
chain/client/tests/cross_shard_tx.rs	93.02% <100.00%> (ø)
chain/client/tests/process_blocks.rs	90.75% <100.00%> (+0.53%)	⬆️
chain/jsonrpc/src/lib.rs	61.81% <0.00%> (-3.64%)	⬇️
chain/client/src/sync.rs	86.21% <0.00%> (-0.46%)	⬇️
chain/chain/src/store.rs	86.27% <0.00%> (-0.06%)	⬇️
chain/chain/src/validate.rs	90.32% <0.00%> (ø)
... and 8 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ddefbf...1aefeca. Read the comment docs.

[SkidanovAlex]

I would rename it to hash2depth or hash2length (and then consistently use length or depth throughout to refer to the number of blocks on the chain), to avoid the confusion with the actual block heights.

[SkidanovAlex]

It was marked as resolved, but the name is still hash2height. Not sure if there's a change that wasn't pushed yet?

[bowenwang1996]

Looks good to me. One small issue I see is that orphans are not handled in add_block. I am not sure whether that is a possibility.

[pmnoxx]

Looks good to me. One small issue I see is that orphans are not handled in add_block. I am not sure whether that is a possibility.

I was thinking about adding code to handle this case. I would add a panic in case we see a situation like this.

However I decided to keep the code simple. In case there is an orphan, we will see a panic due to unwrap. I could replace .unwrap() with expect("....") if you think this code will be more clear.

[bowenwang1996]

Looks good to me. One small issue I see is that orphans are not handled in add_block. I am not sure whether that is a possibility.

I was thinking about adding code to handle this case. I would add a panic in case we see a situation like this.

However I decided to keep the code simple. In case there is an orphan, we will see a panic due to unwrap. I could replace .unwrap() with expect("....") if you think this code will be more clear.

Are you sure? I only see this

        let prev_depth = self.hash2depth.get(block.header().prev_hash()).map(|v| *v).unwrap_or(0);

which doesn't panic.

[SkidanovAlex]

@bowenwang1996 we never have the same block producer twice in a row, and thus a block cannot be created unless its parent was sent over the network. Given the code you are commenting is the callback that is triggered for each network message, it is impossible for a block to be handled before it's parent was handled.

[gitpod-io]