search

BalancerMaxis / ChildGaugeInjectorV2

1 stars 0 forks source link

Low risk (L6): argument `doNotStartBeforeTimestamp` in `addRecipients` could be set into the past #34

Open petrovska-petro opened 4 days ago

petrovska-petro commented 4 days ago

Severity: it can lead to misconfigurations

Context: currently there is not health-check in place to ensure that the timestamp for doNotStartBeforeTimestamp it is not set into the past, which while reviewing it is clear if indeed it is desired or indeed a bug in design, with the exception of 0 which signals as soon as possible.

Recommendation: add a condition to handle the case where doNotStartBeforeTimestamp < block.timestamp && doNotStartBeforeTimestamp != 0 in the case that setting campaigns into the past it's not desired

Review stage

Balancer Maxis:

Onchainification Labs:

Tritium-VLK commented 3 days ago

I imagine a payload that could be waiting with a do not start before, that may execute before or after that time.

petrovska-petro commented 3 days ago

Alright, if the case it's there that a payload can be load as the owner of an injector as in the past. Then, we can dismiss this issue

Still, it may be worth considering to add better NatSpec to highlight that indeed it is possible to set in the past, similar how 0 is explained