Skip to content

Commit

Permalink
Disallow unchecked arithmetic (#1190)
Browse files Browse the repository at this point in the history
* Move away from using `overflow-checks`

* Remove unchecked math from test

* Simplify logic

* Install clippy on the CI

* Fix CI

* fmt

* update stale docs

* Don't run clippy while building

* Run clippy as a mandatory extra pass

* Fix riscv build

* Remove no longer needed arguments from check

* Fix cargo output parsing

* Fix doctest

* Fix clippy

* Remove build steps

* Move linting before building

* Disable warnings for metadata generation

* Add comment about warning supression.

* fmt
  • Loading branch information
athei authored Aug 14, 2023
1 parent 2f51c20 commit 29e5435
Show file tree
Hide file tree
Showing 12 changed files with 146 additions and 224 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ jobs:
toolchain: stable
default: true
target: wasm32-unknown-unknown
components: rust-src
components: rust-src, clippy

- name: Cache
uses: Swatinem/rust-cache@v2
Expand Down Expand Up @@ -183,7 +183,7 @@ jobs:
toolchain: stable
default: true
target: wasm32-unknown-unknown
components: rust-src
components: rust-src, clippy

- name: Cache
uses: Swatinem/rust-cache@v2
Expand Down
2 changes: 1 addition & 1 deletion crates/build/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ let args = contract_build::ExecuteArgs {
unstable_flags: UnstableFlags::default(),
optimization_passes: Some(OptimizationPasses::default()),
keep_debug_symbols: false,
lint: false,
dylint: false,
output_type: OutputType::Json,
skip_wasm_validation: false,
target: Target::Wasm,
Expand Down
49 changes: 5 additions & 44 deletions crates/build/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,52 +120,13 @@ impl BuildArtifacts {
/// Used as output on the cli.
pub fn steps(&self) -> usize {
match self {
BuildArtifacts::All => 4,
BuildArtifacts::CodeOnly => 3,
BuildArtifacts::All => 5,
BuildArtifacts::CodeOnly => 4,
BuildArtifacts::CheckOnly => 1,
}
}
}

/// Track and display the current and total number of steps.
#[derive(Debug, Clone, Copy)]
pub struct BuildSteps {
pub current_step: usize,
pub total_steps: Option<usize>,
}

impl BuildSteps {
pub fn new() -> Self {
Self {
current_step: 1,
total_steps: None,
}
}

pub fn increment_current(&mut self) {
self.current_step += 1;
}

pub fn set_total_steps(&mut self, steps: usize) {
self.total_steps = Some(steps)
}
}

impl Default for BuildSteps {
fn default() -> Self {
Self::new()
}
}

impl fmt::Display for BuildSteps {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let total_steps = self
.total_steps
.map_or("*".to_string(), |steps| steps.to_string());
write!(f, "[{}/{}]", self.current_step, total_steps)
}
}

/// The list of targets that ink! supports.
#[derive(
Eq,
Expand Down Expand Up @@ -199,10 +160,10 @@ impl Target {
}

/// Target specific flags to be set to `CARGO_ENCODED_RUSTFLAGS` while building.
pub fn rustflags(&self) -> &'static str {
pub fn rustflags(&self) -> Option<&'static str> {
match self {
Self::Wasm => "-C\x1flink-arg=-zstack-size=65536\x1f-C\x1flink-arg=--import-memory\x1f-Clinker-plugin-lto\x1f-C\x1ftarget-cpu=mvp",
Self::RiscV => "-Clinker-plugin-lto",
Self::Wasm => Some("-Clink-arg=-zstack-size=65536\x1f-Clink-arg=--import-memory\x1f-Ctarget-cpu=mvp"),
Self::RiscV => None,
}
}

