diff --git a/.coderabbit.yaml b/.coderabbit.yaml deleted file mode 100644 index d5612af9..00000000 --- a/.coderabbit.yaml +++ /dev/null @@ -1,151 +0,0 @@ -# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json -inheritance: true - -tone_instructions: > - Be direct and technical. Focus on security, correctness, and privacy - implications in this Aztec/Noir smart contract codebase. - -reviews: - profile: assertive - commit_status: true - collapse_walkthrough: false - changed_files_summary: true - sequence_diagrams: true - suggested_reviewers: true - - path_filters: - - "!artifacts/**" - - "!target/**" - - "!store/**" - - "!pxe-*/**" - - "!codegenCache.json" - - "!benchmarks/*.json" - - "!.tsbuildinfo" - - path_instructions: - - path: "src/**/main.nr" - instructions: > - This is an Aztec smart contract written in Noir. Review with focus on: - - - **Authorization (CRITICAL):** - - Every function operating on behalf of another address MUST call - `_validate_from_private` or `_validate_from_public` at the start. - - Missing authwit checks are critical security vulnerabilities. - - - **Internal function protection (CRITICAL):** - - All public state-mutating functions called from private functions MUST - have the `#[internal]` attribute. Without it, anyone can call them - directly and bypass private validation. - - - **Commitment pattern (CRITICAL):** - - Private-to-public transitions must use two-phase commit: private function - creates commitment, public `#[internal]` function validates and applies it. - - - **Note handling (CRITICAL):** - - Every spent note must emit a nullifier (via `set.remove()` or `pop_notes`). - Missing nullifiers cause double-spend vulnerabilities. - - All private note creation must use `encode_and_encrypt_note` with the - correct recipient address. - - - **Code organization:** - - Function order: initializers, private, public, view, unconstrained, - internal, then library methods. - - Imports grouped: Aztec core, external libs, custom types, globals. - - Documentation uses `/// @notice`, `/// @dev`, `/// @param`, `/// @return`. - - Library methods prefixed with underscore, internal functions suffixed - with `_internal`. - - - **Error handling:** - - Assert messages should be descriptive and guide users to fix issues. - - Balance checks should use natural underflow or explicit asserts. - - - path: "src/**/types/**/*.nr" - instructions: > - Custom Noir type definitions for Aztec contracts. Verify: - - Note types have correct field layouts for nullifier computation. - - BalanceSet operations handle multi-note edge cases. - - No unnecessary public exposure of private state types. - - - path: "src/**/test/**/*.nr" - instructions: > - Noir unit tests using TestEnvironment. Check for: - - Test coverage of both success and failure paths. - - Authorization failure tests (unauthorized caller should revert). - - Balance validation (insufficient balance should revert). - - Edge cases (zero amounts, max values, self-transfers). - - Proper use of `env.advance_block_by(1)` after private operations. - - `#[test(should_fail_with = "...")]` for expected failures with correct messages. - - - path: "src/**/test/utils.nr" - instructions: > - Shared test utilities. Ensure: - - Setup functions return consistent tuples for destructuring. - - Balance checker functions exist for both private and public state. - - Mint/transfer helpers cover common test scenarios. - - Account registration uses `cheatcodes::add_account(secret)`. - - - path: "src/ts/test/**/*.ts" - instructions: > - TypeScript integration tests using @aztec/aztec.js. Verify: - - Tests use generated artifacts from the artifacts/ directory. - - PXE setup and teardown in lifecycle hooks (beforeAll/afterAll). - - Wallet management uses AccountWalletWithSecretKey. - - Transaction execution follows `.send().wait()` pattern (note: `.wait()` is dropped in v4). - - Error handling tests verify revert messages. - - Flag tests that duplicate Noir unit test coverage to avoid redundancy. - - - path: "benchmarks/**/*.ts" - instructions: > - Performance benchmark files. Ensure benchmark operations are realistic - and cover critical paths (i.e. mint, transfer, burn) for each contract type. - - - path: "src/escrow_contract/**/*.nr" - instructions: > - Escrow contract with logic contract pattern. Pay special attention to: - - `_assert_msg_sender` validates caller matches the logic contract stored - in the deployment salt. - - `_check_escrow()` validates all contract instance properties (class_id, - public_keys, initialization_hash). - - Logic contract binding must be enforced via deployment salt. - - Stateless escrow design requires initialization_hash and deployer to be null. - - - path: "**/*.toml" - instructions: > - Noir project configuration. Verify: - - Dependencies match the project Aztec version (check package.json config.aztecVersion). - - Workspace members in root Nargo.toml include all contract packages. - - Package names follow snake_case naming conventions. - - - path: "package.json" - instructions: > - Project configuration. Verify: - - All @aztec/* dependency versions are aligned with each other. - - packageManager field is present and matches the expected Yarn version. - - Node.js engine requirements are specified (>=22). - - - path: ".github/workflows/**" - instructions: > - CI/CD workflows. Ensure: - - Aztec version is detected dynamically from package.json config. - - Local network setup steps are present for integration tests. - - Test commands match package.json scripts. - - Concurrency groups prevent duplicate runs. - - Node.js version matches project requirements (>=22). - - auto_review: - drafts: false - base_branches: - - dev - - main - - tools: - eslint: - enabled: true - semgrep: - enabled: true diff --git a/src/arc403_interface/src/main.nr b/src/arc403_interface/src/main.nr index 76dc27f8..bf7f673b 100644 --- a/src/arc403_interface/src/main.nr +++ b/src/arc403_interface/src/main.nr @@ -12,14 +12,14 @@ use aztec::macros::aztec; /// `noir-contracts/contracts/protocol_interface` packages (FeeJuice, /// ContractInstanceRegistry), which use empty-bodied `#[aztec]` contracts for the same /// purpose. The generated call interface depends only on function names, parameter -/// types, and the `#[external]` attributes — never on bodies. +/// types, and the `#[external]` attributes - never on bodies. /// /// The ARC-403 specification is a draft under active discussion: /// https://forum.aztec.network/t/arc-403-authtoken/7887 /// This interface tracks the draft as implemented by the tokens in this repository and /// may change if the specification changes. /// -/// The function bodies below are empty stubs — this artifact exists to define the ABI, +/// The function bodies below are empty stubs - this artifact exists to define the ABI, /// not to be deployed. WARNING: deploying this contract as-is and wiring it as a token's /// `auth_contract` would produce an allow-all authorizer that never denies anything. /// Implementations provide their own authorization logic (allowlists, KYC attestations, diff --git a/src/escrow_contract/src/test/test_logic_contract/src/test/utils.nr b/src/escrow_contract/src/test/test_logic_contract/src/test/utils.nr index 0a933274..26067954 100644 --- a/src/escrow_contract/src/test/test_logic_contract/src/test/utils.nr +++ b/src/escrow_contract/src/test/test_logic_contract/src/test/utils.nr @@ -134,7 +134,7 @@ pub unconstrained fn get_private_events_from_last_tx( assert( query_length <= MAX_PRIVATE_EVENTS_PER_TXE_QUERY, - "TXE query_length exceeds MAX_PRIVATE_EVENTS_PER_TXE_QUERY — sync constants with aztec-nr txe_oracles", + "TXE query_length exceeds MAX_PRIVATE_EVENTS_PER_TXE_QUERY - sync constants with aztec-nr txe_oracles", ); let mut events = BoundedVec::new(); @@ -142,7 +142,7 @@ pub unconstrained fn get_private_events_from_last_tx( let len = event_lengths[i]; assert( len <= MAX_EVENT_SERIALIZATION_LENGTH, - "TXE event payload length exceeds MAX_EVENT_SERIALIZATION_LENGTH — sync constants with aztec-nr txe_oracles", + "TXE event payload length exceeds MAX_EVENT_SERIALIZATION_LENGTH - sync constants with aztec-nr txe_oracles", ); events.push(BoundedVec::from_parts(raw_array_storage[i], len)); } diff --git a/src/vault_contract/src/main.nr b/src/vault_contract/src/main.nr index 1c2c9c29..c5ea4695 100644 --- a/src/vault_contract/src/main.nr +++ b/src/vault_contract/src/main.nr @@ -1480,7 +1480,7 @@ pub contract Vault { /// @notice Validates that the caller possesses authwit from the `from` address or the caller is the `from` address /// @dev Deliberately NOT `#[authorize_once("from", "nonce")]`: that macro asserts `nonce == 0` /// whenever `from == msg_sender`, which is sound for the token contracts (their nonce exists - /// only for the token-call authwit) but wrong here. The vault's `nonce` is dual-purpose — it + /// only for the token-call authwit) but wrong here. The vault's `nonce` is dual-purpose - it /// is forwarded as the authwit nonce of the inner token operations the vault performs on the /// user's behalf (asset `transfer_public_to_public`, shares `burn_private`/`burn_public`). /// A self-caller needs no VAULT authwit but still grants token authwits to the vault, and diff --git a/src/vault_contract/src/test/deposit_private_to_private.nr b/src/vault_contract/src/test/deposit_private_to_private.nr index 1d108e69..2fc7ec61 100644 --- a/src/vault_contract/src/test/deposit_private_to_private.nr +++ b/src/vault_contract/src/test/deposit_private_to_private.nr @@ -330,7 +330,7 @@ unconstrained fn deposit_private_to_private_on_behalf_of_other_success() { #[test] unconstrained fn deposit_private_to_private_repeated_identical_self_deposits_distinct_nonces() { // Regression guard for the vault's dual-use `nonce`: it is forwarded as the inner asset - // transfer's authwit nonce, so a SELF-caller must be able to pass a nonzero nonce — distinct + // transfer's authwit nonce, so a SELF-caller must be able to pass a nonzero nonce - distinct // nonces are what let two otherwise-identical private asset authwits both be consumed. // This is why the vault does not use #[authorize_once("from", "nonce")], which asserts // nonce == 0 whenever from == msg_sender and would revert both deposits below.