Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add Held status to exState status #457

Merged
merged 1 commit into from
Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/core/src/internal/batcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export class Batcher {
switch (executionState.status) {
case ExecutionStatus.FAILED:
case ExecutionStatus.TIMEOUT:
case ExecutionStatus.HELD:
case ExecutionStatus.STARTED:
return [f.id, VisitStatus.UNVISITED];
case ExecutionStatus.SUCCESS:
Expand Down
23 changes: 22 additions & 1 deletion packages/core/src/internal/deployer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,26 @@ export class Deployer {
successful: Object.values(deploymentState.executionStates)
.filter((ex) => ex.status === ExecutionStatus.SUCCESS)
.map((ex) => ex.id),
held: Object.values(deploymentState.executionStates)
.filter(canFail)
.filter((ex) => ex.status === ExecutionStatus.HELD)
.map((ex) => {
assertIgnitionInvariant(
ex.result !== undefined,
`Execution state ${ex.id} is marked as held but has no result`
);

assertIgnitionInvariant(
ex.result.type === ExecutionResultType.STRATEGY_HELD,
`Execution state ${ex.id} is marked as held but has ${ex.result.type} instead of a held result`
);

return {
futureId: ex.id,
heldId: ex.result.heldId,
reason: ex.result.reason,
};
}),
timedOut: Object.values(deploymentState.executionStates)
.filter(canTimeout)
.filter((ex) => ex.status === ExecutionStatus.TIMEOUT)
Expand All @@ -304,7 +324,8 @@ export class Deployer {
.map((ex) => {
assertIgnitionInvariant(
ex.result !== undefined &&
ex.result.type !== ExecutionResultType.SUCCESS,
ex.result.type !== ExecutionResultType.SUCCESS &&
ex.result.type !== ExecutionResultType.STRATEGY_HELD,
`Execution state ${ex.id} is marked as failed but has no error result`
);

Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/internal/execution/execution-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
import { DeploymentLoader } from "../deployment-loader/types";
import { getFuturesFromModule } from "../utils/get-futures-from-module";
import { getPendingNonceAndSender } from "../views/execution-state/get-pending-nonce-and-sender";
import { hasExecutionFailed } from "../views/has-execution-failed";
import { hasExecutionSucceeded } from "../views/has-execution-succeeded";
import { isBatchFinished } from "../views/is-batch-finished";

import { applyNewMessage } from "./deployment-state-helpers";
Expand Down Expand Up @@ -108,7 +108,9 @@ export class ExecutionEngine {
deploymentState
);

if (executionBatch.some((f) => hasExecutionFailed(f, deploymentState))) {
if (
!executionBatch.every((f) => hasExecutionSucceeded(f, deploymentState))
) {
return deploymentState;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,5 +72,7 @@ function _mapResultTypeToStatus(
return ExecutionStatus.FAILED;
case ExecutionResultType.STRATEGY_ERROR:
return ExecutionStatus.FAILED;
case ExecutionResultType.STRATEGY_HELD:
return ExecutionStatus.HELD;
}
}
23 changes: 19 additions & 4 deletions packages/core/src/internal/execution/types/execution-result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export enum ExecutionResultType {
REVERTED_TRANSACTION = "REVERTED_TRANSACTION",
STATIC_CALL_ERROR = "STATIC_CALL_ERROR",
STRATEGY_ERROR = "STRATEGY_ERROR",
STRATEGY_HELD = "STRATEGY_HELD",
}

/**
Expand Down Expand Up @@ -59,6 +60,16 @@ export interface StrategyErrorExecutionResult {
error: string;
}

/**
* The execution strategy returned a strategy-specific hold e.g.
* waiting for off-chain multi-sig confirmations.
*/
export interface StrategyHeldExecutionResult {
kanej marked this conversation as resolved.
Show resolved Hide resolved
type: ExecutionResultType.STRATEGY_HELD;
heldId: number;
kanej marked this conversation as resolved.
Show resolved Hide resolved
reason: string;
}

/**
* A deployment was successfully executed.
*/
Expand All @@ -77,7 +88,8 @@ export type DeploymentExecutionResult =
| StrategySimulationErrorExecutionResult
| RevertedTransactionExecutionResult
| FailedStaticCallExecutionResult
| StrategyErrorExecutionResult;
| StrategyErrorExecutionResult
| StrategyHeldExecutionResult;

/**
* A call future was successfully executed.
Expand All @@ -95,7 +107,8 @@ export type CallExecutionResult =
| StrategySimulationErrorExecutionResult
| RevertedTransactionExecutionResult
| FailedStaticCallExecutionResult
| StrategyErrorExecutionResult;
| StrategyErrorExecutionResult
| StrategyHeldExecutionResult;

/**
* A send data future was successfully executed.
Expand All @@ -113,7 +126,8 @@ export type SendDataExecutionResult =
| StrategySimulationErrorExecutionResult
| RevertedTransactionExecutionResult
| FailedStaticCallExecutionResult
| StrategyErrorExecutionResult;
| StrategyErrorExecutionResult
| StrategyHeldExecutionResult;

/**
* A static call future was successfully executed.
Expand All @@ -129,4 +143,5 @@ export interface SuccessfulStaticCallExecutionResult {
export type StaticCallExecutionResult =
| SuccessfulStaticCallExecutionResult
| FailedStaticCallExecutionResult
| StrategyErrorExecutionResult;
| StrategyErrorExecutionResult
| StrategyHeldExecutionResult;
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export enum ExecutionStatus {
STARTED = "STARATED",
TIMEOUT = "TIMEOUT",
SUCCESS = "SUCCESS",
HELD = "HELD",
FAILED = "FAILED",
}

Expand Down
7 changes: 1 addition & 6 deletions packages/core/src/internal/formatters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,14 @@ export function formatExecutionError(
return `Simulating the transaction failed with error: ${formatFailedEvmExecutionResult(
result.error
)}`;

case ExecutionResultType.STRATEGY_SIMULATION_ERROR:
return `Simulating the transaction failed with error: ${result.error}`;

case ExecutionResultType.REVERTED_TRANSACTION: {
case ExecutionResultType.REVERTED_TRANSACTION:
return `Transaction ${result.txHash} reverted`;
}

case ExecutionResultType.STATIC_CALL_ERROR:
return `Static call failed with error: ${formatFailedEvmExecutionResult(
result.error
)}`;

case ExecutionResultType.STRATEGY_ERROR:
return `Execution failed with error: ${result.error}`;
}
Expand Down
18 changes: 16 additions & 2 deletions packages/core/src/internal/journal/utils/emitExecutionEvent.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import {
ExecutionEventListener,
ExecutionEventType,
ExecutionEventNetworkInteractionType,
ExecutionEventResult,
ExecutionEventResultType,
ExecutionEventNetworkInteractionType,
ExecutionEventType,
} from "../../../types/execution-events";
import { SolidityParameterType } from "../../../types/module";
import {
Expand Down Expand Up @@ -217,6 +217,13 @@ function convertExecutionResultToEventResult(
error: "Transaction reverted",
};
}
case ExecutionResultType.STRATEGY_HELD: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for being repetitive, but held shouldn't be considered an error.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added another event result (held), to allow mapping from held to a non-error result. And wired it through the UI.

return {
type: ExecutionEventResultType.HELD,
heldId: result.heldId,
reason: result.reason,
};
}
}
}

Expand All @@ -241,6 +248,13 @@ function convertStaticCallResultToExecutionEventResult(
error: result.error,
};
}
case ExecutionResultType.STRATEGY_HELD: {
kanej marked this conversation as resolved.
Show resolved Hide resolved
return {
type: ExecutionEventResultType.HELD,
heldId: result.heldId,
reason: result.reason,
};
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,21 @@ import { DeploymentState } from "../execution/types/deployment-state";
import { ExecutionStatus } from "../execution/types/execution-state";

/**
* Returns true if the execution of the given future has failed.
* Returns true if the execution of the given future has succeeded.
*
* @param future The future.
* @param deploymentState The deployment state to check against.
* @returns true if it failed.
* @returns true if it succeeded.
*/
export function hasExecutionFailed(
export function hasExecutionSucceeded(
future: Future,
deploymentState: DeploymentState
): boolean {
const exState = deploymentState.executionStates[future.id];

if (exState === undefined) {
return false;
}

return (
exState.status === ExecutionStatus.FAILED ||
exState.status === ExecutionStatus.TIMEOUT
);
return exState.status === ExecutionStatus.SUCCESS;
}
10 changes: 8 additions & 2 deletions packages/core/src/types/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,17 +114,23 @@ export interface ExecutionErrorDeploymentResult {
type: DeploymentResultType.EXECUTION_ERROR;

/**
* A list of all the future that have started executed but have not
* A list of all the futures that have started executing but have not
* finished, neither successfully nor unsuccessfully.
*/
started: string[];

/**
* A list of all the future that have timed out, including details of the
* A list of all the futures that have timed out, including details of the
* network interaction that timed out.
*/
timedOut: Array<{ futureId: string; networkInteractionId: number }>;

/**
* A list of all the futures that are being Held as determined by the execution
* strategy, i.e. an off-chain process is not yet complete.
*/
held: Array<{ futureId: string; heldId: number; reason: string }>;

/**
* A list of all the future that have failed, including the details of
* the network interaction that errored.
Expand Down
17 changes: 16 additions & 1 deletion packages/core/src/types/execution-events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -360,14 +360,18 @@ export enum ExecutionEventNetworkInteractionType {
export enum ExecutionEventResultType {
SUCCESS = "SUCCESS",
ERROR = "ERROR",
HELD = "HELD",
}

/**
* The result of a future's completed execution.
*
* @beta
*/
export type ExecutionEventResult = ExecutionEventSuccess | ExecutionEventError;
export type ExecutionEventResult =
| ExecutionEventSuccess
| ExecutionEventError
| ExecutionEventHeld;

/**
* A successful result of a future's execution.
Expand All @@ -389,6 +393,17 @@ export interface ExecutionEventError {
error: string;
}

/**
* A hold result of a future's execution.
*
* @beta
*/
export interface ExecutionEventHeld {
type: ExecutionEventResultType.HELD;
heldId: number;
reason: string;
}

/**
* A mapping of execution event types to their corresponding event.
*
Expand Down
10 changes: 9 additions & 1 deletion packages/hardhat-plugin/src/ui/UiEventHandler.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {
UiBatches,
UiFuture,
UiFutureErrored,
UiFutureHeld,
UiFutureStatusType,
UiFutureSuccess,
UiState,
Expand Down Expand Up @@ -396,7 +397,7 @@ export class UiEventHandler implements ExecutionEventListener {

private _getFutureStatusFromEventResult(
result: ExecutionEventResult
): UiFutureSuccess | UiFutureErrored {
): UiFutureSuccess | UiFutureErrored | UiFutureHeld {
switch (result.type) {
case ExecutionEventResultType.SUCCESS: {
return {
Expand All @@ -410,6 +411,13 @@ export class UiEventHandler implements ExecutionEventListener {
message: result.error,
};
}
case ExecutionEventResultType.HELD: {
return {
type: UiFutureStatusType.HELD,
heldId: result.heldId,
reason: result.reason,
};
}
}
}

Expand Down
Loading
Loading