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.

Sign up for GitHub

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

Jump to bottom

refactor: fetchHistory on sagas/nanoContract to avoid iteration suspension #569

Merged
merged 7 commits into from
Sep 26, 2024
Merged

refactor: fetchHistory on sagas/nanoContract to avoid iteration suspension #569

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c7ca8c5
refactor: fetchHistory on sagas/nanoContract to avoid iteration suspe…
alexruzenhack Sep 17, 2024
7beefca
docs: improve docstring
alexruzenhack Sep 17, 2024
93c314d
refactor: avoid wildcard import and change local variable name
alexruzenhack Sep 19, 2024
5824541
review: apply suggestion on documentation
alexruzenhack Sep 19, 2024
492af4b
refactor: remove task collect and make it a single list of promises
alexruzenhack Sep 20, 2024
8d45c87
review: adopt name suggestion
alexruzenhack Sep 26, 2024
d929a2c
refactor: move isAddressMine to nanoContract saga
alexruzenhack Sep 26, 2024
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
67 changes: 40 additions & 27 deletions src/sagas/nanoContract.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 { getNanoContractFeatureToggle } from '../utils';

const log = logger('nano-contract-saga');

Expand All @@ -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) {
Expand Down Expand Up @@ -101,13 +120,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;
Expand Down Expand Up @@ -158,6 +177,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.
*
Expand Down Expand Up @@ -215,16 +235,18 @@ 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: {};
* history: NcTxHistory;
* }}
*
* @throws {Error} when request code is greater then 399 or when response's success is false
*/
export async function fetchHistory(req) {
const {
wallet,
useWalletService,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[note] Receive it from the saga effect is more efficient. More efficient than that would be a flag in the wallet instance to indicate the kind of wallet, wither fullnode or walletService. Do you think should add this flag in the wallet interface?

ncId,
count,
after,
Expand All @@ -245,28 +267,17 @@ 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();
// Translate rawNcTxHistory to NcTxHistory
// Prouce a list ordered from newest to oldest
const transformedTxHistory = rawHistory.map(async (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,
}));
const isMine = await isAddressMine(wallet, caller, useWalletService);

const tx = {
txId: rawTx.hash,
Expand All @@ -278,13 +289,13 @@ export async function fetchHistory(req) {
blueprintId: rawTx.nc_blueprint_id,
firstBlock: rawTx.first_block,
caller,
isMine,
actions,
isMine,
};
historyNewestToOldest[idx] = tx;
}
return tx;
});

return { history: historyNewestToOldest };
return { history: await Promise.all(transformedTxHistory) };
}

/**
Expand Down Expand Up @@ -332,9 +343,11 @@ export function* requestHistoryNanoContract({ payload }) {
yield put(nanoContractHistoryClean({ ncId }));
}

const useWalletService = yield select((state) => state.useWalletService);
try {
const req = {
wallet,
useWalletService,
ncId,
before,
after,
Expand Down
12 changes: 0 additions & 12 deletions src/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
Loading