lightningdevkit / rust-lightning

A highly modular Bitcoin Lightning library written in Rust. It's rust-lightning, not Rusty's Lightning!
Other
1.15k stars 363 forks source link

Rust release support #471

Closed jkczyz closed 3 years ago

jkczyz commented 4 years ago

There's some useful features in Rust's stable release. However, we have Travis configured for some older releases.

https://github.com/rust-bitcoin/rust-lightning/blob/f70058ea4c451955e72b15d24b67d941d7f3e467/.travis.yml#L2-L6

15 added 1.22 when adding fuzzing. #220 added 1.29.2 because stable (1.30 at the time) broke fuzzing. #353 replaced 1.29.2 with 1.34.2 to get integer atomics for fuzzing.

Is there a reason to support Rust 1.22 still? What criteria should we use to determine supported releases?

jeandudey commented 4 years ago

Is there a reason to support Rust 1.22 still? What criteria should we use to determine supported releases?

Debian oldstable rustc package

TheBlueMatt commented 4 years ago

See-also https://github.com/rust-bitcoin/rust-bitcoin/issues/338. Probably best to continue the discussion there unless we have an urgent need to support a different version than rust-bitcoin itself.

jkczyz commented 4 years ago

See-also rust-bitcoin/rust-bitcoin#338. Probably best to continue the discussion there unless we have an urgent need to support a different version than rust-bitcoin itself.

I don't see an urgent need to support a different version, but there are features such as native endianness support that would be nice to have rather than rolling it ourselves. Also, my limited understanding of the Rust RFC and release process is that it takes awhile for new features to be released to stable. So we'd have to wait even longer for upcoming features.

Debian oldstable rustc package

Maybe a better question then is: what criteria should we use to determine supported OS distributions? IIUC, developers want to use rust-bitcoin in Bitcoin Core, so the supported distros are limited to those supported by Core. But given rust-bitcoin is a dependency of rust-lightning and not vice versa, why should rust-lightning use similar criteria?

jeandudey commented 4 years ago

there are features such as native endianness support that would be nice to have rather than rolling it ourselves.

And will be used once the MSRV is updated to a more recent version.

But given rust-bitcoin is a dependency of rust-lightning and not vice versa, why should rust-lightning use similar criteria?

The ability to spin up a rust-lightning implementation in older systems that don't support recent Rust versions.


The main goal is to be able to build everywhere (Rust suffers a lot since crates upgrade their MSRV constantly) without using rustup, and building deterministically on a system that you trust.

jkczyz commented 4 years ago

Wanted to circle back to this as we've been running into problems where we have to write code in a more verbose way to placate 1.22.

Debian oldstable is now at 1.34: https://github.com/rust-bitcoin/rust-bitcoin/issues/338#issuecomment-583849580

jeandudey commented 4 years ago

Seems reasonable since Debian oldoldstable got support for 1.34 too.

TheBlueMatt commented 4 years ago

Hmm, I've never had any issues just making things a tinge more verbose to get 1.22 to build, but I'm also open to bumping it. Unless we have a strong reason not to, I'd prefer to have the same MSRV as rust-bitcoin, but maybe we can bump it to 1.29 given mrustc seems to mostly be focusing there. https://github.com/rust-bitcoin/rust-bitcoin/issues/338

jkczyz commented 4 years ago

Here's one example:

https://github.com/rust-bitcoin/rust-lightning/pull/494/commits/b4921e900a31f4a02c12a00c6a4dd26ab6c8d686

There are similar examples in that PR that go away in 1.31, AFAICT. My preference would be to go to 1.34.

TheBlueMatt commented 4 years ago

Past 1.29 would be quite annoying to bootstrap a compiler, AFAIU, as mrustc only supports 1.29.

jkczyz commented 4 years ago

I was not familiar with the requirements around using mrustc for bootstrapping. I found https://github.com/rust-bitcoin/rust-bitcoin/issues/206, but from my reading I don't see how it is a problem. See: https://github.com/rust-bitcoin/rust-bitcoin/issues/206#issuecomment-450159143

TheBlueMatt commented 4 years ago

Right, the only way to build a later version of rust with mrustc is to compile each previous version. Thus, relying on anything much later than 1.29 makes it relatively impractical to do.

jkczyz commented 4 years ago

I'm not sure I understand why this is impractical. Isn't this a one-off event whenever we want to bump releases? Maybe I'm missing something obvious or don't understand what exact use case is being targeted in practice.

jeandudey commented 4 years ago

I'm not sure I understand why this is impractical. Isn't this a one-off event whenever we want to bump releases? Maybe I'm missing something obvious or don't understand what exact use case is being targeted in practice.

