Conversation
test/foundry/unit/StakingRewardsV2/StakingV2Checkpointing.t.sol
Outdated
Show resolved
Hide resolved
tommyrharper
left a comment
tommyrharper
left a comment
There was a problem hiding this comment.
Nice work @Flocqst 💪 🔥 🎉 - glad to see this getting ripped out! Left a few comments here and there, nothing major :)
JaredBorders
left a comment
JaredBorders
left a comment
There was a problem hiding this comment.
preliminary review - just waiting for our sync to go further
|
please add your name/email/gh handle as author to contracts you modified. |
|
Thanks for the review @tommyrharper, took into account almost every comment you had except for the suggestion i did not include because of the incoming #252. |
JaredBorders
left a comment
JaredBorders
left a comment
There was a problem hiding this comment.
Logic-wise, everything looks good to me. Nice work 💪
The only point of concern:
Removing the cooldown could potentially lead to governance attacks, depending on the voting mechanism implementation.
For instance, with snapshot voting, a malicious actor could utilize a flashloan to artificially inflate voting power if they knew the exact block of the snapshot. This risk should be documented in the interface or another appropriate location.
tommyrharper
left a comment
tommyrharper
left a comment
There was a problem hiding this comment.
Nice work, thanks for addressing all my concerns!
3 participants
Remove the staking cooldown for the stakingv2 contracts.
Description
contracts/StakingRewardsV2.sol,contracts/RewardEscrowV2.solandcontracts/EscrowMigrator.solMotivation and Context
KIP-127: Staking V2 Upgrade