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

Add a tendermint-validator crate #1152

Open
wants to merge 58 commits into
base: main
Choose a base branch
from

Conversation

tomtau
Copy link
Contributor

@tomtau tomtau commented Jun 30, 2022

fixes #1134

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

mzabaluev and others added 30 commits November 26, 2021 18:12
0.23.1 was released before informalsystems#1023 or its backport to v0.23.x were merged.
…ems#1020)

* Reformat readme

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Expand README to better reflect current repo state/goals

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Reformat contributing guidelines

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add note about assigning oneself to an issue

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Clarify version/branch correlations

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Fix readme formatting

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Reorganize and deduplicate contributing guidelines

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Format ADR documentation

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Update GitHub issue/PR templates to accommodate new versioning

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Fix spelling of GitHub

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add note on Tendermint Core version support

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Simplify GitHub templates

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add version support matrix table to readme

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Remove mention of `dev` branch for now

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Fix versioning section and add note on pinning to patch version for v0.23.x

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Fix typo in ADR readme

Signed-off-by: Thane Thomson <connect@thanethomson.com>
…0.35.0 (informalsystems#1022)

* tendermint: add From<Infallible> for Error

* Use hex for AppHash Debug formatting

* Regenerate protobuf types using tools/proto-compiler.

* Regenerate protos against tendermint v0.35.0-rc3.

* Remove SetOption-related code in tendermint-abci.

* Update imports to reflect protobuf movements.

The BlockParams and ConsensusParams structs moved out of the ABCI
protos.

* Update tendermint-abci to reflect upstream proto changes.

* p2p: treat all non-Ed25519 keys as unsupported

This fixes a compile error introduced by upstream proto changes that add
an Sr25519 variant.

* Improve ABCI response code modeling with NonZeroU32

The previous data modeling allowed a user to construct an `Err(0)` value that
would be serialized and deserialized as `Ok`.

* Use the Bytes type in ABCI protos.

* Add conversions between protobuf and chrono types.

* tendermint: define Serde for Block in terms of RawBlock

This changes the Serialize/Deserialize implementations for Block to convert
to/from the proto-generated `RawBlock`, and use the derived serialization for
that type.

This is much cleaner and more maintainable than keeping Serde annotations for
each sub-member of the data structure, because all of the serialization code is
kept in one place, and there's only one validation path (the TryFrom conversion
from RawBlock to Block) that applies to both kinds of serialization.

* tendermint: simpler transaction modeling in Block

This changes the Block type to hold the transactions as a plain
`Vec<Vec<u8>>`, rather than as a custom `abci::transaction::Data` type
(which is itself a wrapper for an `Option<Vec<Transaction>>`, the
`Transaction` type being a wrapper for a `Vec<u8>`).

This also updates the `Block` getter functions; it's not clear (to me)
why they're there, since they access the public fields and aren't used
anywhere else.

* rpc: take over tendermint::abci

The existing contents of the `tendermint::abci` module note that they're
only for the purpose of implementing the RPC endpoints, not a general
ABCI implementation.

Moving that code from the `tendermint` crate to the `tendermint-rpc`
crate decouples the RPC interface from improvements to the ABCI domain
modeling. Eventually, it would be good to eliminate this code and align
it with the new ABCI domain types.

* tendermint: add ABCI domain types.

These types mirror the generated types in tendermint_proto, but have better
naming.

The documentation for each request type is adapted from the ABCI Methods and
Types spec document. However, the same logical request may appear three
times, as a struct with the request data, as a Request variant, and as a
CategoryRequest variant.

To avoid duplication, this PR uses the `#[doc = include_str!(...)]`
functionality stabilized in Rust 1.54 to keep common definitions of the
documentation.

* tendermint: eliminate &'static str errors in ABCI domain types.

* Merge `abci::params::ConsensusParams` with `consensus::Params`.

The code in the `abci` module had more complete documentation from the ABCI
docs, so I copied it onto the existing structure.

* Add hex encoding Serde attribute to Sr25519 keys.

* Replace integers with `block::Height`, `vote::Power`

* Replace integer with block::Round

* Fix tools build by using correct imports

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Fix clippy complaints in tools

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Fix clippy warning

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Fix clippy lints

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Fix more clippy lints

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Fix deprecation notices from ed25519 crate

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Regenerate protos for Tendermint v0.35.0

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Fix raw bytes conversion in tests/docs

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add Tendermint v0.35.0 Docker config

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add Tendermint v0.35.0 Docker image for ABCI integration testing

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Remove RPC deserialization tests

