refactor: evidence make equivocation private

[tac0turtle]

Description

Thank you to @ethanfrey for pointing out a footgun of the evidence module. The HandleEquivocation function implemented the evidence handler and could be added as a public route in confusion of setting up the module.

To prevent this we moved the beginblocker function to be a method on the keeper and made the HandleEquivocation private.

---

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

• included the correct type prefix in the PR title
• added ! to the type prefix if API or client breaking change
• targeted the correct branch (see PR Targeting)
• provided a link to the relevant issue or specification
• followed the guidelines for building modules
• included the necessary unit and integration tests
• added a changelog entry to CHANGELOG.md
• included comments for documenting Go code
• updated the relevant documentation or specification
• reviewed "Files changed" and left comments if necessary
• confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

• confirmed the correct type prefix in the PR title
• confirmed ! in the type prefix if API or client breaking change
• confirmed all author checklist items have been addressed
• reviewed state machine logic
• reviewed API design and naming
• reviewed documentation is accurate
• reviewed tests and test coverage
• manually tested (if applicable)

[alexanderbez]

🔥

[alexanderbez]

Ohh, we should get a changelog entry for this.

[ethanfrey]

Thanks for making this update.

My other issue with this, is it mixed "evidence" from tendermint (which can be 100% trusted and is actually more like a "report") with "evidence" from external user (to be registered, but that must contain the actual proof as part of it).

I recommend that these types no longer implement the interfaces for the evidence router

[ethanfrey]

I wouldn't use same evidence interface for req.ByzantineValidators as this module requires from external messages. This was the original confusion that left the interfaces such that one could register an evidence handler that blindly trusted types.Equivocation.

I would go a bit further.

1. types.Equivocation should not implement Evidence interface (those must be defined with full proofs and registered)
2. types.FromABCIEvidence() should return a concrete type types.Equivocation as long as that is the only type we get from Tendermint. If it adds more types later, the module can generalize this.

My key point is Evidence should only be things that include their own proof, that can be validated by a registered handler. Allowing an unprovable type as Evidence is a misuse of the type system, declaring something as fitting a hole it must never enter.

[tac0turtle]

i went with number two otherwise i would have to introduce more boilerplate for sets since we set Evidence in the store. I think this is safe enough for now. I would prefer we remove this module entirely as i dont see other modules using evidence and instead handling it themself

[julienrbrt]

There are still integration tests failing that were using that public method: https://github.com/cosmos/cosmos-sdk/actions/runs/4446568082/jobs/7807047508 so it won't auto-merge.