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.

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

bugfix: Real-time incoming transactions are sorted incorrectly #1720

Merged
merged 2 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion .github/workflows/checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ jobs:
steps:
- name: Checkout repo
uses: actions/checkout@v4
with:
fetch-depth: 0

- name: Setup node
uses: actions/setup-node@v4
Expand All @@ -115,7 +117,7 @@ jobs:
run: yarn --frozen-lockfile --ignore-optional

- name: Run Jest
run: yarn test:jest --onlyChanged=${{ github.event_name == 'pull_request' }} --passWithNoTests
run: yarn test:jest ${{ github.event_name == 'pull_request' && '--changedSince=origin/main' || '' }} --passWithNoTests

pw_affected_tests:
name: Resolve affected Playwright tests
Expand Down
7 changes: 4 additions & 3 deletions ui/address/AddressTxs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import Pagination from 'ui/shared/pagination/Pagination';
import useQueryWithPages from 'ui/shared/pagination/useQueryWithPages';
import getSortParamsFromValue from 'ui/shared/sort/getSortParamsFromValue';
import getSortValueFromQuery from 'ui/shared/sort/getSortValueFromQuery';
import { sortTxsFromSocket } from 'ui/txs/sortTxs';
import TxsWithAPISorting from 'ui/txs/TxsWithAPISorting';
import { SORT_OPTIONS } from 'ui/txs/useTxsSort';

Expand Down Expand Up @@ -85,7 +86,7 @@ const AddressTxs = ({ scrollRef, overloadCount = OVERLOAD_COUNT }: Props) => {
addressTxsQuery.onFilterChange({ filter: newVal });
}, [ addressTxsQuery ]);

const handleNewSocketMessage: SocketMessage.AddressTxs['handler'] = (payload) => {
const handleNewSocketMessage: SocketMessage.AddressTxs['handler'] = React.useCallback((payload) => {
setSocketAlert('');

queryClient.setQueryData(
Expand Down Expand Up @@ -123,10 +124,10 @@ const AddressTxs = ({ scrollRef, overloadCount = OVERLOAD_COUNT }: Props) => {
items: [
...newItems,
...prevData.items,
],
].sort(sortTxsFromSocket(sort)),
};
});
};
}, [ currentAddress, filterValue, overloadCount, queryClient, sort ]);

const handleSocketClose = React.useCallback(() => {
setSocketAlert('Connection is lost. Please refresh the page to load new transactions.');
Expand Down
79 changes: 79 additions & 0 deletions ui/txs/sortTxs.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import type { Transaction } from 'types/api/transaction';

import sortTxs, { sortTxsFromSocket } from './sortTxs';

describe('sortTxs', () => {
it('should sort transactions by value in descending order', () => {
const txs = [
{ value: '42' },
{ value: '11' },
{ value: '24' },
] as Array<Transaction>;
const result = txs.sort(sortTxs('value-desc'));
expect(result).toEqual([
{ value: '42' },
{ value: '24' },
{ value: '11' },
]);
});

it('should sort transactions by value in ascending order', () => {
const txs = [
{ value: '42' },
{ value: '11' },
{ value: '24' },
] as Array<Transaction>;
const result = txs.sort(sortTxs('value-asc'));
expect(result).toEqual([
{ value: '11' },
{ value: '24' },
{ value: '42' },
]);
});

it('should sort transactions by fee in descending order', () => {
const txs = [
{ fee: { value: '42' } },
{ fee: { value: '11' } },
{ fee: { value: '24' } },
] as Array<Transaction>;
const result = txs.sort(sortTxs('fee-desc'));
expect(result).toEqual([
{ fee: { value: '42' } },
{ fee: { value: '24' } },
{ fee: { value: '11' } },
]);
});

it('should sort transactions by fee in ascending order', () => {
const txs = [
{ fee: { value: '42' } },
{ fee: { value: '11' } },
{ fee: { value: '24' } },
] as Array<Transaction>;
const result = txs.sort(sortTxs('fee-asc'));
expect(result).toEqual([
{ fee: { value: '11' } },
{ fee: { value: '24' } },
{ fee: { value: '42' } },
]);
});
});

describe('sortTxsFromSocket', () => {
it('should sort transaction by age in ascending order if sorting is not provided', () => {
const txs = [
{ timestamp: '2022-11-01T12:33:00Z' },
{ timestamp: '2022-11-01T12:00:00Z' },
{ timestamp: null },
{ timestamp: '2022-11-03T03:03:00Z' },
] as Array<Transaction>;
const result = txs.sort(sortTxsFromSocket(undefined));
expect(result).toEqual([
{ timestamp: null },
{ timestamp: '2022-11-03T03:03:00Z' },
{ timestamp: '2022-11-01T12:33:00Z' },
{ timestamp: '2022-11-01T12:00:00Z' },
]);
});
});
38 changes: 38 additions & 0 deletions ui/txs/sortTxs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import type { Transaction, TransactionsSortingValue } from 'types/api/transaction';

import compareBns from 'lib/bigint/compareBns';

export default function sortTxs(sorting: TransactionsSortingValue | undefined) {
return function sortingFn(tx1: Transaction, tx2: Transaction) {
switch (sorting) {
case 'value-desc':
return compareBns(tx2.value, tx1.value);
case 'value-asc':
return compareBns(tx1.value, tx2.value);
case 'fee-desc':
return compareBns(tx2.fee.value || 0, tx1.fee.value || 0);
case 'fee-asc':
return compareBns(tx1.fee.value || 0, tx2.fee.value || 0);
default:
return 0;
}
};
}

export function sortTxsFromSocket(sorting: TransactionsSortingValue | undefined) {
if (sorting) {
return sortTxs(sorting);
}

return function sortingFn(tx1: Transaction, tx2: Transaction) {
if (!tx1.timestamp) {
return -1;
}

if (!tx2.timestamp) {
return 1;
}

return tx2.timestamp.localeCompare(tx1.timestamp);
};
}
20 changes: 3 additions & 17 deletions ui/txs/useTxsSort.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import type { UseQueryResult } from '@tanstack/react-query';
import React from 'react';

import type { Transaction, TransactionsSortingValue, TxsResponse } from 'types/api/transaction';
import type { TransactionsSortingValue, TxsResponse } from 'types/api/transaction';

import type { ResourceError } from 'lib/api/resources';
import compareBns from 'lib/bigint/compareBns';
import * as cookies from 'lib/cookies';
import type { Option } from 'ui/shared/sort/Sort';

import sortTxs from './sortTxs';

export const SORT_OPTIONS: Array<Option<TransactionsSortingValue>> = [
{ title: 'Default', id: undefined },
{ title: 'Value ascending', id: 'value-asc' },
Expand All @@ -23,21 +24,6 @@ type HookResult = UseQueryResult<TxsResponse, ResourceError<unknown>> & {
setSortByValue: (value: SortingValue) => void;
}

const sortTxs = (sorting: SortingValue) => (tx1: Transaction, tx2: Transaction) => {
switch (sorting) {
case 'value-desc':
return compareBns(tx1.value, tx2.value);
case 'value-asc':
return compareBns(tx2.value, tx1.value);
case 'fee-desc':
return compareBns(tx1.fee.value || 0, tx2.fee.value || 0);
case 'fee-asc':
return compareBns(tx2.fee.value || 0, tx1.fee.value || 0);
default:
return 0;
}
};

export default function useTxsSort(
queryResult: UseQueryResult<TxsResponse, ResourceError<unknown>>,
): HookResult {
Expand Down
Loading