diff --git a/.github/workflows/on-pull-request.yml b/.github/workflows/on-pull-request.yml index 7d21e4a6..de7c4553 100644 --- a/.github/workflows/on-pull-request.yml +++ b/.github/workflows/on-pull-request.yml @@ -6,184 +6,72 @@ on: env: CARGO_TERM_COLOR: always NF4_LARGE_BLOCK_TEST: "true" - + jobs: build: name: Check + Lint - runs-on: self-hosted - environment: DEV + runs-on: ubuntu-latest steps: - name: Checkout uses: actions/checkout@v4 - # Add Node so npx exists for OZ upgrades validation + + # Foundry install + - name: Install Foundry (forge, cast) - needed by build.rs + uses: foundry-rs/foundry-toolchain@v1 + with: + version: nightly + + - name: Verify forge + run: | + which forge + forge --version + + - name: Install Rust nightly with rustfmt & clippy + shell: bash + run: | + rustup toolchain install nightly + rustup +nightly component add rustfmt clippy + - name: Setup Node.js (for upgrades-core validator) uses: actions/setup-node@v4 with: node-version: 20 + - name: Install upgrades-core globally and verify npx + shell: bash run: | npm i -g @openzeppelin/upgrades-core@^1.37.0 - which node && node -v - which npm && npm -v - which npx && npx --version - shell: bash - - name: Export PATH for child processes - run: echo "PATH=$PATH" >> $GITHUB_ENV + node -v + npm -v + npx --version + - name: Run cargo fmt run: cargo +nightly fmt + - name: Run clippy - run: cargo clippy --all-targets -- -D warnings + run: cargo +nightly clippy --all-targets -- -D warnings + - name: Generate keys run: | cargo build cargo run --release --bin key_generation + - name: Run test run: cargo test - resync: - name: Synchronisation Test with Mock Prover - needs: [build] - runs-on: self-hosted - env: - NF4_MOCK_PROVER: "true" - NF4_CONTRACTS__DEPLOY_CONTRACTS: "true" - steps: - - name: Checkout - uses: actions/checkout@v4 - - name: Clear previous run - run: docker compose --profile sync_test down -v - - name: Build - run: cargo build - - name: Generate keys - run: cargo run --release --bin key_generation - - name: Build Docker Image - run: docker compose --profile sync_test build - - name: Run Integration Test - run: docker compose --profile development run test - - name: Print logs - run: docker compose --profile development logs - - name: Remove databases - run: docker compose --profile development rm -fvs client client2 proposer db_client db_client2 db_proposer - - name: Shut Down Containers - run: | - docker compose --profile sync_test down -v - - rollup-prover: - name: Rollup prover integration Test - timeout-minutes: 480 - needs: [build, resync] # This test is long so start it after we're sure the others have passed - runs-on: self-hosted - env: - NF4_MOCK_PROVER: "false" - NF4_CONTRACTS__DEPLOY_CONTRACTS: "true" - steps: - - name: Checkout - uses: actions/checkout@v4 - - name: link pptau cache - # This is a workaround to avoid downloading the ppot_26.ptau file every time. It creates a symlink to a cached file (which will be downloaded if it doesn't exist on the testrunner). - run: | - if [ -e /home/testrunner/ptau-cache/ppot_26.ptau ] - then - echo "Using cached ppot_26.ptau" - else - echo "Downloading ppot file to cache" - mkdir -p /home/testrunner/ptau-cache - wget https://pse-trusted-setup-ppot.s3.eu-central-1.amazonaws.com/pot28_0080/ppot_0080_26.ptau - mv ppot_0080_26.ptau /home/testrunner/ptau-cache/ppot_26.ptau - chmod 444 ptau-cache/ppot_26.ptau # Make it read-only - echo "Cached ppot_26.ptau created" - fi - ln -sf /home/testrunner/ptau-cache/ppot_26.ptau /home/testrunner/actions-runner/_work/nightfall_4_PV/nightfall_4_PV/configuration/bin/ppot_26.ptau - - name: link bn254 setup cache - shell: bash - run: | - set -euo pipefail - # Version the cache so you can invalidate by bumping NF4_SRS_VERSION - VERSION_SUFFIX="${NF4_SRS_VERSION:-v1}" - CACHE_DIR="/home/testrunner/srs-cache" - CACHE_FILE="$CACHE_DIR/bn254_setup_26.${VERSION_SUFFIX}.cache" - TARGET_DIR="$GITHUB_WORKSPACE/configuration/bin" - TARGET="$TARGET_DIR/bn254_setup_26.cache" - mkdir -p "$CACHE_DIR" "$TARGET_DIR" - # Prevent two concurrent workflows from trampling the cache - LOCKFILE="$CACHE_DIR/.bn254_setup.lock" - exec 9>"$LOCKFILE" - flock -w 600 9 || (echo "Failed to acquire cache lock" >&2; exit 1) - if [ -e "$CACHE_FILE" ]; then - echo "Using cached bn254_setup_26.cache at $CACHE_FILE" - ln -sf "$CACHE_FILE" "$TARGET" - ls -l "$TARGET" - else - echo "No cached bn254_setup_26.cache found. Generating once, then caching…" - # Ensure keygen is available - cargo build - # Prefer a narrow generation if supported; fall back to full keygen - if ! NF4_MOCK_PROVER=false cargo run --release --bin key_generation -- --only-bn254-setup; then - NF4_MOCK_PROVER=false cargo run --release --bin key_generation - fi - # Find the generated file (expecting TARGET); fallback to a search - if [ ! -e "$TARGET" ]; then - FOUND="$(find "$GITHUB_WORKSPACE" -maxdepth 5 -type f -name 'bn254_setup_26.cache' | head -n1 || true)" - if [ -z "$FOUND" ]; then - echo "ERROR: Could not locate bn254_setup_26.cache after key generation." >&2 - exit 1 - fi - ln -sf "$FOUND" "$TARGET" - fi - # Populate persistent cache and relink from there (immutable) - cp -f "$TARGET" "$CACHE_FILE" - chmod 444 "$CACHE_FILE" - ln -sf "$CACHE_FILE" "$TARGET" - echo "Cached and linked:" - ls -l "$CACHE_FILE" "$TARGET" - fi - - name: Clear previous run - run: docker compose --profile sync_test down -v - - name: Build - run: cargo build - - name: Generate keys - run: NF4_MOCK_PROVER=false cargo run --release --bin key_generation - - name: Hydrate setup files for Docker build (dereference symlinks) - shell: bash - run: | - set -euo pipefail - BIN_DIR="$GITHUB_WORKSPACE/configuration/bin" - mkdir -p "$BIN_DIR" - hydrate() { - local f="$1" - local p="$BIN_DIR/$f" - if [ ! -e "$p" ]; then - echo "ERROR: Expected artifact missing: $p" >&2 - exit 1 - fi - if [ -L "$p" ]; then - echo "Dereferencing symlink: $p" - # Copy the symlink's target contents into the workspace file - cp -fL "$p" "$p.tmp" - chmod 444 "$p.tmp" - mv -f "$p.tmp" "$p" - else - echo "Already a regular file: $p" - fi - ls -l "$p" - } - hydrate "ppot_26.ptau" - hydrate "bn254_setup_26.cache" - - name: Build Docker Image - run: docker compose --profile sync_test build - - name: Run Integration Test - run: docker compose --profile development run test - - name: Print logs - run: docker compose --profile development logs - - name: Shut Down Containers - run: | - docker compose --profile sync_test down -v forge: name: Smart Contract Unit Tests needs: [build] - runs-on: self-hosted + runs-on: ubuntu-latest steps: - name: Checkout uses: actions/checkout@v4 + + # Same action here (jobs do not share state) + - name: Install Foundry (forge, cast) + uses: foundry-rs/foundry-toolchain@v1 + with: + version: nightly + - name: Run Forge Tests - run: forge test \ No newline at end of file + run: forge test diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 00000000..4c041019 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,58 @@ +# Contributing to nightfall_4_Starknet + +Thanks for contributing! 🎉 +This repository is a **fork** of `nightfall_4_CE`, but all development happens **here**. + +Please follow the steps below to avoid accidentally opening PRs against the upstream repo. + +--- + +## Important: Where to open Pull Requests + +Although this repository is forked from `nightfall_4_CE`, **all pull requests must target this repository**: + +**`EYBlockchain/nightfall_4_Starknet`** + +Do **NOT** open PRs against `nightfall_4_CE` unless explicitly requested. + +--- + +## How to open a Pull Request (recommended workflow) + +### 1. Create a feature branch +```bash +git checkout -b / +``` + +### 2. Push your branch to this repo +Make sure your origin remote points to nightfall_4_Starknet. +```bash +git push -u origin HEAD +``` + +### 3. Open the Pull Request in this repo +Open this link in your browser and select your branch: +```bash +https://github.com/EYBlockchain/nightfall_4_Starknet/compare +``` + +## PR quality & review process + +- **Small, focused PRs (single concern).** Split large work into reviewable chunks. Prefer multiple PRs over one mega-PR. + +- **PRs must be self-contained and well described.** Include: + - a clear title, + - brief context (why this change is needed), + - a comprehensive description of what changed and how it was validated (tests, screenshots/logs, etc.). + +- **Reviews are collaborative.** Expect a detailed back-and-forth to reach a high-quality outcome. + +- **Authors should not resolve review comments themselves.** + Comments should be resolved by the **reviewer** once they are satisfied with the fix or clarification. This ensures feedback is validated and closed with reviewer consent. + +- **Respond to each review comment with a fixup commit link.** + When addressing feedback, reply to each comment with a link to the specific fixup commit that implements the change. This lets reviewers verify updates incrementally. + +- **Squash fixups before merge.** + After review is complete, squash fixup commits into the main branch history prior to merge (keep the final history clean). + diff --git a/lib/src/hex_conversion.rs b/lib/src/hex_conversion.rs index 8b51588f..a3c981e1 100644 --- a/lib/src/hex_conversion.rs +++ b/lib/src/hex_conversion.rs @@ -94,7 +94,7 @@ impl HexConvertible for Vec { fn from_hex_string(hex_str: &str) -> Result, HexError> { let s_int = hex_str.strip_prefix("0x").unwrap_or(hex_str); - if s_int.len() % 2 != 0 || s_int.is_empty() { + if !s_int.len().is_multiple_of(2) || s_int.is_empty() { return Err(HexError::InvalidStringLength); } hex::decode(s_int).map_err(|_| HexError::InvalidString) diff --git a/lib/src/merkle_trees/append_only.rs b/lib/src/merkle_trees/append_only.rs index ee872fc4..db9f7d44 100644 --- a/lib/src/merkle_trees/append_only.rs +++ b/lib/src/merkle_trees/append_only.rs @@ -129,7 +129,7 @@ where let sub_tree_capacity = pow2_usize(metadata.sub_tree_height).ok_or_else(|| { Self::Error::Error("sub_tree_capacity too large to compute capacity safely".to_string()) })?; - if leaves.len() % sub_tree_capacity != 0 { + if !leaves.len().is_multiple_of(sub_tree_capacity) { return Err(Self::Error::IncorrectBatchSize); } if leaves.is_empty() { diff --git a/lib/src/merkle_trees/mutable.rs b/lib/src/merkle_trees/mutable.rs index 1ce38686..ada1a393 100644 --- a/lib/src/merkle_trees/mutable.rs +++ b/lib/src/merkle_trees/mutable.rs @@ -425,7 +425,7 @@ where let sub_tree_capacity = pow2_usize(metadata.sub_tree_height).ok_or_else(|| { Self::Error::Error("sub_tree_height too large to compute sub tree capacity".to_string()) })?; - if leaves.len() % sub_tree_capacity != 0 { + if !leaves.len().is_multiple_of(sub_tree_capacity) { return Err(Self::Error::IncorrectBatchSize); } if leaves.is_empty() { @@ -812,7 +812,7 @@ where let sub_tree_leaf_capacity = pow2_usize(metadata.sub_tree_height).ok_or_else(|| { Self::Error::Error("sub_tree_height too large to compute capacity safely".to_string()) })?; - if commitments.len() % sub_tree_leaf_capacity != 0 { + if !commitments.len().is_multiple_of(sub_tree_leaf_capacity) { return Err(Self::Error::IncorrectBatchSize); } // call insert circuit for each sub_tree diff --git a/lib/src/merkle_trees/trees.rs b/lib/src/merkle_trees/trees.rs index 541921bb..b266124b 100644 --- a/lib/src/merkle_trees/trees.rs +++ b/lib/src/merkle_trees/trees.rs @@ -435,7 +435,7 @@ pub(crate) mod helper_functions { let mut pow1: usize = 2; let mut pow2: usize = pow1 << 1; while index == 0 { - if (leaf_index + 1 - pow1) % pow2 == 0 { + if (leaf_index + 1 - pow1).is_multiple_of(pow2) { index = exp1; } else { pow1 = pow2; diff --git a/lib/src/serialization.rs b/lib/src/serialization.rs index 4d5443c9..2203c46e 100644 --- a/lib/src/serialization.rs +++ b/lib/src/serialization.rs @@ -125,7 +125,7 @@ where .map_err(serde::de::Error::custom)?; Ok(fr) } - +#[allow(dead_code)] struct FrWrapper(Fr); impl Serialize for FrWrapper { diff --git a/lib/src/validate_keys.rs b/lib/src/validate_keys.rs index 94af676d..946b3062 100644 --- a/lib/src/validate_keys.rs +++ b/lib/src/validate_keys.rs @@ -196,17 +196,17 @@ async fn handle_keys_validation(params: FormParams) -> Result