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

xla/multi-tc-versionsupport/fix #1254

Merged

Conversation

xla
Copy link
Contributor

@xla xla commented Dec 23, 2022

Addressing checks and warnings on the multi tc base branch.

* Fix lints that trigger clippy 0.1.66

* config: fix clippy lints in tests

* rpc: fix new clippy lints
@codecov-commenter
Copy link

codecov-commenter commented Dec 23, 2022

Codecov Report

❗ No coverage uploaded for pull request base (mikhail/multi-tc-version-support@6a24461). Click here to learn what that means.
The diff coverage is n/a.

@@                        Coverage Diff                         @@
##             mikhail/multi-tc-version-support   #1254   +/-   ##
==================================================================
  Coverage                                    ?   62.3%           
==================================================================
  Files                                       ?     258           
  Lines                                       ?   22419           
  Branches                                    ?       0           
==================================================================
  Hits                                        ?   13970           
  Misses                                      ?    8449           
  Partials                                    ?       0           

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

@xla xla changed the title xla/multi-tc=versionsupport/fix xla/multi-tc-ersionsupport/fix Dec 23, 2022
@xla xla changed the title xla/multi-tc-ersionsupport/fix xla/multi-tc-versionsupport/fix Dec 23, 2022
@romac
Copy link
Member

romac commented Dec 23, 2022

Hi @xla! Good to see you :) do you know what's up with the huge diff?

@xla
Copy link
Contributor Author

xla commented Dec 23, 2022

@romac Hiiiiii. The issue is that I rebased main to pull in some of the fixes already applied there. Gonna look into it.

@xla xla force-pushed the xla/multi-tc-versionsupport/fix branch from 50a4c7d to 73f5326 Compare December 24, 2022 06:10
@xla
Copy link
Contributor Author

xla commented Dec 24, 2022

Hi @xla! Good to see you :) do you know what's up with the huge diff?

Addressed.

@romac
Copy link
Member

romac commented Dec 24, 2022

Thank you :)

@romac romac merged commit a00a69d into mikhail/multi-tc-version-support Dec 24, 2022
@romac romac deleted the xla/multi-tc-versionsupport/fix branch December 24, 2022 09:29
mzabaluev added a commit that referenced this pull request Mar 2, 2023
* proto-compiler: Improve error reporting

Print out the prost-build error to display protoc output line by line
rather than as an unreadable Debug dump.

* Generate proto from tendermint 0.37.0-alpha.1

Also adapted the tendermint-abci crate as necessary.
Committing this helps take stock of the protocol changes since 0.34,
before multi-version support can be implemented.

* proto: sort includes in generated tendermint.rs

This normalizes the content of the module for protocol updates.

* proto: generate structs for 0.34 and 0.37

Modify proto-compiler to generate protobuf modules for both versions
of the Tendermint protocol. The different versions are disambiguated
by module paths, presently tendermint::v0_34 and tendermint::v0_37,
with the latest supported version reimported as tendermint::*.

* tendermint: Remove Evidence::ConflictingHeaders

This variant does not get expressed in RPC or protobuf,
seems like this protocol change was not taken as per
https://github.com/tendermint/tendermint/blob/91fba07e49cee43048fd761c8b2c2ec3c017acc8/docs/architecture/adr-047-handling-evidence-from-light-client.md

* Add serializers::allow_null

This should be used in preference to nullable where `nil` in the format
could be met as a quirk admitted by the Go implementation,
but otherwise the preferred form is some, possibly default value.

* Adapt domain types to v0.37 and add multi-version conversions.

* Multi-version protobuf support for request types

* Add ABCI response types for PrepareProposal, ProcessProposal

* Multi-version protobuf conversions for response types

* Fix conversion from account::Id to Bytes

* Fix compiler warnings

* light-client: Restore Supervisor::report_evidence

Keep it as a stub to avoid more extensive changes.

* Restore serialization of Block and Evidence

Define hand-written Serialize/Deserialize impls for Evidence.

* Fix clippy lints

* proto-compiler: Improve copying of generated files

- Filter out the empty non-Tendermint files.
- Improve use of the WalkDir iterator.
- Use OS-agnostic path construction.

* proto: Regenerate v0_34 with v0.34.22

* Add domain type for RemoteSignerError

With multi-protocol stuff, it should be done properly and conversions
defined from/to proto stuff.

* Define more multi-protocol conversions

Temporarily remove the import of crate::v0_37::* to the root of
tendermint-proto.

* p2p: version-qualify dependencies on protobuf

Where there are direct dependencies on the proto definitions,
clarify them with the protocol version for later audit.

* Make clippy happy

* Derive Eq where clippy suggests

* tendermint: version-specific Request and Response

Separate Request and Response definitions pertaining to
protocol versions v0.34 and v0.37, namespaced in top-level
modules v0_34 and v0_37 respectively.

* Remove last version aliases of Request, Response

Need to check where we depend in downstream branches.

* Re-generate protos with prost-build 0.11.4

Also update v0_34 to 0.34.24.

* rpc: add endpoints header and header_by_hash

Provided by v0.37 nodes.

* rpc: Separate Client traits for v0.34 and v0.37

Add new endpoints /header and /header_by_hash to
the v0_37::Client trait, and re-export it as new crate::Client.

