From 491f99a8cbf49f4463d4fa95fdc5e498f0773dbd Mon Sep 17 00:00:00 2001 From: Roman Petriv Date: Tue, 5 Dec 2023 15:47:07 +0200 Subject: [PATCH] feat: add transaction error and revert reason (#110) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # What ❔ - fetch and store tx error and revert reason; - return tx error and revert reason via API; - display tx error on UI; ## Why ❔ - fast way to see what's the reason of transaction failure; ## Checklist - [+] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [+] Tests for the changes have been added / updated. - [+] Documentation comments have been added / updated. --- .../transaction.controller.spec.ts | 50 +++++++ .../api/transaction/transaction.controller.ts | 2 +- .../src/transaction/dtos/transaction.dto.ts | 18 +++ .../entities/transaction.entity.ts | 6 + packages/api/test/transaction.e2e-spec.ts | 46 ++++++ .../transactions/infoTable/GeneralInfo.vue | 19 ++- packages/app/src/composables/common/Api.d.ts | 2 + .../app/src/composables/useTransaction.ts | 4 + packages/app/src/locales/en.json | 2 + packages/app/src/locales/uk.json | 3 + .../transactions/GeneralInfo.spec.ts | 4 +- .../components/transactions/Table.spec.ts | 2 + .../tests/composables/useTransaction.spec.ts | 4 + .../tests/composables/useTransactions.spec.ts | 2 + .../src/blockchain/blockchain.service.spec.ts | 140 ++++++++++++++++++ .../src/blockchain/blockchain.service.ts | 20 +++ .../worker/src/entities/transaction.entity.ts | 6 + .../1700684231991-AddTransactionError.ts | 15 ++ .../repositories/transaction.repository.ts | 2 + .../transaction/transaction.processor.spec.ts | 52 ++++++- .../src/transaction/transaction.processor.ts | 24 ++- 21 files changed, 411 insertions(+), 12 deletions(-) create mode 100644 packages/worker/src/migrations/1700684231991-AddTransactionError.ts diff --git a/packages/api/src/api/transaction/transaction.controller.spec.ts b/packages/api/src/api/transaction/transaction.controller.spec.ts index a74a252aa0..049fec6b5e 100644 --- a/packages/api/src/api/transaction/transaction.controller.spec.ts +++ b/packages/api/src/api/transaction/transaction.controller.spec.ts @@ -85,6 +85,56 @@ describe("TransactionController", () => { }, }); }); + + it("returns transaction error in errDescription when transaction is failed and transaction error is present", async () => { + jest.spyOn(transactionServiceMock, "findOne").mockResolvedValue({ + status: TransactionStatus.Failed, + error: "Error", + revertReason: "Reverted", + } as TransactionDetails); + + const response = await controller.getTransactionStatus(transactionHash); + expect(response).toEqual({ + status: ResponseStatus.OK, + message: ResponseMessage.OK, + result: { + isError: "1", + errDescription: "Error", + }, + }); + }); + + it("returns transaction revert reason in errDescription when transaction is failed and transaction revert reason is present", async () => { + jest + .spyOn(transactionServiceMock, "findOne") + .mockResolvedValue({ status: TransactionStatus.Failed, revertReason: "Reverted" } as TransactionDetails); + + const response = await controller.getTransactionStatus(transactionHash); + expect(response).toEqual({ + status: ResponseStatus.OK, + message: ResponseMessage.OK, + result: { + isError: "1", + errDescription: "Reverted", + }, + }); + }); + + it("returns empty errDescription when transaction is failed and transaction error and revert reason are not present", async () => { + jest + .spyOn(transactionServiceMock, "findOne") + .mockResolvedValue({ status: TransactionStatus.Failed } as TransactionDetails); + + const response = await controller.getTransactionStatus(transactionHash); + expect(response).toEqual({ + status: ResponseStatus.OK, + message: ResponseMessage.OK, + result: { + isError: "1", + errDescription: "", + }, + }); + }); }); describe("getTransactionReceiptStatus", () => { diff --git a/packages/api/src/api/transaction/transaction.controller.ts b/packages/api/src/api/transaction/transaction.controller.ts index 29d993b78d..cede0e0c59 100644 --- a/packages/api/src/api/transaction/transaction.controller.ts +++ b/packages/api/src/api/transaction/transaction.controller.ts @@ -36,7 +36,7 @@ export class TransactionController { message: ResponseMessage.OK, result: { isError: hasError ? ResponseStatus.OK : ResponseStatus.NOTOK, - errDescription: "", + errDescription: transaction?.error || transaction?.revertReason || "", }, }; } diff --git a/packages/api/src/transaction/dtos/transaction.dto.ts b/packages/api/src/transaction/dtos/transaction.dto.ts index 8b77f54a1d..597ec07753 100644 --- a/packages/api/src/transaction/dtos/transaction.dto.ts +++ b/packages/api/src/transaction/dtos/transaction.dto.ts @@ -186,4 +186,22 @@ export class TransactionDto { examples: ["included", "committed", "proved", "verified", "failed"], }) public readonly status: TransactionStatus; + + @ApiProperty({ + type: String, + description: "Transaction error", + example: "Some test error", + examples: ["Some test error", null], + nullable: true, + }) + public readonly error?: string; + + @ApiProperty({ + type: String, + description: "Transaction revert reason", + example: "Some test revert reason", + examples: ["Some test revert reason", null], + nullable: true, + }) + public readonly revertReason?: string; } diff --git a/packages/api/src/transaction/entities/transaction.entity.ts b/packages/api/src/transaction/entities/transaction.entity.ts index e4a1b357e8..b907d539c5 100644 --- a/packages/api/src/transaction/entities/transaction.entity.ts +++ b/packages/api/src/transaction/entities/transaction.entity.ts @@ -101,6 +101,12 @@ export class Transaction extends BaseEntity { @OneToMany(() => Transfer, (transfer) => transfer.transaction) public readonly transfers: Transfer[]; + @Column({ nullable: true }) + public readonly error?: string; + + @Column({ nullable: true }) + public readonly revertReason?: string; + public get status(): TransactionStatus { if (this.receiptStatus === 0) { return TransactionStatus.Failed; diff --git a/packages/api/test/transaction.e2e-spec.ts b/packages/api/test/transaction.e2e-spec.ts index e38111a34d..28f55e29f1 100644 --- a/packages/api/test/transaction.e2e-spec.ts +++ b/packages/api/test/transaction.e2e-spec.ts @@ -256,6 +256,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 9, commitTxHash: null, data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: null, fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -282,6 +284,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 8, commitTxHash: "0xeb5ead20476b91008c3b6e44005017e697de78e4fd868d99d2c58566655c5aa8", data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: "0xeb5ead20476b91008c3b6e44005017e697de78e4fd868d99d2c58566655c5ab8", fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -308,6 +312,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 7, commitTxHash: "0xeb5ead20476b91008c3b6e44005017e697de78e4fd868d99d2c58566655c5aa7", data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: "0xeb5ead20476b91008c3b6e44005017e697de78e4fd868d99d2c58566655c5ab7", fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -334,6 +340,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 6, commitTxHash: "0xeb5ead20476b91008c3b6e44005017e697de78e4fd868d99d2c58566655c5aa6", data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: null, fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -360,6 +368,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 5, commitTxHash: "0xeb5ead20476b91008c3b6e44005017e697de78e4fd868d99d2c58566655c5aa5", data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: null, fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -386,6 +396,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 4, commitTxHash: "0xeb5ead20476b91008c3b6e44005017e697de78e4fd868d99d2c58566655c5aa4", data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: null, fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -412,6 +424,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 3, commitTxHash: "0xeb5ead20476b91008c3b6e44005017e697de78e4fd868d99d2c58566655c5aa3", data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: null, fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -438,6 +452,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 1, commitTxHash: null, data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: null, fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -464,6 +480,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 1, commitTxHash: null, data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: null, fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -490,6 +508,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 1, commitTxHash: null, data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: null, fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -526,6 +546,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 8, commitTxHash: "0xeb5ead20476b91008c3b6e44005017e697de78e4fd868d99d2c58566655c5aa8", data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: "0xeb5ead20476b91008c3b6e44005017e697de78e4fd868d99d2c58566655c5ab8", fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -552,6 +574,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 7, commitTxHash: "0xeb5ead20476b91008c3b6e44005017e697de78e4fd868d99d2c58566655c5aa7", data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: "0xeb5ead20476b91008c3b6e44005017e697de78e4fd868d99d2c58566655c5ab7", fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -578,6 +602,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 6, commitTxHash: "0xeb5ead20476b91008c3b6e44005017e697de78e4fd868d99d2c58566655c5aa6", data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: null, fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -660,6 +686,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 1, commitTxHash: null, data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: null, fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -711,6 +739,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 1, commitTxHash: null, data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: null, fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -762,6 +792,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 7, commitTxHash: "0xeb5ead20476b91008c3b6e44005017e697de78e4fd868d99d2c58566655c5aa7", data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: "0xeb5ead20476b91008c3b6e44005017e697de78e4fd868d99d2c58566655c5ab7", fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -788,6 +820,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 6, commitTxHash: "0xeb5ead20476b91008c3b6e44005017e697de78e4fd868d99d2c58566655c5aa6", data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: null, fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -875,6 +909,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 8, commitTxHash: "0xeb5ead20476b91008c3b6e44005017e697de78e4fd868d99d2c58566655c5aa8", data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: "0xeb5ead20476b91008c3b6e44005017e697de78e4fd868d99d2c58566655c5ab8", fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -910,6 +946,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 5, commitTxHash: "0xeb5ead20476b91008c3b6e44005017e697de78e4fd868d99d2c58566655c5aa5", data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: null, fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -945,6 +983,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 3, commitTxHash: "0xeb5ead20476b91008c3b6e44005017e697de78e4fd868d99d2c58566655c5aa3", data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: null, fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -980,6 +1020,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 1, commitTxHash: null, data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: null, fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -1015,6 +1057,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 9, commitTxHash: null, data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: null, fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", @@ -1050,6 +1094,8 @@ describe("TransactionController (e2e)", () => { blockNumber: 1, commitTxHash: null, data: "0x000000000000000000000000000000000000000000000000016345785d8a0000", + error: null, + revertReason: null, executeTxHash: null, fee: "0x2386f26fc10000", from: "0xc7e0220d02d549c4846A6EC31D89C3B670Ebe35C", diff --git a/packages/app/src/components/transactions/infoTable/GeneralInfo.vue b/packages/app/src/components/transactions/infoTable/GeneralInfo.vue index ef84402e10..9141e4769f 100644 --- a/packages/app/src/components/transactions/infoTable/GeneralInfo.vue +++ b/packages/app/src/components/transactions/infoTable/GeneralInfo.vue @@ -30,6 +30,19 @@ /> + + + + {{ t("transactions.table.reason") }} + + + {{ t("transactions.table.reasonTooltip") }} + + + + {{ transaction.error || transaction.revertReason || "" }} + + {{ t("transactions.table.block") }} @@ -233,9 +246,6 @@ const tokenTransfers = computed(() => { diff --git a/packages/app/src/composables/common/Api.d.ts b/packages/app/src/composables/common/Api.d.ts index 371236776a..736992256b 100644 --- a/packages/app/src/composables/common/Api.d.ts +++ b/packages/app/src/composables/common/Api.d.ts @@ -86,6 +86,8 @@ declare namespace Api { l1BatchNumber: number | null; isL1BatchSealed: boolean; status: "included" | "committed" | "proved" | "verified" | "failed"; + error: string | null; + revertReason: string | null; }; type Transfer = { diff --git a/packages/app/src/composables/useTransaction.ts b/packages/app/src/composables/useTransaction.ts index fe48c1da0f..f920b7689a 100644 --- a/packages/app/src/composables/useTransaction.ts +++ b/packages/app/src/composables/useTransaction.ts @@ -64,6 +64,8 @@ export type TransactionItem = { status: TransactionStatus; l1BatchNumber: number | null; isL1BatchSealed: boolean; + error?: string | null; + revertReason?: string | null; logs: TransactionLogEntry[]; transfers: TokenTransfer[]; }; @@ -222,6 +224,8 @@ export function mapTransaction( status: transaction.status, l1BatchNumber: transaction.l1BatchNumber, isL1BatchSealed: transaction.isL1BatchSealed, + error: transaction.error, + revertReason: transaction.revertReason, logs: logs.map((item) => ({ address: item.address, diff --git a/packages/app/src/locales/en.json b/packages/app/src/locales/en.json index b62ec696f3..c1dc5faa01 100644 --- a/packages/app/src/locales/en.json +++ b/packages/app/src/locales/en.json @@ -98,6 +98,8 @@ "table": { "status": "Status", "statusTooltip": "The status of the transaction", + "reason": "Reason", + "reasonTooltip": "The failure reason of the transaction", "txnHash": "Txn hash", "transactionHash": "Transaction Hash", "transactionHashTooltip": "Transaction hash is a unique 66-character identifier that is generated whenever a transaction is executed", diff --git a/packages/app/src/locales/uk.json b/packages/app/src/locales/uk.json index 57b0dd91d0..10df6b08ad 100644 --- a/packages/app/src/locales/uk.json +++ b/packages/app/src/locales/uk.json @@ -72,6 +72,9 @@ }, "table": { "status": "Статус", + "statusTooltip": "Статус транзакції", + "reason": "Причина", + "reasonTooltip": "Причина невиконання транзакції", "transactionHash": "Хеш Транзакції", "nonce": "Нонс", "created": "Створено", diff --git a/packages/app/tests/components/transactions/GeneralInfo.spec.ts b/packages/app/tests/components/transactions/GeneralInfo.spec.ts index 9b101af347..d24bf80f66 100644 --- a/packages/app/tests/components/transactions/GeneralInfo.spec.ts +++ b/packages/app/tests/components/transactions/GeneralInfo.spec.ts @@ -310,15 +310,17 @@ describe("Transaction info table", () => { plugins: [i18n, $testId], }, props: { - transaction: { ...transaction, status: "failed" }, + transaction: { ...transaction, status: "failed", revertReason: "Revert reason" }, loading: false, }, }); await nextTick(); const status = wrapper.findAll("tbody tr td:nth-child(2)")[1]; const badges = status.findAllComponents(Badge); + const reason = wrapper.find(".transaction-reason-value"); expect(badges.length).toBe(1); expect(badges[0].text()).toBe(i18n.global.t("transactions.statusComponent.failed")); + expect(reason.text()).toBe("Revert reason"); }); it("renders included transaction status", async () => { const wrapper = mount(Table, { diff --git a/packages/app/tests/components/transactions/Table.spec.ts b/packages/app/tests/components/transactions/Table.spec.ts index 50cc9d4000..afe9966596 100644 --- a/packages/app/tests/components/transactions/Table.spec.ts +++ b/packages/app/tests/components/transactions/Table.spec.ts @@ -57,6 +57,8 @@ const transaction: TransactionListItem = { executeTxHash: null, proveTxHash: null, isL1BatchSealed: false, + error: null, + revertReason: null, }; const contractAbi: AbiFragment[] = [ diff --git a/packages/app/tests/composables/useTransaction.spec.ts b/packages/app/tests/composables/useTransaction.spec.ts index 6ca5c1927e..a93a8f37fe 100644 --- a/packages/app/tests/composables/useTransaction.spec.ts +++ b/packages/app/tests/composables/useTransaction.spec.ts @@ -88,6 +88,8 @@ vi.mock("ohmyfetch", async () => { commitTxHash: "0xe6a7ed0b6bf1c49f27feae3a71e5ba2aa4abaa6e372524369529946eb61a6936", executeTxHash: "0xdd70c8c2f59d88b9970c3b48a1230320f051d4502d0277124db481a42ada5c33", proveTxHash: "0x688c20e2106984bb0ccdadecf01e7bf12088b0ba671d888eca8e577ceac0d790", + error: null, + revertReason: null, }; return { ...mod, @@ -444,6 +446,8 @@ describe("useTransaction:", () => { nonce: 24, receivedAt: "2023-02-28T08:42:08.198Z", status: "verified", + error: null, + revertReason: null, l1BatchNumber: 11014, isL1BatchSealed: true, logs: [ diff --git a/packages/app/tests/composables/useTransactions.spec.ts b/packages/app/tests/composables/useTransactions.spec.ts index bbc526e5fb..a5e3263953 100644 --- a/packages/app/tests/composables/useTransactions.spec.ts +++ b/packages/app/tests/composables/useTransactions.spec.ts @@ -27,6 +27,8 @@ const transaction: TransactionListItem = { executeTxHash: null, proveTxHash: null, isL1BatchSealed: false, + error: null, + revertReason: null, }; vi.mock("ohmyfetch", () => { diff --git a/packages/worker/src/blockchain/blockchain.service.spec.ts b/packages/worker/src/blockchain/blockchain.service.spec.ts index 5593bd3042..bacfa748b1 100644 --- a/packages/worker/src/blockchain/blockchain.service.spec.ts +++ b/packages/worker/src/blockchain/blockchain.service.spec.ts @@ -1471,6 +1471,146 @@ describe("BlockchainService", () => { }); }); + describe("debugTraceTransaction", () => { + const traceTransactionResult = { + type: "Call", + from: "0x0000000000000000000000000000000000000000", + to: "0x0000000000000000000000000000000000008001", + error: null, + revertReason: "Exceed daily limit", + }; + let timeoutSpy; + + beforeEach(() => { + jest.spyOn(provider, "send").mockResolvedValue(traceTransactionResult); + timeoutSpy = jest.spyOn(timersPromises, "setTimeout"); + }); + + it("starts the rpc call duration metric", async () => { + await blockchainService.debugTraceTransaction( + "0xc0ae49e96910fa9df22eb59c0977905864664d495bc95906120695aa26e1710b" + ); + expect(startRpcCallDurationMetricMock).toHaveBeenCalledTimes(1); + }); + + it("gets transaction trace", async () => { + await blockchainService.debugTraceTransaction( + "0xc0ae49e96910fa9df22eb59c0977905864664d495bc95906120695aa26e1710b" + ); + expect(provider.send).toHaveBeenCalledTimes(1); + expect(provider.send).toHaveBeenCalledWith("debug_traceTransaction", [ + "0xc0ae49e96910fa9df22eb59c0977905864664d495bc95906120695aa26e1710b", + { + tracer: "callTracer", + tracerConfig: { onlyTopCall: false }, + }, + ]); + }); + + it("gets transaction trace with only top call", async () => { + await blockchainService.debugTraceTransaction( + "0xc0ae49e96910fa9df22eb59c0977905864664d495bc95906120695aa26e1710b", + true + ); + expect(provider.send).toHaveBeenCalledTimes(1); + expect(provider.send).toHaveBeenCalledWith("debug_traceTransaction", [ + "0xc0ae49e96910fa9df22eb59c0977905864664d495bc95906120695aa26e1710b", + { + tracer: "callTracer", + tracerConfig: { onlyTopCall: true }, + }, + ]); + }); + + it("stops the rpc call duration metric", async () => { + await blockchainService.debugTraceTransaction( + "0xc0ae49e96910fa9df22eb59c0977905864664d495bc95906120695aa26e1710b" + ); + expect(stopRpcCallDurationMetricMock).toHaveBeenCalledTimes(1); + expect(stopRpcCallDurationMetricMock).toHaveBeenCalledWith({ function: "debugTraceTransaction" }); + }); + + it("returns transaction trace", async () => { + const result = await blockchainService.debugTraceTransaction( + "0xc0ae49e96910fa9df22eb59c0977905864664d495bc95906120695aa26e1710b" + ); + expect(result).toEqual(traceTransactionResult); + }); + + describe("if the call throws an error", () => { + beforeEach(() => { + jest + .spyOn(provider, "send") + .mockRejectedValueOnce(new Error("RPC call error")) + .mockRejectedValueOnce(new Error("RPC call error")) + .mockResolvedValueOnce(traceTransactionResult); + }); + + it("retries RPC call with a default timeout", async () => { + await blockchainService.debugTraceTransaction( + "0xc0ae49e96910fa9df22eb59c0977905864664d495bc95906120695aa26e1710b" + ); + expect(provider.send).toHaveBeenCalledTimes(3); + expect(timeoutSpy).toHaveBeenCalledTimes(2); + expect(timeoutSpy).toHaveBeenNthCalledWith(1, defaultRetryTimeout); + expect(timeoutSpy).toHaveBeenNthCalledWith(2, defaultRetryTimeout); + }); + + it("stops the rpc call duration metric only for the successful retry", async () => { + await blockchainService.debugTraceTransaction( + "0xc0ae49e96910fa9df22eb59c0977905864664d495bc95906120695aa26e1710b" + ); + expect(stopRpcCallDurationMetricMock).toHaveBeenCalledTimes(1); + expect(stopRpcCallDurationMetricMock).toHaveBeenCalledWith({ function: "debugTraceTransaction" }); + }); + + it("returns result of the successful RPC call", async () => { + const result = await blockchainService.debugTraceTransaction( + "0xc0ae49e96910fa9df22eb59c0977905864664d495bc95906120695aa26e1710b" + ); + expect(result).toEqual(traceTransactionResult); + }); + }); + + describe("if the call throws a timeout error", () => { + beforeEach(() => { + jest + .spyOn(provider, "send") + .mockRejectedValueOnce({ code: "TIMEOUT" }) + .mockRejectedValueOnce({ code: "TIMEOUT" }) + .mockResolvedValueOnce(traceTransactionResult); + }); + + it("retries RPC call with a quick timeout", async () => { + await blockchainService.debugTraceTransaction( + "0xc0ae49e96910fa9df22eb59c0977905864664d495bc95906120695aa26e1710b" + ); + expect(timeoutSpy).toHaveBeenCalledTimes(2); + expect(timeoutSpy).toHaveBeenNthCalledWith(1, quickRetryTimeout); + expect(timeoutSpy).toHaveBeenNthCalledWith(2, quickRetryTimeout); + }); + }); + + describe("if the call throws a connection refused error", () => { + beforeEach(() => { + jest + .spyOn(provider, "send") + .mockRejectedValueOnce({ code: "TIMEOUT" }) + .mockRejectedValueOnce({ code: "TIMEOUT" }) + .mockResolvedValueOnce(traceTransactionResult); + }); + + it("retries RPC call with a quick timeout", async () => { + await blockchainService.debugTraceTransaction( + "0xc0ae49e96910fa9df22eb59c0977905864664d495bc95906120695aa26e1710b" + ); + expect(timeoutSpy).toHaveBeenCalledTimes(2); + expect(timeoutSpy).toHaveBeenNthCalledWith(1, quickRetryTimeout); + expect(timeoutSpy).toHaveBeenNthCalledWith(2, quickRetryTimeout); + }); + }); + }); + describe("onModuleInit", () => { let bridgeAddresses; beforeEach(() => { diff --git a/packages/worker/src/blockchain/blockchain.service.ts b/packages/worker/src/blockchain/blockchain.service.ts index cf6dbf64b8..741631c8d5 100644 --- a/packages/worker/src/blockchain/blockchain.service.ts +++ b/packages/worker/src/blockchain/blockchain.service.ts @@ -15,6 +15,14 @@ export interface BridgeAddresses { l2Erc20DefaultBridge: string; } +export interface TraceTransactionResult { + type: string; + from: string; + to: string; + error: string | null; + revertReason: string | null; +} + @Injectable() export class BlockchainService implements OnModuleInit { private readonly logger: Logger; @@ -121,6 +129,18 @@ export class BlockchainService implements OnModuleInit { }, "getDefaultBridgeAddresses"); } + public async debugTraceTransaction(txHash: string, onlyTopCall = false): Promise { + return await this.rpcCall(async () => { + return await this.provider.send("debug_traceTransaction", [ + txHash, + { + tracer: "callTracer", + tracerConfig: { onlyTopCall }, + }, + ]); + }, "debugTraceTransaction"); + } + public async on(eventName: EventType, listener: Listener): Promise { this.provider.on(eventName, listener); } diff --git a/packages/worker/src/entities/transaction.entity.ts b/packages/worker/src/entities/transaction.entity.ts index 0ec6d9b067..ca1f3511ec 100644 --- a/packages/worker/src/entities/transaction.entity.ts +++ b/packages/worker/src/entities/transaction.entity.ts @@ -90,4 +90,10 @@ export class Transaction extends CountableEntity { @Column({ type: "int", default: 1 }) public readonly receiptStatus: number; + + @Column({ nullable: true }) + public readonly error?: string; + + @Column({ nullable: true }) + public readonly revertReason?: string; } diff --git a/packages/worker/src/migrations/1700684231991-AddTransactionError.ts b/packages/worker/src/migrations/1700684231991-AddTransactionError.ts new file mode 100644 index 0000000000..c0ccce4efd --- /dev/null +++ b/packages/worker/src/migrations/1700684231991-AddTransactionError.ts @@ -0,0 +1,15 @@ +import { MigrationInterface, QueryRunner } from "typeorm"; + +export class AddTransactionError1700684231991 implements MigrationInterface { + name = "AddTransactionError1700684231991"; + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query(`ALTER TABLE "transactions" ADD "error" character varying`); + await queryRunner.query(`ALTER TABLE "transactions" ADD "revertReason" character varying`); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query(`ALTER TABLE "transactions" DROP COLUMN "revertReason"`); + await queryRunner.query(`ALTER TABLE "transactions" DROP COLUMN "error"`); + } +} diff --git a/packages/worker/src/repositories/transaction.repository.ts b/packages/worker/src/repositories/transaction.repository.ts index b055c40769..610d1469be 100644 --- a/packages/worker/src/repositories/transaction.repository.ts +++ b/packages/worker/src/repositories/transaction.repository.ts @@ -10,6 +10,8 @@ export interface TransactionDto extends types.TransactionResponse { receiptStatus: number; isL1Originated: boolean; receivedAt: Date; + error?: string; + revertReason?: string; } @Injectable() diff --git a/packages/worker/src/transaction/transaction.processor.spec.ts b/packages/worker/src/transaction/transaction.processor.spec.ts index e152da109e..e6b28d30a2 100644 --- a/packages/worker/src/transaction/transaction.processor.spec.ts +++ b/packages/worker/src/transaction/transaction.processor.spec.ts @@ -3,7 +3,7 @@ import { Logger } from "@nestjs/common"; import { mock } from "jest-mock-extended"; import { types } from "zksync-web3"; import { TransactionRepository, TransactionReceiptRepository } from "../repositories"; -import { BlockchainService } from "../blockchain"; +import { BlockchainService, TraceTransactionResult } from "../blockchain"; import { TransactionProcessor } from "./transaction.processor"; import { LogProcessor } from "../log"; @@ -83,11 +83,16 @@ describe("TransactionProcessor", () => { status: 1, }); const transactionDetails = mock(); + const traceTransactionResult = mock({ + error: "Some error", + revertReason: "Some revert reason", + }); beforeEach(() => { jest.spyOn(blockchainServiceMock, "getTransaction").mockResolvedValue(transaction); jest.spyOn(blockchainServiceMock, "getTransactionReceipt").mockResolvedValue(transactionReceipt); jest.spyOn(blockchainServiceMock, "getTransactionDetails").mockResolvedValue(transactionDetails); + jest.spyOn(blockchainServiceMock, "debugTraceTransaction").mockResolvedValue(traceTransactionResult); }); it("starts the transaction duration metric", async () => { @@ -176,5 +181,50 @@ describe("TransactionProcessor", () => { await transactionProcessor.add(transaction.hash, blockDetails); expect(stopTxProcessingDurationMetricMock).toHaveBeenCalledTimes(1); }); + + describe("when transaction has failed status", () => { + beforeEach(() => { + (blockchainServiceMock.getTransactionReceipt as jest.Mock).mockResolvedValueOnce({ + transactionIndex: 0, + logs: [], + status: 0, + }); + }); + + it("reads transaction trace", async () => { + await transactionProcessor.add(transaction.hash, blockDetails); + expect(blockchainServiceMock.debugTraceTransaction).toHaveBeenCalledTimes(1); + expect(blockchainServiceMock.debugTraceTransaction).toHaveBeenCalledWith(transaction.hash, true); + }); + + describe("when transaction trace contains error and revert reason", () => { + it("adds the transaction info with error and revert reason", async () => { + await transactionProcessor.add(transaction.hash, blockDetails); + expect(transactionRepositoryMock.add).toHaveBeenCalledTimes(1); + expect(transactionRepositoryMock.add).toHaveBeenCalledWith({ + ...transaction, + ...transactionDetails, + l1BatchNumber: blockDetails.l1BatchNumber, + receiptStatus: 0, + error: traceTransactionResult.error, + revertReason: traceTransactionResult.revertReason, + }); + }); + }); + + describe("when transaction trace doe not contain error and revert reason", () => { + it("adds the transaction info without error and revert reason", async () => { + (blockchainServiceMock.debugTraceTransaction as jest.Mock).mockResolvedValueOnce(null); + await transactionProcessor.add(transaction.hash, blockDetails); + expect(transactionRepositoryMock.add).toHaveBeenCalledTimes(1); + expect(transactionRepositoryMock.add).toHaveBeenCalledWith({ + ...transaction, + ...transactionDetails, + l1BatchNumber: blockDetails.l1BatchNumber, + receiptStatus: 0, + }); + }); + }); + }); }); }); diff --git a/packages/worker/src/transaction/transaction.processor.ts b/packages/worker/src/transaction/transaction.processor.ts index 6a9f352efb..0f45a00f54 100644 --- a/packages/worker/src/transaction/transaction.processor.ts +++ b/packages/worker/src/transaction/transaction.processor.ts @@ -44,17 +44,29 @@ export class TransactionProcessor { throw new Error(`Some of the blockchain transaction APIs returned null for a transaction ${transactionHash}`); } + const transactionToAdd = { + ...transaction, + ...transactionDetails, + l1BatchNumber: blockDetails.l1BatchNumber, + receiptStatus: transactionReceipt.status, + } as TransactionDto; + + if (transactionReceipt.status === 0) { + const debugTraceTransactionResult = await this.blockchainService.debugTraceTransaction(transactionHash, true); + if (debugTraceTransactionResult?.error) { + transactionToAdd.error = debugTraceTransactionResult.error; + } + if (debugTraceTransactionResult?.revertReason) { + transactionToAdd.revertReason = debugTraceTransactionResult.revertReason; + } + } + this.logger.debug({ message: "Adding transaction data to the DB", blockNumber: blockDetails.number, transactionHash, }); - await this.transactionRepository.add({ - ...transaction, - ...transactionDetails, - l1BatchNumber: blockDetails.l1BatchNumber, - receiptStatus: transactionReceipt.status, - } as TransactionDto); + await this.transactionRepository.add(transactionToAdd); this.logger.debug({ message: "Adding transaction receipt data to the DB",