From 5f5ee35d81c03c7c8e6913a25c528e54052e3ae9 Mon Sep 17 00:00:00 2001 From: Maxence Cornaton <maxence.cornaton@iex.ec> Date: Wed, 6 Mar 2024 16:54:56 +0100 Subject: [PATCH 1/5] Check result has been uploaded for TEE tasks --- CHANGELOG.md | 1 + .../core/replicate/ReplicatesService.java | 9 +-- .../core/replicate/ReplicateServiceTests.java | 57 +++++-------------- 3 files changed, 18 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0dec7965..2e9871a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file. ### Bug Fixes - Keep a single `updateReplicateStatus` method in `ReplicatesService`. (#670) +- Check result has been uploaded for TEE tasks. (#671) ## [[8.4.0]](https://github.com/iExecBlockchainComputing/iexec-core/releases/tag/v8.4.0) 2024-02-29 diff --git a/src/main/java/com/iexec/core/replicate/ReplicatesService.java b/src/main/java/com/iexec/core/replicate/ReplicatesService.java index 76584c96..515885ae 100644 --- a/src/main/java/com/iexec/core/replicate/ReplicatesService.java +++ b/src/main/java/com/iexec/core/replicate/ReplicatesService.java @@ -551,17 +551,12 @@ public boolean isResultUploaded(String chainTaskId) { } public boolean isResultUploaded(TaskDescription task) { - // Offchain computing - basic & tee + // Offchain computing - basic & TEE if (task.containsCallback()) { return true; } - // Cloud computing - tee - if (task.isTeeTask()) { - return true; // pushed from enclave - } - - // Cloud computing - basic + // Cloud computing - basic & TEE return resultService.isResultUploaded(task.getChainTaskId()); } diff --git a/src/test/java/com/iexec/core/replicate/ReplicateServiceTests.java b/src/test/java/com/iexec/core/replicate/ReplicateServiceTests.java index e19ee356..af8151b3 100644 --- a/src/test/java/com/iexec/core/replicate/ReplicateServiceTests.java +++ b/src/test/java/com/iexec/core/replicate/ReplicateServiceTests.java @@ -30,6 +30,8 @@ import io.vavr.control.Either; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.Mockito; @@ -796,12 +798,13 @@ void shouldGetReplicateWithResultUploadedStatus() { // region isResultUploaded - @Test - void shouldCheckResultServiceAndReturnTrue() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void shouldCheckResultServiceAndReturnTrue(boolean isTeeTask) { TaskDescription taskDescription = TaskDescription.builder() .chainTaskId(CHAIN_TASK_ID) .callback(BytesUtils.EMPTY_ADDRESS) - .isTeeTask(false) + .isTeeTask(isTeeTask) .build(); when(iexecHubService.getTaskDescription(CHAIN_TASK_ID)) .thenReturn(taskDescription); @@ -812,12 +815,13 @@ void shouldCheckResultServiceAndReturnTrue() { verify(resultService).isResultUploaded(CHAIN_TASK_ID); } - @Test - void shouldCheckResultServiceAndReturnFalse() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void shouldCheckResultServiceAndReturnFalse(boolean isTeeTask) { TaskDescription taskDescription = TaskDescription.builder() .chainTaskId(CHAIN_TASK_ID) .callback(BytesUtils.EMPTY_ADDRESS) - .isTeeTask(false) + .isTeeTask(isTeeTask) .build(); when(iexecHubService.getTaskDescription(CHAIN_TASK_ID)) .thenReturn(taskDescription); @@ -838,27 +842,13 @@ void shouldReturnFalseSinceTaskNotFound() { verify(resultService, never()).isResultUploaded(CHAIN_TASK_ID); } - @Test - void shouldReturnTrueForCallbackTask() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void shouldReturnTrueForCallbackTask(boolean isTeeTask) { TaskDescription taskDescription = TaskDescription.builder() .chainTaskId(CHAIN_TASK_ID) .callback("callback") - .isTeeTask(false) - .build(); - when(iexecHubService.getTaskDescription(CHAIN_TASK_ID)) - .thenReturn(taskDescription); - - boolean isResultUploaded = replicatesService.isResultUploaded(CHAIN_TASK_ID); - assertThat(isResultUploaded).isTrue(); - verify(resultService, never()).isResultUploaded(CHAIN_TASK_ID); - } - - @Test - void shouldReturnTrueForTeeTask() { - TaskDescription taskDescription = TaskDescription.builder() - .chainTaskId(CHAIN_TASK_ID) - .callback(BytesUtils.EMPTY_ADDRESS) - .isTeeTask(true) + .isTeeTask(isTeeTask) .build(); when(iexecHubService.getTaskDescription(CHAIN_TASK_ID)) .thenReturn(taskDescription); @@ -1017,24 +1007,6 @@ void shouldNotAuthorizeUpdateOnResultUploadFailedSinceResultUploadedWithCallback .isEqualTo(ReplicateStatusUpdateError.GENERIC_CANT_UPDATE); } - @Test - void shouldNotAuthorizeUpdateOnResultUploadFailedSinceResultUploadedWithTee() { - Replicate replicate = new Replicate(WALLET_WORKER_1, CHAIN_TASK_ID); - replicate.updateStatus(RESULT_UPLOADING, ReplicateStatusModifier.WORKER); - - ReplicateStatusUpdate statusUpdate = ReplicateStatusUpdate.builder() - .modifier(WORKER) - .status(RESULT_UPLOAD_FAILED) - .build(); - UpdateReplicateStatusArgs updateReplicateStatusArgs = UpdateReplicateStatusArgs - .builder() - .taskDescription(TaskDescription.builder().isTeeTask(true).build()) - .build(); - - assertThat(replicatesService.canUpdateReplicateStatus(replicate, statusUpdate, updateReplicateStatusArgs)) - .isEqualTo(ReplicateStatusUpdateError.GENERIC_CANT_UPDATE); - } - @Test void shouldNotAuthorizeUpdateOnContributedSinceNoBlockAvailable() { final Replicate replicate = new Replicate(WALLET_WORKER_1, CHAIN_TASK_ID); @@ -1229,6 +1201,7 @@ void shouldAuthorizeUpdateOnContributeAndFinalizeDone() { .thenReturn(true); when(iexecHubService.getTaskDescription(CHAIN_TASK_ID)).thenReturn(task); when(iexecHubService.isTaskInCompletedStatusOnChain(CHAIN_TASK_ID)).thenReturn(true); + when(resultService.isResultUploaded(CHAIN_TASK_ID)).thenReturn(true); assertThat(replicatesService.canUpdateReplicateStatus(replicate, statusUpdate, null)) .isEqualTo(ReplicateStatusUpdateError.NO_ERROR); From 6c31413fd14a63ca4d78cf5ce1406e29ba2bc81e Mon Sep 17 00:00:00 2001 From: Maxence Cornaton <maxence.cornaton@iex.ec> Date: Thu, 7 Mar 2024 10:51:30 +0100 Subject: [PATCH 2/5] Fix private storage case --- .../com/iexec/core/replicate/ReplicatesService.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/iexec/core/replicate/ReplicatesService.java b/src/main/java/com/iexec/core/replicate/ReplicatesService.java index 515885ae..81f6c7ea 100644 --- a/src/main/java/com/iexec/core/replicate/ReplicatesService.java +++ b/src/main/java/com/iexec/core/replicate/ReplicatesService.java @@ -48,6 +48,7 @@ import static com.iexec.common.replicate.ReplicateStatus.*; import static com.iexec.common.replicate.ReplicateStatusCause.REVEAL_TIMEOUT; +import static com.iexec.commons.poco.chain.DealParams.IPFS_RESULT_STORAGE_PROVIDER; @Slf4j @Service @@ -556,8 +557,13 @@ public boolean isResultUploaded(TaskDescription task) { return true; } - // Cloud computing - basic & TEE - return resultService.isResultUploaded(task.getChainTaskId()); + if (IPFS_RESULT_STORAGE_PROVIDER.equals(task.getResultStorageProvider())) { + // Cloud computing, upload to IPFS - basic & TEE + return resultService.isResultUploaded(task.getChainTaskId()); + } + + // Cloud computing, uploading to private storage + return true; } public boolean didReplicateContributeOnchain(String chainTaskId, String walletAddress) { From 1808b215900cf1fd05e0ed41f510e9f776b4146c Mon Sep 17 00:00:00 2001 From: Maxence Cornaton <maxence.cornaton@iex.ec> Date: Thu, 7 Mar 2024 10:51:56 +0100 Subject: [PATCH 3/5] Fix PR number --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e9871a1..f37b45d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ All notable changes to this project will be documented in this file. ### Bug Fixes - Keep a single `updateReplicateStatus` method in `ReplicatesService`. (#670) -- Check result has been uploaded for TEE tasks. (#671) +- Check result has been uploaded for TEE tasks. (#672) ## [[8.4.0]](https://github.com/iExecBlockchainComputing/iexec-core/releases/tag/v8.4.0) 2024-02-29 From e36fda70b5825892c7872947c9ab12d44c87802e Mon Sep 17 00:00:00 2001 From: Maxence Cornaton <maxence.cornaton@iex.ec> Date: Thu, 7 Mar 2024 11:04:57 +0100 Subject: [PATCH 4/5] Fix tests --- .../core/replicate/ReplicateServiceTests.java | 29 +++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/iexec/core/replicate/ReplicateServiceTests.java b/src/test/java/com/iexec/core/replicate/ReplicateServiceTests.java index af8151b3..7f487785 100644 --- a/src/test/java/com/iexec/core/replicate/ReplicateServiceTests.java +++ b/src/test/java/com/iexec/core/replicate/ReplicateServiceTests.java @@ -54,6 +54,8 @@ import static com.iexec.common.replicate.ReplicateStatus.*; import static com.iexec.common.replicate.ReplicateStatusModifier.WORKER; +import static com.iexec.commons.poco.chain.DealParams.DROPBOX_RESULT_STORAGE_PROVIDER; +import static com.iexec.commons.poco.chain.DealParams.IPFS_RESULT_STORAGE_PROVIDER; import static com.iexec.commons.poco.utils.TestUtils.*; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.*; @@ -805,6 +807,7 @@ void shouldCheckResultServiceAndReturnTrue(boolean isTeeTask) { .chainTaskId(CHAIN_TASK_ID) .callback(BytesUtils.EMPTY_ADDRESS) .isTeeTask(isTeeTask) + .resultStorageProvider(IPFS_RESULT_STORAGE_PROVIDER) .build(); when(iexecHubService.getTaskDescription(CHAIN_TASK_ID)) .thenReturn(taskDescription); @@ -822,6 +825,7 @@ void shouldCheckResultServiceAndReturnFalse(boolean isTeeTask) { .chainTaskId(CHAIN_TASK_ID) .callback(BytesUtils.EMPTY_ADDRESS) .isTeeTask(isTeeTask) + .resultStorageProvider(IPFS_RESULT_STORAGE_PROVIDER) .build(); when(iexecHubService.getTaskDescription(CHAIN_TASK_ID)) .thenReturn(taskDescription); @@ -857,6 +861,23 @@ void shouldReturnTrueForCallbackTask(boolean isTeeTask) { assertThat(isResultUploaded).isTrue(); verify(resultService, never()).isResultUploaded(CHAIN_TASK_ID); } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void shouldReturnTrueIfPrivateStorage(boolean isTeeTask) { + TaskDescription taskDescription = TaskDescription.builder() + .chainTaskId(CHAIN_TASK_ID) + .callback(BytesUtils.EMPTY_ADDRESS) + .isTeeTask(isTeeTask) + .resultStorageProvider(DROPBOX_RESULT_STORAGE_PROVIDER) + .build(); + when(iexecHubService.getTaskDescription(CHAIN_TASK_ID)) + .thenReturn(taskDescription); + + boolean isResultUploaded = replicatesService.isResultUploaded(CHAIN_TASK_ID); + assertThat(isResultUploaded).isTrue(); + verify(resultService, never()).isResultUploaded(any()); + } // endregion // didReplicateContributeOnchain @@ -982,7 +1003,7 @@ void shouldAuthorizeUpdateOnResultUploadFailed() { .build(); UpdateReplicateStatusArgs updateReplicateStatusArgs = UpdateReplicateStatusArgs .builder() - .taskDescription(TaskDescription.builder().build()) + .taskDescription(TaskDescription.builder().resultStorageProvider(IPFS_RESULT_STORAGE_PROVIDER).build()) .build(); assertThat(replicatesService.canUpdateReplicateStatus(replicate, statusUpdate, updateReplicateStatusArgs)) @@ -1234,7 +1255,11 @@ void shouldNotAuthorizeUpdateOnContributeAndFinalizeDoneWhenNotUploaded() { .modifier(WORKER) .status(CONTRIBUTE_AND_FINALIZE_DONE) .build(); - final TaskDescription task = TaskDescription.builder().chainTaskId(CHAIN_TASK_ID).build(); + final TaskDescription task = TaskDescription + .builder() + .chainTaskId(CHAIN_TASK_ID) + .resultStorageProvider(IPFS_RESULT_STORAGE_PROVIDER) + .build(); when(iexecHubService.repeatIsRevealedTrue(CHAIN_TASK_ID, WALLET_WORKER_1)) .thenReturn(true); From abeb8241d4d97597d78960f1b3d06eb840fc1250 Mon Sep 17 00:00:00 2001 From: Jeremy Bernard <jeremy.bernard@iex.ec> Date: Fri, 8 Mar 2024 11:10:56 +0100 Subject: [PATCH 5/5] Only enable TEE tasks result upload check when storage is IPFS --- .../iexec/core/replicate/ReplicatesService.java | 15 ++++++--------- .../core/replicate/ReplicateServiceTests.java | 7 +++---- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/main/java/com/iexec/core/replicate/ReplicatesService.java b/src/main/java/com/iexec/core/replicate/ReplicatesService.java index 81f6c7ea..dcbe19bd 100644 --- a/src/main/java/com/iexec/core/replicate/ReplicatesService.java +++ b/src/main/java/com/iexec/core/replicate/ReplicatesService.java @@ -552,18 +552,15 @@ public boolean isResultUploaded(String chainTaskId) { } public boolean isResultUploaded(TaskDescription task) { - // Offchain computing - basic & TEE - if (task.containsCallback()) { - return true; - } + final boolean hasIpfsStorageProvider = IPFS_RESULT_STORAGE_PROVIDER.equals(task.getResultStorageProvider()); - if (IPFS_RESULT_STORAGE_PROVIDER.equals(task.getResultStorageProvider())) { - // Cloud computing, upload to IPFS - basic & TEE - return resultService.isResultUploaded(task.getChainTaskId()); + // Offchain computing or TEE task with private storage + if (task.containsCallback() || (task.isTeeTask() && !hasIpfsStorageProvider)) { + return true; } - // Cloud computing, uploading to private storage - return true; + // Cloud computing, upload to IPFS - basic & TEE + return resultService.isResultUploaded(task.getChainTaskId()); } public boolean didReplicateContributeOnchain(String chainTaskId, String walletAddress) { diff --git a/src/test/java/com/iexec/core/replicate/ReplicateServiceTests.java b/src/test/java/com/iexec/core/replicate/ReplicateServiceTests.java index 7f487785..43502292 100644 --- a/src/test/java/com/iexec/core/replicate/ReplicateServiceTests.java +++ b/src/test/java/com/iexec/core/replicate/ReplicateServiceTests.java @@ -862,13 +862,12 @@ void shouldReturnTrueForCallbackTask(boolean isTeeTask) { verify(resultService, never()).isResultUploaded(CHAIN_TASK_ID); } - @ParameterizedTest - @ValueSource(booleans = {true, false}) - void shouldReturnTrueIfPrivateStorage(boolean isTeeTask) { + @Test + void shouldReturnTrueIfPrivateStorageForTeeTask() { TaskDescription taskDescription = TaskDescription.builder() .chainTaskId(CHAIN_TASK_ID) .callback(BytesUtils.EMPTY_ADDRESS) - .isTeeTask(isTeeTask) + .isTeeTask(true) .resultStorageProvider(DROPBOX_RESULT_STORAGE_PROVIDER) .build(); when(iexecHubService.getTaskDescription(CHAIN_TASK_ID))