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

vls-protocol-signer-0.11.0 compilation error #969

Closed
darioAnongba opened this issue May 8, 2024 · 7 comments
Closed

vls-protocol-signer-0.11.0 compilation error #969

darioAnongba opened this issue May 8, 2024 · 7 comments

Comments

@darioAnongba
Copy link

While updating the dependencies of our project with cargo update, I ran into the following compiling error linked to VLS. This is on MacOS M2 using the Breez SDK v0.4.0 (tried with 0.4.0-rc1 as well).
I'm not sure what the issue could be here:

error[E0507]: cannot move out of dereference of `Octets`
   --> /.../.cargo/registry/src/index.crates.io-6f17d22bba15001f/vls-protocol-signer-0.11.0/src/handler.rs:690:36
    |
690 |                   let data: Vec<_> = m
    |  ____________________________________^
691 | |                     .u5bytes
692 | |                     .clone()
    | |____________________________^ move occurs because value has type `std::vec::Vec<u8>`, which does not implement the `Copy` trait
693 |                       .into_iter()
    |                        ----------- value moved due to this method call
    |
note: `into_iter` takes ownership of the receiver `self`, which moves value
   --> ....rustup/toolchains/1.76-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/iter/traits/collect.rs:268:18
    |
268 |     fn into_iter(self) -> Self::IntoIter;
    |                  ^^^^
help: you can `clone` the value and consume it, but this might not be your desired behavior
    |
690 ~                 let data: Vec<_> = <std::vec::Vec<u8> as Clone>::clone(&m
691 |                     .u5bytes
692 ~                     .clone())    |

Using rustc 1.76.0. Pretty surprised no-one is running into this issue so I'm wondering if I messed up something. Sadly for me, I didn't commit the Cargo.lock to revert so now I'm stuck :/.

I tried cargo clean without success.

@JssDWt
Copy link
Contributor

JssDWt commented May 9, 2024

I ran into this same error when trying to cargo install gl-plugin in a debian bookworm docker container on an M3 mac. cargo build --release does work though. It's an error compiling the vls-protocol-signer. I haven't been able to figure out exactly why the build fails. I think it has something to do with the bitcoinconsensus dev dependency being compiled.

Dockerfile for this gl-plugin compilation:

ARG VERSION=a36fa6a0675fe3d265ef904e51d7fc1e10ecefeb
ARG REPOSITORY=https://github.com/JssDWt/greenlight.git

FROM debian:bookworm-slim as downloader

ARG VERSION
ARG REPOSITORY

WORKDIR /source/

RUN apt-get update -qq && \
    apt-get install -qq -y --no-install-recommends \
        ca-certificates \
        git

RUN git init && \
    git remote add origin "$REPOSITORY" && \
    git fetch --depth 1 origin "$VERSION" && \
    git checkout FETCH_HEAD


FROM rust:1.78-slim-bookworm as plugin-builder

RUN apt-get update -qq && \
    apt-get install -qq -y --no-install-recommends \
        build-essential \
        ca-certificates \
        git \
        protobuf-compiler

WORKDIR /app/

COPY --from=downloader /source/ ./

WORKDIR /app/libs/gl-plugin

# below command works
# RUN cargo build --release
# below command fails
RUN cargo install --path .

fails with

#14 41.47 error[E0507]: cannot move out of dereference of `Octets`
#14 41.47    --> /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/vls-protocol-signer-0.11.0/src/handler.rs:690:36
#14 41.47     |
#14 41.47 690 |                   let data: Vec<_> = m
#14 41.48     |  ____________________________________^
#14 41.48 691 | |                     .u5bytes
#14 41.48 692 | |                     .clone()
#14 41.48     | |____________________________^ move occurs because value has type `std::vec::Vec<u8>`, which does not implement the `Copy` trait
#14 41.48 693 |                       .into_iter()
#14 41.48     |                        ----------- value moved due to this method call
#14 41.48     |
#14 41.48 note: `into_iter` takes ownership of the receiver `self`, which moves value
#14 41.48    --> /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/core/src/iter/traits/collect.rs:311:18
#14 41.48 help: you can `clone` the value and consume it, but this might not be your desired behavior
#14 41.48     |
#14 41.48 690 ~                 let data: Vec<_> = <std::vec::Vec<u8> as Clone>::clone(&m
#14 41.48 691 |                     .u5bytes
#14 41.48 692 ~                     .clone())
#14 41.48     |
#14 41.48 
#14 41.53 For more information about this error, try `rustc --explain E0507`.
#14 41.53 error: could not compile `vls-protocol-signer` (lib) due to 1 previous error