To bootstrap the rustc compiler without using the binaries that the Rust language provides (rustup, etc.) you need to bootstrap from the very first compilers. If you want to create you own trust chain of compilers because you care about security you start the bootstrap on your own without trusting other binaries you need a compiler like mrustc which isn't written in Rust and can compile a Rust compiler, meaning if it can compile 1.29 we can start bootstrapping from there.

jkczyz commented 4 years ago

To bootstrap the rustc compiler without using the binaries that the Rust language provides (rustup, etc.) you need to bootstrap from the very first compilers. If you want to create you own trust chain of compilers because you care about security you start the bootstrap on your own without trusting other binaries you need a compiler like mrustc which isn't written in Rust and can compile a Rust compiler, meaning if it can compile 1.29 we can start bootstrapping from there.

Yes, I understand that part. But what I don't understand is why targeting a version later than 1.29 makes this scenario impractical. Is it (i.e., the impracticality) simply the fact that a user who cares about security needs to bootstrap the compiler up to the later version rather than using mrustc directly?

jeandudey commented 4 years ago

Is it (i.e., the impracticality) simply the fact that a user who cares about security needs to bootstrap the compiler up to the later version rather than using mrustc directly?

IMHO it's more easier to use mrustc directly than starting the bootstrap process, and if rust-lighning supports 1.29 is a win/win scenario.

In any case, if this project is going to be tied to 1.29 (because of mrustc version) it makes sense to setup CI against that rustc and/or mrustc versions respectively.

TheBlueMatt commented 4 years ago

Right, we do have Travis testing 1.22, but we could up it to 1.29, though thats still a ways before NLL became available.

Yes, I understand that part. But what I don't understand is why targeting a version later than 1.29 makes this scenario impractical.

Mostly time invested. You cannot simply build Rust 1.36 with Rust 1.29, you have to build every intermediate version along the way. One or two revs may be within reason, but once you get into a much higher version it gets pretty hairy.

jkczyz commented 4 years ago

(FYI, I'm pushing on this because I want to understand the exact criteria and our reasoning for it.)

IMHO it's more easier to use mrustc directly than starting the bootstrap process, and if rust-lighning supports 1.29 is a win/win scenario.

I don't disagree that it would be easier. We'd probably both agree that's objectively easier to use mrustc directly than it is to bootstrap a compiler from it. But for the former, supporting mrustc in practice seems more like introducing a dev dependency on mrustc, IMHO, despite having a clear workaround (i.e., bootstrapping).

Right, we do have Travis testing 1.22, but we could up it to 1.29, though thats still a ways before NLL became available.

My worry is that for substantial features like NLL, we are depending on a project that may never ship such a feature (and may never plan to). From what I can tell, the vast majority of contributions come from one developer, and the project itself is billed as "not yet suitable for everyday use" in its README.

Mostly time invested. You cannot simply build Rust 1.36 with Rust 1.29, you have to build every intermediate version along the way.

It sounds like the argument is that we don't want to introduce any unnecessary friction in case someone prefers to use mrustc.

One or two revs may be within reason, but once you get into a much higher version it gets pretty hairy.

Is there more to this than successive bootstraps?

TheBlueMatt commented 4 years ago

On Feb 24, 2020, at 17:30, Jeffrey Czyz notifications@github.com wrote: I don't disagree that it would be easier. We'd probably both agree that's objectively easier to use mrustc directly than it is to bootstrap a compiler from it. But for the former, supporting mrustc in practice seems more like introducing a dev dependency on mrustc, IMHO, despite having a clear workaround (i.e., bootstrapping).

Right, I don’t think mrustc is a competitive compiler in the sense that it does not enforce things like the borrow checker fully. That said, I believe most distros that are adding rust more recently or want reproducible builds are using mrustc to bootstrap their release rustc, which is something we want to support.

-snip-

Mostly time invested. You cannot simply build Rust 1.36 with Rust 1.29, you have to build every intermediate version along the way.

It sounds like the argument is that we don't want to introduce any unnecessary friction in case someone prefers to use mrustc.

Essentially, yes, for those who want a greenfield rustc and don’t want to trust $PERSON_WHO_BUILT_BINARIES, we want to make it relatively easy to use your own rustc/llvm bins. One or two revs may be within reason, but once you get into a much higher version it gets pretty hairy.

Is there more to this than successive bootstraps?

Largely, no. It’s definitely not anywhere near as important as “distro X doesn’t even ship rustc Y so we shouldn’t update”, but if we end up in a world where any major users of RL are shipping production binaries holding large sums of users funds built with rustup that would be very sad.

valentinewallace commented 4 years ago

I haven't read the full discussion, but just wanted to note here that 1.22 has been taking up a good amount of my time on #667

TheBlueMatt commented 4 years ago

If it makes it easier, feel free to bump to 1.29, I think.

TheBlueMatt commented 3 years ago

We now require 1.36. If there are further things we need from even newer rustc we should open a new issue.