From b8b2dd6afa377315e2adb4e5b75ac040588d5bfa Mon Sep 17 00:00:00 2001 From: Daniel Simon Date: Thu, 2 Jan 2025 14:44:36 +0700 Subject: [PATCH] feat: don't call `onAfterAllocateLQTY()` on vetos Fixes #125. --- src/Governance.sol | 45 ++++++++++++++++++++++------------ src/interfaces/IGovernance.sol | 14 ++++++++--- 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/src/Governance.sol b/src/Governance.sol index 4d1177a..f89e7c0 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -128,7 +128,7 @@ contract Governance is MultiDelegateCall, UserProxyFactory, ReentrancyGuard, Own _initiatives[i], MIN_GAS_TO_HOOK, 0, abi.encodeCall(IInitiative.onRegisterInitiative, (1)) ); - emit RegisterInitiative(_initiatives[i], msg.sender, 1, success); + emit RegisterInitiative(_initiatives[i], msg.sender, 1, success ? HookStatus.Succeeded : HookStatus.Failed); } _renounceOwnership(); @@ -513,7 +513,9 @@ contract Governance is MultiDelegateCall, UserProxyFactory, ReentrancyGuard, Own _initiative, MIN_GAS_TO_HOOK, 0, abi.encodeCall(IInitiative.onRegisterInitiative, (currentEpoch)) ); - emit RegisterInitiative(_initiative, msg.sender, currentEpoch, success); + emit RegisterInitiative( + _initiative, msg.sender, currentEpoch, success ? HookStatus.Succeeded : HookStatus.Failed + ); } struct ResetInitiativeData { @@ -805,19 +807,30 @@ contract Governance is MultiDelegateCall, UserProxyFactory, ReentrancyGuard, Own vars.userState.allocatedOffset = add(vars.userState.allocatedOffset, (vars.deltaOffsetVotes + vars.deltaOffsetVetos)); - // Replaces try / catch | Enforces sufficient gas is passed - bool success = safeCallWithMinGas( - initiative, - MIN_GAS_TO_HOOK, - 0, - abi.encodeCall( - IInitiative.onAfterAllocateLQTY, - (vars.currentEpoch, msg.sender, vars.userState, vars.allocation, vars.initiativeState) - ) - ); + HookStatus hookStatus; + + // See https://github.com/liquity/V2-gov/issues/125 + // A malicious initiative could try to dissuade voters from casting vetos by consuming as much gas as + // possible in the `onAfterAllocateLQTY` hook when detecting vetos. + // We deem that the risks of calling into malicous initiatives upon veto allocation far outweigh the + // benefits of notifying benevolent initiatives of vetos. + if (vars.allocation.vetoLQTY == 0) { + // Replaces try / catch | Enforces sufficient gas is passed + hookStatus = safeCallWithMinGas( + initiative, + MIN_GAS_TO_HOOK, + 0, + abi.encodeCall( + IInitiative.onAfterAllocateLQTY, + (vars.currentEpoch, msg.sender, vars.userState, vars.allocation, vars.initiativeState) + ) + ) ? HookStatus.Succeeded : HookStatus.Failed; + } else { + hookStatus = HookStatus.NotCalled; + } emit AllocateLQTY( - msg.sender, initiative, vars.deltaLQTYVotes, vars.deltaLQTYVetos, vars.currentEpoch, success + msg.sender, initiative, vars.deltaLQTYVotes, vars.deltaLQTYVetos, vars.currentEpoch, hookStatus ); } @@ -863,7 +876,7 @@ contract Governance is MultiDelegateCall, UserProxyFactory, ReentrancyGuard, Own _initiative, MIN_GAS_TO_HOOK, 0, abi.encodeCall(IInitiative.onUnregisterInitiative, (currentEpoch)) ); - emit UnregisterInitiative(_initiative, currentEpoch, success); + emit UnregisterInitiative(_initiative, currentEpoch, success ? HookStatus.Succeeded : HookStatus.Failed); } /// @inheritdoc IGovernance @@ -907,7 +920,9 @@ contract Governance is MultiDelegateCall, UserProxyFactory, ReentrancyGuard, Own abi.encodeCall(IInitiative.onClaimForInitiative, (votesSnapshot_.forEpoch, claimableAmount)) ); - emit ClaimForInitiative(_initiative, claimableAmount, votesSnapshot_.forEpoch, success); + emit ClaimForInitiative( + _initiative, claimableAmount, votesSnapshot_.forEpoch, success ? HookStatus.Succeeded : HookStatus.Failed + ); return claimableAmount; } diff --git a/src/interfaces/IGovernance.sol b/src/interfaces/IGovernance.sol index 6a28608..b4f0628 100644 --- a/src/interfaces/IGovernance.sol +++ b/src/interfaces/IGovernance.sol @@ -8,6 +8,12 @@ import {ILQTYStaking} from "./ILQTYStaking.sol"; import {PermitParams} from "../utils/Types.sol"; interface IGovernance { + enum HookStatus { + Failed, + Succeeded, + NotCalled + } + /// @notice Emitted when a user deposits LQTY /// @param user The account depositing LQTY /// @param rewardRecipient The account receiving the LUSD/ETH rewards earned from staking in V1, if claimed @@ -49,8 +55,8 @@ interface IGovernance { event SnapshotVotes(uint256 votes, uint256 forEpoch, uint256 boldAccrued); event SnapshotVotesForInitiative(address indexed initiative, uint256 votes, uint256 vetos, uint256 forEpoch); - event RegisterInitiative(address initiative, address registrant, uint256 atEpoch, bool hookSuccess); - event UnregisterInitiative(address initiative, uint256 atEpoch, bool hookSuccess); + event RegisterInitiative(address initiative, address registrant, uint256 atEpoch, HookStatus hookStatus); + event UnregisterInitiative(address initiative, uint256 atEpoch, HookStatus hookStatus); event AllocateLQTY( address indexed user, @@ -58,9 +64,9 @@ interface IGovernance { int256 deltaVoteLQTY, int256 deltaVetoLQTY, uint256 atEpoch, - bool hookSuccess + HookStatus hookStatus ); - event ClaimForInitiative(address indexed initiative, uint256 bold, uint256 forEpoch, bool hookSuccess); + event ClaimForInitiative(address indexed initiative, uint256 bold, uint256 forEpoch, HookStatus hookStatus); struct Configuration { uint256 registrationFee;