Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fill in the LightClientAttackEvidence struct and conversions to/from Protobuf #1259

Closed
wants to merge 4 commits into from

Conversation

romac
Copy link
Member

@romac romac commented Jan 10, 2023

Closes: #1219

  • 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/

@codecov-commenter
Copy link

codecov-commenter commented Jan 10, 2023

Codecov Report

Merging #1259 (1b5d980) into main (5020067) will decrease coverage by 0.1%.
The diff coverage is 6.1%.

@@           Coverage Diff           @@
##            main   #1259     +/-   ##
=======================================
- Coverage   64.7%   64.5%   -0.2%     
=======================================
  Files        245     245             
  Lines      21687   21744     +57     
=======================================
+ Hits       14035   14044      +9     
- Misses      7652    7700     +48     
Impacted Files Coverage Δ
tendermint/src/evidence.rs 29.8% <6.1%> (-10.5%) ⬇️
abci/src/codec.rs 88.8% <0.0%> (-1.3%) ⬇️
abci/src/server.rs 8.8% <0.0%> (-0.4%) ⬇️
testgen/src/light_block.rs 87.5% <0.0%> (+0.5%) ⬆️
testgen/src/header.rs 83.5% <0.0%> (+0.5%) ⬆️
testgen/src/commit.rs 91.3% <0.0%> (+0.6%) ⬆️
testgen/src/validator.rs 87.5% <0.0%> (+0.7%) ⬆️
light-client-verifier/src/types.rs 39.0% <0.0%> (+0.8%) ⬆️
testgen/src/vote.rs 85.7% <0.0%> (+1.6%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ancazamfir
Copy link
Contributor

Besides these changes I think we also need to put an API in place to be used by hermes. imho tendermint-rs should be filling in Evidence::LightClientAttackEvidence (e.g. the byzantine_validators) based on the two conflicting and maybe a common light block.
Also not clear to me if hermes will call the broadcast_evidence() RPC or we do this via the new API (call to report_evidence()?) and in the latter case if there are more changes required.

So maybe we need to work in parallel with the hermes PR here, you probably have better idea.

@romac
Copy link
Member Author

romac commented Jan 11, 2023

Besides these changes I think we also need to put an API in place to be used by hermes. imho tendermint-rs should be filling in Evidence::LightClientAttackEvidence (e.g. the byzantine_validators) based on the two conflicting and maybe a common light block.

Yes, I am working on that right now :)

@romac
Copy link
Member Author

romac commented Jan 11, 2023

Also not clear to me if hermes will call the broadcast_evidence() RPC or we do this via the new API (call to report_evidence()?) and in the latter case if there are more changes required.

What's this new API? Do you have a link to it?

Also should we keep submitting the MisbehaviorEvidence message to the chain like we do currently?

@ancazamfir
Copy link
Contributor

Also not clear to me if hermes will call the broadcast_evidence() RPC or we do this via the new API (call to report_evidence()?) and in the latter case if there are more changes required.

What's this new API? Do you have a link to it?

the new API to be used by hermes to submit evidence to the node.

Also should we keep submitting the MisbehaviorEvidence message to the chain like we do currently?
yes, that should stay.

@romac
Copy link
Member Author

romac commented Jan 17, 2023

Superseded by #1260

@romac romac closed this Jan 17, 2023
@romac romac deleted the romac/1219-light-client-attack-evidence branch January 31, 2023 13:30
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.

Support LightClientAttackEvidence
3 participants