From 8638a7ff136cdfdb7fc0dff4fc8ed096f555d6e7 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 9 Jul 2024 10:53:32 +0200 Subject: [PATCH] Update to Rust v1.79 and fix warnings (#1903) * Update to Rust v1.79 and fix warnings * Forgot a warning * Fix warnings in full node * Should be good this time * More warnings * More warnings --- .github/workflows/ci.yml | 18 +++++----- .github/workflows/deploy.yml | 12 +++---- .github/workflows/periodic-cargo-update.yml | 2 +- full-node/src/json_rpc_service.rs | 5 --- .../chain_head_subscriptions.rs | 5 +-- .../runtime_caches_service.rs | 5 +-- lib/src/executor/allocator.rs | 27 ++++++-------- lib/src/executor/host.rs | 4 +-- light-base/src/json_rpc_service.rs | 2 ++ light-base/src/sync_service.rs | 36 +++++-------------- 10 files changed, 40 insertions(+), 76 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 98df87b751..510d1021ce 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,7 +30,7 @@ jobs: test-64bits: runs-on: ubuntu-latest container: - image: rust:1.77 + image: rust:1.79 steps: - uses: actions/checkout@v4 - uses: Swatinem/rust-cache@v2 @@ -39,7 +39,7 @@ jobs: test-32bits: runs-on: ubuntu-latest container: - image: rust:1.77 + image: rust:1.79 steps: - run: apt-get update && apt install -y libc6-dev-i386 - uses: actions/checkout@v4 @@ -50,7 +50,7 @@ jobs: wasm-node-check: runs-on: ubuntu-latest container: - image: rust:1.77 + image: rust:1.79 steps: - uses: actions/checkout@v4 - run: rustup target add wasm32-unknown-unknown @@ -66,7 +66,7 @@ jobs: check-features: runs-on: ubuntu-latest container: - image: rust:1.77 + image: rust:1.79 steps: - uses: actions/checkout@v4 - uses: Swatinem/rust-cache@v2 @@ -100,7 +100,7 @@ jobs: check-no-std: runs-on: ubuntu-latest container: - image: rust:1.77 + image: rust:1.79 steps: - uses: actions/checkout@v4 - run: rustup target add thumbv7m-none-eabi @@ -112,7 +112,7 @@ jobs: check-rustdoc-links: runs-on: ubuntu-latest container: - image: rust:1.77 + image: rust:1.79 steps: - uses: actions/checkout@v4 - uses: Swatinem/rust-cache@v2 @@ -121,7 +121,7 @@ jobs: fmt: runs-on: ubuntu-latest container: - image: rust:1.77 + image: rust:1.79 steps: # Checks `rustfmt` formatting - uses: actions/checkout@v4 @@ -135,7 +135,7 @@ jobs: clippy: runs-on: ubuntu-latest container: - image: rust:1.77 + image: rust:1.79 steps: - uses: actions/checkout@v4 # Since build artifacts are specific to a nightly version, we pin the specific nightly @@ -176,7 +176,7 @@ jobs: wasm-node-versions-match: runs-on: ubuntu-latest container: - image: rust:1.77 + image: rust:1.79 steps: - uses: actions/checkout@v4 - run: apt-get update && apt install -y jq diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index df1a5b4345..4292ce066a 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -56,7 +56,7 @@ jobs: build-js-doc: runs-on: ubuntu-latest container: - image: rust:1.77 + image: rust:1.79 steps: - uses: actions/checkout@v4 with: @@ -83,7 +83,7 @@ jobs: build-rust-doc: runs-on: ubuntu-latest container: - image: rust:1.77 + image: rust:1.79 steps: - uses: actions/checkout@v4 with: @@ -104,7 +104,7 @@ jobs: build-tests-coverage: runs-on: ubuntu-latest container: - image: rust:1.77 + image: rust:1.79 steps: - run: apt update && apt install -y jq - run: rustup component add llvm-tools-preview @@ -174,7 +174,7 @@ jobs: npm-publish: runs-on: ubuntu-latest container: - image: rust:1.77 + image: rust:1.79 steps: - uses: actions/checkout@v4 - run: rustup target add wasm32-unknown-unknown @@ -217,7 +217,7 @@ jobs: - uses: actions-rs/toolchain@v1 with: # Ideally we don't want to install any toolchain, but the GH action doesn't support this. - toolchain: 1.77 + toolchain: 1.79 profile: minimal - uses: Swatinem/rust-cache@v2 - id: compute-tag # Compute the tag that we might push. @@ -244,7 +244,7 @@ jobs: crates-io-publish: runs-on: ubuntu-latest container: - image: rust:1.77 + image: rust:1.79 steps: - uses: actions/checkout@v4 - run: cargo publish --dry-run --locked diff --git a/.github/workflows/periodic-cargo-update.yml b/.github/workflows/periodic-cargo-update.yml index b816562759..1417d5564d 100644 --- a/.github/workflows/periodic-cargo-update.yml +++ b/.github/workflows/periodic-cargo-update.yml @@ -9,7 +9,7 @@ jobs: cargo-update: runs-on: ubuntu-latest container: - image: rust:1.77 + image: rust:1.79 steps: - uses: actions/checkout@v4 # Note: `cargo update --workspace` doesn't seem to have any effect. diff --git a/full-node/src/json_rpc_service.rs b/full-node/src/json_rpc_service.rs index 0a6d4c3c3d..a4c64e7439 100644 --- a/full-node/src/json_rpc_service.rs +++ b/full-node/src/json_rpc_service.rs @@ -154,7 +154,6 @@ impl JsonRpcService { spawn_client_main_task( config.tasks_executor.clone(), - config.log_callback.clone(), config.consensus_service.clone(), config.database.clone(), to_requests_handlers.clone(), @@ -164,7 +163,6 @@ impl JsonRpcService { let runtime_caches_service = Arc::new(runtime_caches_service::RuntimeCachesService::new( runtime_caches_service::Config { tasks_executor: config.tasks_executor.clone(), - log_callback: config.log_callback.clone(), database: config.database.clone(), num_cache_entries: NonZeroUsize::new(16).unwrap(), // TODO: configurable? }, @@ -363,7 +361,6 @@ impl JsonRpcBackground { ); spawn_client_main_task( self.tasks_executor.clone(), - self.log_callback.clone(), self.consensus_service.clone(), self.database.clone(), self.to_requests_handlers.clone(), @@ -547,7 +544,6 @@ fn spawn_client_io_task( fn spawn_client_main_task( tasks_executor: Arc + Send>>) + Send + Sync>, - log_callback: Arc, consensus_service: Arc, database: Arc, to_requests_handlers: async_channel::Sender, @@ -649,7 +645,6 @@ fn spawn_client_main_task( chain_head_subscriptions::spawn_chain_head_subscription_task( chain_head_subscriptions::Config { tasks_executor: tasks_executor.clone(), - log_callback: log_callback.clone(), receiver: rx, chain_head_follow_subscription: subscription_start, with_runtime, diff --git a/full-node/src/json_rpc_service/chain_head_subscriptions.rs b/full-node/src/json_rpc_service/chain_head_subscriptions.rs index 34981b422d..270165a7ee 100644 --- a/full-node/src/json_rpc_service/chain_head_subscriptions.rs +++ b/full-node/src/json_rpc_service/chain_head_subscriptions.rs @@ -31,7 +31,7 @@ use std::{ sync::Arc, }; -use crate::{consensus_service, database_thread, LogCallback}; +use crate::{consensus_service, database_thread}; pub struct Config { /// Function that can be used to spawn background tasks. @@ -39,9 +39,6 @@ pub struct Config { /// The tasks passed as parameter must be executed until they shut down. pub tasks_executor: Arc + Send>>) + Send + Sync>, - /// Function called in order to notify of something. - pub log_callback: Arc, - /// Receiver for actions that the JSON-RPC client wants to perform. pub receiver: async_channel::Receiver, diff --git a/full-node/src/json_rpc_service/runtime_caches_service.rs b/full-node/src/json_rpc_service/runtime_caches_service.rs index fb443b62b8..280a1e45a8 100644 --- a/full-node/src/json_rpc_service/runtime_caches_service.rs +++ b/full-node/src/json_rpc_service/runtime_caches_service.rs @@ -15,7 +15,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use crate::{database_thread, LogCallback}; +use crate::database_thread; use futures_channel::oneshot; use futures_lite::{Future, StreamExt as _}; @@ -33,9 +33,6 @@ pub struct Config { /// Closure that spawns background tasks. pub tasks_executor: Arc + Send>>) + Send + Sync>, - /// Function called in order to notify of something. - pub log_callback: Arc, - /// Database to access blocks. pub database: Arc, diff --git a/lib/src/executor/allocator.rs b/lib/src/executor/allocator.rs index d0164d36e7..f92933d869 100644 --- a/lib/src/executor/allocator.rs +++ b/lib/src/executor/allocator.rs @@ -84,7 +84,7 @@ pub enum Error { /// The client passed a memory instance which is smaller than previously observed. MemoryShrinked, // TODO: wtf is "Other"? - Other(&'static str), + Other, } /// The maximum number of bytes that can be allocated at one time. @@ -102,11 +102,6 @@ const ALIGNMENT: u32 = 8; // to which it belongs. const HEADER_SIZE: u32 = 8; -/// Create an allocator error. -fn error(msg: &'static str) -> Error { - Error::Other(msg) -} - // The minimum possible allocation size is chosen to be 8 bytes because in that case we would have // easier time to provide the guaranteed alignment of 8. // @@ -144,7 +139,7 @@ impl Order { if order < N_ORDERS as u32 { Ok(Self(order)) } else { - Err(error("invalid order")) + Err(Error::Other) } } @@ -381,7 +376,7 @@ impl FreeingBumpHeapAllocator { /// - `size` - size in bytes of the allocation request pub fn allocate(&mut self, mem: &mut M, size: u32) -> Result { if self.poisoned { - return Err(error("the allocator has been poisoned")); + return Err(Error::Other); } let bomb = PoisonBomb { @@ -402,7 +397,7 @@ impl FreeingBumpHeapAllocator { // Remove this header from the free list. let next_free = Header::read_from(mem, header_ptr)? .into_free() - .ok_or_else(|| error("free list points to a occupied header"))?; + .ok_or_else(|| Error::Other)?; self.free_lists[order] = next_free; header_ptr @@ -443,7 +438,7 @@ impl FreeingBumpHeapAllocator { /// - `ptr` - pointer to the allocated chunk pub fn deallocate(&mut self, mem: &mut M, ptr: u32) -> Result<(), Error> { if self.poisoned { - return Err(error("the allocator has been poisoned")); + return Err(Error::Other); } let bomb = PoisonBomb { @@ -454,11 +449,11 @@ impl FreeingBumpHeapAllocator { let header_ptr = u32::from(ptr) .checked_sub(HEADER_SIZE) - .ok_or_else(|| error("Invalid pointer for deallocation"))?; + .ok_or_else(|| Error::Other)?; let order = Header::read_from(mem, header_ptr)? .into_occupied() - .ok_or_else(|| error("the allocation points to an empty header"))?; + .ok_or_else(|| Error::Other)?; // Update the just freed header and knit it back to the free list. let prev_head = self.free_lists.replace(order, Link::Ptr(header_ptr)); @@ -468,7 +463,7 @@ impl FreeingBumpHeapAllocator { self.total_size = self .total_size .checked_sub(order.size() + HEADER_SIZE) - .ok_or_else(|| error("Unable to subtract from total heap size without overflow"))?; + .ok_or_else(|| Error::Other)?; bomb.disarm(); Ok(()) @@ -520,16 +515,14 @@ pub trait Memory { impl Memory for [u8] { fn read_le_u64(&self, ptr: u32) -> Result { - let range = - heap_range(ptr, 8, self.len()).ok_or_else(|| error("read out of heap bounds"))?; + let range = heap_range(ptr, 8, self.len()).ok_or_else(|| Error::Other)?; let bytes = self[range] .try_into() .expect("[u8] slice of length 8 must be convertible to [u8; 8]"); Ok(u64::from_le_bytes(bytes)) } fn write_le_u64(&mut self, ptr: u32, val: u64) -> Result<(), Error> { - let range = - heap_range(ptr, 8, self.len()).ok_or_else(|| error("write out of heap bounds"))?; + let range = heap_range(ptr, 8, self.len()).ok_or_else(|| Error::Other)?; let bytes = val.to_le_bytes(); self[range].copy_from_slice(&bytes[..]); Ok(()) diff --git a/lib/src/executor/host.rs b/lib/src/executor/host.rs index 52d85c52e5..6d379b5d3d 100644 --- a/lib/src/executor/host.rs +++ b/lib/src/executor/host.rs @@ -4144,7 +4144,7 @@ enum MemAccessVm<'a> { impl<'a> allocator::Memory for MemAccess<'a> { fn read_le_u64(&self, ptr: u32) -> Result { if (ptr + 8) > u32::from(self.memory_total_pages) * 64 * 1024 { - return Err(allocator::Error::Other("out of bounds access")); + return Err(allocator::Error::Other); } // Note that this function (`read_le_u64`) really should take ̀`&mut self` but that is @@ -4215,7 +4215,7 @@ impl<'a> allocator::Memory for MemAccess<'a> { fn write_le_u64(&mut self, ptr: u32, val: u64) -> Result<(), allocator::Error> { if (ptr + 8) > u32::from(self.memory_total_pages) * 64 * 1024 { - return Err(allocator::Error::Other("out of bounds access")); + return Err(allocator::Error::Other); } let bytes = val.to_le_bytes(); diff --git a/light-base/src/json_rpc_service.rs b/light-base/src/json_rpc_service.rs index 42bea1ac79..6efb32e367 100644 --- a/light-base/src/json_rpc_service.rs +++ b/light-base/src/json_rpc_service.rs @@ -71,6 +71,7 @@ pub struct Config { /// This parameter is necessary in order to prevent users from using up too much memory within /// the client. // TODO: unused at the moment + #[allow(unused)] pub max_pending_requests: NonZeroU32, /// Maximum number of active subscriptions. Any additional subscription will be immediately @@ -79,6 +80,7 @@ pub struct Config { /// This parameter is necessary in order to prevent users from using up too much memory within /// the client. // TODO: unused at the moment + #[allow(unused)] pub max_subscriptions: u32, /// Access to the network, and identifier of the chain from the point of view of the network diff --git a/light-base/src/sync_service.rs b/light-base/src/sync_service.rs index a273f3877b..b78288d8b0 100644 --- a/light-base/src/sync_service.rs +++ b/light-base/src/sync_service.rs @@ -501,24 +501,18 @@ pub enum StorageRequestItemTy { /// The list of the descendants of the [`StorageRequestItem::key`] (including the `key` /// itself) that have a storage value is requested. /// - /// Zero or more [`StorageResultItem::DescendantValue`] will be returned where the - /// [`StorageResultItem::DescendantValue::requested_key`] is equal to - /// [`StorageRequestItem::key`]. + /// Zero or more [`StorageResultItem::DescendantValue`] will be returned. DescendantsValues, /// The list of the descendants of the [`StorageRequestItem::key`] (including the `key` /// itself) that have a storage value is requested. /// - /// Zero or more [`StorageResultItem::DescendantHash`] will be returned where the - /// [`StorageResultItem::DescendantHash::requested_key`] is equal to - /// [`StorageRequestItem::key`]. + /// Zero or more [`StorageResultItem::DescendantHash`] will be returned. DescendantsHashes, /// The Merkle value of the trie node that is the closest ancestor to /// [`StorageRequestItem::key`] is requested. - /// A [`StorageResultItem::ClosestDescendantMerkleValue`] will be returned where - /// [`StorageResultItem::ClosestDescendantMerkleValue::requested_key`] is equal to - /// [`StorageRequestItem::key`]. + /// A [`StorageResultItem::ClosestDescendantMerkleValue`] will be returned. ClosestDescendantMerkleValue, } @@ -543,18 +537,14 @@ pub enum StorageResultItem { }, /// Corresponds to a [`StorageRequestItemTy::DescendantsValues`]. DescendantValue { - /// Key that was requested. Equal to the value of [`StorageRequestItem::key`]. - requested_key: Vec, - /// Equal or a descendant of [`StorageResultItem::DescendantValue::requested_key`]. + /// Equal or a descendant of the requested key. key: Vec, /// Storage value associated with [`StorageResultItem::DescendantValue::key`]. value: Vec, }, /// Corresponds to a [`StorageRequestItemTy::DescendantsHashes`]. DescendantHash { - /// Key that was requested. Equal to the value of [`StorageRequestItem::key`]. - requested_key: Vec, - /// Equal or a descendant of [`StorageResultItem::DescendantHash::requested_key`]. + /// Equal or a descendant of the requested key. key: Vec, /// Hash of the storage value associated with [`StorageResultItem::DescendantHash::key`]. hash: [u8; 32], @@ -567,8 +557,7 @@ pub enum StorageResultItem { /// [`StorageResultItem::ClosestDescendantMerkleValue::closest_descendant_merkle_value`] /// is `Some`, then this is always the parent of the requested key. found_closest_ancestor_excluding: Option>, - /// Merkle value of the closest descendant of - /// [`StorageResultItem::DescendantValue::requested_key`]. The key that corresponds + /// Merkle value of the closest descendant of the requested key. The key that corresponds /// to this Merkle value is not included. `None` if the key has no descendant. closest_descendant_merkle_value: Option>, }, @@ -820,11 +809,7 @@ impl StorageQuery { debug_assert!(!full_storage_values_required); self.available_results.push_back(( request_index, - StorageResultItem::DescendantHash { - key, - hash, - requested_key: requested_key.clone(), - }, + StorageResultItem::DescendantHash { key, hash }, )); } prefix_proof::StorageValue::Value(value) @@ -832,11 +817,7 @@ impl StorageQuery { { self.available_results.push_back(( request_index, - StorageResultItem::DescendantValue { - requested_key: requested_key.clone(), - key, - value, - }, + StorageResultItem::DescendantValue { key, value }, )); } prefix_proof::StorageValue::Value(value) => { @@ -850,7 +831,6 @@ impl StorageQuery { hashed_value.as_bytes(), ) .unwrap(), - requested_key: requested_key.clone(), }, )); }