refactor(code): Simplify implementation by only retrieving evidence on decide

[romac]

No description provided.

[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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (bastien/equivocation-evidence@7613bb8). Learn more about missing BASE report.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@                       Coverage Diff                       @@
##             bastien/equivocation-evidence   #1009   +/-   ##
===============================================================
  Coverage                                 ?       0           
===============================================================
  Files                                    ?       0           
  Lines                                    ?       0           
  Branches                                 ?       0           
===============================================================
  Hits                                     ?       0           
  Misses                                   ?       0           
  Partials                                 ?       0           

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

[bastienfaivre]

LGTM, thank you!

[bastienfaivre]

I just read the comment of @ancazamfir here. Should we keep the evidence field in the Decided event?

[romac]

I would suggest to remove it in this PR for now and add logging (via tracing) in the driver whenever we see a misbehavior.

[romac]

Having events would be nice but we need to find a way to implement those nicely and I am not yet sure how to go about that. Having mutable state in the driver that we check and clear every time is brittle, hence why I attempted to remove it here, but maybe there is a better way. Perhaps by changing the EvidenceMap structs to store the latest evidence that was recorded so that we can check if it matches the last vote/proposal that was processed and then emit the event. What do you think? On my side I will think more on this and come back to you.

[bastienfaivre]

I agree that adding a field last_evidence to the data structure would allow us to avoid clearing any value. Moreover, I don't see any other way to inform the engine that such an event occurred in the driver than adding a field somewhere, so this is an ideal solution at first glance.
I suggest the following:

1. We finish this PR by adding the logging required.
2. We merge it into feat(code): Surface proposal and vote equivocation evidence #999 and implement this last_evidence approach.

What do you think?

[romac]

Sounds good to me! @ancazamfir?

[ancazamfir]

Sounds good to me! @ancazamfir?

Can we still keep all evidence and only log the "last" one? Eventually we will send some information to the app if/ when we decide, based on the overall evidence. In Cosmos for example iirc we only send the list of validators that equivocated and not the vote details.

[ancazamfir]

Can we also add evidence metrics. Not sure in which PR we do what but I am ok with either.

[bastienfaivre]

Can we still keep all evidence and only log the "last" one?

That is precisely what we are planning to do. This last_evidence approach is a clean way to inform the engine if an equivocation occurred during the last driver process. In all cases, the evidence_map will keep all evidence that occurred.