-
Notifications
You must be signed in to change notification settings - Fork 51
Description
Description
A refactor suggestion was made to improve the Check-Effects-Interactions (CEI) pattern compliance in the penalty execution logic.
Currently, round.pnkPenalties
is updated in a batch after external calls (unlock/penalize/setJurorInactive). While the modules are trusted, updating storage incrementally after each penalty reduces reliance on local cache across reentrant paths and ensures state consistency at all times.
Suggested Changes
Update the _executePenalties
function to write round.pnkPenalties
to storage immediately after each penalty calculation, rather than batching the update after external calls.
Files affected:
kleros-v2/contracts/src/arbitration/KlerosCoreBase.sol
Lines 754 to 756 in 0f31e0e
if (round.pnkPenalties != pnkPenaltiesInRound) { round.pnkPenalties = pnkPenaltiesInRound; // Reentrancy risk: breaks Check-Effect-Interact } kleros-v2/contracts/src/arbitration/university/KlerosCoreUniversity.sol
Lines 742 to 744 in 0f31e0e
if (round.pnkPenalties != pnkPenaltiesInRound) { round.pnkPenalties = pnkPenaltiesInRound; // Reentrancy risk: breaks Check-Effect-Interact }
Specific changes:
- In
_executePenalties
, updateround.pnkPenalties
immediately after computingavailablePenalty
- Adjust the
execute
function to handle the incremental updates appropriately
Benefits
- Better CEI pattern compliance
- Reduced reentrancy surface area
- More consistent state across function calls
- Defensive programming against future changes
References
- PR: Contracts: multi-dimensional coherence #2090
- Original suggestion: Contracts: multi-dimensional coherence #2090 (comment)
Priority
Low - This is a defensive refactor for improved security patterns, but the current implementation works correctly with trusted modules.