Incorrect Calculation of Remaining Hours in Day B in _calculateIssuance Function

[hats-bug-reporter]

Github username: @0xmahdirostami
Twitter username: 0xmahdirostami
Submission hash (on-chain): 0x27f21c8e50f4ba7fcba30aeb1c79379e795d8b487a744f8fd21f2a7e6f220413
Severity: low

Description:

Vulnerability Detail

The _calculateIssuance function uses a formula to compute the number of incomplete hours remaining in day B from the current timestamp:

Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf/src/circles/Circles.sol

Line 116 in 507e185

int128 l = Math64x64.fromUInt(((dB + 1) * 1 days + inflationDayZero - block.timestamp) / 1 hours + 1);

However, the current implementation assumes that an extra hour is always added, which is not the case when:

((dB + 1) * 1 days + inflationDayZero - block.timestamp) % 1 hours == 0

In this scenario, the function will not calculate the remaining hours correctly, leading to inaccurate results.

Impact

The contract may not work as intended, causing errors in the issuance calculations, which can lead to inaccurate rewards or token minting.

Mitigation

Update the logic to handle cases where the remainder is zero:

/
int128 l;
uint256 middle = (dB + 1) * 1 days + inflationDayZero - block.timestamp;
if ((middle % 1 hours) == 0) {
    l = Math64x64.fromUInt(middle / 1 hours);
} else {
    l = Math64x64.fromUInt(middle / 1 hours + 1);
}

[benjaminbollen]

Thank you for your report on the calculation of remaining hours in the _calculateIssuance function. We've reviewed your submission and agree that this is a valid low-severity issue.

You've correctly identified that when the current time is exactly on the hour, our calculation incorrectly subtracts an extra hour, potentially resulting in a lost hour's worth of minting.

We appreciate your attention to detail in identifying this edge case. Your contribution helps improve the correctness of the system. Thank you for your valuable input.

[aktech297]

potentially resulting in a lost hour's worth of minting. is it low ? why not medium ?
over a period of time, it is going to increase by counting one by one.

[0xmahdirostami]

@aktech297 because the chance of ((dB + 1) * 1 days + inflationDayZero - block.timestamp) % 1 hours == 0 is low.

[benjaminbollen]

Im now coming around to try to write my own tests for this issue;

[benjaminbollen]

@0xmahdirostami for my initial unit tests to cover this issue I am actually (so far) not recovering this as an issue. If you have any tests on your side that highlight this as an issue, it would be good to share.

23 sept. final decision: leaving this as valid, because on first analysis I agreed with the argument, and I need to review this quick test from last week. Leaving this status as 'low' as highest likelihood conclusion