Made setting delegate again a no-op, refactored some common code in T…

…icket.sol
pooltogether · Oct 6, 2021 · 7a3d62d · 7a3d62d
1 parent 0a89dce
commit 7a3d62d
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 53 deletions.
diff --git a/contracts/Ticket.sol b/contracts/Ticket.sol
@@ -226,26 +226,13 @@ contract Ticket is ControlledToken, ITicket {
         uint256 balance = balanceOf(_user);
         address currentDelegate = delegates[_user];
 
-        require(currentDelegate != _to, "Ticket/delegate-already-set");
-
-        delegates[_user] = _to;
-
-        // if we are going from a delegated address to an undelegated address
-        if (currentDelegate != address(0) && _to == address(0)) {
-            _decreaseTotalSupplyTwab(balance);
-        } else
-        // if we are going from undelegated address to a delegated address
-        if (currentDelegate == address(0) && _to != address(0)) {
-            _increaseTotalSupplyTwab(balance);
+        if (currentDelegate == _to) {
+            return;
         }
 
-        if (currentDelegate != address(0)) {
-            _decreaseUserTwab(_user, currentDelegate, balance);
-        }
+        delegates[_user] = _to;
 
-        if (_to != address(0)) {
-            _increaseUserTwab(_user, _to, balance);
-        }
+        _transferTwab(currentDelegate, _to, balance);
 
         emit Delegated(_user, _to);
     }
@@ -300,33 +287,37 @@ contract Ticket is ControlledToken, ITicket {
             _toDelegate = delegates[_to];
         }
 
+        _transferTwab(_fromDelegate, _toDelegate, _amount);
+    }
+
+    /// @notice Transfers the given TWAB balance from one user to another
+    /// @param _from The user to transfer the balance from.  May be zero in the event of a mint.
+    /// @param _to The user to transfer the balance to.  May be zero in the event of a burn.
+    /// @param _amount The balance that is being transferred.
+    function _transferTwab(address _from, address _to, uint256 _amount) internal {
         // If we are transferring tokens from an undelegated account to a delegated account
-        if (_fromDelegate == address(0) && _toDelegate != address(0)) {
+        if (_from == address(0) && _to != address(0)) {
             _increaseTotalSupplyTwab(_amount);
-        } // otherwise, if the from delegate is set, then decrease their twab
-        else if (_fromDelegate != address(0)) {
-            _decreaseUserTwab(_from, _fromDelegate, _amount);
-        }
-
-        // if we are transferring tokens from a delegated account to an undelegated account
-        if (_fromDelegate != address(0) && _toDelegate == address(0)) {
+        } else // if we are transferring tokens from a delegated account to an undelegated account
+        if (_from != address(0) && _to == address(0)) {
             _decreaseTotalSupplyTwab(_amount);
         } // otherwise if the to delegate is set, then increase their twab
-        else if (_toDelegate != address(0)) {
-            _increaseUserTwab(_to, _toDelegate, _amount);
+
+        if (_from != address(0)) {
+            _decreaseUserTwab(_from, _amount);
+        }
+
+        if (_to != address(0)) {
+            _increaseUserTwab(_to, _amount);
         }
     }
 
     /**
-     * @notice Increase `_user` TWAB balance.
-     * @dev If `_user` has not set a delegate address, `_user` TWAB balance will be increased.
-     * @dev Otherwise, `_delegate` TWAB balance will be increased.
-     * @param _user Address of the user.
+     * @notice Increase `_to` TWAB balance.
      * @param _to Address of the delegate.
-     * @param _amount Amount of tokens to be added to `_user` TWAB balance.
+     * @param _amount Amount of tokens to be added to `_to` TWAB balance.
      */
     function _increaseUserTwab(
-        address _user,
         address _to,
         uint256 _amount
     ) internal {
@@ -345,20 +336,16 @@ contract Ticket is ControlledToken, ITicket {
         _account.details = accountDetails;
 
         if (isNew) {
-            emit NewUserTwab(_user, _to, twab);
+            emit NewUserTwab(_to, twab);
         }
     }
 
     /**
-     * @notice Decrease `_user` TWAB balance.
-     * @dev If `_user` has not set a delegate address, `_user` TWAB balance will be decreased.
-     * @dev Otherwise, `_delegate` TWAB balance will be decreased.
-     * @param _user Address of the user.
+     * @notice Decrease `_to` TWAB balance.
      * @param _to Address of the delegate.
-     * @param _amount Amount of tokens to be added to `_user` TWAB balance.
+     * @param _amount Amount of tokens to be added to `_to` TWAB balance.
      */
     function _decreaseUserTwab(
-        address _user,
         address _to,
         uint256 _amount
     ) internal {
@@ -382,7 +369,7 @@ contract Ticket is ControlledToken, ITicket {
         _account.details = accountDetails;
 
         if (isNew) {
-            emit NewUserTwab(_user, _to, twab);
+            emit NewUserTwab(_to, twab);
         }
     }
 

diff --git a/contracts/interfaces/ITicket.sol b/contracts/interfaces/ITicket.sol
@@ -46,12 +46,10 @@ interface ITicket is IControlledToken {
 
     /**
      * @notice Emitted when a new TWAB has been recorded.
-     * @param user The Ticket holder address.
      * @param delegate The recipient of the ticket power (may be the same as the user).
      * @param newTwab Updated TWAB of a ticket holder after a successful TWAB recording.
      */
     event NewUserTwab(
-        address indexed user,
         address indexed delegate,
         ObservationLib.Observation newTwab
     );

diff --git a/test/Ticket.test.ts b/test/Ticket.test.ts
@@ -411,7 +411,7 @@ describe('Ticket', () => {
             await ticket.mint(wallet1.address, insufficientMintAmount);
 
             await expect(ticket.burn(wallet1.address, mintAmount)).to.be.revertedWith(
-                'ERC20: burn amount exceeds balance',
+                'Ticket/burn-amount-exceeds-total-supply-twab',
             );
         });
 
@@ -861,18 +861,13 @@ describe('Ticket', () => {
             expect(await ticket.getBalanceAt(wallet2.address, timestamp)).to.equal(toWei('100'));
         });
 
-        it('should revert if delegate address has already been set to passed address', async () => {
+        it('should be a no-op if delegate address has already been set to passed address', async () => {
             await ticket.mint(wallet1.address, toWei('100'));
             await ticket.delegate(wallet2.address);
 
-            await expect(ticket.delegate(wallet2.address)).to.be.revertedWith(
-                'Ticket/delegate-already-set',
-            );
-
-            const timestamp = (await provider.getBlock('latest')).timestamp;
-
-            expect(await ticket.delegateOf(wallet1.address)).to.equal(wallet2.address);
-            expect(await ticket.getBalanceAt(wallet2.address, timestamp)).to.equal(toWei('100'));
+            await expect(
+                ticket.delegate(wallet2.address)
+            ).to.not.emit(ticket, 'Delegated')
         });
 
         it('should allow the delegate to be reset by passing zero', async () => {