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

Affected by E0034 in from old crate dependency #8083

Closed
gluax opened this issue Nov 4, 2022 · 8 comments
Closed

Affected by E0034 in from old crate dependency #8083

gluax opened this issue Nov 4, 2022 · 8 comments

Comments

@gluax
Copy link

gluax commented Nov 4, 2022

Describe the bug
cargo building in some use cases, for example with the conjunction of the blsttc crate, of this crate causes rust error E0034 caused by a dependency.

To Reproduce
The code:

use near_sdk::borsh::{self, BorshDeserialize, BorshSerialize};
use near_sdk::near_bindgen;
use blsttc::{Signature, PublicKey};

#[near_bindgen]
#[derive(Default, BorshDeserialize, BorshSerialize)]
pub struct VerifySignature;
#[near_bindgen]
impl VerifySignature {
    #[payable]
    pub fn verify_sig(&mut self, message: Vec<u8>, signature: Vec<u8>, public_key: Vec<u8>) -> bool {   
        let signature: [u8; 96] = signature.try_into().expect("invalid signature");
        let public_key: [u8; 48] = public_key.try_into().expect("invalid pk");
        let sig = Signature::from_bytes(signature).expect("couldn't read signature");
        let pk = PublicKey::from_bytes(public_key).expect("couldn't read pk");
        pk.verify(&sig, message)
        
    }
}

#[cfg(not(target_arch = "wasm32"))]
#[cfg(test)]
mod tests {
    use super::*;
    use near_sdk::test_utils::VMContextBuilder;
    use near_sdk::{testing_env, VMContext};

    use blsttc::{PublicKey, SecretKey};


    fn get_context(is_view: bool) -> VMContext {
        VMContextBuilder::new()
            .signer_account_id("bob_near".parse().unwrap())
            .is_view(is_view)
            .build()
    }

    #[test]
    fn verify_msg() {
        let context = get_context(false);
        testing_env!(context);
        let mut contract = VerifySignature::default();

        let sk = SecretKey::random();
        let pk = sk.public_key();
        let message = "hello".try_to_vec().unwrap();
        let sig = sk.sign(message.clone());
        
        println!("sk: {:?}", sk);
        println!("pk: {:?}", pk);
        println!("sig: {:?}", sig);

        println!("pk_bytes: {:?}", pk.as_bytes());
        println!("sig_bytes: {:?}", sig.as_bytes());
        println!("sig_bytes: {:?}", sig.as_bytes());

        let sig_bytes = sig.to_bytes().to_vec();
        let pk_bytes= pk.to_bytes().to_vec();
        let verified = contract.verify_sig(message, sig_bytes, pk_bytes);

        println!("verified: {:?}", verified);

    }

}```

Deps:
```toml
near-sdk = "4.0.0"
blsttc = {version = "7.1.0"}
getrandom = {version = "0.2.8", features = ["js"]}

Expected behavior
A clear and concise description of what you expected to happen.

Screenshots
image

Versions
near-sdk: 4.0
blsttc: 7.1.0(this crate has since updated to version 8.0.0, but this still remains an issue).

Happens on linux and windows.

Additional context
Seems to be a known bug that affects older versions of the bitvec crate. I think since both crates import different versions somewhere down the line the issue is exposed when compiling.

Ideally, both crates should update their deps, but I know it's in sub-dep hell for both. But the latest version of bitvec no longer has this bug.

@austinabell
Copy link
Contributor

Have you tried pinning the funty dependency as suggested in this comment?

I've seen that work for similar situations before.

Otherwise, you should just be able to disable the unit-testing feature that's enabled by default as a workaround and just write e2e tests with https://github.com/near/workspaces-rs instead of unit tests

@gluax
Copy link
Author

gluax commented Nov 4, 2022

Pinning, unfortunately, does not work, and even with default-features = false for near-sdk the same errors from the original post occur when trying to build.

@austinabell
Copy link
Contributor

austinabell commented Nov 4, 2022

Pinning, unfortunately, does not work, and even with default-features = false for near-sdk the same errors from the original post occur when trying to build.

ah, those deps were gated behind that feature in 4.1.0-pre.0 and above versions, that's why that dep is still being pulled in for 4.0.

@gluax
Copy link
Author

gluax commented Nov 17, 2022

@austinabell thanks, setting the version to 4.1.0-pre.0 does work with default-features = false. Hopefully, in the future, we can update those dependencies better?

@gluax
Copy link
Author

gluax commented Nov 17, 2022

Digging further it seems near-sdk depends on near-crypto version 0.13.0 but even the latest version(0.15.0) of near-crypto uses primitives-types version 0.10.0 while upgrading this crate to version 0.12.1 there would then update to a version of bitvec that no longer has this bug.

There's a similar issue in the blsttc crate of them not updating a single dependency and therefore having a buggy version of bitvec. I will similarly open an issue on that crate.

@austinabell
Copy link
Contributor

@austinabell thanks, setting the version to 4.1.0-pre.0 does work with default-features = false. Hopefully, in the future, we can update those dependencies better?

Those dependencies you mention are outside of the control of this repo, but that was the intention of the feature flag, to make this less of a pain point to be able to ignore these deps!

Digging further it seems near-sdk depends on near-crypto version 0.13.0 but even the latest version(0.15.0) of near-crypto uses primitives-types version 0.10.0 while upgrading this crate to version 0.12.1 there would then update to a version of bitvec that no longer has this bug.

There's a similar issue in the blsttc crate of them not updating a single dependency and therefore having a buggy version of bitvec. I will similarly open an issue on that crate.

Perhaps this issue can be moved to nearcore to see if it's possible to update the bitvec dependency. Unfortunately, even in that case, the timeline for that to be updated in the SDK will likely be slow (to avoid breakages).

Would you like me to move this issue?

@gluax
Copy link
Author

gluax commented Nov 17, 2022

Yea, we can work around it for now with that feature flag, but would be ideal to see the dep updates eventually! So I think moving the issue is a good call :). Much appreciated!

@austinabell austinabell transferred this issue from near/near-sdk-rs Nov 18, 2022
@matklad
Copy link
Contributor

matklad commented Nov 18, 2022

Fixed on master with ff8e885

@matklad matklad closed this as completed Nov 18, 2022
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