-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Conversation
“easy” means that there are no major changes required.
Does this supersedes: #3532 ? |
It is not even building and when it includes the other pr, it supersedes it. |
Please fix the build. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as a note the following crates has been updated in there major release:
rpassword
futures-timer
rand
futures-preview
parity-wasm
ed25519-dalek
zeroize
as updated versions have been published to crates.io
Otherwise, rustc complains (correctly) about duplicate lang items.
Rust does not allow duplicate lang items. When compiled without the `std` feature, `sr-io` defines two lang items. Therefore, `sr-io` compiled without `feature = "std"` must not be linked with `std`. However, `pwasm-utils` and `wasmi-validation` both bring in `std` unless compiled with `default-features = "false"`. This caused a duplicate lang item error. Building both with `default-features = "false"` prevents this error. When building with `feature = "std"`, they should both be built with the `std` feature, so this feature needs to be explicitly depended on.
Three tests used 1 less gas than they had previously.
This is ready to be merged. |
@@ -458,7 +458,7 @@ impl TraitPair for Pair { | |||
Err(_) => return false | |||
}; | |||
|
|||
match public_key.verify(message.as_ref(), &sig) { | |||
match public_key.verify::<sha2::Sha512>(message.as_ref(), &sig) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate on those? Are these changes equivalent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tomaka, can you please confirm this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to check it myself and it seems that it should be fine. However, I am not 100%.
Co-Authored-By: Sergei Pepyakin <sergei@parity.io>
dev-dependencies need `default-features = false`, too.
…to demi/upgrade-compatible-deps
…ible-deps Also bump more dependencies by minor versions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just some last nitpicks
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Co-Authored-By: Sergei Pepyakin <sergei@parity.io>
“easy” means that there are no major changes required.
Fixes #3685