Skip to content

Commit

Permalink
fix: better randomInt, and arrayShuffle
Browse files Browse the repository at this point in the history
  • Loading branch information
peter-sanderson committed Oct 14, 2024
1 parent 1a9ead0 commit ee18606
Show file tree
Hide file tree
Showing 12 changed files with 38 additions and 28 deletions.
4 changes: 2 additions & 2 deletions packages/coinjoin/src/client/round/endedRound.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { enumUtils, getRandomNumberInRange } from '@trezor/utils';
import { enumUtils, getWeakRandomNumberInRange } from '@trezor/utils';

import type { CoinjoinRound, CoinjoinRoundOptions } from '../CoinjoinRound';
import { EndRoundState, WabiSabiProtocolErrorCode } from '../../enums';
Expand Down Expand Up @@ -56,7 +56,7 @@ export const ended = (round: CoinjoinRound, { logger, network }: CoinjoinRoundOp
// repeated input-registration will tell if they are really banned,
// make sure that addresses registered in round are recycled (reset Infinity sentence)
const minute = 60 * 1000;
const sentenceEnd = getRandomNumberInRange(5 * minute, 10 * minute);
const sentenceEnd = getWeakRandomNumberInRange(5 * minute, 10 * minute);
[...inputs, ...addresses].forEach(vinvout =>
prison.detain(vinvout, {
sentenceEnd,
Expand Down
4 changes: 2 additions & 2 deletions packages/coinjoin/src/client/round/inputRegistration.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getRandomNumberInRange } from '@trezor/utils';
import { getWeakRandomNumberInRange } from '@trezor/utils';

import * as coordinator from '../coordinator';
import * as middleware from '../middleware';
Expand Down Expand Up @@ -56,7 +56,7 @@ const registerInput = async (
// setup random delay for registration request. we want each input to be registered in different time as different TOR identity
// note that this may cause that the input will not be registered if phase change before expected deadline
const deadline = round.phaseDeadline - Date.now() - ROUND_SELECTION_REGISTRATION_OFFSET;
const delay = deadline > 0 ? getRandomNumberInRange(0, deadline) : 0;
const delay = deadline > 0 ? getWeakRandomNumberInRange(0, deadline) : 0;
logger.info(
`Trying to register ~~${input.outpoint}~~ to ~~${round.id}~~ with delay ${delay}ms and deadline ${round.phaseDeadline}`,
);
Expand Down
4 changes: 2 additions & 2 deletions packages/coinjoin/src/utils/roundUtils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { bufferutils, Transaction, Network } from '@trezor/utxo-lib';
import { getRandomNumberInRange } from '@trezor/utils';
import { getWeakRandomNumberInRange } from '@trezor/utils';

import {
COORDINATOR_FEE_RATE_FALLBACK,
Expand Down Expand Up @@ -88,7 +88,7 @@ export const scheduleDelay = (
// and at most 1 sec before the calculated max (so there's room for randomness)
const min = clamp(minimumDelay, 0, max - 1000);

return getRandomNumberInRange(min, max);
return getWeakRandomNumberInRange(min, max);
};

// NOTE: deadlines are not accurate. phase may change earlier
Expand Down
4 changes: 2 additions & 2 deletions packages/coinjoin/tests/client/CoinjoinRound.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ describe(`CoinjoinRound`, () => {

it('onPhaseChange lock cool off resolved', async () => {
const delayMock = jest
.spyOn(trezorUtils, 'getRandomNumberInRange')
.spyOn(trezorUtils, 'getWeakRandomNumberInRange')
.mockImplementation(() => 800);

const constantsMock = jest
Expand Down Expand Up @@ -396,7 +396,7 @@ describe(`CoinjoinRound`, () => {

it('unregisterAccount when round is locked', async () => {
const delayMock = jest
.spyOn(trezorUtils, 'getRandomNumberInRange')
.spyOn(trezorUtils, 'getWeakRandomNumberInRange')
.mockImplementation(() => 800);

const constantsMock = jest
Expand Down
8 changes: 4 additions & 4 deletions packages/coinjoin/tests/client/transactionSigning.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { networks } from '@trezor/utxo-lib';
import { getRandomNumberInRange } from '@trezor/utils';
import { getWeakRandomNumberInRange } from '@trezor/utils';

import { transactionSigning } from '../../src/client/round/transactionSigning';
import { createServer } from '../mocks/server';
Expand Down Expand Up @@ -529,7 +529,7 @@ describe('transactionSigning signature delay', () => {
);

// signature is sent in range 17-67 sec. (resolve time is less than 50 sec TX_SIGNING_DELAY)
expect(getRandomNumberInRange).toHaveBeenLastCalledWith(17000, 67000);
expect(getWeakRandomNumberInRange).toHaveBeenLastCalledWith(17000, 67000);
expect(response.isSignedSuccessfully()).toBe(true);
});

Expand Down Expand Up @@ -558,7 +558,7 @@ describe('transactionSigning signature delay', () => {
);

// signature is sent in range 0-46.21 sec. (resolve time is greater than 50 sec of TX_SIGNING_DELAY)
expect(getRandomNumberInRange).toHaveBeenLastCalledWith(0, 46210);
expect(getWeakRandomNumberInRange).toHaveBeenLastCalledWith(0, 46210);
expect(response.isSignedSuccessfully()).toBe(true);
});

Expand Down Expand Up @@ -588,7 +588,7 @@ describe('transactionSigning signature delay', () => {
);

// signature is sent in default range 0-1 sec.
expect(getRandomNumberInRange).toHaveBeenLastCalledWith(0, 1000);
expect(getWeakRandomNumberInRange).toHaveBeenLastCalledWith(0, 1000);
expect(response.isSignedSuccessfully()).toBe(true);
});
});
20 changes: 10 additions & 10 deletions packages/coinjoin/tests/utils/roundUtils.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getRandomNumberInRange } from '@trezor/utils';
import { getWeakRandomNumberInRange } from '@trezor/utils';

import {
getCommitmentData,
Expand Down Expand Up @@ -150,38 +150,38 @@ describe('roundUtils', () => {

// default (no min, no max) range 0-10 sec.
resultInRange(scheduleDelay(60000), 0, 10000);
expect(getRandomNumberInRange).toHaveBeenLastCalledWith(0, 10000);
expect(getWeakRandomNumberInRange).toHaveBeenLastCalledWith(0, 10000);

// range 3-10sec.
resultInRange(scheduleDelay(20000, 3000), 3000, 10000);
expect(getRandomNumberInRange).toHaveBeenLastCalledWith(3000, 10000);
expect(getWeakRandomNumberInRange).toHaveBeenLastCalledWith(3000, 10000);

// deadlineOffset < 0, range 0-1 sec.
resultInRange(scheduleDelay(1000, 3000), 0, 1000);
expect(getRandomNumberInRange).toHaveBeenLastCalledWith(0, 1000);
expect(getWeakRandomNumberInRange).toHaveBeenLastCalledWith(0, 1000);

// deadline < min, range 9-10 sec.
resultInRange(scheduleDelay(60000, 61000), 9000, 10000);
expect(getRandomNumberInRange).toHaveBeenLastCalledWith(9000, 10000);
expect(getWeakRandomNumberInRange).toHaveBeenLastCalledWith(9000, 10000);

// deadline < min && deadline < max, range 49-50 sec.
resultInRange(scheduleDelay(60000, 61000, 62000), 49000, 50000);
expect(getRandomNumberInRange).toHaveBeenLastCalledWith(49000, 50000);
expect(getWeakRandomNumberInRange).toHaveBeenLastCalledWith(49000, 50000);

// deadline > min && deadline < max, range 3-20 sec.
resultInRange(scheduleDelay(30000, 3000, 50000), 3000, 20000);
expect(getRandomNumberInRange).toHaveBeenLastCalledWith(3000, 20000);
expect(getWeakRandomNumberInRange).toHaveBeenLastCalledWith(3000, 20000);

// min < 0 && deadline < max && deadlineOffset > 0, range 0-2.5 sec.
resultInRange(scheduleDelay(12500, -3000, 50000), 0, 2500);
expect(getRandomNumberInRange).toHaveBeenLastCalledWith(0, 2500);
expect(getWeakRandomNumberInRange).toHaveBeenLastCalledWith(0, 2500);

// min < 0 && max < 0 && deadlineOffset > 0, range 0-1 sec.
resultInRange(scheduleDelay(12500, -10000, -5000), 0, 1000);
expect(getRandomNumberInRange).toHaveBeenLastCalledWith(0, 1000);
expect(getWeakRandomNumberInRange).toHaveBeenLastCalledWith(0, 1000);

// min < 0 && max < 0 && deadlineOffset < 0, range 0-1 sec.
resultInRange(scheduleDelay(7500, -10000, -5000), 0, 1000);
expect(getRandomNumberInRange).toHaveBeenLastCalledWith(0, 1000);
expect(getWeakRandomNumberInRange).toHaveBeenLastCalledWith(0, 1000);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useState } from 'react';
import styled from 'styled-components';
import { Card, Column, variables } from '@trezor/components';
import { Translation } from 'src/components/suite';
import { getRandomNumberInRange } from '@trezor/utils';
import { getWeakRandomNumberInRange } from '@trezor/utils';
import { typography } from '@trezor/theme';

const NoResults = styled.div`
Expand Down Expand Up @@ -46,7 +46,7 @@ const getTip = (num: number) => {
};

export const NoSearchResults = () => {
const [tip] = useState(getRandomNumberInRange(1, 10));
const [tip] = useState(getWeakRandomNumberInRange(1, 10));

return (
<Card>
Expand Down
4 changes: 3 additions & 1 deletion packages/utils/src/arrayShuffle.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { getRandomInt } from './randomInt';

/**
* Implementation of the Fisher-Yates shuffle algorithm.
* The algorithm produces an unbiased permutation: every permutation is equally likely.
Expand All @@ -8,7 +10,7 @@
export const arrayShuffle = <T>(array: readonly T[]): T[] => {
const shuffled = array.slice();
for (let i = shuffled.length - 1; i > 0; i--) {
const j = Math.floor(Math.random() * (i + 1));
const j = getRandomInt(0, i);
[shuffled[i], shuffled[j]] = [shuffled[j], shuffled[i]];
}

Expand Down
2 changes: 0 additions & 2 deletions packages/utils/src/getRandomNumberInRange.ts

This file was deleted.

2 changes: 2 additions & 0 deletions packages/utils/src/getWeakRandomNumberInRange.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export const getWeakRandomNumberInRange = (min: number, max: number) =>
Math.floor(Math.random() * (max - min + 1)) + min;
3 changes: 2 additions & 1 deletion packages/utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export * from './createTimeoutPromise';
export * from './getLocaleSeparators';
export * from './getMutex';
export * from './getNumberFromPixelString';
export * from './getRandomNumberInRange';
export * from './getWeakRandomNumberInRange';
export * from './getSynchronize';
export * from './getWeakRandomId';
export * from './hasUppercaseLetter';
Expand All @@ -40,6 +40,7 @@ export * from './topologicalSort';
export * from './truncateMiddle';
export * from './typedEventEmitter';
export * from './urlToOnion';
export * from './randomInt';
export * from './logs';
export * from './logsManager';
export * from './bigNumber';
Expand Down
7 changes: 7 additions & 0 deletions packages/utils/src/randomInt.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { randomBytes } from 'crypto';

export const getRandomInt = (min: number, max: number) => {
const randomValue = parseInt(randomBytes(4).toString('hex'), 16);

return min + (randomValue % (max - min + 1));
};

0 comments on commit ee18606

Please sign in to comment.