From c7ca8c516e86838a09afb72bdc76f7c0af8e9e98 Mon Sep 17 00:00:00 2001 From: Alex Ruzenhack Date: Tue, 17 Sep 2024 21:01:14 +0100 Subject: [PATCH 1/7] refactor: fetchHistory on sagas/nanoContract to avoid iteration suspension --- src/sagas/nanoContract.js | 49 ++++++++++++++++++++------------------- src/utils.js | 12 ---------- 2 files changed, 25 insertions(+), 36 deletions(-) diff --git a/src/sagas/nanoContract.js b/src/sagas/nanoContract.js index 16234ba9e..c014315ad 100644 --- a/src/sagas/nanoContract.js +++ b/src/sagas/nanoContract.js @@ -20,6 +20,8 @@ import { } from 'redux-saga/effects'; import { t } from 'ttag'; import { NanoRequest404Error } from '@hathor/wallet-lib/lib/errors'; +import { getRegisteredNanoContracts, safeEffect } from './helpers'; +import { isWalletServiceEnabled } from './wallet'; import { nanoContractBlueprintInfoFailure, nanoContractBlueprintInfoSuccess, @@ -35,9 +37,7 @@ import { } from '../actions'; import { logger } from '../logger'; import { NANO_CONTRACT_TX_HISTORY_SIZE } from '../constants'; -import { consumeGenerator, getNanoContractFeatureToggle } from '../utils'; -import { getRegisteredNanoContracts, safeEffect } from './helpers'; -import { isWalletServiceEnabled } from './wallet'; +import * as utils from '../utils'; const log = logger('nano-contract-saga'); @@ -55,7 +55,7 @@ export const failureMessage = { }; export function* init() { - const isEnabled = yield select(getNanoContractFeatureToggle); + const isEnabled = yield select(utils.getNanoContractFeatureToggle); if (!isEnabled) { log.debug('Halting nano contract initialization because the feature flag is disabled.'); return; @@ -217,7 +217,7 @@ function* registerNanoContractOnError(error) { * @param {Object} req.wallet Wallet instance from redux state * * @returns {{ - * history: {}; + * history: NcTxHistory; * }} * * @throws {Error} when request code is greater then 399 or when response's success is false @@ -225,6 +225,7 @@ function* registerNanoContractOnError(error) { export async function fetchHistory(req) { const { wallet, + useWalletService, ncId, count, after, @@ -245,28 +246,21 @@ export async function fetchHistory(req) { throw new Error('Failed to fetch nano contract history'); } - /* TODO: Make it run concurrently while guaranting the order. - /* see https://github.com/HathorNetwork/hathor-wallet-mobile/issues/514 - */ - const historyNewestToOldest = new Array(rawHistory.length) - for (let idx = 0; idx < rawHistory.length; idx += 1) { - const rawTx = rawHistory[idx]; - const network = wallet.getNetworkObject(); + const network = wallet.getNetworkObject(); + // As isAddressMine call is async we collect the tasks to avoid + // suspend the iteration with an await. + const isMineTasks = []; + // Translate rawNcTxHistory to NcTxHistory and collect isAddressMine tasks + const historyNewestToOldest = rawHistory.map((rawTx) => { const caller = addressUtils.getAddressFromPubkey(rawTx.nc_pubkey, network).base58; - // XXX: Wallet Service doesn't implement isAddressMine. - // See issue: https://github.com/HathorNetwork/hathor-wallet-lib/issues/732 - // Default to `false` if using Wallet Service. - let isMine = false; - const useWalletService = consumeGenerator(isWalletServiceEnabled()); - if (!useWalletService) { - // eslint-disable-next-line no-await-in-loop - isMine = await wallet.isAddressMine(caller); - } const actions = rawTx.nc_context.actions.map((each) => ({ type: each.type, // 'deposit' or 'withdrawal' uid: each.token_uid, amount: each.amount, })); + // As this is a promise, collect as task to hydrate later. + // This strategy avoids an await suspension. + isMineTasks.push(utils.isAddressMine(wallet, caller, useWalletService)); const tx = { txId: rawTx.hash, @@ -278,11 +272,16 @@ export async function fetchHistory(req) { blueprintId: rawTx.nc_blueprint_id, firstBlock: rawTx.first_block, caller, - isMine, actions, }; - historyNewestToOldest[idx] = tx; - } + return tx; + }); + + // Hydrate ncTxHistory with isMine flag + const isMineResults = await Promise.all(isMineTasks); + isMineResults.forEach((isMine, idx) => { + historyNewestToOldest[idx].isMine = isMine; + }); return { history: historyNewestToOldest }; } @@ -332,9 +331,11 @@ export function* requestHistoryNanoContract({ payload }) { yield put(nanoContractHistoryClean({ ncId })); } + const useWalletService = yield select((state) => state.useWalletService); try { const req = { wallet, + useWalletService, ncId, before, after, diff --git a/src/utils.js b/src/utils.js index 156ea08bf..ef137eaf4 100644 --- a/src/utils.js +++ b/src/utils.js @@ -428,18 +428,6 @@ export const getNanoContractFeatureToggle = (state) => ( */ export const getTimestampFormat = (timestamp) => moment.unix(timestamp).format(t`DD MMM YYYY [•] HH:mm`) -/** - * Extract the result of a generator function when it is done. - */ -export const consumeGenerator = (generator) => { - for (;;) { - const { value, done } = generator.next(); - if (done) { - return value; - } - } -} - /** * Extract all the items of an async iterator/generator. * From 7beefca6959dd048a19e9b54256b3aeeb46095d8 Mon Sep 17 00:00:00 2001 From: Alex Ruzenhack Date: Tue, 17 Sep 2024 21:07:19 +0100 Subject: [PATCH 2/7] docs: improve docstring --- src/sagas/nanoContract.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sagas/nanoContract.js b/src/sagas/nanoContract.js index c014315ad..b704fc4c6 100644 --- a/src/sagas/nanoContract.js +++ b/src/sagas/nanoContract.js @@ -215,6 +215,7 @@ function* registerNanoContractOnError(error) { * @param {string} req.before Transaction hash to start to get newer items * @param {string} req.after Transaction hash to start to get older items * @param {Object} req.wallet Wallet instance from redux state + * @param {boolean} req.useWalletService A flag that determines if wallet service is in use * * @returns {{ * history: NcTxHistory; From 93c314d35517de2c07cad993063e4063e91920ef Mon Sep 17 00:00:00 2001 From: Alex Ruzenhack Date: Thu, 19 Sep 2024 21:58:17 +0100 Subject: [PATCH 3/7] refactor: avoid wildcard import and change local variable name --- src/sagas/nanoContract.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/sagas/nanoContract.js b/src/sagas/nanoContract.js index b704fc4c6..511436ff4 100644 --- a/src/sagas/nanoContract.js +++ b/src/sagas/nanoContract.js @@ -37,7 +37,7 @@ import { } from '../actions'; import { logger } from '../logger'; import { NANO_CONTRACT_TX_HISTORY_SIZE } from '../constants'; -import * as utils from '../utils'; +import { getNanoContractFeatureToggle, isAddressMine } from '../utils'; const log = logger('nano-contract-saga'); @@ -55,7 +55,7 @@ export const failureMessage = { }; export function* init() { - const isEnabled = yield select(utils.getNanoContractFeatureToggle); + const isEnabled = yield select(getNanoContractFeatureToggle); if (!isEnabled) { log.debug('Halting nano contract initialization because the feature flag is disabled.'); return; @@ -101,13 +101,13 @@ export function* registerNanoContract({ payload }) { // XXX: Wallet Service doesn't implement isAddressMine. // See issue: https://github.com/HathorNetwork/hathor-wallet-lib/issues/732 // Default to `false` if using Wallet Service. - let isAddressMine = false; + let isAddrMine = false; const useWalletService = yield call(isWalletServiceEnabled); if (!useWalletService) { - isAddressMine = yield call([wallet, wallet.isAddressMine], address); + isAddrMine = yield call([wallet, wallet.isAddressMine], address); } - if (!isAddressMine) { + if (!isAddrMine) { log.debug('Fail registering Nano Contract because address do not belongs to this wallet.'); yield put(nanoContractRegisterFailure(failureMessage.addressNotMine)); return; @@ -158,6 +158,7 @@ export function* registerNanoContract({ payload }) { // emit action NANOCONTRACT_REGISTER_SUCCESS with feedback to user yield put(nanoContractRegisterSuccess({ entryKey: ncId, entryValue: nc, hasFeedback: true })); } + /** * Effect invoked by safeEffect if an unexpected error occurs. * @@ -261,7 +262,7 @@ export async function fetchHistory(req) { })); // As this is a promise, collect as task to hydrate later. // This strategy avoids an await suspension. - isMineTasks.push(utils.isAddressMine(wallet, caller, useWalletService)); + isMineTasks.push(isAddressMine(wallet, caller, useWalletService)); const tx = { txId: rawTx.hash, From 5824541368b7995324630dd6af8b700ebe2c1bd8 Mon Sep 17 00:00:00 2001 From: Alex Ruzenhack Date: Thu, 19 Sep 2024 22:03:36 +0100 Subject: [PATCH 4/7] review: apply suggestion on documentation --- src/sagas/nanoContract.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sagas/nanoContract.js b/src/sagas/nanoContract.js index 511436ff4..0b4bd0785 100644 --- a/src/sagas/nanoContract.js +++ b/src/sagas/nanoContract.js @@ -249,8 +249,8 @@ export async function fetchHistory(req) { } const network = wallet.getNetworkObject(); - // As isAddressMine call is async we collect the tasks to avoid - // suspend the iteration with an await. + // As isAddressMine call is async, we should collect the tasks to avoid + // suspending the iteration with an await. const isMineTasks = []; // Translate rawNcTxHistory to NcTxHistory and collect isAddressMine tasks const historyNewestToOldest = rawHistory.map((rawTx) => { From 492af4ba5ccd9d23f2b73c76c622108e952b6040 Mon Sep 17 00:00:00 2001 From: Alex Ruzenhack Date: Fri, 20 Sep 2024 17:18:04 +0100 Subject: [PATCH 5/7] refactor: remove task collect and make it a single list of promises --- src/sagas/nanoContract.js | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/sagas/nanoContract.js b/src/sagas/nanoContract.js index 0b4bd0785..6196ef2bd 100644 --- a/src/sagas/nanoContract.js +++ b/src/sagas/nanoContract.js @@ -249,20 +249,16 @@ export async function fetchHistory(req) { } const network = wallet.getNetworkObject(); - // As isAddressMine call is async, we should collect the tasks to avoid - // suspending the iteration with an await. - const isMineTasks = []; - // Translate rawNcTxHistory to NcTxHistory and collect isAddressMine tasks - const historyNewestToOldest = rawHistory.map((rawTx) => { + // Translate rawNcTxHistory to NcTxHistory + // Prouce a list ordered from newest to oldest + const historyTransformer = rawHistory.map(async (rawTx) => { const caller = addressUtils.getAddressFromPubkey(rawTx.nc_pubkey, network).base58; const actions = rawTx.nc_context.actions.map((each) => ({ type: each.type, // 'deposit' or 'withdrawal' uid: each.token_uid, amount: each.amount, })); - // As this is a promise, collect as task to hydrate later. - // This strategy avoids an await suspension. - isMineTasks.push(isAddressMine(wallet, caller, useWalletService)); + const isMine = await isAddressMine(wallet, caller, useWalletService); const tx = { txId: rawTx.hash, @@ -275,17 +271,12 @@ export async function fetchHistory(req) { firstBlock: rawTx.first_block, caller, actions, + isMine, }; return tx; }); - // Hydrate ncTxHistory with isMine flag - const isMineResults = await Promise.all(isMineTasks); - isMineResults.forEach((isMine, idx) => { - historyNewestToOldest[idx].isMine = isMine; - }); - - return { history: historyNewestToOldest }; + return { history: await Promise.all(historyTransformer) }; } /** From 8d45c876754f8a539d44c625caca4abd94299c0b Mon Sep 17 00:00:00 2001 From: Alex Ruzenhack Date: Thu, 26 Sep 2024 20:24:46 +0100 Subject: [PATCH 6/7] review: adopt name suggestion --- src/sagas/nanoContract.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sagas/nanoContract.js b/src/sagas/nanoContract.js index 6196ef2bd..b52decafe 100644 --- a/src/sagas/nanoContract.js +++ b/src/sagas/nanoContract.js @@ -251,7 +251,7 @@ export async function fetchHistory(req) { const network = wallet.getNetworkObject(); // Translate rawNcTxHistory to NcTxHistory // Prouce a list ordered from newest to oldest - const historyTransformer = rawHistory.map(async (rawTx) => { + const transformedTxHistory = rawHistory.map(async (rawTx) => { const caller = addressUtils.getAddressFromPubkey(rawTx.nc_pubkey, network).base58; const actions = rawTx.nc_context.actions.map((each) => ({ type: each.type, // 'deposit' or 'withdrawal' @@ -276,7 +276,7 @@ export async function fetchHistory(req) { return tx; }); - return { history: await Promise.all(historyTransformer) }; + return { history: await Promise.all(transformedTxHistory) }; } /** From d929a2c03d2e06573d35a3879af181eaffae7a41 Mon Sep 17 00:00:00 2001 From: Alex Ruzenhack Date: Thu, 26 Sep 2024 21:04:33 +0100 Subject: [PATCH 7/7] refactor: move isAddressMine to nanoContract saga --- src/sagas/nanoContract.js | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/sagas/nanoContract.js b/src/sagas/nanoContract.js index b52decafe..cca332e59 100644 --- a/src/sagas/nanoContract.js +++ b/src/sagas/nanoContract.js @@ -37,7 +37,7 @@ import { } from '../actions'; import { logger } from '../logger'; import { NANO_CONTRACT_TX_HISTORY_SIZE } from '../constants'; -import { getNanoContractFeatureToggle, isAddressMine } from '../utils'; +import { getNanoContractFeatureToggle } from '../utils'; const log = logger('nano-contract-saga'); @@ -54,6 +54,25 @@ export const failureMessage = { nanoContractHistoryFailure: t`Error while trying to download Nano Contract transactions history.`, }; +/** + * Call the async wallet method `isAddressMine` considering the type of wallet. + * + * @param {Object} wallet A wallet instance + * @param {string} address A wallet address to check + * @param {boolean} useWalletService A flag that determines if wallet service is in use + */ +export const isAddressMine = async (wallet, address, useWalletService) => { + // XXX: Wallet Service doesn't implement isAddressMine. + // See issue: https://github.com/HathorNetwork/hathor-wallet-lib/issues/732 + // Default to `false` if using Wallet Service. + if (useWalletService) { + return false; + } + + const isMine = await wallet.isAddressMine(address); + return isMine; +}; + export function* init() { const isEnabled = yield select(getNanoContractFeatureToggle); if (!isEnabled) {