feat: don't call onAfterAllocateLQTY() on vetos

Fixes #125.
liquity · Jan 2, 2025 · b8b2dd6 · b8b2dd6
1 parent f6839c9
commit b8b2dd6
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 19 deletions.
diff --git 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
@@ -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,18 +55,18 @@ 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,
         address indexed initiative,
         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;