Add voucherHub refund tests

contracts/IVoucherHub.sol

@@ -42,7 +42,7 @@ interface IVoucherHub {
         address workerpool,
         uint256 workerpoolPrice
     ) external returns (uint256 sponsoredAmount);
-    function refundVoucherForTask(bytes32 taskId, uint256 amount) external;
+    function refundVoucher(uint256 amount) external;
 
     function getIexecPoco() external view returns (address);
     function getVoucherBeacon() external view returns (address);

contracts/VoucherHub.sol

@@ -3,8 +3,6 @@
 
 pragma solidity ^0.8.20;
 
-import {IexecLibCore_v5} from "@iexec/poco/contracts/libs/IexecLibCore_v5.sol";
-import {IexecPocoAccessors} from "@iexec/poco/contracts/modules/interfaces/IexecPocoAccessors.sol";
 import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import {Create2} from "@openzeppelin/contracts/utils/Create2.sol";
 import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";
@@ -49,8 +47,8 @@ contract VoucherHub is
         bytes32 _voucherCreationCodeHash;
         VoucherType[] voucherTypes;
         mapping(uint256 voucherTypeId => mapping(address asset => bool)) matchOrdersEligibility;
-        // Save refunded tasks to disable replay attack.
-        mapping(bytes32 taskId => bool) _refundedTasks;
+        // Track created vouchers to avoid replay in certain operations such as refund.
+        mapping(address voucherAddress => bool) _isVoucher;
     }
 
     modifier whenVoucherTypeExists(uint256 id) {
@@ -59,6 +57,12 @@ contract VoucherHub is
         _;
     }
 
+    modifier onlyVoucher() {
+        VoucherHubStorage storage $ = _getVoucherHubStorage();
+        require($._isVoucher[msg.sender], "VoucherHub: sender is not voucher");
+        _;
+    }
+
     /// @custom:oz-upgrades-unsafe-allow constructor
     constructor() {
         _disableInitializers();
@@ -170,6 +174,7 @@ contract VoucherHub is
         Voucher(voucherAddress).initialize(owner, address(this), voucherExpiration, voucherType);
         IERC20($._iexecPoco).transfer(voucherAddress, value); // SRLC
         _mint(voucherAddress, value); // VCHR
+        $._isVoucher[voucherAddress] = true;
         emit VoucherCreated(voucherAddress, owner, voucherExpiration, voucherType, value);
     }
 
@@ -183,6 +188,9 @@ contract VoucherHub is
      * possible to try to debit the voucher in best effort mode (In short: "use
      * voucher if possible"), before trying other payment methods.
      *
+     * Note: no need to for "onlyVoucher" modifier because if the sender is not a voucher,
+     * its balance would be null, then "_burn()" would revert.
+     *
      * @param voucherTypeId The type ID of the voucher to debit.
      * @param app The app address.
      * @param appPrice The app price.
@@ -220,20 +228,12 @@ contract VoucherHub is
     }
 
     /**
-     * Refund voucher for a failed task.
-     * @param taskId id of the task
-     * @param amount to be refunded
+     * Refund sender if it is a voucher.
+     * @param amount value to be refunded
      */
