diff --git a/packages/core/src/internal/execution/nonce-management/get-nonce-sync-messages.ts b/packages/core/src/internal/execution/nonce-management/get-nonce-sync-messages.ts index 1ca3fdce8..bf91e8c71 100644 --- a/packages/core/src/internal/execution/nonce-management/get-nonce-sync-messages.ts +++ b/packages/core/src/internal/execution/nonce-management/get-nonce-sync-messages.ts @@ -117,12 +117,29 @@ export async function getNonceSyncMessages( transactions.map((tx) => jsonRpcClient.getTransaction(tx)) ); - if (fetchedTransactions.some((tx) => tx === undefined)) { + // If at least one transaction for the future is still in the mempool, + // we do nothing + if (fetchedTransactions.some((tx) => tx !== undefined)) { continue; } + // If we are here, all the previously pending transactions for this + // future were dropped or replaced. + // Case 1: Confirmed transaction with this nonce + // There are more transactions up to the latest block than our nonce, + // that means that one transaction with our nonce was sent and confirmed + // + // Example: + // + // Ignition sends transaction with nonce 5 + // It is replaced by the user, with user transaction nonce 5 + // The user transaction confirms + // That means there is a block that includes it + // If we look at the latest transaction count, it will be at least 6 if (latestCount > nonce) { + // We know the ignition transaction was replaced, and the replacement + // transaction has at least one confirmation. // We don't continue until the user's transactions have enough confirmations if (safeConfirmationsCount <= nonce) { throw new IgnitionError(ERRORS.EXECUTION.WAITING_FOR_NONCE, { @@ -144,6 +161,14 @@ export async function getNonceSyncMessages( // Case 2: There's a pending transaction with this nonce sent by the user // We first handle confirmed transactions, that'w why this check is safe here + // + // Example: + // + // Ignition has sent a transaction with nonce 5 + // It is replaced by the user, with user transaction nonce 5 + // The user transaction is still in the mempool + // The pending count will show as larger than the nonce, and we know + // from the test above that it has not been confirmed if (pendingCount > nonce) { throw new IgnitionError(ERRORS.EXECUTION.WAITING_FOR_NONCE, { sender, @@ -153,13 +178,18 @@ export async function getNonceSyncMessages( } // Case 3: There's no transaction sent by the user with this nonce, but ours were still dropped - messages.push({ type: JournalMessageType.ONCHAIN_INTERACTION_DROPPED, futureId: executionStateId, networkInteractionId, }); } + + // Case 4: the user sent a set of transactions with nonces higher than + // our highest pending nonce. + + // TODO if they have enough confirmation we continue, otherwise we throw + // and wait for further confirmations } return messages; diff --git a/packages/core/test/execution/nonce-management/get-nonce-sync-messages.ts b/packages/core/test/execution/nonce-management/get-nonce-sync-messages.ts index 6979d8ac2..f90fcd789 100644 --- a/packages/core/test/execution/nonce-management/get-nonce-sync-messages.ts +++ b/packages/core/test/execution/nonce-management/get-nonce-sync-messages.ts @@ -20,7 +20,10 @@ import { OnchainInteractionDroppedMessage, OnchainInteractionReplacedByUserMessage, } from "../../../src/internal/execution/types/messages"; -import { NetworkInteractionType } from "../../../src/internal/execution/types/network-interaction"; +import { + NetworkInteractionType, + OnchainInteraction, +} from "../../../src/internal/execution/types/network-interaction"; import { exampleAccounts } from "../../helpers"; const requiredConfirmations = 5; @@ -122,7 +125,103 @@ describe("execution - getNonceSyncMessages", () => { }); describe("second deployment run", () => { - it("should indicate the user replaced the transaction if the transaction's nonce is less than the latest", async () => { + it("should succeed with an in-flight transaction that has been mined between runs", async () => { + // Assuming a fresh chain, and a deployment that was halted on the first + // future and its deploy transaction is included in a block before the + // next run + + // The in-flight transaction has been included in a block, so latest includes it + const latestCount = 1; + // Safest is x blocks in the past so is zero + const safestCount = 0; + // There are no pending + const pendingCount = latestCount; + // The nonce of the now complete transaction is 0 + const nonce = 0; + + const deploymentState = + setupDeploymentStateBasedOnExampleModuleWithOneTranWith(nonce); + + const inflightTxHash = ( + ( + deploymentState.executionStates[ + "Example#MyContract" + ] as DeploymentExecutionState + ).networkInteractions[0] as OnchainInteraction + ).transactions[0].hash; + + await assertSuccessOnGetNonceSyncResult({ + ignitionModule: exampleModule, + deploymentState, + transactionCountEntries: { + [exampleAccounts[1]]: { + pending: pendingCount, + latest: latestCount, + number: () => safestCount, + }, + }, + // We are saying that the inflight transaction is now included in a block + getTransaction: (txHash: string) => { + if (txHash !== inflightTxHash) { + throw new Error( + `Mock getTransaction was not expecting the getTransaction request for: ${txHash}` + ); + } + + return { _kind: "FAKE_TRANSACTION" }; + }, + }); + }); + + it("should succeed with an in-flight transaction that has not been mined between runs but is in the mempool", async () => { + // Assuming a fresh chain, and a deployment that was halted on the first + // future and its deploy transaction is in the mempool but not mined + // at the start of the second run + + // The in-flight transaction has been included in a block, so latest includes it + const latestCount = 0; + // Safest is x blocks in the past so is zero + const safestCount = 0; + // There are no pending + const pendingCount = latestCount + 1; + // The nonce of the now complete transaction is 0 + const nonce = 0; + + const deploymentState = + setupDeploymentStateBasedOnExampleModuleWithOneTranWith(nonce); + + const inflightTxHash = ( + ( + deploymentState.executionStates[ + "Example#MyContract" + ] as DeploymentExecutionState + ).networkInteractions[0] as OnchainInteraction + ).transactions[0].hash; + + await assertSuccessOnGetNonceSyncResult({ + ignitionModule: exampleModule, + deploymentState, + transactionCountEntries: { + [exampleAccounts[1]]: { + pending: pendingCount, + latest: latestCount, + number: () => safestCount, + }, + }, + // We are saying that the inflight transaction is still in the mempool + getTransaction: (txHash: string) => { + if (txHash !== inflightTxHash) { + throw new Error( + `Mock getTransaction was not expecting the getTransaction request for: ${txHash}` + ); + } + + return { _kind: "FAKE_TRANSACTION" }; + }, + }); + }); + + it("should indicate the user replaced the transaction and the user transaction is safely confirmed", async () => { // Set latest to be an arbitrary nonce const latestCount = 30; // Set safest to be the same as latest, it is not relevant @@ -130,7 +229,7 @@ describe("execution - getNonceSyncMessages", () => { // There are no pending transactions const pendingCount = latestCount; // Set the nonce to be less than latest, indicating it was replaced - const nonce = latestCount - 2; + const nonce = latestCount - 1; await assertGetNonceSyncResult( { @@ -155,13 +254,40 @@ describe("execution - getNonceSyncMessages", () => { ); }); - it("should error if the user has sent a non-ignition pending transaction that has not confirmed on the account", async () => { + it("should throw if the user replaced the transaction and the user transaction is mined but has not yet fully confirmed", async () => { + // set an arbitrary nonce + const nonce = 16; + // put the latest as bigger than the nonce being checked + const latest = nonce + 1; + // there are no pending + const pending = latest; + // the safest is behind the nonce + const safest = nonce - 1; + + await assertGetNonceSyncThrows( + { + ignitionModule: exampleModule, + deploymentState: + setupDeploymentStateBasedOnExampleModuleWithOneTranWith(nonce), + transactionCountEntries: { + [exampleAccounts[1]]: { + pending, + latest, + number: () => safest, + }, + }, + }, + `IGN404: You have sent transactions from 0x3C44CdDdB6a900fa2b585dd299e03d12FA4293BC with nonce 16. Please wait until they get 5 confirmations before running Ignition again.` + ); + }); + + it("should error if the user replace the transaction and the user transaction is in the mempool but not mined (pending)", async () => { // Set latest to an arbitary nonce const latestCount = 30; // Safe is the same as latest const safestCount = latestCount; - // Set the nonce to be larger than latest - const nonce = latestCount + 1; + // Set the nonce to be the latest (nonce's are ids not cardinalities) + const nonce = latestCount; // Set pending larger than the nonce const pendingCount = nonce + 1; @@ -178,19 +304,21 @@ describe("execution - getNonceSyncMessages", () => { }, }, }, - `IGN404: You have sent transactions from ${exampleAccounts[1]} with nonce 31. Please wait until they get 5 confirmations before running Ignition again.` + `IGN404: You have sent transactions from ${exampleAccounts[1]} with nonce 30. Please wait until they get 5 confirmations before running Ignition again.` ); }); - it("should indicate the transaction was dropped if the nonce is higher than the latest", async () => { + it("should indicate if the ignition transaction was dropped from mempool (no user interference)", async () => { // Set an arbitary latest const latestCount = 30; // The safest is exactly the same as latest const safestCount = 40; // Pending isn't relevant so is set to latest const pendingCount = latestCount; - // Set the nonce higher than latest, indicating it was dropped - const nonce = latestCount + 1; + // Set the nonce to latest (note nonce is not a cardinality), + // indicating it was dropped as latest/pending should be larger + // than the nonce + const nonce = latestCount; await assertGetNonceSyncResult( { @@ -262,6 +390,7 @@ async function assertGetNonceSyncThrows( number: (num: number) => number; }; }; + getTransaction?: (txHash: string) => any; }, errorMessage: string ) { @@ -278,6 +407,7 @@ async function assertSuccessOnGetNonceSyncResult(ctx: { number: (num: number) => number; }; }; + getTransaction?: (txHash: string) => any; }) { return assertGetNonceSyncResult(ctx, []); } @@ -287,6 +417,7 @@ async function assertGetNonceSyncResult( ignitionModule, deploymentState = deploymentStateReducer(), transactionCountEntries = {}, + getTransaction, }: { ignitionModule: IgnitionModule< string, @@ -301,6 +432,7 @@ async function assertGetNonceSyncResult( number: (num: number) => number; }; }; + getTransaction?: (txHash: string) => any; }, expectedResult: Array< OnchainInteractionReplacedByUserMessage | OnchainInteractionDroppedMessage @@ -308,7 +440,8 @@ async function assertGetNonceSyncResult( ) { const mockJsonRpcClient = setupJsonRpcClient( latestBlock, - transactionCountEntries + transactionCountEntries, + getTransaction ); const result = await getNonceSyncMessages( @@ -331,12 +464,20 @@ function setupJsonRpcClient( latest: number; number: (num: number) => number; }; - } + }, + getTransaction?: (txHash: string) => any ): JsonRpcClient { const mockJsonRpcClient = { getLatestBlock: () => { return { number: latestBlockNum }; }, + getTransaction: (txHash: string) => { + if (getTransaction !== undefined) { + return getTransaction(txHash); + } + + return undefined; // Inflight transactions are never found by default + }, getTransactionCount: ( address: string, blockTag: "pending" | "latest" | number @@ -396,7 +537,14 @@ function setupDeploymentStateBasedOnExampleModuleWithOneTranWith( to: undefined, data: "0x", value: BigInt(0), - transactions: [], + transactions: [ + { + hash: "0x123", + fees: { + gasPrice: BigInt(10000), + }, + }, + ], shouldBeResent: false, nonce, },