diff --git a/src/abstract/AbstractVotingModule.sol b/src/abstract/AbstractVotingModule.sol index bf51c24..ccd096e 100644 --- a/src/abstract/AbstractVotingModule.sol +++ b/src/abstract/AbstractVotingModule.sol @@ -249,6 +249,24 @@ abstract contract AbstractVotingModule is IVotingModule, Initializable, EIP712Up return _getAbstractVotingModuleStorage().totalCycleVotingPower[cycle]; } + // ============ Events ============ + + /// @notice Emitted when a vote is cast with additional parameters for downstream implementations + /// @param voter The voter who cast the vote + /// @param points The voting points allocated to each recipient + /// @param votingPower The voter's total voting power + /// @param nonce The unique nonce used for replay protection + /// @param signature The EIP-712 signature authorizing the vote + /// @param additionalData Arbitrary bytes data passed for downstream use (e.g., multipliers) + event VoteCastWithParams( + address indexed voter, + uint256[] points, + uint256 votingPower, + uint256 nonce, + bytes signature, + bytes additionalData + ); + // ============ Internal Functions ============ /// @notice Processes a single vote with signature verification @@ -305,6 +323,39 @@ abstract contract AbstractVotingModule is IVotingModule, Initializable, EIP712Up /// @param votingPower Total voting power of the voter function _processVote(address voter, uint256[] calldata points, uint256 votingPower) internal virtual; + /// @notice Processes a vote with additional data for downstream implementations + /// @dev Called by _castSingleVoteWithParams. Default impl calls _processVote then + /// _handleAdditionalVoteData. Override _handleAdditionalVoteData to act on additionalData. + /// @param voter Address of the voter + /// @param points Array of points allocated to each recipient + /// @param votingPower Total voting power of the voter + /// @param additionalData Arbitrary bytes data from the caller + function _processVoteWithParams( + address voter, + uint256[] calldata points, + uint256 votingPower, + bytes calldata additionalData + ) internal virtual { + _processVote(voter, points, votingPower); + _handleAdditionalVoteData(voter, points, votingPower, additionalData); + } + + /// @notice Hook for downstream implementations to act on additional vote data + /// @dev No-op by default. Override in implementations that need to act on + /// the additionalData bytes parameter (e.g., multiplier indices, metadata). + /// @param voter Address of the voter + /// @param points Array of points allocated to each recipient + /// @param votingPower Total voting power of the voter + /// @param additionalData Arbitrary bytes data from the caller + function _handleAdditionalVoteData( + address voter, + uint256[] calldata points, + uint256 votingPower, + bytes calldata additionalData + ) internal virtual { + // Default: no-op + } + /// @notice Validates vote points distribution /// @dev Checks if points array is valid according to module rules /// @param points Array of points to validate diff --git a/src/base/BasisPointsVotingModule.sol b/src/base/BasisPointsVotingModule.sol index 760038c..4682c50 100644 --- a/src/base/BasisPointsVotingModule.sol +++ b/src/base/BasisPointsVotingModule.sol @@ -111,6 +111,66 @@ contract BasisPointsVotingModule is AbstractVotingModule { _castSingleVote(voter, points, nonce, signature); } + /// @notice Casts a vote with an EIP-712 signature and additional bytes parameters + /// @dev Same as castVoteWithSignature but also passes arbitrary bytes data to downstream + /// implementations (e.g., multiplier indices, metadata, conditional voting params). + /// See _handleAdditionalVoteData for the downstream hook. + /// @param voter The address of the voter casting the vote + /// @param points Array of basis points to allocate to each recipient + /// @param nonce Unique nonce for this vote to prevent replay attacks + /// @param signature EIP-712 signature authorizing this vote + /// @param additionalData Arbitrary bytes data passed to downstream implementations + function castVoteWithSignatureAndParams( + address voter, + uint256[] calldata points, + uint256 nonce, + bytes calldata signature, + bytes calldata additionalData + ) external { + _castSingleVoteWithParams(voter, points, nonce, signature, additionalData); + } + + /// @notice Casts a direct vote with additional bytes data (no EIP-712 signature required) + /// @dev msg.sender is the voter. Validates points, records the vote, and invokes the + /// _handleAdditionalVoteData hook. Reverts if the caller already voted this cycle. + /// Emits VoteWithData. See Issue #62. + /// @param points Array of basis points to allocate to each recipient + /// @param data Arbitrary bytes data forwarded to _handleAdditionalVoteData + function voteWithData(uint256[] calldata points, bytes calldata data) external { + if (hasVotedInCurrentCycle(msg.sender)) revert AlreadyVotedInCurrentCycle(); + if (!_validateVotePoints(points)) revert InvalidPointsDistribution(); + + uint256 votingPower = _calculateTotalVotingPower(msg.sender); + _processVoteWithParams(msg.sender, points, votingPower, data); + + emit VoteWithData(msg.sender, points, data); + } + + /// @notice Casts multiple direct votes with additional data in a single transaction + /// @dev Processes each (voter, points, data) tuple atomically. If any entry fails, the + /// entire batch reverts. Limited to MAX_BATCH_SIZE entries. Does NOT require + /// EIP-712 signatures — suitable for keeper / relayer flows where voting power is + /// determined by on-chain token holdings. See Issue #62. + /// @param voters Array of voter addresses + /// @param points Array of point allocations per voter + /// @param data Array of arbitrary bytes data per voter + function voteWithDataBatch(address[] calldata voters, uint256[][] calldata points, bytes[] calldata data) + external + onlyOwner + { + if (voters.length != points.length) revert ArrayLengthMismatch(); + if (voters.length != data.length) revert ArrayLengthMismatch(); + if (voters.length > MAX_BATCH_SIZE) revert BatchTooLarge(); + + for (uint256 i = 0; i < voters.length; i++) { + if (hasVotedInCurrentCycle(voters[i])) revert AlreadyVotedInCurrentCycle(); + if (!_validateVotePoints(points[i])) revert InvalidPointsDistribution(); + uint256 votingPower = _calculateTotalVotingPower(voters[i]); + _processVoteWithParams(voters[i], points[i], votingPower, data[i]); + emit VoteWithData(voters[i], points[i], data[i]); + } + } + /// @notice Casts multiple votes in a single transaction for gas efficiency /// @dev Processes multiple votes atomically. If any vote fails, the entire batch reverts. /// Limited to MAX_BATCH_SIZE votes per transaction to prevent gas limit issues. @@ -252,6 +312,42 @@ contract BasisPointsVotingModule is AbstractVotingModule { return true; } + /// @notice Processes a single vote with additional data for downstream implementations + /// @dev Mirrors _castSingleVote but calls _processVoteWithParams to invoke the additionalData hook. + /// @param voter Address of the voter + /// @param points Array of points to allocate to each recipient + /// @param nonce Unique nonce for replay protection + /// @param signature EIP-712 signature from the voter + /// @param additionalData Arbitrary bytes data for downstream implementations + function _castSingleVoteWithParams( + address voter, + uint256[] calldata points, + uint256 nonce, + bytes calldata signature, + bytes calldata additionalData + ) internal { + AbstractVotingModuleStorage storage $ = _getAbstractVotingModuleStorage(); + + if (isNonceUsed(voter, nonce)) revert NonceAlreadyUsed(); + if (!_validateVotePoints(points)) revert InvalidPointsDistribution(); + if (!validateSignature(voter, points, nonce, signature)) revert InvalidSignature(); + + $.usedNonces[voter][nonce] = true; + + uint256 votingPower = _calculateTotalVotingPower(voter); + _processVoteWithParams(voter, points, votingPower, additionalData); + + emit VoteCastWithParams(voter, points, votingPower, nonce, signature, additionalData); + } + + /// @notice Hook for downstream implementations to act on additional vote data + /// @dev BasisPointsVotingModule does not use additionalData — override this in + /// downstream implementations that need it (e.g., multiplier indices, metadata). + /// Marked virtual so that subclasses can override without mutability restrictions. + function _handleAdditionalVoteData(address, uint256[] calldata, uint256, bytes calldata) internal virtual override { + // BasisPointsVotingModule: no-op. Override in downstream implementations. + } + // Issue #43: Store required votes at proposal creation in VotingRecipientRegistry // https://github.com/BreadchainCoop/breadkit/issues/43 // TODO: Implement when VotingRecipientRegistry is added diff --git a/src/interfaces/IVotingModule.sol b/src/interfaces/IVotingModule.sol index 576f3c3..c214a17 100644 --- a/src/interfaces/IVotingModule.sol +++ b/src/interfaces/IVotingModule.sol @@ -46,6 +46,9 @@ interface IVotingModule { /// @notice Thrown when a zero address is provided error ZeroAddress(); + /// @notice Thrown when a voter tries to vote twice in the same cycle via voteWithData + error AlreadyVotedInCurrentCycle(); + // ============ Events ============ /// @notice Emitted when a vote is cast with a signature @@ -61,6 +64,12 @@ interface IVotingModule { /// @param nonces Array of nonces used event BatchVotesCast(address[] voters, uint256[] nonces); + /// @notice Emitted when a direct vote with additional data is cast + /// @param voter The voter who cast the vote + /// @param points The voting points allocated to each recipient + /// @param data Arbitrary bytes data passed by the caller + event VoteWithData(address indexed voter, uint256[] points, bytes data); + /// @notice Emitted when the voting module is initialized /// @param strategies Array of voting power strategies event VotingModuleInitialized(IVotingPowerStrategy[] strategies); diff --git a/test/VotingModule.t.sol b/test/VotingModule.t.sol index df1625c..20db7b2 100644 --- a/test/VotingModule.t.sol +++ b/test/VotingModule.t.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.20; import {Test} from "forge-std/Test.sol"; import {BasisPointsVotingModule} from "../src/base/BasisPointsVotingModule.sol"; +import {AbstractVotingModule} from "../src/abstract/AbstractVotingModule.sol"; import {IVotingModule} from "../src/interfaces/IVotingModule.sol"; import {TokenBasedVotingPower} from "../src/implementation/strategies/TokenBasedVotingPower.sol"; import {IVotingPowerStrategy} from "../src/interfaces/IVotingPowerStrategy.sol"; @@ -11,10 +12,30 @@ import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; import {ERC20Votes} from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Votes.sol"; import {ERC20Permit} from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Permit.sol"; import {Nonces} from "@openzeppelin/contracts/utils/Nonces.sol"; +import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; import {MockRecipientRegistry} from "./mocks/MockRecipientRegistry.sol"; import {CycleModule} from "../src/implementation/CycleModule.sol"; import {MockDistributionModule} from "./mocks/MockDistributionModule.sol"; +// ============ Mock helper for testing _handleAdditionalVoteData hook ============ + +/// @dev Subclass that records whether _handleAdditionalVoteData was invoked +/// so tests can assert the hook is actually called by voteWithData / voteWithDataBatch. +contract MockBasisPointsVotingModule is BasisPointsVotingModule { + bool public handleAdditionalDataCalled; + address public lastHandledVoter; + bytes public lastHandledData; + + function _handleAdditionalVoteData(address voter, uint256[] calldata, uint256, bytes calldata additionalData) + internal + override + { + handleAdditionalDataCalled = true; + lastHandledVoter = voter; + lastHandledData = additionalData; + } +} + // Mock token implementation for testing contract MockToken is ERC20, ERC20Votes, ERC20Permit { constructor() ERC20("Mock Token", "MOCK") ERC20Permit("Mock Token") {} @@ -33,6 +54,16 @@ contract MockToken is ERC20, ERC20Votes, ERC20Permit { } contract VotingModuleTest is Test { + // Local event declaration so vm.expectEmit can find it + event VoteCastWithParams( + address indexed voter, + uint256[] points, + uint256 votingPower, + uint256 nonce, + bytes signature, + bytes additionalData + ); + // Constants uint256 constant MAX_POINTS = 100; @@ -57,6 +88,7 @@ contract VotingModuleTest is Test { event VoteCast(address indexed voter, uint256[] points, uint256 votingPower, uint256 nonce, bytes signature); event BatchVotesCast(address[] voters, uint256[] nonces); event VotingModuleInitialized(IVotingPowerStrategy[] strategies); + event VoteWithData(address indexed voter, uint256[] points, bytes data); function setUp() public { // Setup test accounts @@ -458,4 +490,242 @@ contract VotingModuleTest is Test { return keccak256(abi.encodePacked("\x19\x01", votingModule.DOMAIN_SEPARATOR(), structHash)); } + + // ============ Issue #62: voteWithData / voteWithDataBatch tests ============ + + /// @notice voteWithData emits VoteWithData(voter, points, data) + function testVoteWithDataEmitsEvent() public { + uint256[] memory points = new uint256[](3); + points[0] = 50; + points[1] = 30; + points[2] = 20; + bytes memory data = hex"1234"; + + vm.expectEmit(true, false, false, true); + emit VoteWithData(voter1, points, data); + + vm.prank(voter1); + votingModule.voteWithData(points, data); + } + + /// @notice voteWithData records the vote identically to castVoteWithSignature + function testVoteWithDataRecordsVote() public { + uint256[] memory points = new uint256[](3); + points[0] = 50; + points[1] = 30; + points[2] = 20; + + vm.prank(voter1); + votingModule.voteWithData(points, ""); + + // hasVotedInCurrentCycle should be true + assertTrue(votingModule.hasVotedInCurrentCycle(voter1), "voter1 should have voted in current cycle"); + + // Project distributions should match the points allocation + uint256 votingPower = votingModule.getVotingPower(voter1); + uint256[] memory dist = votingModule.getCurrentVotingDistribution(); + assertEq(dist.length, 3, "distribution should have 3 entries"); + + uint256 totalPoints = 100; // 50 + 30 + 20 + assertEq(dist[0], (50 * votingPower * 1e18) / totalPoints / 1e18, "project 0 allocation mismatch"); + assertEq(dist[1], (30 * votingPower * 1e18) / totalPoints / 1e18, "project 1 allocation mismatch"); + assertEq(dist[2], (20 * votingPower * 1e18) / totalPoints / 1e18, "project 2 allocation mismatch"); + } + + /// @notice _handleAdditionalVoteData hook is called by voteWithData with the correct args + function testVoteWithDataCallsHandleAdditionalVoteData() public { + // Deploy and initialise the mock subclass that records hook invocations + MockBasisPointsVotingModule mockModule = new MockBasisPointsVotingModule(); + IVotingPowerStrategy[] memory strategies = new IVotingPowerStrategy[](1); + strategies[0] = IVotingPowerStrategy(address(tokenStrategy)); + mockModule.initialize( + MAX_POINTS, strategies, address(distributionModule), address(recipientRegistry), address(cycleModule) + ); + + uint256[] memory points = new uint256[](3); + points[0] = 50; + points[1] = 30; + points[2] = 20; + bytes memory data = hex"deadbeef"; + + vm.prank(voter1); + mockModule.voteWithData(points, data); + + assertTrue(mockModule.handleAdditionalDataCalled(), "_handleAdditionalVoteData was not called"); + assertEq(mockModule.lastHandledVoter(), voter1, "hook received wrong voter address"); + assertEq(mockModule.lastHandledData(), data, "hook received wrong data bytes"); + } + + /// @notice voteWithDataBatch processes multiple voters in one call + function testVoteWithDataBatch() public { + // msg.sender is the test contract (owner) — batch voting is owner-only + address[] memory voters = new address[](2); + voters[0] = voter1; + voters[1] = voter2; + + uint256[][] memory pointsArray = new uint256[][](2); + pointsArray[0] = new uint256[](3); + pointsArray[0][0] = 50; + pointsArray[0][1] = 30; + pointsArray[0][2] = 20; + + pointsArray[1] = new uint256[](3); + pointsArray[1][0] = 60; + pointsArray[1][1] = 25; + pointsArray[1][2] = 15; + + bytes[] memory data = new bytes[](2); + data[0] = hex"1234"; + data[1] = hex"5678"; + + // Expect VoteWithData events for each voter + vm.expectEmit(true, false, false, true); + emit VoteWithData(voter1, pointsArray[0], data[0]); + vm.expectEmit(true, false, false, true); + emit VoteWithData(voter2, pointsArray[1], data[1]); + + votingModule.voteWithDataBatch(voters, pointsArray, data); + + assertTrue(votingModule.hasVotedInCurrentCycle(voter1), "voter1 should have voted in current cycle"); + assertTrue(votingModule.hasVotedInCurrentCycle(voter2), "voter2 should have voted in current cycle"); + + uint256[] memory dist = votingModule.getCurrentVotingDistribution(); + assertEq(dist.length, 3, "distribution should have 3 entries"); + } + + /// @notice voteWithData succeeds and records the vote when data is empty bytes + function testVoteWithDataWithEmptyBytes() public { + uint256[] memory points = new uint256[](3); + points[0] = 50; + points[1] = 30; + points[2] = 20; + + vm.prank(voter1); + votingModule.voteWithData(points, ""); + + assertTrue(votingModule.hasVotedInCurrentCycle(voter1), "voter1 should have voted in current cycle"); + } + + /// @notice voteWithData reverts when the points array length does not match the recipient count + function testVoteWithDataRevertsInvalidPointsLength() public { + // Registry has 3 recipients — submitting 2 points should revert + uint256[] memory points = new uint256[](2); + points[0] = 50; + points[1] = 50; + + vm.prank(voter1); + vm.expectRevert(IVotingModule.InvalidPointsDistribution.selector); + votingModule.voteWithData(points, ""); + } + + /// @notice voteWithData reverts if the caller already voted in the current cycle + function testVoteWithDataRevertsAlreadyVoted() public { + uint256[] memory points = new uint256[](3); + points[0] = 50; + points[1] = 30; + points[2] = 20; + + // First vote — should succeed + vm.prank(voter1); + votingModule.voteWithData(points, ""); + + // Second vote in the same cycle — should revert with AlreadyVotedInCurrentCycle + vm.prank(voter1); + vm.expectRevert(IVotingModule.AlreadyVotedInCurrentCycle.selector); + votingModule.voteWithData(points, ""); + } + + /// @notice voteWithDataBatch reverts when called by non-owner + function testVoteWithDataBatchRevertsNonOwner() public { + address[] memory voters = new address[](1); + voters[0] = voter1; + + uint256[][] memory pointsArray = new uint256[][](1); + pointsArray[0] = new uint256[](3); + pointsArray[0][0] = 50; + pointsArray[0][1] = 30; + pointsArray[0][2] = 20; + + bytes[] memory data = new bytes[](1); + data[0] = hex"1234"; + + // Non-owner should not be able to batch-vote on behalf of others + vm.prank(voter1); + vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, voter1)); + votingModule.voteWithDataBatch(voters, pointsArray, data); + } + + /// @notice voteWithDataBatch reverts when a voter has already voted this cycle + function testVoteWithDataBatchRevertsAlreadyVoted() public { + // voter1 votes first via voteWithData + uint256[] memory points = new uint256[](3); + points[0] = 50; + points[1] = 30; + points[2] = 20; + + vm.prank(voter1); + votingModule.voteWithData(points, ""); + + // Now try to include voter1 in a batch — should revert + address[] memory voters = new address[](1); + voters[0] = voter1; + + uint256[][] memory pointsArray = new uint256[][](1); + pointsArray[0] = new uint256[](3); + pointsArray[0][0] = 60; + pointsArray[0][1] = 25; + pointsArray[0][2] = 15; + + bytes[] memory data = new bytes[](1); + data[0] = ""; + + vm.expectRevert(IVotingModule.AlreadyVotedInCurrentCycle.selector); + votingModule.voteWithDataBatch(voters, pointsArray, data); + } + + /// @notice voteWithDataBatch reverts when duplicate voters appear in the same batch + function testVoteWithDataBatchRevertsDuplicateVoter() public { + address[] memory voters = new address[](2); + voters[0] = voter1; + voters[1] = voter1; // duplicate + + uint256[][] memory pointsArray = new uint256[][](2); + pointsArray[0] = new uint256[](3); + pointsArray[0][0] = 50; + pointsArray[0][1] = 30; + pointsArray[0][2] = 20; + pointsArray[1] = new uint256[](3); + pointsArray[1][0] = 60; + pointsArray[1][1] = 25; + pointsArray[1][2] = 15; + + bytes[] memory data = new bytes[](2); + data[0] = ""; + data[1] = ""; + + vm.expectRevert(IVotingModule.AlreadyVotedInCurrentCycle.selector); + votingModule.voteWithDataBatch(voters, pointsArray, data); + } + + /// @notice castVoteWithSignatureAndParams emits VoteCastWithParams and records the vote + function testCastVoteWithSignatureAndParams() public { + uint256[] memory points = new uint256[](3); + points[0] = 50; + points[1] = 30; + points[2] = 20; + bytes memory additionalData = hex"cafe"; + uint256 nonce = 1; + + bytes32 digest = _createVoteDigest(voter1, points, nonce); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(voter1PrivateKey, digest); + bytes memory signature = abi.encodePacked(r, s, v); + + vm.expectEmit(true, false, false, true); + emit VoteCastWithParams(voter1, points, votingModule.getVotingPower(voter1), nonce, signature, additionalData); + + votingModule.castVoteWithSignatureAndParams(voter1, points, nonce, signature, additionalData); + + assertTrue(votingModule.hasVotedInCurrentCycle(voter1), "voter1 should have voted"); + assertTrue(votingModule.isNonceUsed(voter1, nonce), "nonce should be used"); + } }