The support files for these tests were manually generated some time ago.
We should rather favour testing with the `kvstore_fixtures` tests, whose
fixtures are automatically generated by our rpc-probe tool.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Reformat Docker folder readme

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Bump version of Tendermint used in ABCI integration test

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Bump version of Tendermint used in rpc probe

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Bump version of Tendermint used in kvstore integration test

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Bump version of Tendermint used in proto-compiler

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Regenerate kvstore fixtures with rpc-probe for Tendermint v0.35.0

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Update kvstore integration test to accommodate newly generated fixtures

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Update RPC tests and data structures to accommodate Tendermint v0.35.0 changes

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Update ABCI encoding scheme to accommodate breaking change in tendermint/tendermint#5783

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Bump Tendermint version in GitHub Actions kvstore integration test

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add changelog entries to capture breaking changes

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Change tx hash encoding from base64 to hex and update tests

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add changelog entry for /tx endpoint change

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Co-authored-by: Henry de Valence <hdevalence@penumbra.zone>
Co-authored-by: Henry de Valence <hdevalence@hdevalence.ca>
This does not seem to be produced anywhere.
* Fix trivial lints reported by clippy 0.1.57

* Boxing to reduce oversized enum variants

* abci: fix one more clippy 0.1.57 lint

* Changelog on breaking changes in informalsystems#1041
* pbt-gen: Converted from chrono to time 0.3

chrono has soundness issues (see RUSTSEC-2020-0159) and does not
seem to be maintained.

* Replace dependency on chrono with time 0.3

Change Time implementation to crate time: chrono has soundness issues
(see RUSTSEC-2020-0159) and does not seem to be actively maintained.

* Add Time methods checked_add and checked_sub

These should be used instead of the overloaded operators that broke
the operator convention by returning a Result.

* proto: Don't use formatting methods of time

Drop the "formatting" feature of time, as this brings in std.

* pbt-gen: Add arb_datetime_for_rfc3339

With crate time, the range of supported dates stretches to the ancient
past beyond the common era, which is not representable in RFC 3339.
Add a generator to produce `OffsetDateTime` values that are
within the RFC 3339 spec.

* Ensure Time can only have values good for RFC 3339

As the time values are meant for human-readable representation in
the RFC 3339 format, make it not possible to construct Time with values
that fall outside this standard representation.
Conversion from time::OffsetDateTime is made fallible with
a TryFrom impl replacing the From impl.
Conversion from Unix timestamps is also affected.

* rpc: Replaced chrono with time 0.3

* testgen: Replaced chrono with time 0.3

* Less allocatey ways of formatting date-times

Provide and use another helper in proto mod serializers::timestamp,
one that formats into a provided fmt::Write object rather than
allocating a string.

* light-client: port from chrono to time

* Revert to Add/Sub returning Result

* light-client: changed the MBTs from chrono to time

* tendermint: Remove a comment referencing chrono

We use ErrorDetail::DurationOutOfRange without the source error
from the time library because that is uninformative in the context.
Also move the DateOutOfRange close with DurationOutOfRange
since they can occur in the same operations and are generally similar.

* light-client: add std feature for time

The clock needs OffsetDateTime::now_utc().

* tendermint: Simplify Time::duration_since

* testgen: minor code cleanup

* Restrict valid Time years to 1-9999

Exclude year 0 because the spec on Google protobuf Timestamp forbids it.

Add more helpers in pbt-gen to produce protobuf-safe values in both
OffsetDateTime and RFC 3339 strings.

Restrict the property tests to only use the protobuf-safe values
where expected.

* Test Time::checked_add and Time::checked_sub

* Changelog entries for informalsystems#1030

* Improve documentation of tendermint::Time

* proto: remove the chrono type conversions

The From impls are panicky and do not enforce compliance with
protobuf message value restrictions.

* tendermint: remove direct uses of chrono types

Replace with tendermint::Time.

* Harden Timestamp conversions and serde

Require the timestamp to be in the validity range (years 1-9999 in UTC)
and the nanosecond member value to not exceed  999_999_999.

Rename ErrorDetail::TimestampOverflow to TimestampNanosOutOfRange,
because the old variant was not very informative (the chained
TryFromIntError did not help) and we also use it for the above-range
case now which is not an overflow.

* Changelog entry about changed error variants

* Restore nanosecond range check in Time::from_unix_timestamp

Add a unit test to exercise the check.

* proto: Improve timestamp::fmt_as_rfc3339_nanos

- More ergonomic signature
- A non-allocating implementation

