Incorrect error revert in `operateFlowMatrix()` function

[hats-bug-reporter[bot]]

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

Description: Description\ in the operateFlowMatrix() function:

for (uint16 i = 0; i < _streams.length; i++) {
            if (!isApprovedForAll(_flowVertices[_streams[i].sourceCoordinate], msg.sender)) {
                // Operator not approved for source.
                revert CirclesHubOperatorNotApprovedForSource(
                    msg.sender, _flowVertices[_streams[i].sourceCoordinate], i, 0
                );
            }
        }

i starts from 0, like the _stream.length array. However, when the code reverts, incorrect error message is passed in the streamid parameter. Since streamid in this case starts from 1, using the i variable in streamid will be incorrect because i is always 1 unit behind the streamid, as it starts from 0.

Recommendation\ Consider implementing i + 1 in the error message to fix this issue.

    for (uint16 i = 0; i < _streams.length; i++) {
            if (!isApprovedForAll(_flowVertices[_streams[i].sourceCoordinate], msg.sender)) {
                // Operator not approved for source.
                revert CirclesHubOperatorNotApprovedForSource(
-                   msg.sender, _flowVertices[_streams[i].sourceCoordinate], i, 0
+                   msg.sender, _flowVertices[_streams[i].sourceCoordinate], i + 1, 0

                );
            }
        }

[0xarshia]

thanks for your comment

however my point is not about the flow vertices my point is about third param of error which is StreamID is being returned wrongfully as it indexed from1 but in error its like it indexed from 0 always id parameter would return wrong streams id

Stream 1: Alice intends to send 3 Circles from her to David. sourceCoordinate = 0 (Alice) flowEdgeIds = [3] (fourth edge B-D is the single terminal edge for stream 1) data (some message Alice sends along to David) Stream 2: Bob intends to send 4 Circles to David. sourceCoordinate = 1 (Bob) flowEdgeIds = [1] (second edge B-D is single terminal edge for stream 2) data Stream 3: Alice intends to send 5 Circles to Eva. sourceCoorindate = 0 flowEdgeIds = [4] (fifth edge C-E terimates stream 3) data

so shouldnt i be +1 in error instead? as the doc says thanks

[benjaminbollen]

for the actually array index Solidity does not care about this "custom definition of indexing", so it's up for debate whether the returned error value should be the array index or the custom convention. It is my judgement that it is more logical to return the array index.

[benjaminbollen]

maybe to put more colour on this: a developer is likely never exposed to these internals of "building a path data structure" as we have code that analyses the graph and produces this data. From a developer perspective, they would know that they have n streams, and that solidity indexes from zero;

so you're right that the docs make this confusing because I detail this for people to understand here the internals of the code; but for a developer using Circles contracts, would likely never know we internally shift the index of the streamIds;

I will take note to make this more clear in the docs though, so I appreciate your issue, but I will stick with this code.

[benjaminbollen]

Most likely we'll keep the code as is, but the docs should have been clearer, so thanks for the issue