Expand Down
46 changes: 8 additions & 38 deletions crates/build/src/docker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,7 @@ use tokio_stream::{

use crate::{
verbose_eprintln,
BuildArtifacts,
BuildResult,
BuildSteps,
CrateMetadata,
ExecuteArgs,
Verbosity,
Expand Down Expand Up @@ -119,21 +117,13 @@ pub fn docker_build(args: ExecuteArgs) -> Result<BuildResult> {
verbosity,
output_type,
target,
build_artifact,
image,
..
} = args;
tokio::runtime::Builder::new_multi_thread()
.enable_all()
.build()?
.block_on(async {
let mut build_steps = BuildSteps::new();

build_steps.set_total_steps(3);
if build_artifact == BuildArtifacts::CodeOnly {
build_steps.set_total_steps(2);
}

let crate_metadata = CrateMetadata::collect(&manifest_path, target)?;
let host_folder = std::env::current_dir()?;
let args = compose_build_args()?;
Expand All @@ -159,23 +149,19 @@ pub fn docker_build(args: ExecuteArgs) -> Result<BuildResult> {
&crate_metadata.contract_artifact_name,
&host_folder,
&verbosity,
&mut build_steps,
)
.await?;

let mut build_result =
run_build(&client, &container, &verbosity, &mut build_steps).await?;
let mut build_result = run_build(&client, &container, &verbosity).await?;

update_build_result(&host_folder, &mut build_result)?;

update_metadata(&build_result, &verbosity, &mut build_steps, &image, &client)
.await?;
update_metadata(&build_result, &verbosity, &image, &client).await?;

build_steps.increment_current();
verbose_eprintln!(
verbosity,
" {} {}",
format!("{build_steps}").bold(),
"[==]".bold(),
"Displaying results".bright_cyan().bold(),
);

Expand Down Expand Up @@ -228,7 +214,6 @@ fn update_build_result(host_folder: &Path, build_result: &mut BuildResult) -> Re
async fn update_metadata(
build_result: &BuildResult,
verbosity: &Verbosity,
build_steps: &mut BuildSteps,
build_image: &str,
client: &Docker,
) -> Result<()> {
Expand All @@ -250,13 +235,7 @@ async fn update_metadata(

metadata.image = Some(image_tag);

crate::metadata::write_metadata(
metadata_artifacts,
metadata,
build_steps,
verbosity,
true,
)?;
crate::metadata::write_metadata(metadata_artifacts, metadata, verbosity, true)?;
}
Ok(())
}
Expand Down Expand Up @@ -287,7 +266,6 @@ async fn create_container(
contract_name: &str,
host_folder: &Path,
verbosity: &Verbosity,
build_steps: &mut BuildSteps,
) -> Result<String> {
let entrypoint = vec!["cargo".to_string(), "contract".to_string()];

Expand Down Expand Up @@ -378,8 +356,7 @@ async fn create_container(
}
) {
// no such image locally, so pull and try again
pull_image(client, build_image.to_string(), verbosity, build_steps)
.await?;
pull_image(client, build_image.to_string(), verbosity).await?;
client
.create_container(options, config)
.await
Expand All @@ -397,7 +374,6 @@ async fn run_build(
client: &Docker,
container_name: &str,
verbosity: &Verbosity,
build_steps: &mut BuildSteps,
) -> Result<BuildResult> {
client
.start_container::<String>(container_name, None)
Expand All @@ -418,7 +394,7 @@ async fn run_build(
verbose_eprintln!(
verbosity,
" {} {}",
format!("{build_steps}").bold(),
"[==]".bold(),
format!("Started the build inside the container: {}", container_name)
.bright_cyan()
.bold(),
Expand Down Expand Up @@ -487,12 +463,7 @@ fn compose_build_args() -> Result<Vec<String>> {
}

/// Pulls the docker image from the registry.
async fn pull_image(
client: &Docker,
image: String,
verbosity: &Verbosity,
build_steps: &mut BuildSteps,
) -> Result<()> {
async fn pull_image(client: &Docker, image: String, verbosity: &Verbosity) -> Result<()> {
let mut pull_image_stream = client.create_image(
Some(CreateImageOptions {
from_image: image,
Expand All @@ -505,12 +476,11 @@ async fn pull_image(
verbose_eprintln!(
verbosity,
" {} {}",
format!("{build_steps}").bold(),
"[==]".bold(),
"Image does not exist. Pulling one from the registry"
.bright_cyan()
.bold()
);
build_steps.increment_current();

if verbosity.is_verbose() {
show_pull_progress(pull_image_stream).await?
Expand Down
Loading

0 comments on commit 29e5435

Please sign in to comment.