* Fix component name in changelog for 1030-remove-chrono

Co-authored-by: Thane Thomson <thane@informal.systems>

* time: Use Self instead of the type name in methods

Co-authored-by: Thane Thomson <thane@informal.systems>

* Comment on the inner representation of `Time`

* Don't alias crate time in testgen

* Document the Time::from_utc helper

Co-authored-by: Thane Thomson <thane@informal.systems>
tendermint uses checked_add/checked_sub methods which were added
in time 0.3.5.
)

* Remove unnecessary cdylib artifacts from the build

* Purge redundant [lib] sections from Cargo.toml
* Derive Hash on tendermint::Time

Now that we're backed by a data structure with unambiguously unique
values, we can make time moments hashable.

* Changelog entry for informalsystems#1054

* Update changelog entry with package name

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Co-authored-by: Thane Thomson <connect@thanethomson.com>
…#1067)

* Use ed25519-consensus instead of ed25519-dalek

Closes informalsystems#355 (see that issue for more context; `ed25519-consensus` is a fork of
`ed25519-zebra` that's Zcash-independent).

This change ensures that `tendermint-rs` has the same signature verification as
`tendermint`, which uses the validation criteria provided by the Go
`ed25519consensus` library.

* clippy fixes

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* Remove redundant dependency

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* cargo fmt

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add changelog entries

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Co-authored-by: Henry de Valence <hdevalence@penumbra.zone>
* rpc: Switch hyper-proxy to use rustls

The rest of the stack already uses rustls for their TLS needs.

* Changelog entry for informalsystems#1068
…light-client-verifier` (informalsystems#1071)

* Split out verifier parts of tendermint-light-client to tendermint-light-client-verifier

* Add Time::now() method behind clock feature

* light-client refactor: Re-export new verifier crate from `tendermint-light-client` (informalsystems#1074)

* Re-export tendermint_light_client_verifier as verifier from tendermint_light_client

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Remove unnecessary tendermint-light-client import

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Fix crate docs to reflect verifier extraction

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add changelog entry

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Re-export the original exports in light-client crate

Co-authored-by: Thane Thomson <thane@informal.systems>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
…informalsystems#1081)

* Use Time instead of u64 seconds in tendermint-testgen Header

* Fix testgen time serialization for MBT
…ems#1082)

* Implement Clone for tendermint::PrivateKey

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add changelog entry

Signed-off-by: Thane Thomson <connect@thanethomson.com>
* Changelog release

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Build changelog

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Bump versions to v0.24.0-pre.1

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add tendermint-light-client-verifier crate to release script

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Rebuild changelog

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Update date in changelog and rebuild

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add readme for tendermint-light-client-verifier

Signed-off-by: Thane Thomson <connect@thanethomson.com>
…s#1084)

* Rename kvstore module to common, since it provides common RPC requests

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Rename quick module to kvstore, since it caters exclusively for the kvstore

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Move attribute after comment

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add support for simple query plan for Gaia

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Enable rustls for wss support

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Fix clippy lint

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add changelog entry

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
 (informalsystems#1086)

* Add convenience method to decode RPC requests from strings

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add deserialization workaround for validator updates

This introduces a (somewhat hacky, yet temporary) approach to being able
to deserialize public keys in validator updates until such time that
Tendermint addresses the problem.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add changelog entry

Signed-off-by: Thane Thomson <connect@thanethomson.com>
…formalsystems#1087) (informalsystems#1088)

* Remove tendermint-rpc dependency from tendermint-light-client-verifier

* Add CI check for no-std compliance

* Fix path to no-std-check

Co-authored-by: Soares Chen <soares.chen@gmail.com>
* Make encode_vec() infallible

* Add .changelog entry

* Undo changes to kvstore

* Expand on changelog entry

Co-authored-by: Thane Thomson <thane@informal.systems>
* Add block_by_hash RPC endpoint and tests

* rpc-probe part of this

* Fix comments

* Update .changelog/unreleased/features/832-block-by-hash.md

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* cargo fmt

Co-authored-by: Thane Thomson <connect@thanethomson.com>
Signed-off-by: Thane Thomson <connect@thanethomson.com>
* Fix clippy warnings from Rust v1.59.0

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Workaround for flex-error-related clippy warning

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Upgrade to contracts v0.6.2 and fix clippy errors

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add changelog entry

Signed-off-by: Thane Thomson <connect@thanethomson.com>
In PeggyJV/ocular#18 we are trying to create a registry of "well-known"
chain IDs. I proposed using `tendermint::chain::Id` for this, but to do
so we'd need to have a const initializer support.

This commit changes the internal representation of `chain::Id` to
`Cow<'static, str>`. This permits a `const fn` initializer, but avoids
adding a lifetime to `chain::Id` itself.

