From cb3c9ee7505b4f75a3b4d7dcf386ddca9c6f213a Mon Sep 17 00:00:00 2001 From: Umair Sarfraz Date: Thu, 27 Dec 2018 23:57:22 +0500 Subject: [PATCH 1/2] Refactor shouldAllowSendingToAddress implementation - Rename shouldAllowSendingToAddress to isAnyAddressSpent - Returns Promise if any provided address is spent --- .../__tests__/libs/iota/addresses.spec.js | 107 ++++++++++++++++++ src/shared/actions/transfers.js | 24 ++-- src/shared/libs/iota/addresses.js | 14 +-- 3 files changed, 123 insertions(+), 22 deletions(-) diff --git a/src/shared/__tests__/libs/iota/addresses.spec.js b/src/shared/__tests__/libs/iota/addresses.spec.js index 1fdb425303..88e1588940 100644 --- a/src/shared/__tests__/libs/iota/addresses.spec.js +++ b/src/shared/__tests__/libs/iota/addresses.spec.js @@ -1467,4 +1467,111 @@ describe('libs: iota/addresses', () => { expect(result).to.eql([true, false, false]); }); }); + + describe('#isAnyAddressSpent', () => { + let addresses; + + before(() => { + addresses = map(['U', 'A', 'S', '9'], (char) => char.repeat(81)); + }); + + describe('when all addresses are unspent', () => { + beforeEach(() => { + nock('http://localhost:14265', { + reqheaders: { + 'Content-Type': 'application/json', + 'X-IOTA-API-Version': IRI_API_VERSION, + }, + }) + .filteringRequestBody(() => '*') + .persist() + .post('/', '*') + .reply(200, (_, body) => { + const { addresses, command } = body; + + if (command === 'wereAddressesSpentFrom') { + return { states: map(addresses, () => false) }; + } + + return {}; + }); + }); + + afterEach(() => { + nock.cleanAll(); + }); + + it('should return false', () => { + return addressesUtils + .isAnyAddressSpent()(addresses) + .then((isSpent) => expect(isSpent).to.equal(false)); + }); + }); + + describe('when all addresses are spent', () => { + beforeEach(() => { + nock('http://localhost:14265', { + reqheaders: { + 'Content-Type': 'application/json', + 'X-IOTA-API-Version': IRI_API_VERSION, + }, + }) + .filteringRequestBody(() => '*') + .persist() + .post('/', '*') + .reply(200, (_, body) => { + const { addresses, command } = body; + + if (command === 'wereAddressesSpentFrom') { + return { states: map(addresses, () => true) }; + } + + return {}; + }); + }); + + afterEach(() => { + nock.cleanAll(); + }); + + it('should return true', () => { + return addressesUtils + .isAnyAddressSpent()(addresses) + .then((isSpent) => expect(isSpent).to.equal(true)); + }); + }); + + describe('when some addresses are spent', () => { + beforeEach(() => { + nock('http://localhost:14265', { + reqheaders: { + 'Content-Type': 'application/json', + 'X-IOTA-API-Version': IRI_API_VERSION, + }, + }) + .filteringRequestBody(() => '*') + .persist() + .post('/', '*') + .reply(200, (_, body) => { + const { addresses, command } = body; + + if (command === 'wereAddressesSpentFrom') { + return { states: map(addresses, (_, idx) => idx % 2 === 0) }; + } + + return {}; + }); + }); + + afterEach(() => { + nock.cleanAll(); + }); + + it('should return true', () => { + return addressesUtils + .isAnyAddressSpent()(addresses) + .then((isSpent) => expect(isSpent).to.equal(true)); + }); + }); + }); }); diff --git a/src/shared/actions/transfers.js b/src/shared/actions/transfers.js index cba5fa5ee5..7377dab6da 100644 --- a/src/shared/actions/transfers.js +++ b/src/shared/actions/transfers.js @@ -55,11 +55,7 @@ import { markBundleBroadcastStatusComplete, markBundleBroadcastStatusPending, } from './accounts'; -import { - shouldAllowSendingToAddress, - getAddressesUptoRemainder, - categoriseAddressesBySpentStatus, -} from '../libs/iota/addresses'; +import { isAnyAddressSpent, getAddressesUptoRemainder, categoriseAddressesBySpentStatus } from '../libs/iota/addresses'; import { getStartingSearchIndexToPrepareInputs, getUnspentInputs } from '../libs/iota/inputs'; import { generateAlert, @@ -434,17 +430,17 @@ export const makeTransaction = (seedStore, receiveAddress, value, message, accou // Progressbar step => (Validating receive address) dispatch(setNextStepAsActive()); - // Make sure that the address a user is about to send to is not already used. - return shouldAllowSendingToAddress()([address]) - .then((shouldAllowSending) => { - if (shouldAllowSending) { - // Progressbar step => (Syncing account) - dispatch(setNextStepAsActive()); - - return syncAccount()(accountState, seedStore); + // Make sure that the address a user is about to send to is not already spent. + return isAnyAddressSpent()([address]) + .then((isSpent) => { + if (isSpent) { + throw new Error(Errors.KEY_REUSE); } - throw new Error(Errors.KEY_REUSE); + // Progressbar step => (Syncing account) + dispatch(setNextStepAsActive()); + + return syncAccount()(accountState, seedStore); }) .then((newState) => { // Assign latest account but do not update the local store yet. diff --git a/src/shared/libs/iota/addresses.js b/src/shared/libs/iota/addresses.js index a036927f57..71cb17aff4 100644 --- a/src/shared/libs/iota/addresses.js +++ b/src/shared/libs/iota/addresses.js @@ -405,19 +405,17 @@ export const filterSpentAddresses = (provider) => (inputs, addressData, normalis }; /** - * Communicates with ledger and checks if the addresses are spent from. + * Communicates with ledger and checks if the any address is spent. * - * @method shouldAllowSendingToAddress + * @method isAnyAddressSpent * @param {string} [provider] * * @returns {function(array): Promise} **/ -export const shouldAllowSendingToAddress = (provider) => (addresses) => { - return wereAddressesSpentFromAsync(provider)(addresses).then((wereSpent) => { - const spentAddresses = filter(addresses, (address, idx) => wereSpent[idx]); - - return !spentAddresses.length; - }); +export const isAnyAddressSpent = (provider) => (addresses) => { + return wereAddressesSpentFromAsync(provider)(addresses).then((spendStatuses) => + some(spendStatuses, (spendStatus) => spendStatus === true), + ); }; /** From a0df118abe88fe2b0753422bb7d9a5719ec14237 Mon Sep 17 00:00:00 2001 From: Umair Sarfraz Date: Fri, 28 Dec 2018 13:07:42 +0500 Subject: [PATCH 2/2] Add additional spend status check before broadcast (value transactions) --- src/mobile/src/libs/progressSteps.js | 1 + .../__tests__/actions/transfers.spec.js | 4 ++-- src/shared/actions/transfers.js | 21 +++++++++++++++++++ src/shared/containers/wallet/Send.js | 1 + src/shared/locales/en/translation.json | 1 + 5 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/mobile/src/libs/progressSteps.js b/src/mobile/src/libs/progressSteps.js index 4a0ff13607..6d5c6099ac 100644 --- a/src/mobile/src/libs/progressSteps.js +++ b/src/mobile/src/libs/progressSteps.js @@ -6,6 +6,7 @@ export default { 'progressSteps:preparingTransfers', 'progressSteps:gettingTransactionsToApprove', 'progressSteps:proofOfWork', + 'progressSteps:validatingTransactionAddresses', 'progressSteps:broadcasting', ], zeroValueTransaction: [ diff --git a/src/shared/__tests__/actions/transfers.spec.js b/src/shared/__tests__/actions/transfers.spec.js index 74551e2568..b313314a73 100644 --- a/src/shared/__tests__/actions/transfers.spec.js +++ b/src/shared/__tests__/actions/transfers.spec.js @@ -383,7 +383,7 @@ describe('actions: transfers', () => { }); describe('when transaction is successful', () => { - it('should create eight actions of type IOTA/PROGRESS/SET_NEXT_STEP_AS_ACTIVE', () => { + it('should create nine actions of type IOTA/PROGRESS/SET_NEXT_STEP_AS_ACTIVE', () => { const wereAddressesSpentFrom = sinon.stub(iota.api, 'wereAddressesSpentFrom').yields(null, [false]); const store = mockStore({ accounts }); @@ -425,7 +425,7 @@ describe('actions: transfers', () => { .getActions() .map((action) => action.type) .filter((type) => type === 'IOTA/PROGRESS/SET_NEXT_STEP_AS_ACTIVE').length, - ).to.equal(8); + ).to.equal(9); syncAccountAfterSpending.restore(); syncAccount.restore(); diff --git a/src/shared/actions/transfers.js b/src/shared/actions/transfers.js index 7377dab6da..e6cc4a3c82 100644 --- a/src/shared/actions/transfers.js +++ b/src/shared/actions/transfers.js @@ -10,6 +10,7 @@ import some from 'lodash/some'; import size from 'lodash/size'; import every from 'lodash/every'; import includes from 'lodash/includes'; +import uniq from 'lodash/uniq'; import { iota } from '../libs/iota'; import { replayBundleAsync, @@ -649,6 +650,26 @@ export const makeTransaction = (seedStore, receiveAddress, value, message, accou }); }); }) + // Re-check spend statuses of all addresses in bundle + .then(({ trytes, transactionObjects }) => { + // Skip this check if it's a zero value transaction + if (isZeroValue) { + return Promise.resolve({ trytes, transactionObjects }); + } + + // Progressbar step => (Validating transaction addresses) + dispatch(setNextStepAsActive()); + + const addresses = uniq(map(transactionObjects, (transaction) => transaction.address)); + + return isAnyAddressSpent()(addresses).then((isSpent) => { + if (isSpent) { + throw new Error(Errors.KEY_REUSE); + } + + return { trytes, transactionObjects }; + }); + }) .then(({ trytes, transactionObjects }) => { cached.trytes = trytes; cached.transactionObjects = transactionObjects; diff --git a/src/shared/containers/wallet/Send.js b/src/shared/containers/wallet/Send.js index 1958024ad9..9972a15066 100644 --- a/src/shared/containers/wallet/Send.js +++ b/src/shared/containers/wallet/Send.js @@ -82,6 +82,7 @@ export default function withSendData(SendComponent) { t('progressSteps:preparingTransfers'), t('progressSteps:gettingTransactionsToApprove'), t('progressSteps:proofOfWork'), + t('progressSteps:validatingTransactionAddresses'), t('progressSteps:broadcasting'), ]; diff --git a/src/shared/locales/en/translation.json b/src/shared/locales/en/translation.json index 4736067ed2..3f2f378797 100644 --- a/src/shared/locales/en/translation.json +++ b/src/shared/locales/en/translation.json @@ -705,6 +705,7 @@ "progressSteps": { "checkingNodeHealth": "Checking node health", "validatingReceiveAddress": "Validating receive address", + "validatingTransactionAddresses": "Validating transaction addresses", "syncingAccount": "Syncing account", "preparingInputs": "Preparing inputs", "preparingTransfers": "Preparing transfers",