Skip to content

refactor: duplicate vote detection#1499

Closed
rnbguy wants to merge 8 commits intomainfrom
rano/refactor/duplicate-vote-count
Closed

refactor: duplicate vote detection#1499
rnbguy wants to merge 8 commits intomainfrom
rano/refactor/duplicate-vote-count

Conversation

@rnbguy
Copy link

@rnbguy rnbguy commented Apr 10, 2025

This PR introduces a minor refactor to optimize the logic to check for duplicate votes.

  • Referenced an issue explaining the need for the change
  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Added entry in .changelog/

@rnbguy rnbguy requested a review from melekes April 10, 2025 11:32
fn test_successful_verify_malicious_update_header_2() {
// This test is almost same as the previous one.
// The only difference is order of the validator set iteration.
// In this case, the verifier will correctly detect invalid signature.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the other case?

Copy link
Author

@rnbguy rnbguy Apr 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_successful_verify_malicious_update_header_1 (the existing test) is the other case.

Basically, a malicious validator could only reuse its vote if only it came earlier than the targeted validator in this following loop (test_successful_verify_malicious_update_header_1).

https://github.com/informalsystems/tendermint-rs/blob/71c946084eceed3a2da91b78921d0345b46401e5/light-client-verifier/src/operations/voting_power.rs#L419-L420

If the targeted validator came earlier in this loop, the verifier will see vote.verified is false and (in)validate the signature (test_successful_verify_malicious_update_header_2).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check 73dec22 for the changes I made to the existing test to trigger the invalid signature error.

@rnbguy
Copy link
Author

rnbguy commented Apr 10, 2025

Interesting that other tests started failing 😅 I will take a look at it later.

@rnbguy rnbguy marked this pull request as draft April 10, 2025 12:27
@rnbguy rnbguy closed this Aug 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants