*: feerecipient cmd, wip

[pinebit]

Introducing new feerecipient commands and Obol API model.

category: feature
ticket: #4293

[sonarqubecloud]

Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
6.7% Duplication on New Code

See analysis details on SonarQube Cloud

General approach: Avoid parsing into a type larger than the one you eventually use, or add explicit bounds ensuring the parsed value fits in the target type before casting. Here, shareIndex is only ever compared to len(lock.Operators) and then converted to int. We can safely parse it directly as a bounded signed integer (int64 with 32‑bit size limit) or a bounded unsigned integer (uint64 with 32‑bit size), and/or add a check that the value is within the range of int.

Best targeted fix without changing behaviour: keep using strconv.ParseUint but restrict the bit size to 32 (since indices and int conversions here are logically small) and ensure the value remains in the int range. Alternatively, we can switch to strconv.ParseInt with bit size 32, store in an int64, and then cast to int. That directly matches the recommendation and removes any narrowing from a larger integer type. This is slightly clearer and avoids unsigned/signed confusion.

Concretely:

• Change the declaration of shareIndex from uint64 to int64, and parse with strconv.ParseInt(shareIndexVar, 10, 32).
• Update the bounds check to use the signed shareIndex and ensure it’s positive and ≤ int64(len(lock.Operators)).
• Adjust the cast when calling PublicShare and when indexing the partials map / setting ShareIdx so they all cast from int64 to int. Because we parsed with bit size 32 and ensured positivity, this is safe.
• No new imports are required; strconv is already imported and math is not needed because ParseInt’s bitSize argument ensures the value fits a 32‑bit signed int.

All changes are local to HandleSubmitPartialFeeRecipient in testutil/obolapimock/feerecipient.go.

---

In general, the problem is converting a parsed uint64 into a smaller integer type without an explicit bound check against that smaller type’s limits. The cleanest fix here is to avoid the problematic conversion by using int (or another appropriately sized type) consistently instead of uint64, or alternatively by adding an explicit check that the parsed value is within the target type’s range before casting.

For this specific code, the best minimal‑change fix is:

• Parse shareIndex directly as an int using strconv.Atoi, since the allowed range is already constrained by len(lock.Operators), which is itself an int. This removes the need for any uint64→int conversion.
• Replace all uses of shareIndex that currently rely on it being uint64 (the > uint64(len(lock.Operators)) comparison and the casts at lines 87 and 117–118) with the int-typed variable.
• This keeps functionality identical: the semantic checks (shareIndex == 0, shareIndex > len(lock.Operators)) and the indices passed to PublicShare and used as map keys and ShareIdx remain the same, just without unsafe casting.

No new imports are needed; strconv is already imported and used.

Concretely (within testutil/obolapimock/feerecipient.go):

• Change line 61 to shareIndex, err := strconv.Atoi(shareIndexVar).
• Adjust the upper bound check on line 68 to compare shareIndex > len(lock.Operators) instead of using uint64.
• Remove the casts int(shareIndex) on lines 87, 117, and 118, and use shareIndex directly since it will now be an int.

In general, to fix this kind of issue you either: (1) parse into a type that matches the eventual use (e.g., parse with a smaller bit size if you will store into a smaller type), or (2) add explicit upper and lower bound checks before converting to a smaller integer type. The bounds should ensure that the parsed number is within the representable range of the destination type.

Here, shareIndex is already range-checked against len(lock.Operators), but CodeQL does not recognize that as a safety guarantee for the conversion to int. The simplest robust fix that does not change behavior is to avoid storing shareIndex as an int key at all. Instead, keep it as uint64 for the map key and for ShareIdx, and only convert to int in contexts where an int is required and where we have an additional, explicit bound that guarantees safety. Concretely:

• Change the feeRecipientBlob type so partials is keyed by uint64 instead of int.
• When storing, use shareIndex directly as the map key.
• Keep ShareIdx typed as uint64 (if that matches the underlying definition) or convert to the correct concrete type; to avoid assumptions, we will use uint64(shareIndex) so there is no narrowing conversion.
• For the v.PublicShare call, which needs an int, we can safely convert because shareIndex is guaranteed to be within [1, len(lock.Operators)]. Since len(lock.Operators) is of type int, the condition shareIndex > uint64(len(lock.Operators)) ensures shareIndex-1 fits in int. To make this clear and avoid repeated casts, we can introduce a local shareIndexInt := int(shareIndex) once the check has passed and use it both in the PublicShare call and, if needed, elsewhere. However, we must keep changes minimal; therefore, we will just leave the existing int(shareIndex) - 1 (which is already safe given the check) and focus on removing the unnecessary int(shareIndex) conversions used for the map key and ShareIdx.

These changes are all within testutil/obolapimock/feerecipient.go and do not require new imports or additional helper methods.