Custom relay strategy

[fewensa]

Help all bridgers implement their own relay strategy

[aurexav]

RelayStrategy sounds better

[svyatonik]

The reason of this change is to ease process of adding new strategy, right? So now we have altruistic and rational, you need to add 3rd, but it requires forking messages-relay, right? If not - please explain :) If so, then idea seems good to me.

I've left some comments - one issue with accumulators && couple of design suggestions.

[fewensa]

Yes, it is, we want to add another strategy to our bridger.

[fewensa]

@svyatonik changed to follow your suggestions

[svyatonik]

There are two types of checks in this loop - hard and soft. You can't violate hard checks in any strategy - i.e. if target chain weight has max transaction size set to 1MB, it means that you can't deliver 2MB of messages in a single tx. That's the 'hard' limit. The 'soft' limit is defined by the strategy. So I'd expect *_strategy.rs files to only contain soft checks. Hard checks must be done at the upper level and stay the same for all strategies.

That said, I expect altruistic_strategy.rs to only have something like return true or return <something-that-says-Im-ready-to-deliver-all-nonces> in its only method. And the rational_strategy.rs should only have checks related to rewards/losses. Is it possible to fix that, please?

[fewensa]

I changed it. can be reviewed.

[fewensa]

Oh, sorry, This is still wrong, please wait. let me fix it

[fewensa]

Done

[fewensa]

I don't know if there is a better solution, there is currently a big struct

[svyatonik]

Thanks! I'll take a look in next few days - the change is non-trivial && I'll need to review it in details.

[svyatonik]

The code seems to be correct - I'll do a brief run before approving + merging, though :)

Couple of comments:

1. there are multiple warnings in the clippy + fmt jobs for this PR. They're introduced here - please fix it. I can't hit merge until everything is green :)
2. the MixStrategy is very similar to what've been DefaultStrategy in the very beginning :) It looks like a code that links two implementations - enum-based old and trait-based new. I'd prefer to have AltruisticStrategy + RationalStrategy only;
3. big structs that are passed between strategies. I've suggested strategies to have some context for their own fields. Like total_cost and total_reward fields of the RelayerReference struct are only used by the RationalStrategy. They're never touched by the AltruisticStrategy and they're never read or updated by that EnforcementStrategy. So it isn't clear when I look at all these strategies code - who's responsible for updating these fields and how they're related to selecting nonces.

Re (2) and (3) - we may add a separate issue for that and fix it in later PRs if you're blocked on this one.

[fewensa]

About 2
I think MixStrategy is a simple solution. If we use AltruisticStrate or RationalStrategy directly, the code will become more complicated. We need to change
pub relay_strategy: Strategy, to pub relay_strategy: Box<Strategy>.
and there
https://github.com/darwinia-network/parity-bridges-common/blob/eaa4a1f4b9112be052ef501d456d3cf980e43929/relays/messages/src/message_race_delivery.rs?_pjax=%23js-repo-pjax-container%2C%20div%5Bitemtype%3D%22http%3A%2F%2Fschema.org%2FSoftwareSourceCode%22%5D%20main%2C%20%5Bdata-pjax-container%5D#L250
there
https://github.com/darwinia-network/parity-bridges-common/blob/eaa4a1f4b9112be052ef501d456d3cf980e43929/relays/bin-substrate/src/chains/kusama_messages_to_polkadot.rs?_pjax=%23js-repo-pjax-container%2C%20div%5Bitemtype%3D%22http%3A%2F%2Fschema.org%2FSoftwareSourceCode%22%5D%20main%2C%20%5Bdata-pjax-container%5D#L185
etc.

Maybe we can remove RelauerMode in messages-relay module. just in substrate-relay module. and move MixStrategy to substrate-relay (No, Can't move it now, because some tests in message_race_delivery.rs are required, Can only be modified to Box) . Or be sure to change to use Box?

About 3.
I'm sorry, I don't have a good idea to perfect this question, There is only one thing I think can be optimized,

merge selected_reward and total_reward
merge selected_cost and total_cost

[fewensa]

All tests and code format fixed.

[svyatonik]

#1202 and #2483