It also adds `chain::Id::new` which is defined as a `const fn`, which
can be used to define chain ID constants.
@tomtau
Copy link
Contributor Author

tomtau commented Jul 4, 2022

Thanks @tony-iqlusion ! Yes, tendermint-privval may be a better name.

As for the SignerProvider trait, I guess there are a few options:

  1. use async_signature::Signer directly instead and require public keys as a separate explicit input argument to the privval service (this may be a bit more error-prone and one will find out any signer-related issues only after the first signing request arrives)
  2. a new version of async_signature that will add public key retrieval functions and use that directly instead
  3. change SignerProvider to take async_signature::Signer as a type parameter

What do you think?

@tony-iqlusion
Copy link
Collaborator

@tomtau for "public key retrieval", in cosmrs we use a helper trait with a blanket impl:

https://docs.rs/cosmrs/latest/cosmrs/crypto/secp256k1/trait.EcdsaSigner.html

Upstreaming something like this into signature/async-signature is something I've considered.

@codecov-commenter
Copy link

codecov-commenter commented Jul 22, 2022

Codecov Report

Merging #1152 (97a0b92) into master (6a28510) will increase coverage by 1.0%.
The diff coverage is 86.0%.

@@           Coverage Diff            @@
##           master   #1152     +/-   ##
========================================
+ Coverage    64.7%   65.7%   +1.0%     
========================================
  Files         232     238      +6     
  Lines       15667   16210    +543     
========================================
+ Hits        10138   10664    +526     
- Misses       5529    5546     +17     
Impacted Files Coverage Δ
privval/src/error.rs 0.0% <0.0%> (ø)
tendermint/src/signature.rs 54.9% <0.0%> (-5.1%) ⬇️
privval/src/config.rs 60.0% <60.0%> (ø)
privval/src/server.rs 84.8% <84.8%> (ø)
privval/src/state.rs 97.1% <97.1%> (ø)
privval/src/lib.rs 100.0% <100.0%> (ø)
privval/src/signer.rs 100.0% <100.0%> (ø)
tendermint/src/consensus/state.rs 99.4% <100.0%> (+10.0%) ⬆️
tendermint/src/private_key.rs 63.4% <100.0%> (+6.6%) ⬆️
tendermint/src/public_key.rs 75.2% <100.0%> (+0.2%) ⬆️
... and 15 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

@tomtau tomtau marked this pull request as ready for review July 22, 2022 03:31
@tomtau
Copy link
Contributor Author

tomtau commented Jul 22, 2022

@tony-iqlusion that's the third option and it seems good: if it's upstreamed into signature/async-signature in the future, it shouldn't be difficult to change. So, I switched SignerProvider to be a helper trait.

@thanethomson this PR should be good to go (sans some questions on the other PR: #1147 (comment) ) -- I could possibly add those signature trait implementations in #1147 if it's preferable to split it that way.

@tony-iqlusion
Copy link
Collaborator

Did a quick pass and this is looking good now!

I'll try to more thoroughly review this next week.

@tony-iqlusion
Copy link
Collaborator

@tomtau here's a PR to add a Keypair trait to the signature crate:

RustCrypto/traits#1080

It could have a corresponding AsyncKeypair trait in the async-signature crate.

@tony-iqlusion
Copy link
Collaborator

Added AsyncKeypair: https://docs.rs/async-signature/0.2.0/async_signature/trait.AsyncKeypair.html

@tony-iqlusion
Copy link
Collaborator

It seems this is blocked on #1225

@romac romac changed the base branch from master to main September 29, 2023 12:17
@romac romac changed the title added a validator crate Add a tendermint-validator crate Sep 29, 2023
Copy link
Member

@romac romac left a comment

Choose a reason for hiding this comment

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

I switched the base branch from master to main so now this PR needs a rebase.

@tomtau
Copy link
Contributor Author

tomtau commented Sep 29, 2023

I'm not sure when I'll get to this PR. Are you looking into merging it soon or is anyone interested in taking over it?

@romac
Copy link
Member

romac commented Sep 29, 2023

No rush from our side, so feel free to come back to it whenever it suits you if you are still interested in getting this merged. And sorry for not being very reactive with this PR, we've had and still have limited capacity to work on tendermint-rs this past quarter.

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.

generate privval gRPC server and provide a high-level pluggable default implementation