ElementsProject / lightning

Core Lightning — Lightning Network implementation focusing on spec compliance and performance
Other
2.81k stars 889 forks source link

Rust-bins: Upgrade tonic to v0.12 for v0.1.9 #7598

Open nepet opened 1 month ago

nepet commented 1 month ago

Upgrade tonic to v0.12 for rust bins v0.1.9 Description This PR upgrades the tonic dependency of the rust libraries to v0.12. Latest version used was v0.8. Tonic had some major bugfixes in between these versions. We need to upgrade tonic for downstream consumers of our libs to be able to upgrade tonic as well.

This PR also introduces a new versioning scheme where we bump the minor semver version on any breaking api change such as the removal of deprecated calls as well as changes to the requirements of fields.

The whole rust lib stack cln-grpc cln-rpc cln-plugin and cln-grpc-plugin is now on v0.2.x, on top of tag cln v24.02.2 for the underlying message scheme.

nepet commented 1 week ago

This PR changed a bit now:

We don't want to keep different branches for different minor versions of the rust crates for now, just update on master. This PR now does:

If you are ok with these changes @daywalker90, #7623 becomes obsolete.

daywalker90 commented 1 week ago

I'm fine with it, but i think @cdecker had concerns with upgrading dependencies when i made a PR for it.

nepet commented 1 week ago

I'm fine with it, but i think @cdecker had concerns with upgrading dependencies when i made a PR for it.

Yeah, I remember that upgrading some crate led to a lot of pain in the past, but I can't remember which crate it was. Upgrading tonic, on the other hand, brings some fixes that help us get better transport errors and is both wanted and needed by Greenlight, so this upgrade should be fine :)

nepet commented 1 week ago

So, I could locally reproduce the error CI runs into. The error appears after we try to establish a connection with a certificate unknown to the cln-grpc plugin. From what we can tell, the plugin stops listening on the grpc port after a tls hanshake error.

I am going to invert the commit that upgrades tonic to v0.12.2 which caused the error to appear. We still should bump the version to a new minor version giving us the oportunity to uprade tonic once this problem got solved.