Inclusions struct unit tests #1518
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tdimitrov
reviewed
Sep 12, 2023
tdimitrov
reviewed
Sep 12, 2023
tdimitrov
reviewed
Sep 12, 2023
tdimitrov
reviewed
Sep 12, 2023
tdimitrov
reviewed
Sep 12, 2023
tdimitrov
approved these changes
Sep 12, 2023
There was a problem hiding this comment.
Very good work @Overkillus! A few nits from me, most of them are a matter of personal test.
test_duplicate_insertion_same_height_different_blocks and inclusions_remove_with_empty_maps are great tests to have!
eskimor
reviewed
Sep 13, 2023
| } | ||
|
|
||
| #[test] | ||
| fn inclusions_duplicate_insertion_same_height_and_block() { |
There was a problem hiding this comment.
I would be more interested if pruning works as expected for same height/same block/different block. As this was precisely what went wrong in the Kusama incident.
Ideally we would also test whether pruning still works as desired if the same candidate was included at different heights.
Overkillus
added a commit
that referenced
this pull request
Sep 14, 2023
In follow-up to #1518 Adding extra tests for inclusion pruning. Primarily focusing on various cases surrounding candidates included in different forks (with different relay parents). All cases fall into a few buckets based on 3 degrees of freedom - number of candidates, number of blocks (height), number of forks + extra case for pruning multiple heights at once. Added small tweak to the original pruning function to disregard stale candidate duplicates which should keep the same behaviour. --------- Co-authored-by: Tsvetomir Dimitrov <tsvetomir@parity.io>
bkchr
pushed a commit
that referenced
this pull request
Apr 10, 2024
* send messages using xcm pallet * XcmBridge && XcmBridgeAdapter + (untested) config in RialtoParachain * impl encode_send_xcm for the rest * remove duplicate code * some fixes * cleanup * some more tests * cleanup * cleanup * send Rialto -> Millau messages using bridge-messages pallet * fmt * some clippy fixes * more clippy
bkchr
pushed a commit
that referenced
this pull request
Apr 10, 2024
* Adjustments for the xcm messages sending logic Signed-off-by: Serban Iorga <serban@parity.io> * Deduplicate XCM destination Signed-off-by: Serban Iorga <serban@parity.io> * [send_message] small changes Signed-off-by: Serban Iorga <serban@parity.io> * Define CustomNetworkId Right now we use some associations between Rialto, RialtoParachain and Millau chains and chains defined in the NetworkId enum. But if we are not carreful we might do mistakes like: In Millau: pub const ThisNetwork: NetworkId = Kusama; pub const RialtoNetwork: NetworkId = Polkadot; In Rialto: pub const ThisNetwork: NetworkId = Kusama; pub const MillauNetwork: NetworkId = Polkadot; We're introducing CustomNetworkId to have a centralized mapping between NetworkId chains and our custom chains. Signed-off-by: Serban Iorga <serban@parity.io> * Revert "Deduplicate XCM destination" This reverts commit 3a0a950e1d7484e3ecac45f5c00b152f0485cd11. Signed-off-by: Serban Iorga <serban@parity.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In follow-up to #1432
Some additional unit tests for the inclusion struct used in the scraper.