feat(code): Surface proposal and vote equivocation evidence

[bastienfaivre]

Closes: #911

---

PR author checklist

For all contributors

• Reference a GitHub issue
• Ensure the PR title follows the conventional commits spec
• Add a release note in RELEASE_NOTES.md if the change warrants it
• Add an entry in BREAKING_CHANGES.md if the change warrants it

For external contributors

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

[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

[bastienfaivre]

If we want to inform the engine and the application of the equivocation that occurred during a height, we can simply add the evidence map to the Decided event. This is done in commit 3b83e0f.
Now, if we want to know about the equivocation as soon as it occurs, we can throw an error for the vote and the proposal, respectively. This works well for the vote, since the engine (or more specifically, the driver) does not have to do anything other than return if it is an equivocation. However, this approach does not work for the proposal since, even if it is an equivocation, the driver still needs to execute multiplex_proposal (c.f. mux.rs), which, depending on the output, will need to perform something else in the driver that can also lead to an error. Therefore, we may need to carry two errors, which is inconvenient.
Another approach would be to define proper data structures at the same level as the evidence map so that the engine can check and clear them every time it processes a vote or a proposal.

What do you think? @romac @ancazamfir

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (01420b4) to head (e85f987).

✅ All tests successful. No failed tests found.

Additional details and impacted files

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

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

[ancazamfir]

Can you add some evidence metrics above and also some error/warn logs?

[ancazamfir]

If we want to inform the engine and the application of the equivocation that occurred during a height, we can simply add the evidence map to the Decided event. This is done in commit 3b83e0f.

Let't do this in a separate PR. We need to figure out if we send the actual equivocating votes/ proposals or just a statement "This node/address has equivocated". It depends on the app -- consensus trust level and if the app is "ready" to handle votes and proposals.

Now, if we want to know about the equivocation as soon as it occurs, we can throw an error for the vote and the proposal, respectively. This works well for the vote, since the engine (or more specifically, the driver) does not have to do anything other than return if it is an equivocation. However, this approach does not work for the proposal since, even if it is an equivocation, the driver still needs to execute multiplex_proposal (c.f. mux.rs), which, depending on the output, will need to perform something else in the driver that can also lead to an error. Therefore, we may need to carry two errors, which is inconvenient. Another approach would be to define proper data structures at the same level as the evidence map so that the engine can check and clear them every time it processes a vote or a proposal.

Yes, the latter is better. Not sure about clearing it though. So maybe we can check after applying a proposal or vote if "new" evidence is recorded in the driver. This is just for logging purposes at this point.

What do you think? @romac @ancazamfir

[romac]

I opened https://github.com/informalsystems/malachite/pull/1009 which addresses some of my comments above and simplifies the implementation by only retrieving the evidence on decide, rather than emit events at the time when equivocation happens.

[ancazamfir]

I opened #1009 which addresses some of my comments above and simplifies the implementation by only retrieving the evidence on decide, rather than emit events at the time when equivocation happens.

I think we should postpone sending the evidence to the app in decide. It is not clear to me the evidence form that the app could digest. I was proposing above to do this in a different PR once we have a conversation with app devs.

We should also log the evidence in consensus and update metrics. Ideally when it happens as the height may not decide and this information may prove important in debugging.

[bastienfaivre]

@ancazamfir @romac What should this branch implement, then? Based on your perspectives, #1009 is already in better shape than this one.

[romac]

See my comment in #1009.

[bastienfaivre]

@romac, the inject feature is not enough/complete. Indeed, injecting a ProposeValue message does not work as the engine/driver will not consider two propose values; therefore, no conflicting proposals are sent. If we want to force receiving a proposal via a NetworkEvent, it also does not work since we do not have access to the peer id and cannot sign the proposal.
Do you have an idea on how to fix this?

[romac]

Ah bummer… Let me think about it and I'll come back to you

[romac]

Maybe we can pass an unsigned message to inject and use Node::get_signing_provider to sign the message in the test framework?

[bastienfaivre]

For that, we need to have access to the app instance (on which we can actually call get_signing_provider), but I don't see where we can access it in the framework.

[ancazamfir]

Maybe an overkill but can you run once and collect the proposals from that run? And then run again (with same keys) and inject in the second run the proposal from the first?

[ancazamfir]

The other question, does it make sense to do the tests in a separate PR and merge this one as is?

[bastienfaivre]

Maybe an overkill but can you run once and collect the proposals from that run? And then run again (with same keys) and inject in the second run the proposal from the first?

Do you mean hardcoding the messages? For example, for votes, retrieve data from such logs:

2025-05-13T12:08:41.926664Z DEBUG test{id=1}:node{id=2}: Event: Published(msg: Vote(SignedMessage { message: Vote { typ: Precommit, height: Height(2), round: Some(0), value: Val(ValueId(81418)), validator_address: Address(AD52C99C243AAB9755836D3055A4ECAA5A5D76C7), extension: None }, signature: Signature(Signature { R_bytes: "f75cf3e1a3ddafe8c340f8700b5e792f47c0fe3a0d364446ae206bf7f8c54e4b", s_bytes: "4610ac2c9868b57f50bdb9064d503e2145e22978dfbc67da9994bcf38224ce09" }) }))

That might work, but does the framework support specifying a key seed? @romac

I would likely prefer the inject approach, which might also be useful for other tests.

The other question, does it make sense to do the tests in a separate PR and merge this one as is?

That is technically possible, yes.

[ancazamfir]

Do you mean hardcoding the messages? For example, for votes, retrieve data from such logs:

I thought you could intercept the proposal (or vote) in the first run, save it in some test state and then inject it in the second run.

I would likely prefer the inject approach, which might also be useful for other tests.

You still do the inject but you need a properly signed (second) proposal

[romac]

Nifty idea, I'll give it a go next week!