refactor: use requires macros instead of if-else for validations

[snawaz]

With these require! macros, we log not only the error code but also the failing condition as a string.
This makes it much easier to identify the exact check that failed, even when error codes are not fully granular.

[snawaz]

• refactor: use requires macros instead of if-else for validations #129 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[coderabbitai]

Actionable comments posted: 2

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a2c0490 and e40a546.

📒 Files selected for processing (4)

• src/diff/algorithm.rs
• src/error.rs
• src/processor/fast/commit_diff.rs
• src/processor/fast/utils/requires.rs

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-10-15T11:45:25.802Z

Learnt from: snawaz
Repo: magicblock-labs/delegation-program PR: 107
File: src/entrypoint.rs:141-144
Timestamp: 2025-10-15T11:45:25.802Z
Learning: In the delegation-program codebase, prefer using `log!` (from pinocchio_log) over `msg!` for error and panic scenarios in the entrypoint code, as per maintainer preference.

Applied to files:

• src/processor/fast/utils/requires.rs

🧬 Code graph analysis (2)

src/processor/fast/utils/requires.rs (1)

src/entrypoint.rs (1)

• pinocchio (19-19)

src/diff/algorithm.rs (1)

src/diff/types.rs (1)

• changed_len (121-123)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: test

🔇 Additional comments (11)

src/error.rs (1)

5-149: LGTM! Error enum expansion is well-structured.

The new error variants follow a consistent naming pattern, have clear descriptive messages, and use sequential non-overlapping error codes (0-41, with InfallibleError at 100). The conversions to both standard and pinocchio ProgramError types are correctly implemented.

src/diff/algorithm.rs (3)

13-13: LGTM! Macro imports are correctly placed.

The require_eq and require_le macros are properly imported for use in the validation logic below.

---

263-267: LGTM! Precondition validation using macro.

The require_eq! macro correctly validates that the destination length matches the diffset's changed length, replacing the previous manual validation with a more consistent approach that includes automatic logging.

---

274-274: LGTM! Range validation using macro.

The require_le! macro correctly validates that start doesn't exceed the original length, providing better error diagnostics than the previous manual check.

src/processor/fast/utils/requires.rs (7)

30-40: LGTM! Basic require macro is correctly implemented.

The macro correctly negates the condition, logs the failing expression using stringify!, and returns the error. The implementation is clean and follows Rust macro best practices.

---

42-57: LGTM! Key comparison macro is well-implemented.

The macro correctly uses pinocchio's pubkey_eq for comparison and logs both keys when they don't match, providing excellent debugging information.

---

59-74: LGTM! Equality assertion macro is well-designed.

The macro provides excellent debugging output by logging both the expression names (via stringify!) and the actual values, making it easy to diagnose failures.

---

76-91: LGTM! Less-than-or-equal macro is correctly implemented.

The macro correctly validates the <= relationship and provides clear diagnostic output consistent with the other comparison macros.

---

93-108: LGTM! Less-than macro is correctly implemented.

The macro correctly validates the < relationship with clear diagnostic output.

---

110-125: LGTM! Greater-than-or-equal macro is correctly implemented.

The macro correctly validates the >= relationship with clear diagnostic output consistent with the other comparison macros.

---

127-151: Verify error handling asymmetry.

The macro has asymmetric error handling:

• Less case (line 137): Returns ProgramError::NotEnoughAccountKeys (standard error)
• Greater case (line 147): Returns DlpError::TooManyAccountKeys (custom error)

This asymmetry might be intentional (NotEnoughAccountKeys is a standard Solana error), but it's worth confirming this design choice is intentional.

Additionally, line 140 returns DlpError::InfallibleError when the TryInto conversion fails in the Equal case. Since we've already verified the length matches exactly, this conversion should never fail unless there's a logic error. Consider adding a comment explaining this is a defensive check.

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

src/processor/fast/commit_diff.rs (1)

52-62: Remove unprofessional comment.

The comment "// force multi-line" on line 53 is unnecessary and unprofessional for production code. The array destructuring is already clear without this comment.

🔎 Proposed fix

     let [
-        validator, // force multi-line
+        validator,
         delegated_account,

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e40a546 and 93a41b6.

📒 Files selected for processing (4)

• src/diff/algorithm.rs
• src/error.rs
• src/processor/fast/commit_diff.rs
• src/processor/fast/utils/requires.rs

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-10-15T11:45:25.802Z

Learnt from: snawaz
Repo: magicblock-labs/delegation-program PR: 107
File: src/entrypoint.rs:141-144
Timestamp: 2025-10-15T11:45:25.802Z
Learning: In the delegation-program codebase, prefer using `log!` (from pinocchio_log) over `msg!` for error and panic scenarios in the entrypoint code, as per maintainer preference.

Applied to files:

• src/processor/fast/utils/requires.rs

🧬 Code graph analysis (2)

src/diff/algorithm.rs (1)

src/diff/types.rs (1)

• changed_len (121-123)

src/processor/fast/utils/requires.rs (1)

src/entrypoint.rs (1)

• pinocchio (19-19)

🪛 GitHub Actions: Run Tests

src/processor/fast/commit_diff.rs

[error] 64-64: cannot find macro require in this scope. help: consider importing this macro through its public re-export. 1 + use crate::require;

🔇 Additional comments (6)

src/diff/algorithm.rs (2)

13-13: LGTM: Macro imports are correct.

The import of require_eq and require_le macros aligns with the PR objective to replace manual validations with macros.

---

263-267: LGTM: Validation macros improve error diagnostics.

The replacement of manual checks with require_eq! and require_le! macros provides better debugging information by logging both the error code and the failing condition, which aligns perfectly with the PR objectives.

Also applies to: 274-274

src/processor/fast/commit_diff.rs (1)

64-67: Validation macro usage is correct (pending import fix).

The require! macro usage correctly validates the data length. Once the import issue on line 13 is resolved, this validation will provide improved error diagnostics.

src/processor/fast/utils/requires.rs (2)

30-125: LGTM: Validation macros are well-implemented.

The validation macros (require!, require_eq!, require_le!, require_lt!, require_ge!, require_eq_keys!) follow a consistent pattern and provide excellent debugging information by logging both the stringified condition and actual values on failure. This aligns perfectly with the PR objective to make it easier to identify failing checks.

---

127-151: LGTM: Account count validation is comprehensive.

The require_n_accounts! macro correctly handles all three cases (too few, exact, too many) with appropriate error codes and diagnostic logging. The exact-match case properly converts to a fixed-size array reference, enabling compile-time length checking for the destructuring pattern.

src/error.rs (1)

11-132: LGTM: Error enum expansion is well-structured.

The addition of 41 new error variants provides granular error handling for the new validation macros. The consistent naming pattern for account-specific errors (e.g., CommitState*, DelegationRecord*) and clear error messages will improve debugging and error reporting.

[GabrielePicco]

LGTM!