refactor: update isDatasetCompatibleWithDeal function to return detailed results

Author: Le-Caignec

contracts/facets/IexecPoco1Facet.sol

@@ -57,99 +57,55 @@ contract IexecPoco1Facet is IexecPoco1, FacetBase, IexecEscrow, SignatureVerifie
      *
      * @param datasetOrder The dataset order to verify
      * @param dealid The deal ID to check against
-     * @return true if the dataset order is compatible with the deal, false otherwise
+     * @return result true if the dataset order is compatible with the deal, false otherwise
+     * @return reason the specific reason why the compatibility check failed, empty string if successful
      */
     function isDatasetCompatibleWithDeal(
         IexecLibOrders_v5.DatasetOrder calldata datasetOrder,
         bytes32 dealid
-    ) external view override returns (bool) {
+    ) external view override returns (bool result, string memory reason) {
         PocoStorageLib.PocoStorage storage $ = PocoStorageLib.getPocoStorage();
-
-        // Track status of all checks
-        bool dealExists;
-        bool dealHasNoDataset;
-        bool signatureValid;
-        bool notFullyConsumed;
-        bool appRestrictionValid;
-        bool workerpoolRestrictionValid;
-        bool requesterRestrictionValid;
-        bool tagCompatible;
-
         // Check if deal exists
         IexecLibCore_v5.Deal storage deal = $.m_deals[dealid];
-        dealExists = (deal.requester != address(0));
-
+        if (deal.requester == address(0)) {
+            return (false, "Deal does not exist");
+        }
         // The specified deal should not have a dataset.
-        dealHasNoDataset = (deal.dataset.pointer == address(0));
-
+        if (deal.dataset.pointer != address(0)) {
+            return (false, "Deal already has a dataset");
+        }
         // Check dataset order owner signature (including presign and EIP1271)
         bytes32 datasetOrderHash = _toTypedDataHash(datasetOrder.hash());
         address datasetOwner = IERC5313(datasetOrder.dataset).owner();
-        signatureValid = _verifySignatureOrPresignature(
-            datasetOwner,
-            datasetOrderHash,
-            datasetOrder.sign
-        );
-
+        if (!_verifySignatureOrPresignature(datasetOwner, datasetOrderHash, datasetOrder.sign)) {
+            return (false, "Invalid dataset order signature");
+        }
         // Check if dataset order is not fully consumed
-        notFullyConsumed = ($.m_consumed[datasetOrderHash] < datasetOrder.volume);
-
+        if ($.m_consumed[datasetOrderHash] >= datasetOrder.volume) {
+            return (false, "Dataset order is fully consumed");
+        }
         // Check if deal app is allowed by dataset order apprestrict (including whitelist)
-        appRestrictionValid = _isAccountAuthorizedByRestriction(
-            datasetOrder.apprestrict,
-            deal.app.pointer
-        );
-
+        if (!_isAccountAuthorizedByRestriction(datasetOrder.apprestrict, deal.app.pointer)) {
+            return (false, "App restriction not satisfied");
+        }
         // Check if deal workerpool is allowed by dataset order workerpoolrestrict (including whitelist)
-        workerpoolRestrictionValid = _isAccountAuthorizedByRestriction(
-            datasetOrder.workerpoolrestrict,
-            deal.workerpool.pointer
-        );
-
+        if (
+            !_isAccountAuthorizedByRestriction(
+                datasetOrder.workerpoolrestrict,
+                deal.workerpool.pointer
+            )
+        ) {
+            return (false, "Workerpool restriction not satisfied");
+        }
         // Check if deal requester is allowed by dataset order requesterrestrict (including whitelist)
-        requesterRestrictionValid = _isAccountAuthorizedByRestriction(
-            datasetOrder.requesterrestrict,
-            deal.requester
-        );
-
+        if (!_isAccountAuthorizedByRestriction(datasetOrder.requesterrestrict, deal.requester)) {
+            return (false, "Requester restriction not satisfied");
+        }
         // Check if deal tag fulfills all the tag bits of the dataset order
-        tagCompatible = ((deal.tag & datasetOrder.tag) == datasetOrder.tag);
-
-        // Check if all conditions are met
-        bool allChecksPassed = dealExists &&
-            dealHasNoDataset &&
-            signatureValid &&
-            notFullyConsumed &&
-            appRestrictionValid &&
-            workerpoolRestrictionValid &&
-            requesterRestrictionValid &&
-            tagCompatible;
-
-        if (!allChecksPassed) {
-            string memory status = string(
-                abi.encodePacked(
-                    "DatasetCompatibilityCheck: ",
-                    dealExists ? "1" : "0",
-                    "-",
-                    dealHasNoDataset ? "1" : "0",
-                    "-",
-                    signatureValid ? "1" : "0",
-                    "-",
-                    notFullyConsumed ? "1" : "0",
-                    "-",
-                    appRestrictionValid ? "1" : "0",
-                    "-",
-                    workerpoolRestrictionValid ? "1" : "0",
-                    "-",
-                    requesterRestrictionValid ? "1" : "0",
-                    "-",
-                    tagCompatible ? "1" : "0"
-                )
-            );
-            revert(status);
+        if ((deal.tag & datasetOrder.tag) != datasetOrder.tag) {
+            return (false, "Tag compatibility not satisfied");
         }
-
-        return true;
+        return (true, "");
     }
 
     /***************************************************************************

contracts/interfaces/IexecPoco1.sol

@@ -50,5 +50,5 @@ interface IexecPoco1 {
     function isDatasetCompatibleWithDeal(
         IexecLibOrders_v5.DatasetOrder calldata datasetOrder,
         bytes32 dealid
-    ) external view returns (bool);
+    ) external view returns (bool result, string memory reason);
 }

contracts/interfaces/IexecPoco1.v8.sol

@@ -55,5 +55,5 @@ interface IexecPoco1 {
     function isDatasetCompatibleWithDeal(
         IexecLibOrders_v5.DatasetOrder calldata datasetOrder,
         bytes32 dealid
-    ) external view returns (bool);
+    ) external view returns (bool result, string memory reason);
 }

test/byContract/IexecPoco/IexecPoco1.test.ts

@@ -1112,22 +1112,25 @@ describe('IexecPoco1', () => {
         });
 
         it('Should return true for compatible dataset order', async () => {
-            expect(
-                await iexecPoco.isDatasetCompatibleWithDeal(
-                    compatibleDatasetOrder,
-                    dealIdWithoutDataset,
-                ),
-            ).to.be.true;
+            const [result, reason] = await iexecPoco.isDatasetCompatibleWithDeal(
+                compatibleDatasetOrder,
+                dealIdWithoutDataset,
+            );
+            expect(result).to.be.true;
+            expect(reason).to.equal('');
         });
 
-        it('Should revert with status for non-existent deal', async () => {
+        it('Should return false with reason for non-existent deal', async () => {
             const nonExistentDealId = ethers.keccak256(ethers.toUtf8Bytes('non-existent-deal'));
-            await expect(
-                iexecPoco.isDatasetCompatibleWithDeal(compatibleDatasetOrder, nonExistentDealId),
-            ).to.be.revertedWith('DatasetCompatibilityCheck: 0-1-1-1-0-0-0-0');
+            const [result, reason] = await iexecPoco.isDatasetCompatibleWithDeal(
+                compatibleDatasetOrder,
+                nonExistentDealId,
+            );
+            expect(result).to.be.false;
+            expect(reason).to.equal('Deal does not exist');
         });
 
-        it('Should revert with status for deal with a dataset', async () => {
+        it('Should return false with reason for deal with a dataset', async () => {
             // Use the original orders that include a dataset to create a deal with dataset
             const ordersWithDataset = buildOrders({
                 assets: ordersAssets, // This includes the dataset
@@ -1157,56 +1160,62 @@ describe('IexecPoco1', () => {
             );
             await iexecPocoAsRequester.matchOrders(...ordersWithDataset.toArray());
 
-            await expect(
-                iexecPoco.isDatasetCompatibleWithDeal(compatibleDatasetOrder, dealIdWithDataset),
-            ).to.be.revertedWith('DatasetCompatibilityCheck: 1-0-1-1-1-1-1-1');
+            const [result, reason] = await iexecPoco.isDatasetCompatibleWithDeal(
+                compatibleDatasetOrder,
+                dealIdWithDataset,
+            );
+            expect(result).to.be.false;
+            expect(reason).to.equal('Deal already has a dataset');
         });
 
-        it('Should revert with status for dataset order with invalid signature', async () => {
+        it('Should return false with reason for dataset order with invalid signature', async () => {
             // Create dataset order with invalid signature
             const invalidSignatureDatasetOrder = {
                 ...compatibleDatasetOrder,
                 sign: randomSignature, // Invalid signature
             };
 
-            await expect(
-                iexecPoco.isDatasetCompatibleWithDeal(
-                    invalidSignatureDatasetOrder,
-                    dealIdWithoutDataset,
-                ),
-            ).to.be.revertedWith('DatasetCompatibilityCheck: 1-1-0-1-1-1-1-1');
+            const [result, reason] = await iexecPoco.isDatasetCompatibleWithDeal(
+                invalidSignatureDatasetOrder,
+                dealIdWithoutDataset,
+            );
+            expect(result).to.be.false;
+            expect(reason).to.equal('Invalid dataset order signature');
         });
 
-        it('Should revert with status for fully consumed dataset order', async () => {
+        it('Should return false with reason for fully consumed dataset order', async () => {
             // Create dataset order with volume 0 (fully consumed)
             const consumedDatasetOrder = {
                 ...compatibleDatasetOrder,
                 volume: 0n,
             };
             await signOrder(iexecWrapper.getDomain(), consumedDatasetOrder, datasetProvider);
 
-            await expect(
-                iexecPoco.isDatasetCompatibleWithDeal(consumedDatasetOrder, dealIdWithoutDataset),
-            ).to.be.revertedWith('DatasetCompatibilityCheck: 1-1-1-0-1-1-1-1');
+            const [result, reason] = await iexecPoco.isDatasetCompatibleWithDeal(
+                consumedDatasetOrder,
+                dealIdWithoutDataset,
+            );
+            expect(result).to.be.false;
+            expect(reason).to.equal('Dataset order is fully consumed');
         });
 
-        it('Should revert with status for dataset order with incompatible app restriction', async () => {
+        it('Should return false with reason for dataset order with incompatible app restriction', async () => {
             // Create dataset order with incompatible app restriction
             const incompatibleAppDatasetOrder = {
                 ...compatibleDatasetOrder,
                 apprestrict: randomAddress, // Different app restriction
             };
             await signOrder(iexecWrapper.getDomain(), incompatibleAppDatasetOrder, datasetProvider);
 
-            await expect(
-                iexecPoco.isDatasetCompatibleWithDeal(
-                    incompatibleAppDatasetOrder,
-                    dealIdWithoutDataset,
-                ),
-            ).to.be.revertedWith('DatasetCompatibilityCheck: 1-1-1-1-0-1-1-1');
+            const [result, reason] = await iexecPoco.isDatasetCompatibleWithDeal(
+                incompatibleAppDatasetOrder,
+                dealIdWithoutDataset,
+            );
+            expect(result).to.be.false;
+            expect(reason).to.equal('App restriction not satisfied');
         });
 
-        it('Should revert with status for dataset order with incompatible workerpool restriction', async () => {
+        it('Should return false with reason for dataset order with incompatible workerpool restriction', async () => {
             // Create dataset order with incompatible workerpool restriction
             const incompatibleWorkerpoolDatasetOrder = {
                 ...compatibleDatasetOrder,
@@ -1218,15 +1227,15 @@ describe('IexecPoco1', () => {
                 datasetProvider,
             );
 
-            await expect(
-                iexecPoco.isDatasetCompatibleWithDeal(
-                    incompatibleWorkerpoolDatasetOrder,
-                    dealIdWithoutDataset,
-                ),
-            ).to.be.revertedWith('DatasetCompatibilityCheck: 1-1-1-1-1-0-1-1');
+            const [result, reason] = await iexecPoco.isDatasetCompatibleWithDeal(
+                incompatibleWorkerpoolDatasetOrder,
+                dealIdWithoutDataset,
+            );
+            expect(result).to.be.false;
+            expect(reason).to.equal('Workerpool restriction not satisfied');
         });
 
-        it('Should revert with status for dataset order with incompatible requester restriction', async () => {
+        it('Should return false with reason for dataset order with incompatible requester restriction', async () => {
             // Create dataset order with incompatible requester restriction
             const incompatibleRequesterDatasetOrder = {
                 ...compatibleDatasetOrder,
@@ -1238,28 +1247,28 @@ describe('IexecPoco1', () => {
                 datasetProvider,
             );
 
-            await expect(
-                iexecPoco.isDatasetCompatibleWithDeal(
-                    incompatibleRequesterDatasetOrder,
-                    dealIdWithoutDataset,
-                ),
-            ).to.be.revertedWith('DatasetCompatibilityCheck: 1-1-1-1-1-1-0-1');
+            const [result, reason] = await iexecPoco.isDatasetCompatibleWithDeal(
+                incompatibleRequesterDatasetOrder,
+                dealIdWithoutDataset,
+            );
+            expect(result).to.be.false;
+            expect(reason).to.equal('Requester restriction not satisfied');
         });
 
-        it('Should revert with status for dataset order with incompatible tag', async () => {
+        it('Should return false with reason for dataset order with incompatible tag', async () => {
             // Create dataset order with incompatible tag
             const incompatibleTagDatasetOrder = {
                 ...compatibleDatasetOrder,
                 tag: '0x0000000000000000000000000000000000000000000000000000000000000002', // Different tag
             };
             await signOrder(iexecWrapper.getDomain(), incompatibleTagDatasetOrder, datasetProvider);
 
-            await expect(
-                iexecPoco.isDatasetCompatibleWithDeal(
-                    incompatibleTagDatasetOrder,
-                    dealIdWithoutDataset,
-                ),
-            ).to.be.revertedWith('DatasetCompatibilityCheck: 1-1-1-1-1-1-1-0');
+            const [result, reason] = await iexecPoco.isDatasetCompatibleWithDeal(
+                incompatibleTagDatasetOrder,
+                dealIdWithoutDataset,
+            );
+            expect(result).to.be.false;
+            expect(reason).to.equal('Tag compatibility not satisfied');
         });
     });