feat: add handler for consumer double voting

[sainoe]

Description

Closes: #1227

---

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 state-machine breaking change (i.e., requires coordinated upgrade)
• Confirmed this PR does not introduce changes requiring state migrations, OR migration code has been added to consumer and/or provider modules
• Targeted the correct branch (see PR Targeting)
• Provided a link to the relevant issue or specification
• Followed the guidelines for building SDK 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 this PR does not introduce changes requiring state migrations, OR confirmed migration code has been added to consumer and/or provider modules
• 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

[sainoe]

This definition stays unchanged bc the Hermes relayer is supposedly already using it.

[sainoe]

Only used for the consumer chain ID atm

[mpoke]

Nice work @sainoe. Approve the main logic. See my comments bellow.

[insumity]

Is my understanding correct that GetSigners builds the addresses that need to have signed the MsgSubmitConsumerDoubleVoting in order for the message to be valid?
For example, in the bank module, you need to verify that someone sending money from an account from has indeed signed the message, otherwise you could potentially steal tokens.

If the above is true, then in the case of MsgSubmitConsumerDoubleVoting why do we need the message to be signed by addr?

[sainoe]

Good question. AFAIU all messages must be signed by the submitter and should satisfy the msg interface .

[insumity]

If my understanding is correct that those tests check the order of operations (i.e., method calls) taking place in JailAndTombstoneValidator. So, the first test checks that GetValidatorByConsAddr is called. Then, the third test checks that if GetValidatorByConsAddr passes and since we are not unbonded, then we call IsTombstoned, etc.

Maybe it would make sense for JailAndTombstoneValidator to return an error on whether it succeeds or not and then test depending (on the mocks we have) on whether JailAndTombstoneValidator succeeds? Or is this not possible because the underlying JailUntil and Tombstonefrom theslashingmodule do not return anerr`or?

[sainoe]

JailAndTombstoneValidator success is tested by the number of expected mocks per test case.
For instance, when the validator is unbonded, the test expects to have one unique call to GetValidatorByConsAddr or fails.

[insumity]

evidence.Time() uses the Timestamp from DuplicateVoteEvidence that in my understanding could be easily faked? If that's the case, someone could send old evidence with a recent timestamp.
Would it make sense to get the time out of one of the votes instead? I guess even that would assume some decently-bounded clock drift between provider and consumer chains.

[sainoe]

Good catch! The evidence timestamp isn't signed so it can be faked.
Maybe using a vote timestamp works, we should check how it changes from using the evidence timestamp which is set to the infraction block time in CometBFT.

[insumity]

We talked about this offline but adding here for visibility. It seems that with the changes in this PR, a validator V could prevent getting slashed in the provider chain by doing the following:

1. V performs equivocation E in the provider chain
2. V performs double signing in the consumer chain (the double signing could have also been created in the past and V is just storing it to use it in such a scenario)
3. V submits a MsgSubmitConsumerDoubleVoting to the provider chain
4. V gets jailed and tombstoned in the provider chain but not slashed
5. Now if equivocation E shows up in the provider chain, V is tombstoned and won't be slashed

The above scenario seems unlikely because V would have to hide equivocation E for some time, but I guess it is still be technically possible.