chore(code): Move mode check for effect handling in engine

[ancazamfir]

Closes: #XXX

---

PR author checklist

For all contributors

• Reference GitHub issue
• Ensure PR title follows the conventional commits spec

For external contributors

• Maintainers of Malachite are allowed to push changes to the branch

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (5fb4820) to head (23c5a2b).
Report is 2 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@     Coverage Diff     @@
##   main   #942   +/-   ##
===========================
===========================

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

[romac]

Let's refactor this into a function that we can use here and for the WAL below

[ancazamfir]

I think for the WAL we will have to remove the check when we fix #897. So we have three options

• refactor now and undo when we fix code: WAL scenarios for Proposal #897
• leave things as they are
• fix code: WAL scenarios for Proposal #897 in this PR

[github-actions]

✅ Semver Check Passed

Great job! All semver violations have been resolved. This PR now complies with semantic versioning rules.

If you made version updates, please ensure that:

• The version in Cargo.toml accurately reflects the nature of your changes
• Any breaking changes are properly documented in BREAKING_CHANGES.md

[romac]

I think this approach puts too much burden on users integrating at the core-consensus level, and we should postpone this until we have a higher-level wrapper around the core-consensus API. What do you think @ancazamfir?

[ancazamfir]

I think this approach puts too much burden on users integrating at the core-consensus level, and we should postpone this until we have a higher-level wrapper around the core-consensus API. What do you think @ancazamfir?

We should clarify with the integrators if this is the case. Having less Effects to implement is good but it comes at a cost of integrators having to verify all sigs before sending messages to core. But I assume core API integrators will look at the engine and see what they need to do.

For me the bigger concern is that here we do the expensive checks early, for example before we verify that messages are for lower heights and discard. For that reason I would also move some of the checks in the engine...and then yes it's extra work on integration that need to be done and a wrapper would be good.

[ancazamfir]

Let's close this until we have a way to verify signatures for future heights in the engine, or we decide to optimistically verify with current height validator set.