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

bump nightly to 2023-08-25 #33048

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions accounts-db/src/append_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,7 @@ pub mod tests {
fn set_data_len_unsafe(&self, new_data_len: u64) {
// UNSAFE: cast away & (= const ref) to &mut to force to mutate append-only (=read-only) AppendVec
unsafe {
#[allow(invalid_reference_casting)]
std::ptr::write(
std::mem::transmute::<*const u64, *mut u64>(&self.meta.data_len),
new_data_len,
Expand All @@ -711,6 +712,7 @@ pub mod tests {
fn set_executable_as_byte(&self, new_executable_byte: u8) {
// UNSAFE: Force to interpret mmap-backed &bool as &u8 to write some crafted value;
unsafe {
#[allow(invalid_reference_casting)]
std::ptr::write(
std::mem::transmute::<*const bool, *mut u8>(&self.account_meta.executable),
new_executable_byte,
Expand Down
2 changes: 1 addition & 1 deletion ci/rust-version.sh
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ fi
if [[ -n $RUST_NIGHTLY_VERSION ]]; then
nightly_version="$RUST_NIGHTLY_VERSION"
else
nightly_version=2023-04-19
nightly_version=2023-08-25
fi


Expand Down
2 changes: 2 additions & 0 deletions ci/test-checks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ _ scripts/cargo-for-all-lock-files.sh -- "+${rust_nightly}" clippy --workspace -
--deny=clippy::integer_arithmetic \
--deny=clippy::manual_let_else \
--deny=clippy::used_underscore_binding \
--allow=clippy::items-after-test-module \
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rust-lang/rust-clippy#11153

allowing this one globally Feels Bad Man ™️, but the offending code is emitted by a proc-macro and we can't put it in the right place locally 🤕

"${nightly_clippy_allows[@]}"

# temporarily run stable clippy as well to scan the codebase for
Expand All @@ -89,6 +90,7 @@ _ scripts/cargo-for-all-lock-files.sh -- clippy --workspace --tests --bins --ex
--deny=clippy::default_trait_access \
--deny=clippy::integer_arithmetic \
--deny=clippy::manual_let_else \
--allow=clippy::items-after-test-module \
--deny=clippy::used_underscore_binding

if [[ -n $CI ]]; then
Expand Down
26 changes: 13 additions & 13 deletions clap-v3-utils/src/keypair.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1003,7 +1003,8 @@ pub fn keypair_from_path(
keypair_name: &str,
confirm_pubkey: bool,
) -> Result<Keypair, Box<dyn error::Error>> {
let keypair = encodable_key_from_path(matches, path, keypair_name)?;
let skip_validation = matches.is_present(SKIP_SEED_PHRASE_VALIDATION_ARG.name);
let keypair = encodable_key_from_path(path, keypair_name, skip_validation)?;
if confirm_pubkey {
confirm_encodable_keypair_pubkey(&keypair, "pubkey");
}
Expand Down Expand Up @@ -1050,7 +1051,8 @@ pub fn elgamal_keypair_from_path(
elgamal_keypair_name: &str,
confirm_pubkey: bool,
) -> Result<ElGamalKeypair, Box<dyn error::Error>> {
let elgamal_keypair = encodable_key_from_path(matches, path, elgamal_keypair_name)?;
let skip_validation = matches.is_present(SKIP_SEED_PHRASE_VALIDATION_ARG.name);
let elgamal_keypair = encodable_key_from_path(path, elgamal_keypair_name, skip_validation)?;
if confirm_pubkey {
confirm_encodable_keypair_pubkey(&elgamal_keypair, "ElGamal pubkey");
}
Expand Down Expand Up @@ -1104,29 +1106,27 @@ pub fn ae_key_from_path(
path: &str,
key_name: &str,
) -> Result<AeKey, Box<dyn error::Error>> {
encodable_key_from_path(matches, path, key_name)
let skip_validation = matches.is_present(SKIP_SEED_PHRASE_VALIDATION_ARG.name);
encodable_key_from_path(path, key_name, skip_validation)
}

fn encodable_key_from_path<K: EncodableKey + SeedDerivable>(
matches: &ArgMatches,
path: &str,
keypair_name: &str,
skip_validation: bool,
) -> Result<K, Box<dyn error::Error>> {
let SignerSource {
kind,
derivation_path,
legacy,
} = parse_signer_source(path)?;
match kind {
SignerSourceKind::Prompt => {
let skip_validation = matches.is_present(SKIP_SEED_PHRASE_VALIDATION_ARG.name);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so there's some bug with const refs in generic contexts that triggers ICE in the targeted toolchain. took a day to isolate and stumble into this "fix" via introducing replicode.

didn't see any related issues on github and haven't had time to try to minimize repro yet

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you reported this already? Even without minimal reproduction it might be worth giving them a heads up on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not yet. if we can put this commit in, i'll have something to permalink. don't think the pr links persist after i delete the branch. otherwise hoping to have time to attempt a minimization tonight.

Ok(encodable_key_from_seed_phrase(
keypair_name,
skip_validation,
derivation_path,
legacy,
)?)
}
SignerSourceKind::Prompt => Ok(encodable_key_from_seed_phrase(
keypair_name,
skip_validation,
derivation_path,
legacy,
)?),
SignerSourceKind::Filepath(path) => match K::read_from_file(&path) {
Err(e) => Err(std::io::Error::new(
std::io::ErrorKind::Other,
Expand Down
1 change: 1 addition & 0 deletions gossip/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ num-traits = { workspace = true }
rand = { workspace = true }
rand_chacha = { workspace = true }
rayon = { workspace = true }
rustversion = { workspace = true }
serde = { workspace = true }
serde_bytes = { workspace = true }
serde_derive = { workspace = true }
Expand Down
4 changes: 4 additions & 0 deletions gossip/src/cluster_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1972,6 +1972,10 @@ impl ClusterInfo {
// Returns a predicate checking if the pull request is from a valid
// address, and if the address have responded to a ping request. Also
// appends ping packets for the addresses which need to be (re)verified.
//
// allow lint false positive trait bound requirement (`CryptoRng` only
// implemented on `&'a mut T`
#[rustversion::attr(since(1.73), allow(clippy::needless_pass_by_ref_mut))]
Comment on lines +1976 to +1978
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment pretty much covers it. allegedly this lint was introduced in 1.72, but that version was complaining that it doesn't know about it, hence the toolchain version gating.

i'm guessing this one won't be contentious and we can just put it in with a corresponding tracking issue

fn check_pull_request<'a, R>(
&'a self,
now: Instant,
Expand Down
1 change: 1 addition & 0 deletions programs/sbf/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions sdk/program/src/account_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ impl<'a> AccountInfo<'a> {
pub fn assign(&self, new_owner: &Pubkey) {
// Set the non-mut owner field
unsafe {
#[allow(invalid_reference_casting)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Lichtso says the compiler's being hyperbolic in calling this UB 😉

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From Rust perspective it is UB, but in LLVM the subsequent volatile write will become pretty well defined again.

std::ptr::write_volatile(
self.owner as *const Pubkey as *mut [u8; 32],
new_owner.to_bytes(),
Expand Down