Finschia / ostracon

Ostracon, a consensus algorithm, is forked from Tendermint Core. We have added VRF to Tendermint BFT. It adds randomness to PoS Validator elections and improves security.
Apache License 2.0
70 stars 28 forks source link

fix: modify proto pubkey regisiter name for IBC relayer compatibility #762

Closed tkxkd0159 closed 4 months ago

tkxkd0159 commented 4 months ago

Description

  1. IBC relayer(hermes) parse events based on tendermint-rs. It renames fields as tendermint.crypto.* via serde for PublicKey. (ref)
  2. To fix above issue, we should modify register name for our proto pubkey. This is only used for converting to JSON. And Ostracon doesn't save these values anywhere so there are no breaking changes even if we change this. (saved consensus key with internal type, not proto type)
  3. There is no breaking change for finschia-sdk. sdk registers their amino types here.

Closes: #XXX

CLAassistant commented 4 months ago

CLA assistant check
All committers have signed the CLA.

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 66.64%. Comparing base (2aa7d4c) to head (489777b). Report is 2 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #762 +/- ## ========================================== + Coverage 66.54% 66.64% +0.10% ========================================== Files 285 285 Lines 37919 37919 ========================================== + Hits 25232 25271 +39 + Misses 10883 10850 -33 + Partials 1804 1798 -6 ``` | [Files](https://app.codecov.io/gh/Finschia/ostracon/pull/762?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Finschia) | Coverage Δ | | |---|---|---| | [crypto/encoding/codec.go](https://app.codecov.io/gh/Finschia/ostracon/pull/762?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Finschia#diff-Y3J5cHRvL2VuY29kaW5nL2NvZGVjLmdv) | `47.61% <100.00%> (ø)` | | ... and [14 files with indirect coverage changes](https://app.codecov.io/gh/Finschia/ostracon/pull/762/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Finschia)
ulbqb commented 4 months ago

https://github.com/tendermint/tendermint/blob/main/crypto/encoding/codec.go Is it possible to use tendermint package in your situation? It looks like codec file should be removed in Ostracon because of duplication.

tkxkd0159 commented 4 months ago

https://github.com/tendermint/tendermint/blob/main/crypto/encoding/codec.go Is it possible to use tendermint package in your situation? It looks like codec file should be removed in Ostracon because of duplication.

Yeah I think it's possible. But I have heard that our repo will be converted with cometbft soon. So I just tried to not change as much as possible. Is it better to replace with tendermint now? @ulbqb

ulbqb commented 4 months ago

So I just tried to not change as much as possible.

That’s make sense.