L-01: tryDecrease return initialized value if delta is initialized

[arr00]

Summary by CodeRabbit

• Bug Fixes

  • Corrected edge-case handling for uninitialized values in safe arithmetic operations, affecting the success condition and output value behavior.

• Tests

  • Updated test cases to validate corrected edge-case scenarios.

[netlify]

✅ Deploy Preview for confidential-tokens ready!

Name	Link
🔨 Latest commit	c13536c
🔍 Latest deploy log	https://app.netlify.com/projects/confidential-tokens/deploys/6914ed4baf6a63000802eb7d
😎 Deploy Preview	https://deploy-preview-241--confidential-tokens.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The pull request modifies the tryDecrease function in FHESafeMath to change how uninitialized values are handled. When oldValue is not initialized, the function now returns success only if delta equals 0 and yields 0 as the result, replacing the previous logic that checked equality and preserved oldValue.

Changes

Cohort / File(s)	Summary
Safe Math Logic contracts/utils/FHESafeMath.sol	Modified tryDecrease edge-case handling for uninitialized oldValue: changed success condition from oldValue == delta to delta == 0, and updated result value from oldValue to 0
Test Suite Updates test/utils/FHESafeMath.test.ts	Updated test cases for tryIncrease and tryDecrease where operand is undefined, replacing undefined expected results with explicit values (e.g., [undefined, 0, 0, true] and [undefined, 1, 0, false])

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Extra attention to the logic change in tryDecrease: verify the new edge-case condition (delta == 0) correctly reflects intended behavior for uninitialized values
• Cross-check test case assertions against the updated logic to ensure test expectations match the new behavior
• Consider whether this change affects other safe math functions with similar uninitialized-value patterns

Possibly related PRs

• Support non-initialized inputs with tryIncrease(..)/tryDecrease(..) #183: Directly related modification to FHESafeMath's handling of uninitialized euint64 inputs in tryDecrease/tryIncrease with corresponding test updates.

Poem

🐰 When values hide in uninitialized shade,
We check if delta's whispers match the trade,
Now zero shines where secrets once did sleep,
Safe math keeps promises we deeply keep! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the specific function being modified (tryDecrease) and the key behavioral change (returning initialized value when delta is initialized).
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

contracts/utils/FHESafeMath.sol (1)

32-36: Inconsistent handling when both values are uninitialized.

Line 36 correctly returns an initialized value (FHE.asEuint64(0)) when oldValue is uninitialized but delta is initialized. However, line 34 still returns the uninitialized oldValue when both parameters are uninitialized. This is inconsistent with the PR's objective of returning initialized values.

Semantically, when treating uninitialized values as 0, the operation 0 - 0 = 0 should return an initialized zero value.

Apply this diff to fix the inconsistency:

         if (!FHE.isInitialized(delta)) {
-            return (FHE.asEbool(true), oldValue);
+            return (FHE.asEbool(true), FHE.asEuint64(0));
         }

🧹 Nitpick comments (1)

contracts/utils/FHESafeMath.sol (1)

17-19: Consider similar fix in tryIncrease for consistency.

The tryIncrease function has a similar pattern where it returns delta (line 19) without verifying whether delta is initialized. If delta is uninitialized, this would return an uninitialized value, which is inconsistent with the approach taken in this PR for tryDecrease.

For consistency across the library, consider updating this function to handle the case where both oldValue and delta are uninitialized.

Suggested fix:

     function tryIncrease(euint64 oldValue, euint64 delta) internal returns (ebool success, euint64 updated) {
         if (!FHE.isInitialized(oldValue)) {
+            if (!FHE.isInitialized(delta)) {
+                return (FHE.asEbool(true), FHE.asEuint64(0));
+            }
             return (FHE.asEbool(true), delta);
         }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b2c6388 and fc61c16.

📒 Files selected for processing (2)

• contracts/utils/FHESafeMath.sol (1 hunks)
• test/utils/FHESafeMath.test.ts (1 hunks)

⏰ 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). (4)

• GitHub Check: boostsecurity - boostsecurityio/semgrep-pro
• GitHub Check: coverage
• GitHub Check: tests
• GitHub Check: slither

🔇 Additional comments (1)

test/utils/FHESafeMath.test.ts (1)

64-67: LGTM! Test cases correctly validate the new behavior.

The test cases properly verify that when oldValue is uninitialized:

• undefined - 0 = 0 with success=true (line 64)
• undefined - 1 = 0 with success=false (line 67)

This correctly matches the contract logic at line 36 in FHESafeMath.sol where success = FHE.eq(delta, 0) and updated = FHE.asEuint64(0).