Skip to content

Add helper methods to AssignedCollators to avoid is_empty checks#43

Draft
tmpolaczyk wants to merge 8 commits intomasterfrom
tomasz-refactor-empty-assignment-master
Draft

Add helper methods to AssignedCollators to avoid is_empty checks#43
tmpolaczyk wants to merge 8 commits intomasterfrom
tomasz-refactor-empty-assignment-master

Conversation

@tmpolaczyk
Copy link
Contributor

@tmpolaczyk tmpolaczyk commented Jul 22, 2025

AssignedCollators struct has this field:

container_chains: BTreeMap<ParaId, Vec<AccountId>>,

It is a map of container chain para id to list of collators assigned to that chain. A common query is "does this chain have any assigned collators?". The naive solution is to do container_chains.contains(para_id), but that's wrong because the chain could be in the BTreeMap but with an empty collator list. The correct solution is something like container_chains.get(para_id).unwrap_or(&vec![]).is_empty() == false, which is more complex and easy to forget.

This PR tries to avoid that problem by making the container_chains field private, and only allow reading through getters that automatically check the empty vector case and convert it into None. Example:

/// Get collators assigned to container chain `para_id`. Handles the edge case of an empty list.
/// If this returns Some, the Vec can be assumed to not be empty.
pub fn get_container_chain(&self, para_id: &ParaId) -> Option<&Vec<AccountId>> {
    let x = self.container_chains.get(para_id);

    // Filter out empty assignment, return None instead
    match x {
        Some(x) if x.is_empty() => None,
        x => x,
    }
}

And now callers can simply do assigned_collators.get_container_chain(para_id).is_some() to check if a chain has any collators.

Also added more helper methods that are needed because the field is private.

@tmpolaczyk
Copy link
Contributor Author

On hold because making the field private made too many tests fail to compile, but I can't think of any better alternatives

@tmpolaczyk tmpolaczyk marked this pull request as draft December 1, 2025 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant