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

feat: add rustfmt imports_granularity #1077

Merged
merged 11 commits into from
Jan 5, 2024
92 changes: 59 additions & 33 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,17 @@ commands:
- run:
name: Install libclang for rocksdb
command: sudo apt install clang
install-depends-nightly:
steps:
- run:
name: Prepare for apt upgrades
command: apt update
- run:
name: Install libssl-dev for openssl-sys
command: apt install -y libssl-dev
- run:
name: Install libclang for rocksdb
command: apt install -y clang
orbs:
rust: circleci/[email protected]
win: circleci/[email protected]
Expand All @@ -49,6 +60,9 @@ executors:
IMAGE_NAME: portalnetwork/trin
docker:
- image: cimg/rust:1.71.1
docker-nightly:
docker:
- image: rustlang/rust:nightly
jobs:
docker-build:
resource_class: xlarge
Expand Down Expand Up @@ -118,30 +132,41 @@ jobs:
command: |
echo "$DOCKERHUB_PASS" | docker login -u "$DOCKERHUB_USERNAME" --password-stdin
docker push $IMAGE_NAME:latest-bridge
lint:
description: |
Check linting with Clippy and rustfmt.
resource_class: xlarge
executor:
name: rust/default
tag: 1.71.1
environment:
RUSTFLAGS: '-D warnings'
RUST_LOG: 'debug'
steps:
- checkout
- install-depends
- run:
name: Run rustfmt
command: cargo fmt --all -- --check
- run:
name: Install Clippy
command: rustup component add clippy
- setup-and-restore-sccache-cache
- run:
name: Run Clippy
command: cargo clippy --all --all-targets --all-features --no-deps -- --deny warnings
- save-sccache-cache
cargo-fmt:
description: |
Check linting with rustfmt.
resource_class: xlarge
executor: docker-nightly
environment:
RUSTFLAGS: "-D warnings"
RUST_LOG: "debug"
steps:
- checkout
- install-depends-nightly
- run:
name: Run rustfmt
command: cargo +nightly fmt --all -- --check
cargo-clippy:
description: |
Check linting with Clippy.
resource_class: xlarge
executor:
name: rust/default
tag: 1.71.1
environment:
RUSTFLAGS: "-D warnings"
RUST_LOG: "debug"
steps:
- checkout
- install-depends
- run:
name: Install Clippy
command: rustup component add clippy
- setup-and-restore-sccache-cache
- run:
name: Run Clippy
command: cargo clippy --all --all-targets --all-features --no-deps -- --deny warnings
Copy link
Member

Choose a reason for hiding this comment

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

Looking at this now, I assume here clippy is running with the nightly version, but because we don't pin Trin to nightly there may be some discrepancies between nightly clippy and stable clippy in the future.

It would be better if we split the lint job to cargo-fmt and cargo-clippy and run clippy with the default stable toolchain.

If you don't feel like doing it in this PR, we can leave it for a future fix (this will not be a blocker for merging this PR).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could try it in this PR, seems like a simple change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

all done!

- save-sccache-cache
build:
description: |
Build the crate.
Expand All @@ -150,8 +175,8 @@ jobs:
name: rust/default
tag: 1.71.1
environment:
RUSTFLAGS: '-D warnings'
RUST_LOG: 'debug'
RUSTFLAGS: "-D warnings"
RUST_LOG: "debug"
steps:
- checkout
- install-depends
Expand All @@ -167,8 +192,8 @@ jobs:
name: win/default
size: xlarge
environment:
RUSTFLAGS: '-D warnings'
RUST_LOG: 'debug'
RUSTFLAGS: "-D warnings"
RUST_LOG: "debug"
steps:
- checkout
- run:
Expand All @@ -187,8 +212,8 @@ jobs:
name: rust/default
tag: 1.71.1
environment:
RUSTFLAGS: '-D warnings'
RUST_LOG: 'debug'
RUSTFLAGS: "-D warnings"
RUST_LOG: "debug"
steps:
- checkout
- install-depends
Expand All @@ -204,8 +229,8 @@ jobs:
image: ubuntu-2204:current
resource_class: xlarge
environment:
RUSTFLAGS: '-D warnings'
RUST_LOG: 'debug'
RUSTFLAGS: "-D warnings"
RUST_LOG: "debug"
steps:
- checkout
- run:
Expand Down Expand Up @@ -276,7 +301,8 @@ workflows:
filters:
branches:
only: master
- lint
- cargo-fmt
- cargo-clippy
- build
- test
- check-windows
Expand Down
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ help:
@echo "create-docker-image - create docker image"
@echo "push-docker-image - push docker image"

lint:
lint:
cargo clippy --all --all-targets --all-features --no-deps -- --deny warnings
cargo fmt --all -- --check
cargo +nightly fmt --all -- --check

lint-unstable:
cargo clippy --all --all-targets --all-features --no-deps -- -Wclippy::cargo
cargo fmt --all -- --check
cargo +nightly fmt --all -- --check
RUSTFLAGS="-W unused_crate_dependencies" cargo build

