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

Compilation error when using neard crate outside of the workspace #3010

Closed
frol opened this issue Jul 20, 2020 · 7 comments
Closed

Compilation error when using neard crate outside of the workspace #3010

frol opened this issue Jul 20, 2020 · 7 comments
Assignees
Labels
A-CI Area: Continuous Integration C-bug Category: This is a bug T-SRE Team: issues relevant to the SRE team

Comments

@frol
Copy link
Collaborator

frol commented Jul 20, 2020

Describe the bug

Currently, we rely on Cargo.lock to get reliable builds of nearcore, but when it comes to using nearcore as a crate (library), e.g. in NEAR Indexer, we bump into an issue of that nearcore may fail to compile due to releases of external dependencies since Cargo.lock is ignored for dependencies, and they are only effective on the binary builds.

To Reproduce

The simplest way to reproduce the issue is to remove Cargo.lock from the nearcore root folder and try to compile, you will see:

error[E0599]: no method named `sign` found for struct `ed25519_dalek::keypair::Keypair` in the current scope
   --> core/crypto/src/signature.rs:354:44
    |
354 |                 Signature::ED25519(keypair.sign(data))
    |                                            ^^^^ method not found in `ed25519_dalek::keypair::Keypair`
    |
   ::: /home/frol/.cargo/registry/src/github.com-1ecc6299db9ec823/signature-1.1.0/src/signer.rs:15:8
    |
15  |     fn sign(&self, msg: &[u8]) -> S {
    |        ----
    |        |
    |        the method is available for `std::boxed::Box<ed25519_dalek::keypair::Keypair>` here
    |        the method is available for `std::sync::Arc<ed25519_dalek::keypair::Keypair>` here
    |        the method is available for `std::rc::Rc<ed25519_dalek::keypair::Keypair>` here
    |
    = help: items from traits can only be used if the trait is in scope
    = note: the following trait is implemented but not in scope; perhaps add a `use` for it:
            `use signature::signer::Signer;`

error[E0599]: no method named `verify` found for struct `ed25519_dalek::public::PublicKey` in the current scope
   --> core/crypto/src/signature.rs:497:50
    |
497 |                     Ok(public_key) => public_key.verify(data, signature).is_ok(),
    |                                                  ^^^^^^ method not found in `ed25519_dalek::public::PublicKey`
    |
   ::: /home/frol/.cargo/registry/src/github.com-1ecc6299db9ec823/signature-1.1.0/src/verifier.rs:14:8
    |
14  |     fn verify(&self, msg: &[u8], signature: &S) -> Result<(), Error>;
    |        ------
    |        |
    |        the method is available for `std::boxed::Box<ed25519_dalek::public::PublicKey>` here
    |        the method is available for `std::sync::Arc<ed25519_dalek::public::PublicKey>` here
    |        the method is available for `std::rc::Rc<ed25519_dalek::public::PublicKey>` here
    |
    = help: items from traits can only be used if the trait is in scope
    = note: the following trait is implemented but not in scope; perhaps add a `use` for it:
            `use signature::verifier::Verifier;`

...

The root cause of this is that ed25519-dalek crate we use introduced a breaking change between 1.0.0-pre.3 (pinned in our Cargo.lock) and 1.0.0-pre.4 (which gets picked up by cargo since our core/crypto/Cargo.toml allows the newer semver-compatible versions).

Expected behavior

The project should be compilable without Cargo.lock to enable usage of nearcore as a crate dependency (in NEAR Indexer).

Solution

Introduce a CI check that compiles nearcore without Cargo.lock, so we can identify and pin our dependencies reliably.

The fix for this specific compilation error is:

diff --git a/core/crypto/Cargo.toml b/core/crypto/Cargo.toml
index d8b3559f..d85fbd8d 100644
--- a/core/crypto/Cargo.toml
+++ b/core/crypto/Cargo.toml
@@ -12,7 +12,7 @@ bs58 = "0.3"
 c2-chacha = "0.2"
 curve25519-dalek = "2"
 digest = "0.8"
-ed25519-dalek = "1.0.0-pre.3"
+ed25519-dalek = "=1.0.0-pre.3"
 lazy_static = "1.4"
 libc = "0.2"
 parity-secp256k1 = "0.7"

Version (please complete the following information):

  • nearcore commit/branch: any
  • rust version: any
@frol frol added C-bug Category: This is a bug A-CI Area: Continuous Integration T-SRE Team: issues relevant to the SRE team labels Jul 20, 2020
@bowenwang1996
Copy link
Collaborator

Should we really pin .pre versions? seems unreliable.

@frol
Copy link
Collaborator Author

frol commented Jul 20, 2020

@bowenwang1996 Why do you consider pinning to be unreliable? Alternatively, someone should now spend their time refactoring the usage of ed25519-dalek to resolve the compilation error, and require 1.0.0-pre.4 version. Do you see another solution?

@chefsale
Copy link
Contributor

Hmm, so if I just added a new step which does the build, but deletes Cargo.lock that should be good enough I assume?

@frol
Copy link
Collaborator Author

frol commented Jul 21, 2020

@chefsale yep 👍

@chefsale
Copy link
Contributor

Added the cargo crate release: https://buildkite.com/nearprotocol/nearcore-private/builds/45#9a65d68b-5a70-4687-812f-dda6d504b587, but it fails as mentioned above, not sure should I disable it for now or not and we should make sure it gets fixed?

@frol
Copy link
Collaborator Author

frol commented Jul 21, 2020

@chefsale can you submit a PR with the proposed fix and confirm that CI gets fixed? Then we just merge it

@chefsale
Copy link
Contributor

Added the PR: #3015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CI Area: Continuous Integration C-bug Category: This is a bug T-SRE Team: issues relevant to the SRE team
Projects
None yet
Development

No branches or pull requests

3 participants