From 91ecfa8ef4112ba84f5b066b2acec495a9598393 Mon Sep 17 00:00:00 2001 From: Gabriel Fournier Date: Mon, 29 Apr 2024 10:37:41 +0200 Subject: [PATCH] factorize match orders / boost functions with modifiers --- contracts/beacon/Voucher.sol | 124 ++++++++++++++++++++++------------- test/beacon/Voucher.test.ts | 85 +++++++++++++++++++++--- 2 files changed, 153 insertions(+), 56 deletions(-) diff --git a/contracts/beacon/Voucher.sol b/contracts/beacon/Voucher.sol index ef3be44..6544f2c 100644 --- a/contracts/beacon/Voucher.sol +++ b/contracts/beacon/Voucher.sol @@ -30,6 +30,21 @@ contract Voucher is OwnableUpgradeable, IVoucher { mapping(bytes32 dealId => uint256) _sponsoredAmounts; } + modifier onlyAuthorized() { + VoucherStorage storage $ = _getVoucherStorage(); + require( + msg.sender == owner() || $._authorizedAccounts[msg.sender], + "Voucher: sender is not authorized" + ); + _; + } + + modifier onlyNotExpired() { + VoucherStorage storage $ = _getVoucherStorage(); + require(block.timestamp < $._expiration, "Voucher: voucher is expired"); + _; + } + /// @custom:oz-upgrades-unsafe-allow constructor constructor() { _disableInitializers(); @@ -92,32 +107,17 @@ contract Voucher is OwnableUpgradeable, IVoucher { ) external returns (bytes32 dealId) { // TODO add onlyAuthorized // TODO check expiration - uint256 appPrice = appOrder.appprice; - uint256 datasetPrice = datasetOrder.datasetprice; - uint256 workerpoolPrice = workerpoolOrder.workerpoolprice; VoucherStorage storage $ = _getVoucherStorage(); IVoucherHub voucherHub = IVoucherHub($._voucherHub); - uint256 sponsoredAmount = voucherHub.debitVoucher( + uint256 sponsoredAmount = _debitVoucherAndTransfer( $._type, - appOrder.app, - appPrice, - datasetOrder.dataset, - datasetPrice, - workerpoolOrder.workerpool, - workerpoolPrice + $._voucherHub, + appOrder, + datasetOrder, + workerpoolOrder, + requestOrder ); - uint256 dealPrice = appPrice + datasetPrice + workerpoolPrice; - address iexecPoco = voucherHub.getIexecPoco(); - if (sponsoredAmount != dealPrice) { - // Transfer non-sponsored amount from the iExec account of the - // requester to the iExec account of the voucher - IERC20(iexecPoco).transferFrom( - requestOrder.requester, - address(this), - dealPrice - sponsoredAmount - ); - } - dealId = IexecPoco1(iexecPoco).sponsorMatchOrders( + dealId = IexecPoco1(voucherHub.getIexecPoco()).sponsorMatchOrders( appOrder, datasetOrder, workerpoolOrder, @@ -143,35 +143,18 @@ contract Voucher is OwnableUpgradeable, IVoucher { IexecLibOrders_v5.DatasetOrder calldata datasetOrder, IexecLibOrders_v5.WorkerpoolOrder calldata workerpoolOrder, IexecLibOrders_v5.RequestOrder calldata requestOrder - ) external returns (bytes32 dealId) { - // TODO add onlyAuthorized - // TODO check expiration - uint256 appPrice = appOrder.appprice; - uint256 datasetPrice = datasetOrder.datasetprice; - uint256 workerpoolPrice = workerpoolOrder.workerpoolprice; + ) external onlyAuthorized onlyNotExpired returns (bytes32 dealId) { VoucherStorage storage $ = _getVoucherStorage(); IVoucherHub voucherHub = IVoucherHub($._voucherHub); - uint256 sponsoredAmount = voucherHub.debitVoucher( + uint256 sponsoredAmount = _debitVoucherAndTransfer( $._type, - appOrder.app, - appPrice, - datasetOrder.dataset, - datasetPrice, - workerpoolOrder.workerpool, - workerpoolPrice + $._voucherHub, + appOrder, + datasetOrder, + workerpoolOrder, + requestOrder ); - uint256 dealPrice = appPrice + datasetPrice + workerpoolPrice; - address iexecPoco = voucherHub.getIexecPoco(); - if (sponsoredAmount != dealPrice) { - // Transfer non-sponsored amount from the iExec account of the - // requester to the iExec account of the voucher - IERC20(iexecPoco).transferFrom( - requestOrder.requester, - address(this), - dealPrice - sponsoredAmount - ); - } - dealId = IexecPocoBoost(iexecPoco).sponsorMatchOrdersBoost( + dealId = IexecPocoBoost(voucherHub.getIexecPoco()).sponsorMatchOrdersBoost( appOrder, datasetOrder, workerpoolOrder, @@ -251,4 +234,51 @@ contract Voucher is OwnableUpgradeable, IVoucher { $.slot := VOUCHER_STORAGE_LOCATION } } + + /** + * @dev Debit voucher and transfer non-sponsored amount from requester's account. + * + * @param appOrder The app order. + * @param datasetOrder The dataset order. + * @param workerpoolOrder The workerpool order. + * @param requestOrder The request order. + * + * @return sponsoredAmount The amount sponsored by the voucher. + */ + function _debitVoucherAndTransfer( + uint256 _type, + address _voucherHub, + IexecLibOrders_v5.AppOrder calldata appOrder, + IexecLibOrders_v5.DatasetOrder calldata datasetOrder, + IexecLibOrders_v5.WorkerpoolOrder calldata workerpoolOrder, + IexecLibOrders_v5.RequestOrder calldata requestOrder + ) private returns (uint256 sponsoredAmount) { + uint256 appPrice = appOrder.appprice; + uint256 datasetPrice = datasetOrder.datasetprice; + uint256 workerpoolPrice = workerpoolOrder.workerpoolprice; + IVoucherHub voucherHub = IVoucherHub(_voucherHub); + + sponsoredAmount = voucherHub.debitVoucher( + _type, + appOrder.app, + appPrice, + datasetOrder.dataset, + datasetPrice, + workerpoolOrder.workerpool, + workerpoolPrice + ); + + uint256 dealPrice = appPrice + datasetPrice + workerpoolPrice; + address iexecPoco = voucherHub.getIexecPoco(); + + if (sponsoredAmount != dealPrice) { + // Transfer non-sponsored amount from the iExec account of the + // requester to the iExec account of the voucher + IERC20(iexecPoco).transferFrom( + requestOrder.requester, + address(this), + dealPrice - sponsoredAmount + ); + } + } } diff --git a/test/beacon/Voucher.test.ts b/test/beacon/Voucher.test.ts index c374edf..3344d2a 100644 --- a/test/beacon/Voucher.test.ts +++ b/test/beacon/Voucher.test.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { SignerWithAddress } from '@nomicfoundation/hardhat-ethers/signers'; -import { loadFixture } from '@nomicfoundation/hardhat-toolbox/network-helpers'; +import { loadFixture, mine, time } from '@nomicfoundation/hardhat-toolbox/network-helpers'; import { expect } from 'chai'; import { ethers } from 'hardhat'; import * as commonUtils from '../../scripts/common'; @@ -61,6 +61,7 @@ describe('Voucher', function () { voucherHubWithAssetEligibilityManagerSigner = voucherHub.connect(assetEligibilityManager); voucherHubWithAnyoneSigner = voucherHub.connect(anyone); await voucherHubWithAssetEligibilityManagerSigner.createVoucherType(description, duration); + await iexecPocoInstance .transfer(await voucherHub.getAddress(), initVoucherHubBalance) .then((tx) => tx.wait()); @@ -315,18 +316,20 @@ describe('Voucher', function () { workerpoolprice: workerpoolPrice, }; let requestOrder = { ...mockOrder }; - let [voucherOwner1, requester]: SignerWithAddress[] = []; + let [voucherOwner1, requester, anyone]: SignerWithAddress[] = []; let voucherHub: VoucherHub; let voucher: Voucher; + let voucherWithOwnerSigner: Voucher; beforeEach(async () => { - ({ voucherHub, voucherOwner1, requester } = await loadFixture(deployFixture)); + ({ voucherHub, voucherOwner1, requester, anyone } = await loadFixture(deployFixture)); requestOrder.requester = requester.address; voucher = await voucherHubWithVoucherManagerSigner .createVoucher(voucherOwner1, voucherType, voucherValue) .then((tx) => tx.wait()) .then(() => voucherHub.getVoucher(voucherOwner1)) .then((voucherAddress) => commonUtils.getVoucher(voucherAddress)); + voucherWithOwnerSigner = voucher.connect(voucherOwner1); }); it('Should match orders with full sponsored amount', async () => { @@ -417,7 +420,7 @@ describe('Voucher', function () { const requesterInitialSrlcBalanceBefore = await getRequesterBalanceOnIexecPoco(); expect( - await voucher.matchOrdersBoost.staticCall( + await voucherWithOwnerSigner.matchOrdersBoost.staticCall( appOrder, datasetOrder, workerpoolOrder, @@ -425,7 +428,12 @@ describe('Voucher', function () { ), ).to.be.equal(dealId); await expect( - voucher.matchOrdersBoost(appOrder, datasetOrder, workerpoolOrder, requestOrder), + voucherWithOwnerSigner.matchOrdersBoost( + appOrder, + datasetOrder, + workerpoolOrder, + requestOrder, + ), ) .to.emit(voucher, 'OrdersBoostMatchedWithVoucher') .withArgs(dealId); @@ -453,7 +461,12 @@ describe('Voucher', function () { const requesterInitialSrlcBalance = await getRequesterBalanceOnIexecPoco(); await expect( - voucher.matchOrdersBoost(appOrder, datasetOrder, workerpoolOrder, requestOrder), + voucherWithOwnerSigner.matchOrdersBoost( + appOrder, + datasetOrder, + workerpoolOrder, + requestOrder, + ), ) .to.emit(iexecPocoInstance, 'Transfer') .withArgs(requester.address, await voucher.getAddress(), dealPrice) @@ -468,9 +481,38 @@ describe('Voucher', function () { expect(await voucher.getSponsoredAmount(dealId)).to.equal(0); }); + it('Should match orders boost with an authorized account', async () => { + for (const asset of [app, dataset, workerpool]) { + await voucherHubWithAssetEligibilityManagerSigner + .addEligibleAsset(voucherType, asset) + .then((x) => x.wait()); + } + const authorizationTx = await voucherWithOwnerSigner.authorizeAccount( + anyone.address, + ); + await authorizationTx.wait(); + + const voucherWithAnyoneSigner = voucher.connect(anyone); + await expect( + voucherWithAnyoneSigner.matchOrdersBoost( + appOrder, + datasetOrder, + workerpoolOrder, + requestOrder, + ), + ) + .to.emit(voucher, 'OrdersBoostMatchedWithVoucher') + .withArgs(dealId); + }); + it('Should not match orders boost when non-sponsored amount not transferable', async () => { await expect( - voucher.matchOrdersBoost(appOrder, datasetOrder, workerpoolOrder, requestOrder), + voucherWithOwnerSigner.matchOrdersBoost( + appOrder, + datasetOrder, + workerpoolOrder, + requestOrder, + ), ) .to.be.revertedWithCustomError(iexecPocoInstance, 'ERC20InsufficientAllowance') .withArgs(await voucher.getAddress(), 0, dealPrice); @@ -480,9 +522,8 @@ describe('Voucher', function () { await iexecPocoInstance .willRevertOnSponsorMatchOrdersBoost() .then((tx) => tx.wait()); - await expect( - voucher.matchOrdersBoost( + voucherWithOwnerSigner.matchOrdersBoost( { ...appOrder, appprice: 0 }, { ...datasetOrder, datasetprice: 0 }, { ...workerpoolOrder, workerpoolprice: 0 }, @@ -490,6 +531,32 @@ describe('Voucher', function () { ), ).to.be.revertedWith('IexecPocoMock: Failed to sponsorMatchOrdersBoost'); }); + it('Should not match orders boost when sender is not allowed', async () => { + const voucherWithAnyoneSigner = voucher.connect(anyone); + await expect( + voucherWithAnyoneSigner.matchOrdersBoost( + appOrder, + datasetOrder, + workerpoolOrder, + requestOrder, + ), + ).to.be.revertedWith('Voucher: sender is not authorized'); + }); + + it('Should not match orders boost when voucher is expired', async () => { + const expirationDate = (await voucher.getExpiration()) + BigInt(10); + await time.setNextBlockTimestamp(expirationDate); + // Mine empty block to get to voucher expiration time. + await mine(); + await expect( + voucherWithOwnerSigner.matchOrdersBoost( + appOrder, + datasetOrder, + workerpoolOrder, + requestOrder, + ), + ).to.be.revertedWith('Voucher: voucher is expired'); + }); }); }); });