feat: don't call onAfterAllocateLQTY() on vetos

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;
     }

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;

test/InitiativeHooks.t.sol

@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.24;
+
+import {IGovernance} from "../src/interfaces/IGovernance.sol";
+import {IInitiative} from "../src/interfaces/IInitiative.sol";
+import {Governance} from "../src/Governance.sol";
+import {MockERC20Tester} from "./mocks/MockERC20Tester.sol";
+import {MockStakingV1} from "./mocks/MockStakingV1.sol";
+import {MockStakingV1Deployer} from "./mocks/MockStakingV1Deployer.sol";
+
+contract MockInitiative is IInitiative {
+    struct OnAfterAllocateLQTYParams {
+        uint256 currentEpoch;
+        address user;
+        IGovernance.UserState userState;
+        IGovernance.Allocation allocation;
+        IGovernance.InitiativeState initiativeStat;
+    }
+
+    OnAfterAllocateLQTYParams[] public onAfterAllocateLQTYCalls;
+
+    function numOnAfterAllocateLQTYCalls() external view returns (uint256) {
+        return onAfterAllocateLQTYCalls.length;
+    }
+
+    function onAfterAllocateLQTY(
+        uint256 _currentEpoch,
+        address _user,
+        IGovernance.UserState calldata _userState,
+        IGovernance.Allocation calldata _allocation,
+        IGovernance.InitiativeState calldata _initiativeState
+    ) external override {
+        onAfterAllocateLQTYCalls.push(
+            OnAfterAllocateLQTYParams(_currentEpoch, _user, _userState, _allocation, _initiativeState)
+        );
+    }
+
+    function onRegisterInitiative(uint256) external override {}
+    function onUnregisterInitiative(uint256) external override {}
+    function onClaimForInitiative(uint256, uint256) external override {}
+}
+
+contract InitiativeHooksTest is MockStakingV1Deployer {
+    uint32 constant START_TIME = 1732873631;
+    uint32 constant EPOCH_DURATION = 7 days;
+    uint32 constant EPOCH_VOTING_CUTOFF = 6 days;
+
+    IGovernance.Configuration config = IGovernance.Configuration({
+        registrationFee: 0,
+        registrationThresholdFactor: 0,
+        unregistrationThresholdFactor: 4 ether,
+        unregistrationAfterEpochs: 4,
+        votingThresholdFactor: 0,
+        minClaim: 0,
+        minAccrual: 0,
+        epochStart: START_TIME - EPOCH_DURATION,
+        epochDuration: EPOCH_DURATION,
+        epochVotingCutoff: EPOCH_VOTING_CUTOFF
+    });
+
+    MockStakingV1 stakingV1;
+    MockERC20Tester lqty;
+    MockERC20Tester lusd;
+    MockERC20Tester bold;
+    Governance governance;
+    MockInitiative initiative;
+    address[] noInitiatives; // left empty
+    address[] initiatives;
+    int256[] votes;
+    int256[] vetos;
+    address voter;
+
+    function setUp() external {
+        vm.warp(START_TIME);
+
+        (stakingV1, lqty, lusd) = deployMockStakingV1();
+
+        bold = new MockERC20Tester("BOLD Stablecoin", "BOLD");
+        vm.label(address(bold), "BOLD");
+
+        governance = new Governance({
+            _lqty: address(lqty),
+            _lusd: address(lusd),
+            _stakingV1: address(stakingV1),
+            _bold: address(bold),
+            _config: config,
+            _owner: address(this),
+            _initiatives: new address[](0)
+        });
+
+        initiative = new MockInitiative();
+        initiatives.push(address(initiative));
+        governance.registerInitialInitiatives(initiatives);
+
+        voter = makeAddr("voter");
+        lqty.mint(voter, 1 ether);
+
+        vm.startPrank(voter);
+        lqty.approve(governance.deriveUserProxyAddress(voter), type(uint256).max);
+        governance.depositLQTY(1 ether);
+        vm.stopPrank();
+
+        votes.push();
+        vetos.push();
+    }
+
+    function test_OnAfterAllocateLQTY_IsCalled_WhenCastingVotes() external {
+        vm.startPrank(voter);
+        votes[0] = 123;
+        governance.allocateLQTY(noInitiatives, initiatives, votes, vetos);
+        vm.stopPrank();
+
+        assertEq(initiative.numOnAfterAllocateLQTYCalls(), 1, "onAfterAllocateLQTY should have been called once");
+        (,,, IGovernance.Allocation memory allocation,) = initiative.onAfterAllocateLQTYCalls(0);
+        assertEq(allocation.voteLQTY, 123, "wrong voteLQTY 1");
+
+        vm.startPrank(voter);
+        votes[0] = 456;
+        governance.allocateLQTY(initiatives, initiatives, votes, vetos);
+        vm.stopPrank();
+
+        assertEq(initiative.numOnAfterAllocateLQTYCalls(), 3, "onAfterAllocateLQTY should have been called twice more");
+        (,,, allocation,) = initiative.onAfterAllocateLQTYCalls(1);
+        assertEq(allocation.voteLQTY, 0, "wrong voteLQTY 2");
+        (,,, allocation,) = initiative.onAfterAllocateLQTYCalls(2);
+        assertEq(allocation.voteLQTY, 456, "wrong voteLQTY 3");
+    }
+
+    function test_OnAfterAllocateLQTY_IsNotCalled_WhenCastingVetos() external {
+        vm.startPrank(voter);
+        vetos[0] = 123;
+        governance.allocateLQTY(noInitiatives, initiatives, votes, vetos);
+        vm.stopPrank();
+
+        assertEq(initiative.numOnAfterAllocateLQTYCalls(), 0, "onAfterAllocateLQTY should not have been called once");
+    }
+
+    function test_OnAfterAllocateLQTY_IsCalledOnceWithZeroVotes_WhenCastingVetosAfterHavingCastVotes() external {
+        vm.startPrank(voter);
+        votes[0] = 123;
+        governance.allocateLQTY(noInitiatives, initiatives, votes, vetos);
+        vm.stopPrank();
+
+        assertEq(initiative.numOnAfterAllocateLQTYCalls(), 1, "onAfterAllocateLQTY should have been called once");
+
+        vm.startPrank(voter);
+        votes[0] = 0;
+        vetos[0] = 456;
+        governance.allocateLQTY(initiatives, initiatives, votes, vetos);
+        vm.stopPrank();
+
+        assertEq(initiative.numOnAfterAllocateLQTYCalls(), 2, "onAfterAllocateLQTY should have been called once more");
+        (,,, IGovernance.Allocation memory allocation,) = initiative.onAfterAllocateLQTYCalls(1);
+        assertEq(allocation.voteLQTY, 0, "wrong voteLQTY");
+    }
+}