Skip to content

Commit 0ddd62d

Browse files
jbern0rdjbern0rd
authored
fix: verify conditions before querying blockchain adapter for finalization (#757)
1 parent 5e873b9 commit 0ddd62d

File tree

2 files changed

+67
-23
lines changed

2 files changed

+67
-23
lines changed

‎src/main/java/com/iexec/core/task/update/TaskUpdateManager.java‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import org.springframework.data.mongodb.core.query.Update;
4141
import org.springframework.stereotype.Service;
4242

43+
import java.time.Instant;
4344
import java.util.ArrayList;
4445
import java.util.Date;
4546
import java.util.List;
@@ -667,12 +668,21 @@ void resultUploaded2Finalizing(final ChainTask chainTask, final Task task) {
667668
return;
668669
}
669670

671+
final long now = Instant.now().toEpochMilli();
672+
final boolean isNotRevealingOnChain = chainTask.getStatus() != ChainTaskStatus.REVEALING;
673+
final boolean isAfterFinalDeadline = chainTask.getFinalDeadline() < now;
674+
// We have enough reveals on-chain when:
675+
// 'all winners have revealed' OR 'at least one winner has revealed when the reveal deadline has been reached'
676+
// The inverted condition is:
677+
// 'not all winners have revealed' AND 'the reveal deadline has not been reached, or it has and no winner has successfully revealed'
678+
final boolean hasNotEnoughRevealsOnChain = chainTask.getWinnerCounter() != chainTask.getRevealCounter()
679+
&& (chainTask.getRevealCounter() == 0 || now < chainTask.getRevealDeadline());
670680
final int onChainReveal = chainTask.getRevealCounter();
671681
final int offChainReveal = replicatesService.getNbReplicatesContainingStatus(task.getChainTaskId(), ReplicateStatus.REVEALED);
672682

673-
if (onChainReveal != offChainReveal) {
674-
log.debug("Cannot finalize, mismatch between on-chain and off-chain data [chainTaskId:{}, onChainReveal:{}, offChainReveal:{}]",
675-
task.getChainTaskId(), onChainReveal, offChainReveal);
683+
if (isNotRevealingOnChain || isAfterFinalDeadline || hasNotEnoughRevealsOnChain || onChainReveal != offChainReveal) {
684+
log.info("Cannot finalize, mismatch between on-chain and off-chain data [chainTaskId:{}, not-revealing-on-chain:{}, after-final-deadline:{}, not-enough-reveals:{}, onChainReveal:{}, offChainReveal:{}]",
685+
task.getChainTaskId(), isNotRevealingOnChain, isAfterFinalDeadline, hasNotEnoughRevealsOnChain, onChainReveal, offChainReveal);
676686
return;
677687
}
678688

‎src/test/java/com/iexec/core/task/update/TaskUpdateManagerTests.java‎

Lines changed: 54 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@
4545
import org.junit.jupiter.api.BeforeEach;
4646
import org.junit.jupiter.api.Test;
4747
import org.junit.jupiter.api.extension.ExtendWith;
48+
import org.junit.jupiter.params.ParameterizedTest;
49+
import org.junit.jupiter.params.provider.Arguments;
50+
import org.junit.jupiter.params.provider.MethodSource;
4851
import org.mockito.ArgumentCaptor;
4952
import org.mockito.Captor;
5053
import org.mockito.Mock;
@@ -71,8 +74,10 @@
7174
import java.util.Date;
7275
import java.util.List;
7376
import java.util.Optional;
77+
import java.util.stream.Stream;
7478

7579
import static com.iexec.common.replicate.ReplicateStatus.RESULT_UPLOAD_REQUESTED;
80+
import static com.iexec.common.replicate.ReplicateStatus.REVEALED;
7681
import static com.iexec.core.TestUtils.*;
7782
import static com.iexec.core.task.TaskStatus.*;
7883
import static org.assertj.core.api.Assertions.assertThat;
@@ -85,6 +90,8 @@
8590
@ExtendWith(MockitoExtension.class)
8691
@ExtendWith(OutputCaptureExtension.class)
8792
class TaskUpdateManagerTests {
93+
private static final long ACTIVE_DEADLINE = Instant.now().plus(5, ChronoUnit.MINUTES).toEpochMilli();
94+
private static final long FAILED_DEADLINE = Instant.now().minus(5, ChronoUnit.MINUTES).toEpochMilli();
8895
private static final String SMS_URL = "smsUrl";
8996
private static final String ERROR_MSG = "Cannot update task [chainTaskId:%s, currentStatus:%s, expectedStatus:%s, method:%s]";
9097

@@ -143,6 +150,13 @@ void afterEach() {
143150
Metrics.globalRegistry.clear();
144151
}
145152

153+
private final ChainTask finalizableTask = ChainTask.builder()
154+
.status(ChainTaskStatus.REVEALING)
155+
.finalDeadline(ACTIVE_DEADLINE)
156+
.winnerCounter(3)
157+
.revealCounter(3)
158+
.build();
159+
146160
// region consensusReached2Reopening
147161

148162
@Test
@@ -1051,19 +1065,18 @@ void shouldUpdateResultUploading2Uploaded2Finalizing2Finalized() { //one worker
10511065
final Task task = getStubTask(RESULT_UPLOADING);
10521066
taskRepository.save(task);
10531067

1054-
final ChainTask chainTask = ChainTask.builder().revealCounter(1).build();
10551068
final Replicate replicate = new Replicate(WALLET_WORKER_1, CHAIN_TASK_ID);
10561069
replicate.setResultLink(RESULT_LINK);
10571070

1071+
when(iexecHubService.getChainTask(CHAIN_TASK_ID))
1072+
.thenReturn(Optional.of(finalizableTask))
1073+
.thenReturn(Optional.of(ChainTask.builder()
1074+
.status(ChainTaskStatus.COMPLETED)
1075+
.build()));
10581076
when(replicatesService.getReplicateWithResultUploadedStatus(CHAIN_TASK_ID)).thenReturn(Optional.of(replicate));
1059-
when(replicatesService.getNbReplicatesContainingStatus(CHAIN_TASK_ID, ReplicateStatus.REVEALED)).thenReturn(1);
1060-
when(iexecHubService.getChainTask(any())).thenReturn(Optional.of(chainTask));
1077+
when(replicatesService.getNbReplicatesContainingStatus(CHAIN_TASK_ID, ReplicateStatus.REVEALED)).thenReturn(finalizableTask.getRevealCounter());
10611078
when(blockchainAdapterService.requestFinalize(any(), any(), any())).thenReturn(Optional.of(CHAIN_TASK_ID));
10621079
when(blockchainAdapterService.isFinalized(any())).thenReturn(Optional.of(true));
1063-
when(iexecHubService.getChainTask(CHAIN_TASK_ID)).thenReturn(Optional.of(ChainTask.builder()
1064-
.status(ChainTaskStatus.COMPLETED)
1065-
.revealCounter(1)
1066-
.build()));
10671080

10681081
taskUpdateManager.updateTask(task.getChainTaskId());
10691082

@@ -1080,12 +1093,9 @@ void shouldUpdateResultUploading2Uploaded2Finalizing2FinalizeFail() { //one work
10801093
replicate.updateStatus(ReplicateStatus.RESULT_UPLOADED, ReplicateStatusModifier.POOL_MANAGER);
10811094

10821095
when(replicatesService.getReplicateWithResultUploadedStatus(task.getChainTaskId())).thenReturn(Optional.of(replicate));
1083-
when(replicatesService.getNbReplicatesContainingStatus(CHAIN_TASK_ID, ReplicateStatus.REVEALED)).thenReturn(1);
1096+
when(iexecHubService.getChainTask(CHAIN_TASK_ID)).thenReturn(Optional.of(finalizableTask));
1097+
when(replicatesService.getNbReplicatesContainingStatus(CHAIN_TASK_ID, ReplicateStatus.REVEALED)).thenReturn(finalizableTask.getRevealCounter());
10841098
when(blockchainAdapterService.requestFinalize(any(), any(), any())).thenReturn(Optional.empty());
1085-
when(iexecHubService.getChainTask(CHAIN_TASK_ID)).thenReturn(Optional.of(ChainTask.builder()
1086-
.status(ChainTaskStatus.FAILED)
1087-
.revealCounter(1)
1088-
.build()));
10891099

10901100
taskUpdateManager.updateTask(task.getChainTaskId());
10911101

@@ -1223,12 +1233,11 @@ void shouldUpdateResultUploaded2Finalizing() {
12231233
final Task task = getStubTask(RESULT_UPLOADED);
12241234
taskRepository.save(task);
12251235

1226-
final ChainTask chainTask = ChainTask.builder().revealCounter(1).build();
12271236
final Replicate replicate = new Replicate(WALLET_WORKER_1, CHAIN_TASK_ID);
12281237
replicate.setResultLink(RESULT_LINK);
12291238

1230-
when(iexecHubService.getChainTask(CHAIN_TASK_ID)).thenReturn(Optional.of(chainTask));
1231-
when(replicatesService.getNbReplicatesContainingStatus(CHAIN_TASK_ID, ReplicateStatus.REVEALED)).thenReturn(1);
1239+
when(iexecHubService.getChainTask(CHAIN_TASK_ID)).thenReturn(Optional.of(finalizableTask));
1240+
when(replicatesService.getNbReplicatesContainingStatus(CHAIN_TASK_ID, ReplicateStatus.REVEALED)).thenReturn(finalizableTask.getRevealCounter());
12321241
when(blockchainAdapterService.requestFinalize(any(), any(), any())).thenReturn(Optional.of(CHAIN_TASK_ID));
12331242

12341243
when(blockchainAdapterService.isFinalized(any())).thenReturn(Optional.of(true));
@@ -1249,16 +1258,42 @@ void shouldNotUpdateResultUploaded2FinalizingSinceNotResultUploaded(CapturedOutp
12491258
assertThat(output.getOut()).contains(String.format(ERROR_MSG, CHAIN_TASK_ID, RESULT_UPLOADING, RESULT_UPLOADED, "resultUploaded2Finalizing"));
12501259
}
12511260

1261+
@ParameterizedTest
1262+
@MethodSource("provideInvalidChainTask")
1263+
void shouldNotUpdateResultUploaded2FinalizingSinceCantFinalize(final ChainTask chainTask) {
1264+
final Task task = getStubTask(RESULT_UPLOADED);
1265+
taskRepository.save(task);
1266+
1267+
final Replicate replicate = new Replicate(WALLET_WORKER_1, CHAIN_TASK_ID);
1268+
replicate.setResultLink(RESULT_LINK);
1269+
1270+
when(iexecHubService.getChainTask(CHAIN_TASK_ID)).thenReturn(Optional.of(chainTask));
1271+
when(replicatesService.getNbReplicatesContainingStatus(CHAIN_TASK_ID, REVEALED)).thenReturn(chainTask.getRevealCounter());
1272+
1273+
taskUpdateManager.updateTask(task.getChainTaskId());
1274+
1275+
final Task resultTask = taskRepository.findByChainTaskId(CHAIN_TASK_ID).orElseThrow();
1276+
assertThat(resultTask.getCurrentStatus()).isEqualTo(RESULT_UPLOADED);
1277+
}
1278+
1279+
private static Stream<Arguments> provideInvalidChainTask() {
1280+
return Stream.of(
1281+
Arguments.of(ChainTask.builder().status(ChainTaskStatus.ACTIVE).finalDeadline(ACTIVE_DEADLINE).build()), // not REVEALING
1282+
Arguments.of(ChainTask.builder().status(ChainTaskStatus.REVEALING).finalDeadline(FAILED_DEADLINE).build()), // after final deadline
1283+
Arguments.of(ChainTask.builder().status(ChainTaskStatus.REVEALING).revealDeadline(ACTIVE_DEADLINE).finalDeadline(ACTIVE_DEADLINE).winnerCounter(3).revealCounter(1).build()), // before reveal deadline and not all winners have revealed
1284+
Arguments.of(ChainTask.builder().status(ChainTaskStatus.REVEALING).revealDeadline(FAILED_DEADLINE).finalDeadline(ACTIVE_DEADLINE).winnerCounter(3).revealCounter(0).build()) // after reveal deadline and no winner has revealed
1285+
);
1286+
}
1287+
12521288
@Test
12531289
void shouldNotUpdateResultUploaded2FinalizingSinceRevealsNotEqual() {
12541290
final Task task = getStubTask(RESULT_UPLOADED);
12551291
taskRepository.save(task);
12561292

1257-
final ChainTask chainTask = ChainTask.builder().revealCounter(1).build();
12581293
final Replicate replicate = new Replicate(WALLET_WORKER_1, CHAIN_TASK_ID);
12591294
replicate.setResultLink(RESULT_LINK);
12601295

1261-
when(iexecHubService.getChainTask(CHAIN_TASK_ID)).thenReturn(Optional.of(chainTask));
1296+
when(iexecHubService.getChainTask(CHAIN_TASK_ID)).thenReturn(Optional.of(finalizableTask));
12621297
when(replicatesService.getNbReplicatesContainingStatus(CHAIN_TASK_ID, ReplicateStatus.REVEALED)).thenReturn(2);
12631298

12641299
taskUpdateManager.updateTask(task.getChainTaskId());
@@ -1272,12 +1307,11 @@ void shouldNotUpdateResultUploaded2FinalizingSinceCantRequestFinalize() {
12721307
final Task task = getStubTask(RESULT_UPLOADED);
12731308
taskRepository.save(task);
12741309

1275-
final ChainTask chainTask = ChainTask.builder().revealCounter(1).build();
12761310
final Replicate replicate = new Replicate(WALLET_WORKER_1, CHAIN_TASK_ID);
12771311
replicate.setResultLink(RESULT_LINK);
12781312

1279-
when(iexecHubService.getChainTask(CHAIN_TASK_ID)).thenReturn(Optional.of(chainTask));
1280-
when(replicatesService.getNbReplicatesContainingStatus(CHAIN_TASK_ID, ReplicateStatus.REVEALED)).thenReturn(1);
1313+
when(iexecHubService.getChainTask(CHAIN_TASK_ID)).thenReturn(Optional.of(finalizableTask));
1314+
when(replicatesService.getNbReplicatesContainingStatus(CHAIN_TASK_ID, ReplicateStatus.REVEALED)).thenReturn(finalizableTask.getRevealCounter());
12811315
when(blockchainAdapterService.requestFinalize(eq(CHAIN_TASK_ID), any(), any())).thenReturn(Optional.empty());
12821316

12831317
taskUpdateManager.updateTask(task.getChainTaskId());

0 commit comments

Comments
 (0)