rust-bitcoin / bitcoind

Utility to launch a regtest bitcoind process in a rust test
MIT License
40 stars 33 forks source link

Nix friendly `build.rs` by avoiding direct downloads #125

Closed RCasatta closed 1 year ago

RCasatta commented 1 year ago

To achieve deterministic builds, the nix build process prevents build scripts to hit the internet. This means the auto-download feature is a no-go.

A solution is obviously to use the nix package manager to provide the bitcoind executable and use the BITCOIND_EXE env var. But this prevents having a Cargo.toml configuration that is fine for both nix and cargo users.

Including all versions executables in the bitcoind crate would make it too big, but having a crate for every version of bitcoind would allow downloading only the one specified by the feature and it would work from nix.

This would be very tedious work requiring publishing 36 crates at the moment supporting all the versions currently available in the features (12 versions * 3 arch)

Is it worth it? Can we drop support from older bitcoin version #120 ?

@apoelstra

apoelstra commented 1 year ago

But this prevents having a Cargo.toml configuration that is fine for both nix and cargo users.

Tihis is sorta inherent to Cargo anyway, because Cargo is crappy and not very Nixy. It also connects to the Internet without a bunch of prodding (e.g. obtaining an up-to-date lockfile from somewhere and patching it into the codebase), and anyway wants to build everything in a single opaque command.

Since I already have to jump through a bunch of hoops to build Rust projects at all, setting an extra env var is really no big deal to me.

RCasatta commented 1 year ago

Since I already have to jump through a bunch of hoops to build Rust projects at all, setting an extra env var is really no big deal to me.

but using the env var it's not enough if you have the dep specified this way:

bitcoind = { version = "0.30.0", features = ["23_0"] }
apoelstra commented 1 year ago

Ah! Yes, that is true. I understand now what you're trying to accomplish.

I think rather than trying to publish 36 (or any number) of crates it would be better to somehow make the env var and the features compatible. Maybe it's enough to just always try the env var first when looking for a bitcoind, and trusting that the user-provided one is actually the correct version. We could maybe do some sanity checks via RPC or by checking the hash of the provided EXE against a whitelist (which could be overridden by yet another env var :P) or something.

RCasatta commented 1 year ago

I think I have found the right approach: If build.rs find the BITCOIND_EXE set find BITCOIND_TARBALL_FILE set, it uses it as a source instead of downloading it. The sha256 check would also prevent the user to mistake the version

apoelstra commented 1 year ago

That sounds great to me.

RCasatta commented 1 year ago

I am using this in https://github.com/RCasatta/fbbe/pull/21/commits

it partially works but not entirely because in the nix build https://github.com/RCasatta/bitcoind/blob/7ea2f44748e1e802811bd7cf9a203a391cf87e6d/src/lib.rs#L461

resolves to:

"/build/source/target/release/build/bitcoind-1996c37c5bf70096/out/bitcoin/bitcoin-23.1/bin/bitcoind\"

but nix stores it in

"/nix/store/c1zd8rs5vjhzzmmihr9y9bzli56zi4gw-fbbe-deps-0.1.4/target/release/build/bitcoind-1996c37c5bf70096/out/bitcoin/bitcoin-23.1/bin/bitcoind"

apoelstra commented 1 year ago

Where are you setting the environment variable?

RCasatta commented 1 year ago

I am sorry, I forgot to push the latest wip commit, now it's there

apoelstra commented 1 year ago

I'm sorry, I don't know flakes well enough to understand what's going on. With stdenv.mkDerivation you could just add the line export BITCOIND_TARBALL_FILE=${BITCOIND_TARBALL_FILE} to your bash code and it'd work.

RCasatta commented 1 year ago

Neither do I :) I am trying to learn...

Thanks for looking into it