hats-finance / Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf

Circles Protocol contracts
https://aboutcircles.com
GNU Affero General Public License v3.0
0 stars 0 forks source link

Incorrect `error` revert in `_matchNettedFlows()` function #41

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x1ba3e28bee5090b61d7a9fc379a6ec1de3218c283307c6d43ad9f1588a22f411 Severity: low

Description: Description\ In _matchNettedFlows() of Hub.sol:

    function _matchNettedFlows(int256[] memory _streamsNettedFlow, int256[] memory _matrixNettedFlow) internal pure {
        if (_streamsNettedFlow.length != _matrixNettedFlow.length) {
            // Mismatch in netted flow length.
            revert CirclesArraysLengthMismatch(_streamsNettedFlow.length, _matrixNettedFlow.length, 5);
        }
        for (uint16 i = 0; i < _streamsNettedFlow.length; i++) {
            if (_streamsNettedFlow[i] != _matrixNettedFlow[i]) {
                // Intended flow does not match verified flow.
@>                revert CirclesHubNettedFlowMismatch(i, _streamsNettedFlow[i], _matrixNettedFlow[i]);
            }
        }
    }

When streamsNettedFlow is not equal to matrixNettedFlow, the error CirclesHubNettedFlowMismatch is reverted.

The issue is that, error CirclesHubNettedFlowMismatch() is incorrectly implemented and it is not correct. error CirclesHubNettedFlowMismatch() in Errors.sol is implemented as:

    error CirclesHubNettedFlowMismatch(uint16 vertexPosition, int256 matrixNettedFlow, int256 streamNettedFlow);

Here, in _matchNettedFlows() function, the values of matrixNettedFlow and streamNettedFlow are interchanged and wont be correctly returned while the error would be reverted due to incorrect implementation. Due to this issue, the function caller would be mislead with incorrect values of matrix and stream netted flow.

Recommendations\ Correct the error CirclesHubNettedFlowMismatch() revert implementation as below:

    function _matchNettedFlows(int256[] memory _streamsNettedFlow, int256[] memory _matrixNettedFlow) internal pure {
        if (_streamsNettedFlow.length != _matrixNettedFlow.length) {
            // Mismatch in netted flow length.
            revert CirclesArraysLengthMismatch(_streamsNettedFlow.length, _matrixNettedFlow.length, 5);
        }
        for (uint16 i = 0; i < _streamsNettedFlow.length; i++) {
            if (_streamsNettedFlow[i] != _matrixNettedFlow[i]) {
                // Intended flow does not match verified flow.
-                revert CirclesHubNettedFlowMismatch(i, _streamsNettedFlow[i], _matrixNettedFlow[i]);
+                 revert CirclesHubNettedFlowMismatch(i, _matrixNettedFlow[i], _streamsNettedFlow[i]);
            }
        }
    }
benjaminbollen commented 1 month ago

Thank you for your detailed report on the incorrect error revert in the _matchNettedFlows() function. We've reviewed your submission and agree that this is a valid low-severity issue.

You've correctly identified that the error definition and the actual revert statement have the matrixNettedFlow and streamNettedFlow values swapped. This inconsistency, while not affecting the functionality of the code, could potentially lead to confusion during debugging or error handling.

We appreciate your attention to detail in spotting this discrepancy. Your report helps us maintain consistency and clarity in our error handling, which is crucial for effective debugging and system maintenance.

Thank you for your valuable contribution to improving the quality and reliability of our codebase.