refactor: split out composite electoral system trait

[kylezs]

Pull Request

Closes: PRO-1675

Checklist

Please conduct a thorough self-review before opening the PR.

• I am confident that the code works.
• I have written sufficient tests.
• I have written and tested required migrations.
• I have updated documentation where appropriate.

Summary

Rather than have a composite "electoral system" impl Electoral system, we now have a new ElectoralSystemRunner trait for the composite, which is then used by the elections pallet directly. This removes some prior hacks that were required to get the traits to work for both a single and composite electoral system.

It also makes it easier to write migration utilities in the future, since storage will always be enum wrapped, as a composite, and we don't need to consider the individual electoral system case.

As a consequence there were other simplifications that fell out:

• Simpler Composite electoral system macro
• Some trait methods now have simpler signatures (e.g. no Result necessary in some cases, fewer params).
• Removes unnecessary complex borrowing in macros by simply using the traits and associated functions directly - which also maps better to how substrate works, data is written into a static storage layer.
• Removes translators 🙌 - much clearer what's happening here now, as we can directly use a CompositeElectoralSystem and provide it with the ElectoralSystem we want to access.
• For the tests I just took the approach of most flexibility for now, the model in the tests also maps nicely to how it actually works with respect to the storage, so can save building a separate mental model.

[codecov]

Codecov Report

Attention: Patch coverage is 79.43596% with 175 lines in your changes missing coverage. Please review.

Project coverage is 71%. Comparing base (9842dea) to head (c6907d9).
Report is 12 commits behind head on main.

Files with missing lines	Patch %	Lines
...allets/cf-elections/src/electoral_system_runner.rs	13%	40 Missing ⚠️
state-chain/pallets/cf-elections/src/lib.rs	85%	21 Missing and 9 partials ⚠️
...ts/cf-elections/src/electoral_systems/composite.rs	56%	20 Missing and 7 partials ⚠️
...cf-elections/src/electoral_systems/mocks/access.rs	91%	23 Missing ⚠️
...lectoral_systems/blockchain/delta_based_ingress.rs	38%	9 Missing and 1 partial ⚠️
...te-chain/runtime/src/chainflip/solana_elections.rs	90%	2 Missing and 8 partials ⚠️
...ate-chain/pallets/cf-elections/src/benchmarking.rs	57%	6 Missing ⚠️
engine/src/elections.rs	0%	5 Missing ⚠️
...e/src/state_chain_observer/client/electoral_api.rs	0%	5 Missing ⚠️
...chain/pallets/cf-elections/src/electoral_system.rs	0%	1 Missing and 3 partials ⚠️
... and 7 more

Additional details and impacted files

@@          Coverage Diff           @@
##            main   #5334    +/-   ##
======================================
- Coverage     72%     71%    -0%     
======================================
  Files        486     487     +1     
  Lines      85863   86047   +184     
  Branches   85863   86047   +184     
======================================
- Hits       61515   61435    -80     
- Misses     21533   21689   +156     
- Partials    2815    2923   +108     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dandanlen]

I'm not really convinced of the benefit of mocking the storage access trait... still feel like we could just use the actual pallet.

[dandanlen]

We can probably define this within the generate macro? (Doesn't have to be now)

And then maybe we can give these sensible names instead of A, B, C etc..

[kylezs]

Sensible names is a little difficult, because every chain could be different, so if we named them "block height", "ingress" etc. depending on the chain, it could be different. Though, it's certainly on the radar as something that would be nice to improve if possible

[dandanlen]

I guess what I mean is that we would use this macro at the runtime-level (ie. solana_elections.rs). We don't really need to define a generic that is only used once. But yeah - can wait for later ofc.

[dandanlen]

I don't really understand the comment here. How are these 'assigned to an election'?

[kylezs]

Made comment more explicit.