@darioAnongba
Copy link
Author

I opened an issue directly at Greenlight here: Blockstream/greenlight#434 but this issue makes the Rust Breez SDK unusable at the moment as it does not compile.

After some research I'm pretty sure the problem is unrelated to MacOS as I tried on my Linux machine (Ubuntu) and it fails with same error.
To be absolutely sure I created a new sample project that you can find here: https://github.com/bitcoin-numeraire/breez-sdk-sample-project. It does absolutely nothing and simply has 1 dependency, the Breez SDK (latest version but will fail with all versions that use greenlight rev=97e2f418c331653330f9fa928ed10ed1538c27d0). I didn't manage to compile it using rust 1.73, 1.76 and 1.78 so I'm pretty convinced it's not rustc either. Maybe someone can try to compile this project?

I'm pretty new to Rust so might be wrong but I think the Breez SDK is currently broken for anyone trying to use the Rust version.

@ok300
Copy link
Contributor

ok300 commented May 10, 2024

Can you please try pinning serde_bolt by updating your Cargo.toml to this:

[dependencies]
breez-sdk-core = { git = "https://github.com/breez/breez-sdk", tag = "0.4.1-rc2" }
serde_bolt = "=0.3.4"

Then

cargo clean
cargo clippy

@darioAnongba
Copy link
Author

well thank you very much this seems to work! I'm a bit out of my depth here but would be nice if I could get an explanation of the issue so I understand better as this fix is a mystery to me. Also to know at what level this should be fixed.

Cheers.

@ok300
Copy link
Contributor

ok300 commented May 10, 2024

Great, glad it fixed it for you!

From what I can tell, the issue was a combination of

  • A) how Rust handles dependencies in general
  • B) the Cargo syntax for versioning in Cargo.toml

Rust handling of dependencies

Libraries (like the Breez SDK, gl-client or vls-protocol-signer) cannot expose their Cargo.lock to callers. Cargo.lock contains the full dependency tree, with the exact version of every direct and transitive dependency. Instead, libraries can only expose their Cargo.toml which mainly specifies the direct dependencies. Based on this, Rust will then generate a fresh Cargo.lock from the available transitive dependencies.

So if a library's Cargo.toml permits a range of possible versions for one of their sub-dependencies, importing this library in a new project will likely use the latest available sub-dependency version that fits that range.

See here for more details: https://doc.rust-lang.org/cargo/faq.html#why-have-cargolock-in-version-control

... this determinism can give a false sense of security because Cargo.lock does not affect the consumers of your package, only Cargo.toml does that.

Cargo syntax for versioning in Cargo.toml

The syntax for specifying versions is a bit unituitive.

For example, VLS 0.11.0 used serde_bolt v0.3.4: https://gitlab.com/lightning-signer/validating-lightning-signer/-/blob/2770b86448cee1a2842d3d1f664a60436dcdbfbf/vls-core/Cargo.toml#L51

However, specifying serde_bolt = { version = "0.3.4" } does not mean "use version 0.3.4". Instead, according to the Cargo syntax, that means "use any version in the range >=0.3.4, <0.4.0".

See here for more details: https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-dependencies-from-cratesio

This meant that when you build your project, Cargo tries to use the latest available serde_bolt below v0.4.0, which at this time is v0.3.5.

The vls-protocol-signer was tested and released with serde_bolt v0.3.4, but Cargo tries to use serde_bolt v0.3.5 instead, which unfortunately failed.

The workaround here was to pin serde_bolt to =v0.3.4, telling Cargo to specifically use this version and not the >=0.3.4, <0.4.0 range.

@darioAnongba
Copy link
Author

Thank you very much for the explanation. For some time I thought the root of the issue was the default ^ instead of = in the dependencies and the fix was to downgrade from vls-protocol-signer v0.11.0 to v0.11.0-rc.2, which didn't work (thinking that Octet was defined by VLS when it was defined by serde_bolt).

Really great catch! Thanks again! I'll let the VLS team know that they should fix the serde_bolt version.

@ok300
Copy link
Contributor

ok300 commented May 10, 2024

Thank you for the feedback and for reporting it in the first place!

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

No branches or pull requests

3 participants