-    function refundVoucherForTask(bytes32 taskId, uint256 amount) external {
-        VoucherHubStorage storage $ = _getVoucherHubStorage();
-        require(!$._refundedTasks[taskId], "VoucherHub: task already refunded");
-        require(
-            IexecPocoAccessors($._iexecPoco).viewTask(taskId).status ==
-                IexecLibCore_v5.TaskStatusEnum.FAILED,
-            "VoucherHub: invalid task id or status"
-        );
+    function refundVoucher(uint256 amount) external onlyVoucher {
+        // TODO ?? check if balanceOf(msg.sender) + amount < initialBalance
         _mint(msg.sender, amount);
-        $._refundedTasks[taskId] = true;
         emit VoucherRefunded(msg.sender, amount);
     }
 

contracts/beacon/Voucher.sol

@@ -36,6 +36,8 @@ contract Voucher is OwnableUpgradeable, IVoucher {
         // Save all deals matched by the voucher to be able to
         // refund requesters for non sponsored tasks.
         mapping(bytes32 dealId => VoucherMatchedDeal) _voucherMatchedDeals;
+        // Save refunded tasks to disable replay attacks.
+        mapping(bytes32 taskId => bool) _claimedTasks;
     }
 
     struct VoucherMatchedDeal {
@@ -194,6 +196,7 @@ contract Voucher is OwnableUpgradeable, IVoucher {
         address iexecPoco = voucherHub.getIexecPoco();
         // Check if task is already claimed.
         bytes32 taskId = keccak256(abi.encodePacked(dealId, index));
+        require(!$._claimedTasks[taskId], "Voucher: task already claimed");
         // Get deal details from PoCo.
         (bool isBoostDeal, address requester, uint256 volume, uint256 dealPrice) = _getDealDetails(
             iexecPoco,
@@ -226,7 +229,7 @@ contract Voucher is OwnableUpgradeable, IVoucher {
                 if (taskSponsoredAmount != 0) {
                     // If the voucher did fully/partially sponsor the deal then mint voucher
                     // credits back.
-                    voucherHub.refundVoucherForTask(taskId, taskSponsoredAmount);
+                    voucherHub.refundVoucher(taskSponsoredAmount);
                 }
                 if (taskSponsoredAmount < taskPrice) {
                     // If the deal was not sponsored or partially sponsored
@@ -235,6 +238,7 @@ contract Voucher is OwnableUpgradeable, IVoucher {
                     IERC20(iexecPoco).transfer(requester, taskPrice - taskSponsoredAmount);
                 }
             }
+            $._claimedTasks[taskId] = true;
         } else {
             // If the deal was not matched by the voucher then
             // forward the claim request to PoCo and do nothing.

contracts/mocks/VoucherHubV2Mock.sol

@@ -8,6 +8,9 @@ pragma solidity ^0.8.20;
 /**
  * @notice This contract is for upgradeability testing purposes only.
  */
+
+// TODO add the same storage structure to enable storage check in upgrade tests.
+
 contract VoucherHubV2Mock is VoucherHub {
     // For production use, please instead use ERC7201 when adding new variables
     string public foo;

contracts/mocks/VoucherV2Mock.sol

@@ -13,6 +13,7 @@ contract VoucherV2Mock is OwnableUpgradeable {
         uint256 _type;
         mapping(address => bool) _authorizedAccounts;
         mapping(bytes32 dealId => MatchedDeal) _voucherMatchedDeals;
+        mapping(bytes32 taskId => bool) _claimedTasks;
         uint256 _newStateVariable;
     }
 

test/VoucherHub.test.ts

@@ -21,6 +21,8 @@ const asset = random();
 const assetPrice = 1;
 const initVoucherHubBalance = 10 * voucherValue; // arbitrary value, but should support couple voucher creations
 
+// TODO use global variables (signers, addresses, ...).
+
 describe('VoucherHub', function () {
     let iexecPoco: string;
     let iexecPocoInstance: IexecPocoMock;
@@ -790,10 +792,64 @@ describe('VoucherHub', function () {
     });
 
     describe('Refund voucher', function () {
-        it('Should refund voucher for task', async function () {});
-        it('Should not refund voucher twice', async function () {});
-        it('Should not refund voucher when task is not found', async function () {});
-        it('Should not refund voucher when task is not failed', async function () {});
+        let [voucherOwner1, voucher, anyone]: SignerWithAddress[] = [];
+        let voucherHub: VoucherHub;
+
+        beforeEach(async function () {
+            ({ voucherHub, voucherOwner1, anyone } = await loadFixture(deployFixture));
+            // Create voucher type
+            await voucherHubWithAssetEligibilityManagerSigner
+                .createVoucherType(description, duration)
+                .then((tx) => tx.wait());
+            // Add eligible asset
+            await voucherHubWithAssetEligibilityManagerSigner
+                .addEligibleAsset(voucherType, asset)
+                .then((tx) => tx.wait());
+            // Create voucher
+            voucher = await voucherHubWithVoucherManagerSigner
+                .createVoucher(voucherOwner1, voucherType, voucherValue)
+                .then((tx) => tx.wait())
+                .then(() => voucherHub.getVoucher(voucherOwner1))
+                .then((voucherAddress) => ethers.getImpersonatedSigner(voucherAddress));
+        });
+
+        it('Should refund voucher', async function () {
+            const debitedValue = BigInt(assetPrice * 3);
+            const voucherInitialCreditBalance = await voucherHub.balanceOf(voucher.address);
+            await voucherHub
+                .connect(voucher)
+                .debitVoucher(voucherType, asset, assetPrice, asset, assetPrice, asset, assetPrice)
+                .then((tx) => tx.wait());
+            expect(await voucherHub.balanceOf(voucher.address)).equals(
+                voucherInitialCreditBalance - debitedValue,
+            );
+            // Refund voucher.
+            const refundAmount = debitedValue / 2n; // any amount < sponsoredValue
+            await expect(voucherHub.connect(voucher).refundVoucher(refundAmount))
+                .to.emit(voucherHub, 'Transfer')
+                .withArgs(ethers.ZeroAddress, voucher.address, refundAmount)
+                .to.emit(voucherHub, 'VoucherRefunded')
+                .withArgs(voucher.address, refundAmount);
+        });
+
+        it('Should not refund when sender is not a voucher', async function () {
+            const debitedValue = BigInt(assetPrice * 3);
+            const voucherInitialCreditBalance = await voucherHub.balanceOf(voucher.address);
+            await voucherHub
+                .connect(voucher)
+                .debitVoucher(voucherType, asset, assetPrice, asset, assetPrice, asset, assetPrice)
+                .then((tx) => tx.wait());
+            expect(await voucherHub.balanceOf(voucher.address)).equals(
+                voucherInitialCreditBalance - debitedValue,
+            );
+            // Refund voucher.
+            const refundAmount = debitedValue / 2n; // any amount < sponsoredValue
+            await expect(voucherHub.connect(anyone).refundVoucher(refundAmount)).to.be.revertedWith(
+                'VoucherHub: sender is not voucher',
+            );
+        });
+
+        it.skip('TODO ?? - Should not refund voucher when amount is greater than voucher value', async function () {});
     });
 
     describe('Get voucher', function () {

test/beacon/Voucher.test.ts

@@ -848,7 +848,7 @@ describe('Voucher', function () {
 
                 // Second claim should revert.
                 await expect(voucher.claim(dealId, taskIndex)).to.be.revertedWith(
-                    'VoucherHub: task already refunded',
+                    'Voucher: task already claimed',
                 );
             }
         });