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

abci: Change hashes' type from bytes::Bytes to tendermint::Hash #1095

Closed
thanethomson opened this issue Feb 16, 2022 · 1 comment · Fixed by #1232
Closed

abci: Change hashes' type from bytes::Bytes to tendermint::Hash #1095

thanethomson opened this issue Feb 16, 2022 · 1 comment · Fixed by #1232
Assignees
Labels
abci domain-types Anything relating to the creation, modification or removal of domain types enhancement New feature or request

Comments

@thanethomson
Copy link
Member

Version(s) of tendermint-rs: master

Description

Related to #1090.

At the moment, several data structures in the tendermint crate's abci module have hash members of type bytes::Bytes (e.g. BeginBlock). I'd like to rather make these of type tendermint::Hash.

We can easily provide a conversion for non-empty hashes to bytes::Bytes.

cc @hdevalence

Definition of "done"

When all hash fields in structs in the tendermint::abci module are of type tendermint::Hash.

@thanethomson thanethomson added enhancement New feature or request abci domain-types Anything relating to the creation, modification or removal of domain types labels Feb 16, 2022
@thanethomson thanethomson self-assigned this Feb 16, 2022
@mzabaluev
Copy link
Contributor

Questions:

  • The response structures Info and InitChain implement Default (Info also defaults the missing field values to the ones provided by the Default impl). If we change the app hash fields to AppHash, is it OK to have the default value of an empty byte array, like it already is de-facto?
  • The hash field of Snapshot is documented as an arbitrary value compared by equality, so the semantics for neither tendermint::Hash nor tendermint::AppHash obviously apply. I think it's ok to leave it as Bytes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abci domain-types Anything relating to the creation, modification or removal of domain types enhancement New feature or request
Projects
None yet
2 participants