From 153d4078e1d57255000be18b8c4b14888256a83c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Abadesso?= Date: Tue, 13 Jul 2021 14:44:39 -0300 Subject: [PATCH 1/3] feat: validating tx outputs before sending transactions --- package-lock.json | 6 +++ package.json | 2 + src/types.ts | 16 +++++--- src/utils.ts | 53 +++++++++++++++++++++---- test/utils.test.ts | 11 ++++++ test/utils.ts | 99 +++++++++++++++++++++++++++++++++++++++++++++- 6 files changed, 172 insertions(+), 15 deletions(-) diff --git a/package-lock.json b/package-lock.json index 8ee97ef0..ae761131 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2080,6 +2080,12 @@ "integrity": "sha1-7ihweulOEdK4J7y+UnC86n8+ce4=", "dev": true }, + "@types/lodash": { + "version": "4.14.171", + "resolved": "https://registry.npmjs.org/@types/lodash/-/lodash-4.14.171.tgz", + "integrity": "sha512-7eQ2xYLLI/LsicL2nejW9Wyko3lcpN6O/z0ZLHrEQsg280zIdCv1t/0m6UtBjUHokCGBQ3gYTbHzDkZ1xOBwwg==", + "dev": true + }, "@types/node": { "version": "14.14.37", "resolved": "https://registry.npmjs.org/@types/node/-/node-14.14.37.tgz", diff --git a/package.json b/package.json index 1708ccc9..f024c77a 100644 --- a/package.json +++ b/package.json @@ -46,6 +46,7 @@ ], "devDependencies": { "@size-limit/preset-small-lib": "^4.10.2", + "@types/lodash": "^4.14.171", "husky": "^6.0.0", "size-limit": "^4.10.2", "tsdx": "^0.14.1", @@ -57,6 +58,7 @@ "aws-sdk": "^2.878.0", "axios": "^0.21.1", "dotenv": "^8.2.0", + "lodash": "^4.17.21", "websocket": "^1.0.33", "winston": "^3.3.3", "xstate": "^4.17.1" diff --git a/src/types.ts b/src/types.ts index 5266b07f..57d670ae 100644 --- a/src/types.ts +++ b/src/types.ts @@ -11,8 +11,8 @@ export interface Block { } export interface DecodedScript { - type: string; - address: string; + type?: string; + address?: string; timelock?: number | undefined | null; value?: number | undefined | null; tokenData?: number | undefined | null; @@ -222,12 +222,15 @@ export interface RawDecodedInput { token_data: number; } +/* Everything is optional because scripts that were not able to + * be decoded will be returned as {} + */ export interface RawDecodedOutput { - type: string; - address: string; + type?: string; + address?: string; timelock?: number | null; - value: number; - token_data: number; + value?: number; + token_data?: number; } export interface RawInput { @@ -276,6 +279,7 @@ export interface Meta { accumulated_weight: number; score: number; height: number; + validation?: string; first_block?: string | null; } diff --git a/src/utils.ts b/src/utils.ts index 3e2a762a..deb15748 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -39,6 +39,7 @@ import dotenv from 'dotenv'; // @ts-ignore import { wallet } from '@hathor/wallet-lib'; import logger from './logger'; +import { isNumber, isNil, get } from 'lodash'; dotenv.config(); @@ -79,8 +80,8 @@ export const downloadTxFromId = async (txId: string): Promise => const txData: RawTxResponse = await downloadTx(txId); const { tx, meta } = txData; - return parseTx(tx); + return parseTx(tx); }; /** @@ -137,7 +138,37 @@ export const recursivelyDownloadTx = async (blockId: string, txIds: string[] = [ !data.has(parent) }); - return recursivelyDownloadTx(blockId, [...txIds, ...newParents], data.set(parsedTx.txId, parsedTx)); + let newData; + + // We need to validate that the outputs scripts have been parsed successfully + if (validateTxOutputs(parsedTx)) { + newData = data.set(parsedTx.txId, parsedTx) + } else { + // Ignore the current transaction (but not its parents) + logger.warn(`Ignoring tx ${parsedTx.txId} because it has invalid output(s)`); + newData = data; + } + + return recursivelyDownloadTx(blockId, [...txIds, ...newParents], newData); +}; + +/** + * Validates a transaction by checking if all tx outputs scripts have been successfully decoded + * + * @param tx - `RawTx` to be validated + * @returns True or False depending on transaction validity + */ +export const validateTxOutputs = (tx: FullTx): boolean => { + for (let i = 0; i < tx.outputs.length; i++) { + /* get will not crash if decoded is undefined and isNil will check for + * both null and undefined + */ + if (isNil(get(tx.outputs[i], 'decoded.type'))) { + return false; + } + } + + return true; }; /** @@ -237,9 +268,9 @@ export const parseTx = (tx: RawTx): FullTx => { const typedDecodedScript: DecodedScript = { type: input.decoded.type as string, address: input.decoded.address as string, - timelock: input.decoded.timelock ? input.decoded.timelock as number : null, - value: input.decoded.value ? input.decoded.value as number : null, - tokenData: input.decoded.token_data ? input.decoded.token_data as number : null, + timelock: isNumber(input.decoded.timelock) ? input.decoded.timelock as number : null, + value: isNumber(input.decoded.value) ? input.decoded.value as number : null, + tokenData: isNumber(input.decoded.token_data) ? input.decoded.token_data as number : null, }; const typedInput: Input = { txId: input.tx_id as string, @@ -256,9 +287,9 @@ export const parseTx = (tx: RawTx): FullTx => { const typedDecodedScript: DecodedScript = { type: output.decoded.type as string, address: output.decoded.address as string, - timelock: output.decoded.timelock ? output.decoded.timelock as number : null, - value: output.decoded.value ? output.decoded.value as number : null, - tokenData: output.decoded.token_data ? output.decoded.token_data as number : null, + timelock: isNumber(output.decoded.timelock) ? output.decoded.timelock as number : null, + value: isNumber(output.decoded.value) ? output.decoded.value as number : null, + tokenData: isNumber(output.decoded.token_data) ? output.decoded.token_data as number : null, }; const typedOutput: Output = { @@ -311,6 +342,12 @@ export async function* syncLatestMempool(): AsyncGenerator { return; } + + if (!validateTxOutputs(tx)) { + logger.warn(`Ignoring tx ${tx.txId} because it has invalid output(s)`); + continue; + } + const preparedTx: PreparedTx = prepareTx(tx); try { diff --git a/test/utils.test.ts b/test/utils.test.ts index d7da4bd5..d77930d4 100644 --- a/test/utils.test.ts +++ b/test/utils.test.ts @@ -14,6 +14,7 @@ import { BLOCK_BY_HEIGHT, MOCK_TXS, MOCK_FULL_TXS, + MOCK_NFT_TX, MOCK_CREATE_TOKEN_TX, generateBlock, } from './utils'; @@ -24,6 +25,7 @@ import { import { prepareTx, parseTx, + validateTxOutputs, } from '../src/utils'; import * as Utils from '../src/utils'; import * as FullNode from '../src/api/fullnode'; @@ -263,3 +265,12 @@ test('prepareTx on a CREATE_TOKEN tx should have token_name and token_symbol', a expect(preparedTx.token_name).toStrictEqual('XCoin'); expect(preparedTx.token_symbol).toStrictEqual('XCN'); }, 500); + +test('validateTxOutputs on NFT transaction', async () => { + expect.hasAssertions(); + + const { tx } = MOCK_NFT_TX; + const parsedTx = parseTx(tx); + + expect(validateTxOutputs(parsedTx)).toStrictEqual(false); +}); diff --git a/test/utils.ts b/test/utils.ts index cb1b1308..cc712c9c 100644 --- a/test/utils.ts +++ b/test/utils.ts @@ -20,6 +20,7 @@ export interface DecodedScript { address: string, timelock?: number, } + export const MOCK_FULL_TXS: FullTx[] = [{ txId: '0000000033a3bb347e0401d85a70b38f0aa7b5e37ea4c70d7dacf8e493946e64', nonce: '2553516830', @@ -269,4 +270,100 @@ export const MOCK_CREATE_TOKEN_TX: RawTxResponse = { first_block: "000000bd45ecc5119963cc3fa03e894f574e69811eef266ed7c6a0d4c1e1806c" }, spent_outputs: {} -} +}; + +export const MOCK_NFT_TX: RawTxResponse = { + "success": true, + "tx": { + "hash": "0055c424b9038b0a8888b574ccdb1933a007fdfc15b91a4b38a48cc883b540bf", + "nonce": "389", + "timestamp": 1626187098, + "version": 2, + "weight": 8.0, + "parents": [ + "0055b20066e8168ad8f05e82d66a34d19970cfb1861281735215cdd84744d842", + "00bb42880bd1183ce34df2185d1431f531a0a95af3556e368fa72e462edf7a9f" + ], + "inputs": [{ + "value": 2, + "token_data": 0, + "script": "dqkU8uf1ieRE8taN5bCNug5z5UHMO6eIrA==", + "decoded": { + "type": "P2PKH", + "address": "WkpQH9t4ue4LbTQKAEWssiXnYHC8CyMp7J", + "timelock": null, + "value": 2, + "token_data": 0 + }, + "tx_id": "0055b20066e8168ad8f05e82d66a34d19970cfb1861281735215cdd84744d842", + "index": 1 + }], + "outputs": [{ + "value": 1, + "token_data": 0, + "script": "TFFodHRwczovL2lwZnMuaW8vaXBmcy9RbWJIdEZrWWlGSG5XdEV6bm01RFFHTVNOSmdwTExXeDdRNlBxdHAxb0NiQlpwL21ldGFkYXRhLmpzb26s", + "decoded": {} + }, + { + "value": 2, + "token_data": 129, + "script": "dqkUYpULlr3iJ6sZbP3YIfgL52fasneIrA==", + "decoded": { + "type": "P2PKH", + "address": "WXfHeaEtr3fS9ex42V5chr2jY7wb5tdcWD", + "timelock": null, + "value": 2, + "token_data": 129 + } + }, + { + "value": 1, + "token_data": 1, + "script": "dqkUYpULlr3iJ6sZbP3YIfgL52fasneIrA==", + "decoded": { + "type": "P2PKH", + "address": "WXfHeaEtr3fS9ex42V5chr2jY7wb5tdcWD", + "timelock": null, + "value": 1, + "token_data": 1 + } + } + ], + "tokens": [{ + "uid": "0055c424b9038b0a8888b574ccdb1933a007fdfc15b91a4b38a48cc883b540bf", + "name": "Furia Special Edition", + "symbol": "DPL9" + }], + "token_name": "Furia Special Edition", + "token_symbol": "DPL9", + "raw": "000201030055b20066e8168ad8f05e82d66a34d19970cfb1861281735215cdd84744d8420100694630440220692c2a95bbb335729520bc1717d9b6da7361ebfc5fb500e6ac45ed4243b4912202201980930659406e06a0f0117a9eb01a29f06faaebf9e4ec58cc96941681043a2e21020377708f22ac1e829c9cfbfd891bb99a47f460bf45d71f4841db404cbefdcb93000000010000544c5168747470733a2f2f697066732e696f2f697066732f516d624874466b596946486e5774457a6e6d354451474d534e4a67704c4c577837513650717470316f4362425a702f6d657461646174612e6a736f6eac0000000281001976a91462950b96bde227ab196cfdd821f80be767dab27788ac0000000101001976a91462950b96bde227ab196cfdd821f80be767dab27788ac01154675726961205370656369616c2045646974696f6e0444504c39402000000000000060eda55a020055b20066e8168ad8f05e82d66a34d19970cfb1861281735215cdd84744d84200bb42880bd1183ce34df2185d1431f531a0a95af3556e368fa72e462edf7a9f00000185" + }, + "meta": { + "hash": "0055c424b9038b0a8888b574ccdb1933a007fdfc15b91a4b38a48cc883b540bf", + "spent_outputs": [ + [ + 0, + [] + ], + [ + 1, + [] + ], + [ + 2, + [] + ] + ], + "received_by": [], + "children": [], + "conflict_with": [], + "voided_by": [], + "twins": [], + "accumulated_weight": 8.0, + "score": 0, + "height": 0, + "first_block": "000000b17b22dd27fb1205a1f810a2c4d40de1e20af140e001529642c4b173a1", + "validation": "full" + }, + "spent_outputs": {} +}; From 3249fc1927dc10c5a35cfffc9f631d6778ced7e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Abadesso?= Date: Tue, 13 Jul 2021 15:39:04 -0300 Subject: [PATCH 2/3] refactor: ignoring output instead of whole transaction --- src/utils.ts | 45 +++++++++++++++------------------------------ test/utils.test.ts | 8 +++++--- 2 files changed, 20 insertions(+), 33 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index deb15748..c9709403 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -138,38 +138,29 @@ export const recursivelyDownloadTx = async (blockId: string, txIds: string[] = [ !data.has(parent) }); - let newData; - - // We need to validate that the outputs scripts have been parsed successfully - if (validateTxOutputs(parsedTx)) { - newData = data.set(parsedTx.txId, parsedTx) - } else { - // Ignore the current transaction (but not its parents) - logger.warn(`Ignoring tx ${parsedTx.txId} because it has invalid output(s)`); - newData = data; - } + const newData = data.set(parsedTx.txId, cleanInvalidOutputs(parsedTx)); return recursivelyDownloadTx(blockId, [...txIds, ...newParents], newData); }; /** - * Validates a transaction by checking if all tx outputs scripts have been successfully decoded + * Removes invalid tx outputs from the received tx * - * @param tx - `RawTx` to be validated - * @returns True or False depending on transaction validity + * @param tx - `FullTx` to remove the invalid outputs */ -export const validateTxOutputs = (tx: FullTx): boolean => { - for (let i = 0; i < tx.outputs.length; i++) { - /* get will not crash if decoded is undefined and isNil will check for - * both null and undefined - */ - if (isNil(get(tx.outputs[i], 'decoded.type'))) { - return false; +export const cleanInvalidOutputs = (tx: FullTx) => ({ + ...tx, + // Filter outputs that we can't handle (script was unable to be decoded) + outputs: tx.outputs.filter((output) => { + const validDecoded = !isNil(get(output, 'decoded.type')); + + if (!validDecoded) { + logger.warn(`Ignoring tx output from tx ${tx.txId} as script couldn't be decoded.`); } - } - return true; -}; + return validDecoded; + }), +}); /** * Prepares a transaction to be sent to the wallet-service `onNewTxRequest` @@ -342,13 +333,7 @@ export async function* syncLatestMempool(): AsyncGenerator { return; } - - if (!validateTxOutputs(tx)) { - logger.warn(`Ignoring tx ${tx.txId} because it has invalid output(s)`); - continue; - } - - const preparedTx: PreparedTx = prepareTx(tx); + const preparedTx: PreparedTx = prepareTx(cleanInvalidOutputs(tx)); try { const sendTxResponse: ApiResponse = await sendTx(preparedTx); diff --git a/test/utils.test.ts b/test/utils.test.ts index d77930d4..79be568f 100644 --- a/test/utils.test.ts +++ b/test/utils.test.ts @@ -25,7 +25,7 @@ import { import { prepareTx, parseTx, - validateTxOutputs, + cleanInvalidOutputs, } from '../src/utils'; import * as Utils from '../src/utils'; import * as FullNode from '../src/api/fullnode'; @@ -266,11 +266,13 @@ test('prepareTx on a CREATE_TOKEN tx should have token_name and token_symbol', a expect(preparedTx.token_symbol).toStrictEqual('XCN'); }, 500); -test('validateTxOutputs on NFT transaction', async () => { +test('cleanInvalidOutputs on NFT transaction', async () => { expect.hasAssertions(); const { tx } = MOCK_NFT_TX; const parsedTx = parseTx(tx); - expect(validateTxOutputs(parsedTx)).toStrictEqual(false); + const cleanedTx = cleanInvalidOutputs(parsedTx); + + expect(cleanedTx.outputs.length).toStrictEqual(2); }); From 65d9a5a613fe35fc73c0c1fee05f83f74c30b324 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Abadesso?= Date: Thu, 15 Jul 2021 11:46:50 -0300 Subject: [PATCH 3/3] chore: logging ignored tx output index --- src/utils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index c9709403..fdea37f8 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -151,11 +151,11 @@ export const recursivelyDownloadTx = async (blockId: string, txIds: string[] = [ export const cleanInvalidOutputs = (tx: FullTx) => ({ ...tx, // Filter outputs that we can't handle (script was unable to be decoded) - outputs: tx.outputs.filter((output) => { + outputs: tx.outputs.filter((output, index) => { const validDecoded = !isNil(get(output, 'decoded.type')); if (!validDecoded) { - logger.warn(`Ignoring tx output from tx ${tx.txId} as script couldn't be decoded.`); + logger.warn(`Ignoring tx output with index ${index} from tx ${tx.txId} as script couldn't be decoded.`); } return validDecoded;