* rpc: feature-gate the Client traits

The same way as the old Client trait was.

* xla/multi-tc-versionsupport/fix (#1254)

* tendermint: post-merge fix

* Add missing ConsensusRequest and ConsensusResponse mappings.

* abci: change serialization to unsigned varint

As the protocol used by tendermint-abci has been updated
to 0.37, (with no compat mode for 0.34 at the moment), the encoding
is changed to match.

* abci: changelog notice about varint encoding

This is a breaking change.

* rpc: multi-version support through generics

Introduce Dialect trait and data structures in modules dialect,
v0_34::dialect, v0_37::dialect, to realize differences in serialization
between RPC versions. Parameterize data structures that need to be
differently serialized by generic parameters containing the differences.

* rpc: remove generic default from request::Wrapper

* Implement both Client dialect traits for websocket

* Rework Event serialization with helper types

DialectEvent and its sub-field structs take over the dialectal generics,
while Event and its inner structs remain public types free from
serialization concerns.

* rpc: Refactor subscriptions to support dialects

* rpc: fix the fixture tests

Have to make DialectEvent and friends public for these
"integration" tests.

* rpc: fix websocket tests

* tendermint: Fix test_sign_bytes_compatibility

Badly merged code broke the test.

* Versioned type aliases for WebSocketClient

Also for WebSocketClientDriver.
The specializations for v0_37 are also re-exported under crate root,
providing some backward compatibility.

* Fix tools build

* Remove dialect parameter for SubscriptionClient

The trait does not need it and the Subscription public API is erased.
Only WebSocketClient needs the dialect parameter.

* Derive PartialEq, Eq on ProdCommitValidator

Also add test code that ensures ProdVerifier supports the common derived
traits.

* tendermint: restore Serialize impl on Event

Also on ABCI domain types containing Event instances.
This is needed for CLI.

* rpc: de-genericized result types

Add associated type Output to SimpleRequest to designate the
output type produced from the response. Implement so that the
output type is a generics-free Response, while the Result type for
the Request trait is a DialectResponse where it needs to be.

* rpc: make the client module public

The purpose is to expose the generic WebSocketClient and
WebSocketClientDriver types, though not under the crate root
that's reserved for the type aliases specialized for the
latest protocol version.

* rpc: rename DefaultDialect to LatestDialect

* rpc: dynamic compat mode for HttpClient

Instead of using statically selected, but mostly identical
Client traits to specify the protocol version, add the dynamic
compatibility mode parameter with enum type CompatMode,
and implement v0.34 RPC compatibility mode with its different
JSON serialization if the parameter specifies so.

* rpc: eliminate dialect Client traits

Get back to the single Client trait, with the clients supporting
protocol dialects through the compatibility mode selected at runtime.
Add CompatMode support to WebSocketClient and make the web socket
client types back non-generic.

HttpClient gets a set_compat_mode method to update the mode
on the fly. No such thing is provided for WebSocketClient, though,
because the compat mode is set into the subscription driver
and cannot be updated.

* rpc: unit test for CompatMode version parsing

* rpc: make WebSocketConfig struct more usable

Make the fileds public, derive Debug.

* rpc: expose CompatMode::from_version

* rpc: debug received events in WebSocketTransport

* Quick fix for running `rpc-probe` against a Comet 0.37 node

* Fix doc for overriding env variable in rpc-probe README

* rpc: added kvstore test fixtures for 0.37

Obtained with rpc-probe against a Docker image cometbft/cometbft:v0.37.x
(ID 4ab97039d4c42dd67e62ecfe72307e1552f9b9c0e48ec15958197d637f1fdde9).

The fixtures for 0.34 have been moved alongside.

* rpc: adjust some v0_37 tests to match fixtures

Some of the fixtures needed adjusting as well, due to
time-dependent assumptions made by tests, or randomness
that could not be guessed ahead of time (incoming/block_by_hash).

* rpc: fudge data in subscribe_newblock_1

Same as was done in the v0_34 fixtures in the past, it lets us test
parsing of the struct containing events.

* rpc: adjust kvstore_fixtures tests for v0_37

* rpc: fixed and expanded unit tests using fixtures

Use the versioned path to the fixture files,
and add a copy of tests for v0_37 which was previously not covered.

* abci: more sensible default impl of ABCI++ methods

Returning blank responses does not make a good migration path
for applications that are yet unaware of the ABCI++ proposal handling
phases.
Implement the simplest sensible behaviors by default for
Application::prepare_proposal and Application::process_proposal
to fulfill the specification.

* abci: overflow-proof impl of prepare_proposal

* rpc: improve CompatMode enum

Instead of Latest as a variant, have only the explicit list of supported
protocol versions as the enum variants. Get the latest via the associated const fn
CompatMode::latest().

* rpc: builder API for clients

Replace the *Config structs with a proper builder API for better
extensibility.

* Add missing serde bounds on WebSocketClientUrl

* Add missing Display impl to WebSocketClientUrl

* rpc: do version discovery in CLI

---------

Co-authored-by: xla <[email protected]>
Co-authored-by: Akosh Farkash <[email protected]>
Co-authored-by: Romain Ruetschi <[email protected]>
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.

4 participants