test: reshardingV3 test loop for buffered receipts #12490
Conversation
Trisfald
requested review from
saketh-are,
wacban,
Longarithm and
shreyan-gupta
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12490 +/- ##
=======================================
Coverage 69.96% 69.96%
=======================================
Files 838 838
Lines 169135 169139 +4
Branches 169135 169139 +4
=======================================
+ Hits 118338 118342 +4
- Misses 45666 45669 +3
+ Partials 5131 5128 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
|
@shreyan-gupta I've added outgoing receipts tests (I hope I'm testing them correctly) and these seems to work fine out of the box. |
| shard_id, | ||
| &epoch_manager.get_shard_layout(&tip.epoch_id).unwrap(), | ||
| ); | ||
| let congestion_info = &client_actor |
There was a problem hiding this comment.
Along with checks for congestion_info I think what we should independently do is to verify whether the correct trie key entries are present or not.
- For delayed receipts we can check the
DelayedReceiptIndicestrie key and see if the start_index is not same as end_index. - For buffered receipts it would be the
BufferedReceiptIndicestrie key and we can do something similar for the expected shard.
There was a problem hiding this comment.
Given this difference in checks, we can consider splitting checking delayed receipt and buffered receipt into separate functions if it looks more appropriate, else I'm fine with having them in the same function as well.
There was a problem hiding this comment.
Interesting! Let me see if I can expand this check easily (here in another PR)
| txs_vec.push((tx.get_hash(), tip.height)); | ||
| txs.set(txs_vec); | ||
| submit_tx(&node_datas, &"account0".parse().unwrap(), tx); | ||
| for _ in 0..(CALLS_PER_BLOCK_HEIGHT / signer_ids.len() |
There was a problem hiding this comment.
Would it benefit to do round robin across signer_ids?
for i in 0..CALLS_PER_BLOCK_HEIGHT
signer_id = signer_ids[i % signer_ids.len()]
...
There was a problem hiding this comment.
It should not make any difference I guess 🤔
There was a problem hiding this comment.
The current logic 0..(CALLS_PER_BLOCK_HEIGHT / signer_ids.len() + (CALLS_PER_BLOCK_HEIGHT % signer_ids.len()).min(1)) can exceeds CALLS_PER_BLOCK_HEIGHT right?
There was a problem hiding this comment.
It does, yes, trying to be fair to all accounts. Maybe it should respect more importantly CALLS_PER_BLOCK_HEIGHT rather than fairness to help with making expectations clearer .. let me modify this part
New test loop test for buffered receipts during resharding.
New state before resharding:
test_resharding_v3_split_parent_buffered_receiptsfails when buffered receipts are present in resharded shard, it works when target shard is not changing at epoch boundaryloop_actionto execute multiple functions, not just one