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

Domain Type implementations needed by IBC #634

Closed
ancazamfir opened this issue Oct 13, 2020 · 4 comments · Fixed by #639
Closed

Domain Type implementations needed by IBC #634

ancazamfir opened this issue Oct 13, 2020 · 4 comments · Fixed by #639
Milestone

Comments

@ancazamfir
Copy link
Contributor

ancazamfir commented Oct 13, 2020

Following domain types and TryFrom/ From implementations are needed for (de)serialization of client IBC messages:

tendermint_proto::types::SignedHeader <===> tendermint::block::signed_header::SignedHeader
tendermint_proto::types::ValidatorSet <===> tendermint::validator::Set
tendermint_proto::abci::ConsensusParams <===> tendermint::consensus::params::Params

Required for informalsystems/hermes#277

@greg-szabo
Copy link
Member

I'm working on this but it's going to take a bit more time than I expected. All these types have fields with other types that also need to be created. I'll try to measure how much this work is, so I can give more appropriate status updates.

@greg-szabo
Copy link
Member

greg-szabo commented Oct 13, 2020

Types to implement: (indentation shows dependency on the type above)

SignedHeader
  block::Header
    ConsensusVersion
  block::Commit (fix: Round is still u32 here for some reason)
    CommitSigs (what is the purpose of this? It's not doing any validation or filtering)
      CommitSig (fix: figure out what to do with the RawCommitSig implemented for serde)

validator::Set
  Info
  vote::Power
  ProposerPriority

consensus::params::Params
  block::Size
  evidence::Params
    evidence::Duration
  evidence::ValidatorParams
    public_key::Algorithm

I haven't found any proof that these were amino-encoded before. If these types are broken in the JSON encoding, then this is something we haven't encountered before and we were only vaguely referring to "fixing JSON-encoding" in the v0.34 upgrade. (If this is not a JSON thing, please indicate it.)

Upgrading JSON-encoding was supposed to be a later little project where we harmonize the validation given by domain types with the JSON validation. It seems we'll have to prioritize this.

I'm going to get back to this PR as soon as I have a better picture what a "JSON-harmonization" would entail.

@greg-szabo
Copy link
Member

SignedHeader and validator::Set are implemented on the greg/research-json branch.

@greg-szabo
Copy link
Member

All three are now implemented in #639 . Some of it was unused before, so if you find any issues we can fix / change the structs in followup issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants