Cleaned up code and added Donation event

script/HookDonation.s.sol

@@ -15,7 +15,7 @@ contract CounterScript is Script, Deployers {
     function run() public {
         vm.startBroadcast();
 
-        // test.setUp();
+
 
         vm.stopBroadcast();
     }

src/HookDonation.sol

@@ -3,7 +3,7 @@ pragma solidity ^0.8.13;
 
 import {BaseHook} from "lib/v4-periphery/src/base/hooks/BaseHook.sol";
 import {PoolKey} from "lib/v4-periphery/lib/v4-core/src/types/PoolKey.sol";
-import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol";
+import {IPoolManager} from "lib/v4-core/src/interfaces/IPoolManager.sol";
 import {BalanceDelta} from "lib/v4-periphery/lib/v4-core/src/types/BalanceDelta.sol";
 import {Hooks} from "lib/v4-periphery/lib/v4-core/src/libraries/Hooks.sol";
 import {CurrencyLibrary, Currency} from "lib/v4-periphery/lib/v4-core/src/types/Currency.sol";
@@ -21,6 +21,8 @@ contract AfterSwapDonationHook is BaseHook {
     address public owner;
     mapping(address => DonationMapping) donationMap;
 
+    event Donation(address indexed payee, address indexed recipient, uint donatedAmount);
+
     // -------------- begin donation associated functions ---------------
     /// Disables donation for msg.sender
     function disableDonation() public {
@@ -33,9 +35,6 @@ contract AfterSwapDonationHook is BaseHook {
         donationMap[tx.origin] = DonationMapping(payable(recipient), percent);
     }
 
-    // the following should all have internal view, not public
-    // but have been changed to public view for testing
-
     function donationEnabled(address addr) public view returns (bool) {
         bool result = donationMap[addr].recipient != payable(0x0);
         return result;
@@ -87,7 +86,6 @@ contract AfterSwapDonationHook is BaseHook {
         BalanceDelta delta,
         bytes calldata // userdata
     ) external override returns (bytes4, int128) {
-        // require(msg.sender == address(poolManager), "Unauthorized caller");
 
         // Check that donation is enabled for the tx.origin, otherwise, return early
         if (!donationEnabled(tx.origin)) {
@@ -104,13 +102,14 @@ contract AfterSwapDonationHook is BaseHook {
 
         IERC20Minimal token = IERC20Minimal(Currency.unwrap(key.currency0));
         uint256 allowance = token.allowance(tx.origin, address(this));
-        assert(allowance > 0); // check that we're allowed to spend on behalf of tx.origin
+        assert(allowance >= donationAmount); // check that we're allowed to spend on behalf of tx.origin
 
         // Track the balance before the transfer
         uint256 balanceOriginBefore = token.balanceOf(tx.origin);
         uint256 balanceRecipientBefore = token.balanceOf(recipient);
 
         token.transferFrom(tx.origin, recipient, donationAmount);
+        emit Donation(tx.origin, recipient, donationAmount);
 
         // Track the balance after the transfer
         uint256 balanceOriginAfter = token.balanceOf(tx.origin);

test/HookDonation.t.sol

@@ -18,17 +18,14 @@ import {IHooks} from "lib/v4-core/src/interfaces/IHooks.sol";
 
 contract DonationTest is
     Test,
-    Deployers //, ISwap
+    Deployers
 {
     using CurrencyLibrary for Currency;
 
     AfterSwapDonationHook donationHook;
 
-    // Mock token
-    // MockERC20 token;
-
     // The two currencies (tokens) from the pool
-    Currency token0 = Currency.wrap(address(0));
+    Currency token0;
     Currency token1;
     PoolKey globalKey;
     address constant RECIPIENT = address(0x01);
@@ -76,15 +73,14 @@ contract DonationTest is
         uint256 enabledPercent = 10;
         donationHook.enableDonation(RECIPIENT, enabledPercent);
 
-        // Now, verify that the recipent, the enabled status and percentage is correctly set
+        // Now, verify that the recipent, the enabled status, percentage and recipient is correctly set
         recipient = donationHook.donationRecipient();
         enabled = donationHook.donationEnabled();
-        uint256 fetchedPercent = donationHook.donationPercent();
+        uint256 setPercent = donationHook.donationPercent();
 
-        payee = donationHook.donationPayee();
         assert(enabled);
         assert(recipient == RECIPIENT);
-        assert(fetchedPercent == enabledPercent);
+        assert(setPercent == enabledPercent);
         vm.stopPrank();
     }
 
@@ -97,12 +93,14 @@ contract DonationTest is
         uint256 percent = 20;
         if (!enabled) {
             donationHook.enableDonation(RECIPIENT, percent);
-        }
 
-        enabled = donationHook.donationEnabled();
-        recipient = donationHook.donationRecipient();
-        assert(enabled);
-        assert(recipient == RECIPIENT);
+            enabled = donationHook.donationEnabled();
+            recipient = donationHook.donationRecipient();
+            uint setPercent = donationHook.donationPercent();
+            assert(enabled);
+            assert(recipient == RECIPIENT);
+            assert(setPercent == percent);
+        }
 
         donationHook.disableDonation();
         enabled = donationHook.donationEnabled();
@@ -113,6 +111,9 @@ contract DonationTest is
         vm.stopPrank();
     }
 
+    /// @param account The account to mint to
+    /// @param amount1 The number of units to mint to, for key.currency0
+    /// @param amount2 The amount of units to mint to, for key.currency1
     function mint(address account, uint256 amount1, uint256 amount2) internal {
         MockERC20 t0 = MockERC20(Currency.unwrap(token0));
         MockERC20 t1 = MockERC20(Currency.unwrap(token1));
@@ -121,7 +122,6 @@ contract DonationTest is
     }
 
     function test_Swap() public {
-        //
         vm.startPrank(tx.origin);
 
         MockERC20 t0 = MockERC20(Currency.unwrap(token0));
@@ -132,7 +132,8 @@ contract DonationTest is
         uint256 percent = 10;
         donationHook.enableDonation(RECIPIENT, percent);
 
-        // Approve this contract to spend on behalf of tx.origin, which is the user / EOA
+        // Approve the donationHook contract to spend on behalf of tx.origin, which is the user / EOA
+        // This is essential, otherwise, in afterSwap, token.transferFrom will fail
         t0.approve(address(donationHook), t0.balanceOf(tx.origin));
         t1.approve(address(donationHook), t1.balanceOf(tx.origin));