create-docker-image:
Expand Down
2 changes: 1 addition & 1 deletion book/src/developers/contributing/git/pull_requests.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ through github via pull requests.
* Mark unfinished pull requests with the "Work in Progress" label.
* Before submitting a pr for review, you should run the following commands
locally and make sure they are passing, otherwise CI will raise an error.
* `cargo fmt --all -- --check` and `cargo clippy --all --all-targets --all-features -- --deny warnings` for linting checks
* `cargo +nightly fmt --all -- --check` and `cargo clippy --all --all-targets --all-features -- --deny warnings` for linting checks
* `RUSTFLAGS='-D warnings' cargo test --workspace` to run all tests
* Pull requests **should** always be reviewed by another member of the team
prior to being merged.
Expand Down
33 changes: 18 additions & 15 deletions ethportal-api/src/beacon.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
use crate::types::content_key::beacon::BeaconContentKey;
use crate::types::enr::Enr;
use crate::types::portal::FindNodesInfo;
use crate::types::portal::{
AcceptInfo, ContentInfo, DataRadius, PaginateLocalContentInfo, PongInfo, TraceContentInfo,
TraceGossipInfo,
use crate::{
types::{
content_key::beacon::BeaconContentKey,
enr::Enr,
portal::{
AcceptInfo, ContentInfo, DataRadius, FindNodesInfo, PaginateLocalContentInfo, PongInfo,
TraceContentInfo, TraceGossipInfo,
},
},
BeaconContentValue, PossibleBeaconContentValue, RoutingTableInfo,
};
use crate::RoutingTableInfo;
use crate::{BeaconContentValue, PossibleBeaconContentValue};
use discv5::enr::NodeId;
use jsonrpsee::{core::RpcResult, proc_macros::rpc};

Expand Down Expand Up @@ -41,8 +43,8 @@ pub trait BeaconNetworkApi {
#[method(name = "beaconPing")]
async fn ping(&self, enr: Enr) -> RpcResult<PongInfo>;

/// Send a FINDNODES request for nodes that fall within the given set of distances, to the designated
/// peer and wait for a response
/// Send a FINDNODES request for nodes that fall within the given set of distances, to the
/// designated peer and wait for a response
#[method(name = "beaconFindNodes")]
async fn find_nodes(&self, enr: Enr, distances: Vec<u16>) -> RpcResult<FindNodesInfo>;

Expand Down Expand Up @@ -77,17 +79,17 @@ pub trait BeaconNetworkApi {
limit: u64,
) -> RpcResult<PaginateLocalContentInfo>;

/// Send the provided content value to interested peers. Clients may choose to send to some or all peers.
/// Return the number of peers that the content was gossiped to.
/// Send the provided content value to interested peers. Clients may choose to send to some or
/// all peers. Return the number of peers that the content was gossiped to.
#[method(name = "beaconGossip")]
async fn gossip(
&self,
content_key: BeaconContentKey,
content_value: BeaconContentValue,
) -> RpcResult<u32>;

/// Send the provided content value to interested peers. Clients may choose to send to some or all peers.
/// Return tracing info detailing the gossip propagation.
/// Send the provided content value to interested peers. Clients may choose to send to some or
/// all peers. Return tracing info detailing the gossip propagation.
#[method(name = "beaconTraceGossip")]
async fn trace_gossip(
&self,
Expand All @@ -96,7 +98,8 @@ pub trait BeaconNetworkApi {
) -> RpcResult<TraceGossipInfo>;

/// Send an OFFER request with given ContentKey, to the designated peer and wait for a response.
/// Returns the content keys bitlist upon successful content transmission or empty bitlist receive.
/// Returns the content keys bitlist upon successful content transmission or empty bitlist
/// receive.
#[method(name = "beaconOffer")]
async fn offer(
&self,
Expand Down
8 changes: 5 additions & 3 deletions ethportal-api/src/dashboard/grafana.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ impl GrafanaAPI {

// If this is failing with a 409 error, it means that there is likely a problem with the
// template, make sure that ...
// - All "uid" fields inside a prometheus datasource object contain `"uid": "{prometheus_id}"`
// - All "uid" fields inside a prometheus datasource object contain `"uid":
// "{prometheus_id}"`
// - The root level `"id"` field is set to `null` (`"id": null`)
// - The root level `"title"` field is set to "Trin Metrics" (`"title": "Trin Metrics"`)
// - The root level `"uid"` field is set to "trin-metrics" (`"uid": "trin-metrics"`)
Expand All @@ -85,8 +86,9 @@ impl GrafanaAPI {
&template_string[..],
[
("prometheus_uid", prometheus_uid),
("", "{}"), // The templating library picks up an empty json object as a placeholder,
// so replace it with another empty json object.
("", "{}"), /* The templating library picks up an empty json object as a
* placeholder, so replace it with
* another empty json object. */
],
)?;
Ok(populated_template)
Expand Down
6 changes: 4 additions & 2 deletions ethportal-api/src/discv5.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::types::discv5::{NodeInfo, RoutingTableInfo};
use crate::types::enr::Enr;
use crate::types::{
discv5::{NodeInfo, RoutingTableInfo},
enr::Enr,
};
use discv5::enr::NodeId;
use jsonrpsee::{core::RpcResult, proc_macros::rpc};

Expand Down
33 changes: 18 additions & 15 deletions ethportal-api/src/history.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
use crate::types::content_key::history::HistoryContentKey;
use crate::types::enr::Enr;
use crate::types::portal::FindNodesInfo;
use crate::types::portal::{
AcceptInfo, ContentInfo, DataRadius, PaginateLocalContentInfo, PongInfo, TraceContentInfo,
TraceGossipInfo,
use crate::{
types::{
content_key::history::HistoryContentKey,
enr::Enr,
portal::{
AcceptInfo, ContentInfo, DataRadius, FindNodesInfo, PaginateLocalContentInfo, PongInfo,
TraceContentInfo, TraceGossipInfo,
},
},
HistoryContentValue, PossibleHistoryContentValue, RoutingTableInfo,
};
use crate::RoutingTableInfo;
use crate::{HistoryContentValue, PossibleHistoryContentValue};
use discv5::enr::NodeId;
use jsonrpsee::{core::RpcResult, proc_macros::rpc};

Expand Down Expand Up @@ -41,8 +43,8 @@ pub trait HistoryNetworkApi {
#[method(name = "historyPing")]
async fn ping(&self, enr: Enr) -> RpcResult<PongInfo>;

/// Send a FINDNODES request for nodes that fall within the given set of distances, to the designated
/// peer and wait for a response
/// Send a FINDNODES request for nodes that fall within the given set of distances, to the
/// designated peer and wait for a response
#[method(name = "historyFindNodes")]
async fn find_nodes(&self, enr: Enr, distances: Vec<u16>) -> RpcResult<FindNodesInfo>;

Expand Down Expand Up @@ -80,17 +82,17 @@ pub trait HistoryNetworkApi {
limit: u64,
) -> RpcResult<PaginateLocalContentInfo>;

/// Send the provided content value to interested peers. Clients may choose to send to some or all peers.
/// Return the number of peers that the content was gossiped to.
/// Send the provided content value to interested peers. Clients may choose to send to some or
/// all peers. Return the number of peers that the content was gossiped to.
#[method(name = "historyGossip")]
async fn gossip(
&self,
content_key: HistoryContentKey,
content_value: HistoryContentValue,
) -> RpcResult<u32>;

/// Send the provided content value to interested peers. Clients may choose to send to some or all peers.
/// Return tracing info detailing the gossip propagation.
/// Send the provided content value to interested peers. Clients may choose to send to some or
/// all peers. Return tracing info detailing the gossip propagation.
#[method(name = "historyTraceGossip")]
async fn trace_gossip(
&self,
Expand All @@ -99,7 +101,8 @@ pub trait HistoryNetworkApi {
) -> RpcResult<TraceGossipInfo>;

/// Send an OFFER request with given ContentKey, to the designated peer and wait for a response.
/// Returns the content keys bitlist upon successful content transmission or empty bitlist receive.
/// Returns the content keys bitlist upon successful content transmission or empty bitlist
/// receive.
#[method(name = "historyOffer")]
async fn offer(
&self,
Expand Down
22 changes: 10 additions & 12 deletions ethportal-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,19 @@ pub use types::content_key::{
state::StateContentKey,
};

pub use types::consensus;
pub use types::consensus::light_client;
pub use types::content_value::{
beacon::{BeaconContentValue, PossibleBeaconContentValue},
error::ContentValueError,
history::{HistoryContentValue, PossibleHistoryContentValue},
pub use types::{
consensus,
consensus::light_client,
content_value::{
beacon::{BeaconContentValue, PossibleBeaconContentValue},
error::ContentValueError,
history::{HistoryContentValue, PossibleHistoryContentValue},
},
execution::{block_body::*, header::*, receipts::*},
};
pub use types::execution::block_body::*;
pub use types::execution::header::*;
pub use types::execution::receipts::*;

// Re-exports jsonrpsee crate
pub use jsonrpsee;
pub use types::content_value::ContentValue;

pub use types::discv5::*;
pub use types::enr::*;
pub use types::node_id::*;
pub use types::{discv5::*, enr::*, node_id::*};
3 changes: 2 additions & 1 deletion ethportal-api/src/types/bootnodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ lazy_static! {
pub enum Bootnodes {
#[default]
Default,
// use explicit None here instead of Option<Bootnodes>, since default value is DEFAULT_BOOTNODES
// use explicit None here instead of Option<Bootnodes>, since default value is
// DEFAULT_BOOTNODES
None,
Custom(Vec<Bootnode>),
}
Expand Down
7 changes: 5 additions & 2 deletions ethportal-api/src/types/cli.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use clap::error::{Error, ErrorKind};
use clap::{arg, Args, Parser, Subcommand};
use clap::{
arg,
error::{Error, ErrorKind},
Args, Parser, Subcommand,
};
use ethereum_types::H256;
use std::{env, ffi::OsString, fmt, net::SocketAddr, path::PathBuf, str::FromStr};
use url::Url;
Expand Down
Loading