diff --git a/src/IVotesPartialDelegation.sol b/src/IVotesPartialDelegation.sol index d7f9208..e76eb25 100644 --- a/src/IVotesPartialDelegation.sol +++ b/src/IVotesPartialDelegation.sol @@ -25,7 +25,9 @@ interface IVotesPartialDelegation is IERC6372 { /** * @dev Emitted when an account changes their delegate. */ - event DelegateChanged(address indexed delegator, address indexed delegatee, uint96 numerator); + event DelegateChanged( + address indexed delegator, PartialDelegation[] oldDelegatees, PartialDelegation[] newDelegatees + ); /** * @dev Emitted when a token transfer or delegate change results in changes to a delegate's number of voting units. diff --git a/src/VotesPartialDelegationUpgradeable.sol b/src/VotesPartialDelegationUpgradeable.sol index 0245912..3bfdd07 100644 --- a/src/VotesPartialDelegationUpgradeable.sol +++ b/src/VotesPartialDelegationUpgradeable.sol @@ -352,37 +352,12 @@ abstract contract VotesPartialDelegationUpgradeable is _lastDelegatee = _newDelegations[i]._delegatee; } // remove any remaining old delegatees - if (_oldDelegateLength > _newDelegationsLength) { - for (uint256 i = _newDelegationsLength; i < _oldDelegateLength; i++) { + if (_oldDelegateLength > _newDelegations.length) { + for (uint256 i = _newDelegations.length; i < _oldDelegateLength; i++) { $._delegatees[_delegator].pop(); } } - // emit events - _emitDelegationEvents(_delegator, _oldDelegations, _newDelegations); - } - - /// @dev Emits {DelegateChanged} events for each change in delegation, taking care to avoid duplicates and no-ops. - function _emitDelegationEvents(address _delegator, PartialDelegation[] memory _old, PartialDelegation[] memory _new) - internal - virtual - { - uint256 i; - uint256 j; - while (i < _old.length || j < _new.length) { - if (i < _old.length && j < _new.length && _old[i]._delegatee == _new[j]._delegatee) { - if (_old[i]._numerator != _new[j]._numerator) { - emit DelegateChanged(_delegator, _new[j]._delegatee, _new[j]._numerator); - } - i++; - j++; - } else if (j == _new.length || (i != _old.length && _old[i]._delegatee < _new[j]._delegatee)) { - emit DelegateChanged(_delegator, _old[i]._delegatee, 0); - i++; - } else { - emit DelegateChanged(_delegator, _new[j]._delegatee, _new[j]._numerator); - j++; - } - } + emit DelegateChanged(_delegator, _oldDelegations, _newDelegations); } /** diff --git a/test/ERC20VotesPartialDelegationUpgradeable.t.sol b/test/ERC20VotesPartialDelegationUpgradeable.t.sol index 23b5660..1097236 100644 --- a/test/ERC20VotesPartialDelegationUpgradeable.t.sol +++ b/test/ERC20VotesPartialDelegationUpgradeable.t.sol @@ -94,43 +94,6 @@ contract PartialDelegationTest is DelegationAndEventHelpers { return delegations; } - function _expectEmitDelegateChangedEvents( - address _delegator, - PartialDelegation[] memory _oldDelegations, - PartialDelegation[] memory _newDelegations - ) internal { - uint256 i; - uint256 j; - while (i < _oldDelegations.length || j < _newDelegations.length) { - // If both delegations have the same delegatee - if ( - i < _oldDelegations.length && j < _newDelegations.length - && _oldDelegations[i]._delegatee == _newDelegations[j]._delegatee - ) { - // if the numerator is different - if (_oldDelegations[i]._numerator != _newDelegations[j]._numerator) { - vm.expectEmit(); - emit DelegateChanged(_delegator, _newDelegations[j]._delegatee, _newDelegations[j]._numerator); - } - i++; - j++; - // Old delegatee comes before the new delegatee OR new delegatees have been exhausted - } else if ( - j == _newDelegations.length - || (i != _oldDelegations.length && _oldDelegations[i]._delegatee < _newDelegations[j]._delegatee) - ) { - vm.expectEmit(); - emit DelegateChanged(_delegator, _oldDelegations[i]._delegatee, 0); - i++; - // If new delegatee comes before the old delegatee OR old delegatees have been exhausted - } else { - vm.expectEmit(); - emit DelegateChanged(_delegator, _newDelegations[j]._delegatee, _newDelegations[j]._numerator); - j++; - } - } - } - function _expectEmitDelegateVotesChangedEvents( uint256 _amount, uint256 _toExistingBalance, @@ -312,7 +275,8 @@ contract Delegate is PartialDelegationTest { vm.startPrank(_actor); tokenProxy.mint(_amount); - _expectEmitDelegateChangedEvents(_actor, new PartialDelegation[](0), delegations); + vm.expectEmit(); + emit DelegateChanged(_actor, new PartialDelegation[](0), delegations); tokenProxy.delegate(delegations); vm.stopPrank(); } @@ -351,7 +315,8 @@ contract Delegate is PartialDelegationTest { newDelegations[i] = oldDelegations[i]; } - _expectEmitDelegateChangedEvents(_actor, oldDelegations, newDelegations); + vm.expectEmit(); + emit DelegateChanged(_actor, oldDelegations, newDelegations); tokenProxy.delegate(newDelegations); vm.stopPrank(); } @@ -394,7 +359,8 @@ contract Delegate is PartialDelegationTest { _totalNumerator += _numerator; } - _expectEmitDelegateChangedEvents(_actor, oldDelegations, newDelegations); + vm.expectEmit(); + emit DelegateChanged(_actor, oldDelegations, newDelegations); tokenProxy.delegate(newDelegations); vm.stopPrank(); } @@ -417,7 +383,8 @@ contract Delegate is PartialDelegationTest { PartialDelegation[] memory newDelegations = _createValidPartialDelegation(_newN, uint256(keccak256(abi.encode(_seed)))); - _expectEmitDelegateChangedEvents(_actor, oldDelegations, newDelegations); + vm.expectEmit(); + emit DelegateChanged(_actor, oldDelegations, newDelegations); tokenProxy.delegate(newDelegations); vm.stopPrank(); } @@ -1446,132 +1413,6 @@ contract GetPastTotalSupply is PartialDelegationTest { } } -// This contract strengthens our confidence in our test helper, `_expectEmitDelegateChangedEvents` -contract ExpectEmitDelegateChangedEvents is PartialDelegationTest { - function test_EmitsWhenFromNoDelegateeToANewDelegateeIsAdded() public { - address _actor = address(this); - PartialDelegation[] memory _oldDelegations = new PartialDelegation[](0); - PartialDelegation[] memory _newDelegations = new PartialDelegation[](1); - _newDelegations[0] = PartialDelegation({_delegatee: address(0x1), _numerator: 5000}); - - vm.recordLogs(); - _expectEmitDelegateChangedEvents(_actor, _oldDelegations, _newDelegations); - uint256 _logLength = vm.getRecordedLogs().length; - - emit DelegateChanged(_actor, address(0x1), 5000); - assertEq(_logLength, 1); - } - - function test_EmitsWhenBothDelegationsHaveTheSameDelegateeButTheNumeratorIsDifferent() public { - address _actor = address(this); - PartialDelegation[] memory _oldDelegations = new PartialDelegation[](1); - _oldDelegations[0] = PartialDelegation({_delegatee: address(0x1), _numerator: 10_000}); - PartialDelegation[] memory _newDelegations = new PartialDelegation[](1); - _newDelegations[0] = PartialDelegation({_delegatee: address(0x1), _numerator: 5000}); - - vm.recordLogs(); - _expectEmitDelegateChangedEvents(_actor, _oldDelegations, _newDelegations); - uint256 _logLength = vm.getRecordedLogs().length; - - emit DelegateChanged(_actor, address(0x1), 5000); - assertEq(_logLength, 1); - } - - function test_EmitsWhenOldDelegateeComesBeforeTheNewDelegatee() public { - address _actor = address(this); - PartialDelegation[] memory _oldDelegations = new PartialDelegation[](1); - _oldDelegations[0] = PartialDelegation({_delegatee: address(0x1), _numerator: 10_000}); - PartialDelegation[] memory _newDelegations = new PartialDelegation[](2); - _newDelegations[0] = PartialDelegation({_delegatee: address(0x1), _numerator: 5000}); - _newDelegations[1] = PartialDelegation({_delegatee: address(0x2), _numerator: 5000}); - - vm.recordLogs(); - _expectEmitDelegateChangedEvents(_actor, _oldDelegations, _newDelegations); - uint256 _logLength = vm.getRecordedLogs().length; - - emit DelegateChanged(_actor, address(0x1), 5000); - emit DelegateChanged(_actor, address(0x2), 5000); - assertEq(_logLength, 2); - } - - function test_EmitsWhenNewDelegateeComesBeforeTheOldDelegatee() public { - address _actor = address(this); - PartialDelegation[] memory _oldDelegations = new PartialDelegation[](1); - _oldDelegations[0] = PartialDelegation({_delegatee: address(0x2), _numerator: 10_000}); - PartialDelegation[] memory _newDelegations = new PartialDelegation[](2); - _newDelegations[0] = PartialDelegation({_delegatee: address(0x1), _numerator: 5000}); - _newDelegations[1] = PartialDelegation({_delegatee: address(0x2), _numerator: 5000}); - - vm.recordLogs(); - _expectEmitDelegateChangedEvents(_actor, _oldDelegations, _newDelegations); - uint256 _logLength = vm.getRecordedLogs().length; - - emit DelegateChanged(_actor, address(0x1), 5000); - emit DelegateChanged(_actor, address(0x2), 5000); - assertEq(_logLength, 2); - } - - function test_CrazyCase1() public { - address _actor = address(this); - PartialDelegation[] memory _oldDelegations = new PartialDelegation[](8); - _oldDelegations[0] = PartialDelegation({_delegatee: address(0x1), _numerator: 5}); - _oldDelegations[1] = PartialDelegation({_delegatee: address(0x2), _numerator: 7}); - _oldDelegations[2] = PartialDelegation({_delegatee: address(0x3), _numerator: 9}); - _oldDelegations[3] = PartialDelegation({_delegatee: address(0x4), _numerator: 11}); - _oldDelegations[4] = PartialDelegation({_delegatee: address(0xA), _numerator: 13}); - _oldDelegations[5] = PartialDelegation({_delegatee: address(0xB), _numerator: 15}); - _oldDelegations[6] = PartialDelegation({_delegatee: address(0xC), _numerator: 17}); - _oldDelegations[7] = PartialDelegation({_delegatee: address(0xD), _numerator: 19}); - - PartialDelegation[] memory _newDelegations = new PartialDelegation[](7); - // prepend - _newDelegations[0] = PartialDelegation({_delegatee: address(0x0), _numerator: 5}); - // leave 0x1 the same - _newDelegations[1] = PartialDelegation({_delegatee: address(0x1), _numerator: 5}); - // change numerator for 0x2 - _newDelegations[2] = PartialDelegation({_delegatee: address(0x2), _numerator: 19}); - // remove 0x3 - // remove 0x4, add 0x9 - _newDelegations[3] = PartialDelegation({_delegatee: address(0x9), _numerator: 1150}); - // keep 0xA the same - _newDelegations[4] = PartialDelegation({_delegatee: address(0xA), _numerator: 13}); - // remove 0xB - // keep 0xC the same - // change 0xD numerator - _newDelegations[5] = PartialDelegation({_delegatee: address(0xD), _numerator: 170}); - // add 0xE - _newDelegations[6] = PartialDelegation({_delegatee: address(0xE), _numerator: 190}); - - vm.recordLogs(); - _expectEmitDelegateChangedEvents(_actor, _oldDelegations, _newDelegations); - uint256 _logLength = vm.getRecordedLogs().length; - - // Source of truth events: The events that we want the expect helper to expect, given the delegation changes. - // 0x0 emitted because new delegate - emit DelegateChanged(_actor, address(0x0), 5); - // skip 0x1 as no change - // 0x2 emitted because numerator change - emit DelegateChanged(_actor, address(0x2), 19); - // 0x3 emitted because removed - emit DelegateChanged(_actor, address(0x3), 0); - // 0x4 emitted because removed - emit DelegateChanged(_actor, address(0x4), 0); - // 0x9 emitted because added - emit DelegateChanged(_actor, address(0x9), 1150); - // 0xA skipped, no change - // 0xB emitted because removed - emit DelegateChanged(_actor, address(0xB), 0); - // 0xC emitted because removed - emit DelegateChanged(_actor, address(0xC), 0); - // 0xD emitted because numerator change - emit DelegateChanged(_actor, address(0xD), 170); - // 0xE emitted because added - emit DelegateChanged(_actor, address(0xE), 190); - uint256 _expectedLength = vm.getRecordedLogs().length; - assertEq(_logLength, _expectedLength); - } -} - // This contract strengthens our confidence in our test helper, `_expectEmitDelegateVotesChangedEvents` contract ExpectEmitDelegateVotesChangedEvents is PartialDelegationTest { /// An Ethereum log. Returned by `getRecordedLogs`. @@ -1681,7 +1522,8 @@ contract ExpectEmitDelegateVotesChangedEvents is PartialDelegationTest { } } -// This contract strengthens our confidence in our test helper, `_expectEmitDelegateChangedEvents` (the 4 param version) +// This contract strengthens our confidence in our test helper, `_expectEmitDelegateVotesChangedEvents` (the 4 param +// version) contract ExpectEmitDelegateVotesChangedEventsDuringTransfer is PartialDelegationTest { function test_EmitsWhenTransferringTokensFromAnAddressWithNoDelegationsToAnAddressWithNoDelegations() public { address from = address(0x10); diff --git a/test/helpers/DelegationAndEventHelpers.sol b/test/helpers/DelegationAndEventHelpers.sol index 058fb3d..59840d5 100644 --- a/test/helpers/DelegationAndEventHelpers.sol +++ b/test/helpers/DelegationAndEventHelpers.sol @@ -10,7 +10,9 @@ contract DelegationAndEventHelpers is Test { ERC20VotesPartialDelegationUpgradeable token; /// @dev Emitted when an account changes their delegate. - event DelegateChanged(address indexed delegator, address indexed delegatee, uint96 numerator); + event DelegateChanged( + address indexed delegator, PartialDelegation[] oldDelegatees, PartialDelegation[] newDelegatees + ); /// @dev Emitted when a token transfer or delegate change results in changes to a delegate's number of voting units. event DelegateVotesChanged(address indexed delegate, uint256 previousVotes, uint256 newVotes);