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

chalabi/safe truncation #204

Merged
merged 4 commits into from
Jan 13, 2025
Merged

chalabi/safe truncation #204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
416aaf5
fix: no manual chainNames
chalabi2 Jan 11, 2025
361197a
fix: safe truncation
chalabi2 Jan 11, 2025
d5ce8dd
chore code rabbit suggestions
chalabi2 Jan 11, 2025
6af1718
fix: truncated string/address rendering
fmorency Jan 13, 2025
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
2 changes: 1 addition & 1 deletion components/admins/components/validatorList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ export default function ValidatorList({
</td>

<td className="py-4 bg-secondary group-hover:bg-base-300 hidden lg:table-cell">
<TruncatedAddressWithCopy slice={10} address={validator.operator_address} />
<TruncatedAddressWithCopy slice={24} address={validator.operator_address} />
</td>
<td className="py-4 bg-secondary group-hover:bg-base-300 hidden md:table-cell">
{validator.consensus_power?.toString() ?? 'N/A'}
Expand Down
11 changes: 10 additions & 1 deletion components/bank/components/__tests__/historyBox.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { test, expect, afterEach, describe, mock } from 'bun:test';
import { test, expect, afterEach, describe, mock, jest } from 'bun:test';
import { screen, cleanup, fireEvent } from '@testing-library/react';
import { HistoryBox } from '../historyBox';
import { renderWithChainProvider } from '@/tests/render';
Expand All @@ -7,6 +7,15 @@ import matchers from '@testing-library/jest-dom/matchers';

expect.extend(matchers);

// Mock next/router
const m = jest.fn();
mock.module('next/router', () => ({
useRouter: m.mockReturnValue({
query: {},
push: jest.fn(),
}),
}));

// Mock the hooks
mock.module('@/hooks', () => ({
useTokenFactoryDenomsMetadata: () => ({
Expand Down
2 changes: 1 addition & 1 deletion components/bank/components/historyBox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@
? tx.data.to_address
: tx.data.from_address
}
slice={6}
slice={24}

Check warning on line 251 in components/bank/components/historyBox.tsx

View check run for this annotation

Codecov / codecov/patch

components/bank/components/historyBox.tsx#L251 

Added line #L251 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Add test coverage for address truncation behavior

The existing test suite in components/bank/components/__tests__/historyBox.test.tsx lacks coverage for the address truncation functionality. Consider adding a test case that verifies addresses are correctly truncated to 24 characters.

🔗 Analysis chain

LGTM! Consider adding test coverage.

The change to standardize address truncation to 24 characters aligns with the broader effort to ensure consistent address display across the application.

The line is flagged as not covered by tests. Consider adding test coverage to ensure the address truncation behavior works as expected:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for existing test files and patterns
# Test: Search for test files that might need updating
fd -e test.tsx -e spec.tsx -e test.ts -e spec.ts | rg -i "historybox|address"

Length of output: 136

Script:

#!/bin/bash
# Check the content of historyBox test file for address-related test cases
cat components/bank/components/__tests__/historyBox.test.tsx

Length of output: 3679

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 251-251: components/bank/components/historyBox.tsx#L251
Added line #L251 was not covered by tests

/>
) : (
<div className="text-[#00000099] dark:text-[#FFFFFF99]">
Expand Down
2 changes: 1 addition & 1 deletion components/bank/modals/txInfo.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ function InfoItem({
<div className="bg-[#FFFFFF66] dark:bg-[#FFFFFF1A] rounded-[16px] p-4">
{isAddress ? (
<div className="flex items-center">
<TruncatedAddressWithCopy address={value} slice={8} />
<TruncatedAddressWithCopy address={value} slice={24} />
<a
href={`${env.explorerUrl}/${label === 'TRANSACTION HASH' ? 'transaction' : 'account'}/${label?.includes('TRANSACTION') ? value?.toUpperCase() : value}`}
target="_blank"
Expand Down
3 changes: 2 additions & 1 deletion components/groups/components/groupControls.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { TokenList } from '@/components';
import { CombinedBalanceInfo, ExtendedMetadataSDKType } from '@/utils';
import DenomList from '@/components/factory/components/DenomList';
import { useResponsivePageSize } from '@/hooks/useResponsivePageSize';
import env from '@/config/env';

type GroupControlsProps = {
policyAddress: string;
Expand Down Expand Up @@ -231,7 +232,7 @@ export default function GroupControls({
.trim();
}

const { address } = useChain('manifest');
const { address } = useChain(env.chain);
const { groupByMemberData } = useGroupsByMember(address ?? '');

useEffect(() => {
Expand Down
4 changes: 2 additions & 2 deletions components/groups/components/myGroups.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export function YourGroups({
const [selectedGroupName, setSelectedGroupName] = useState<string>('Untitled Group');

const router = useRouter();
const { address } = useChain('manifest');
const { address } = useChain(env.chain);

const filteredGroups = groups.groups.filter(group => {
try {
Expand Down Expand Up @@ -635,7 +635,7 @@ function GroupRow({
</td>
<td className="bg-secondary group-hover:bg-base-300 hidden lg:table-cell w-1/6">
<div onClick={e => e.stopPropagation()}>
<TruncatedAddressWithCopy address={policyAddress} slice={12} />
<TruncatedAddressWithCopy address={policyAddress} slice={24} />
</div>
</td>
<td className="bg-secondary group-hover:bg-base-300 rounded-r-[12px] sm:rounded-l-none w-1/6">
Expand Down
15 changes: 10 additions & 5 deletions components/groups/forms/groups/ConfirmationForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { ThresholdDecisionPolicy } from '@liftedinit/manifestjs/dist/codegen/cosmos/group/v1/types';
import { Duration } from '@liftedinit/manifestjs/dist/codegen/google/protobuf/duration';
import { secondsToHumanReadable } from '@/utils/string';

import env from '@/config/env';
export default function ConfirmationForm({
nextStep,
prevStep,
Expand All @@ -28,10 +28,15 @@
};

// Convert the object to a JSON string
const jsonString = JSON.stringify(groupMetadata);

const { tx, isSigning, setIsSigning } = useTx('manifest');
const { estimateFee } = useFeeEstimation('manifest');
let jsonString: string;
try {
jsonString = JSON.stringify(groupMetadata);
} catch (error) {
console.error('Failed to serialize group metadata:', error);

Check warning on line 35 in components/groups/forms/groups/ConfirmationForm.tsx

View check run for this annotation

Codecov / codecov/patch

components/groups/forms/groups/ConfirmationForm.tsx#L34-L35 

Added lines #L34 - L35 were not covered by tests
throw new Error('Invalid group metadata format');
}
Comment on lines +31 to +37
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Add test coverage for JSON serialization error handling

The test file lacks coverage for the JSON serialization error handling. Consider adding these test cases:

  • Test successful serialization of valid group metadata
  • Test error handling when serialization fails (e.g., with circular references)
🔗 Analysis chain

Add test coverage for error handling.

The error handling for JSON serialization is good, but it lacks test coverage.

Could you add test cases to cover the error handling scenario? Here's a verification script to check current test coverage:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check test coverage for the error handling block
rg -l "test.*ConfirmationForm.*metadata.*serialization" -t ts

Length of output: 63

Script:

#!/bin/bash
# Find test files related to ConfirmationForm
fd "ConfirmationForm.*test" --type f

# If no direct test file found, check for test files in the same directory
fd ".*test.*\.(ts|tsx)$" --type f --exec grep -l "ConfirmationForm"

Length of output: 366

Script:

#!/bin/bash
# Check content of the relevant test file
cat "components/groups/forms/groups/__tests__/ConfirmationForm.test.tsx"

Length of output: 2752

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 34-35: components/groups/forms/groups/ConfirmationForm.tsx#L34-L35
Added lines #L34 - L35 were not covered by tests

const { tx, isSigning, setIsSigning } = useTx(env.chain);
const { estimateFee } = useFeeEstimation(env.chain);

const minExecutionPeriod: Duration = {
seconds: BigInt(0),
Expand Down
6 changes: 3 additions & 3 deletions components/groups/forms/groups/__tests__/Success.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ describe('Success Component', () => {
screen.getByText('Your transaction was successfully signed and broadcasted.')
).toBeInTheDocument();
expect(screen.getByText('Group Information')).toBeInTheDocument();
expect(screen.getByText('manifest1autho...author')).toBeInTheDocument();
expect(screen.getByText('manifest1efd63...m6rp3z')).toBeInTheDocument();
expect(screen.getByText('manifest1hj5fv...8ws9ct')).toBeInTheDocument();
expect(screen.getByText('manifest1autho...')).toBeInTheDocument();
expect(screen.getByText('manifest1efd63...')).toBeInTheDocument();
expect(screen.getByText('manifest1hj5fv...')).toBeInTheDocument();
const normalizer = getDefaultNormalizer({ collapseWhitespace: true, trim: true });
expect(screen.getByText('2 / 2', { normalizer })).toBeInTheDocument();
});
Expand Down
2 changes: 2 additions & 0 deletions components/groups/modals/memberManagementModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { CopyIcon, TrashIcon } from '@/components/icons';
import { MdContacts } from 'react-icons/md';
import { TailwindModal } from '@/components/react/modal';
import env from '@/config/env';
import { truncateAddress } from '@/utils';

interface ExtendedMember extends MemberSDKType {
isNew: boolean;
Expand Down Expand Up @@ -321,6 +322,7 @@ export function MemberManagementModal({
}`}
placeholder="manifest1..."
disabled={!member.isNew || member.markedForDeletion}
value={truncateAddress(field.value)}
/>
{member.isNew && !member.markedForDeletion && (
<button
Expand Down
7 changes: 4 additions & 3 deletions components/groups/modals/voteModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { cosmos } from '@liftedinit/manifestjs';
import { useChain } from '@cosmos-kit/react';
import React, { useState } from 'react';
import { CloseIcon } from '@/components/icons';
import env from '@/config/env';
function VotingPopup({
proposalId,
refetch,
Expand All @@ -13,9 +14,9 @@ function VotingPopup({
refetch: () => void;
setIsSigning: (isSigning: boolean) => void;
}) {
const { estimateFee } = useFeeEstimation('manifest');
const { tx } = useTx('manifest');
const { address } = useChain('manifest');
const { estimateFee } = useFeeEstimation(env.chain);
const { tx } = useTx(env.chain);
const { address } = useChain(env.chain);
const [error, setError] = useState<string | null>(null);

const { vote } = cosmos.group.v1.MessageComposer.withTypeUrl;
Expand Down
37 changes: 3 additions & 34 deletions components/react/addressCopy.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import React, { useState, useEffect } from 'react';
import { FiCopy, FiCheck } from 'react-icons/fi';
import { truncateAddress } from '@/utils';

export const TruncatedAddressWithCopy = ({
address,
address = '',
slice,
size,
}: {
Expand Down Expand Up @@ -31,7 +32,7 @@ export const TruncatedAddressWithCopy = ({
}
};

const truncatedAddress = `${address?.slice(0, slice)}...${address?.slice(-6)}`;
const truncatedAddress = truncateAddress(address, slice);
const iconSize = size === 'small' ? 10 : 16;

return (
Expand All @@ -45,35 +46,3 @@ export const TruncatedAddressWithCopy = ({
</span>
);
};

export const AddressWithCopy = ({ address }: { address: string }) => {
const [copied, setCopied] = useState(false);

useEffect(() => {
let timer: ReturnType<typeof setTimeout>;
if (copied) {
timer = setTimeout(() => setCopied(false), 2000);
}
return () => clearTimeout(timer);
}, [copied]);

const handleCopy = async () => {
try {
await navigator.clipboard.writeText(address);
setCopied(true);
} catch (err) {
console.error('Failed to copy: ', err);
}
};

return (
<span
className="flex items-center space-x-2"
onClick={handleCopy}
style={{ cursor: 'pointer' }}
>
<span className="hover:text-primary">{address}</span>
{copied ? <FiCheck size="16" /> : <FiCopy size="16" />}
</span>
);
};
4 changes: 2 additions & 2 deletions components/react/views/Connected.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import ProfileAvatar from '@/utils/identicon';
import { useBalance } from '@/hooks/useQueries';
import { CopyIcon } from '@/components/icons';
import { getRealLogo, shiftDigits, truncateString } from '@/utils';
import { getRealLogo, shiftDigits, truncateAddress } from '@/utils';
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Add basic test coverage for truncateAddress utility

The truncateAddress function in utils/string.ts lacks test coverage. While the implementation is straightforward, tests should verify:

  • Handling of addresses shorter than the limit
  • Truncation of addresses longer than the default 24 characters
  • Empty string handling
🔗 Analysis chain

Add test coverage for address truncation.

The address truncation implementation looks good, but the static analysis indicates that line 86 lacks test coverage. Consider adding test cases to verify the truncation behavior.

Would you like me to help generate test cases for the address truncation functionality?

Also applies to: 86-86

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for existing tests of Connected component
fd -e tsx -e ts "Connected.test" components/react/views/

Length of output: 56

Script:

#!/bin/bash
# Find all test files in views directory
echo "=== Test files in views ==="
fd -e test.tsx -e test.ts . components/react/views/

# Find truncateAddress tests
echo -e "\n=== truncateAddress tests ==="
fd -e test.tsx -e test.ts . utils/

# Find actual usage of truncateAddress
echo -e "\n=== truncateAddress usage in Connected ==="
rg -C 2 "truncateAddress" components/react/views/Connected.tsx

Length of output: 848

Script:

#!/bin/bash
# Find truncateAddress tests using ripgrep
echo "=== Searching for truncateAddress tests ==="
rg -l "truncateAddress.*test"

# Find truncateAddress implementation
echo -e "\n=== truncateAddress implementation ==="
rg -C 4 "export.*truncateAddress"

Length of output: 628

import Image from 'next/image';
import { MdContacts } from 'react-icons/md';
import { Contacts } from './Contacts';
Expand Down Expand Up @@ -83,7 +83,7 @@
<p className="text-lg font-semibold">{username || 'Anonymous'}</p>
<div className="flex items-center">
<p className="text-sm text-gray-500 dark:text-gray-400">
{truncateString(address || '', 12)}
{truncateAddress(address || '')}

Check warning on line 86 in components/react/views/Connected.tsx

View check run for this annotation

Codecov / codecov/patch

components/react/views/Connected.tsx#L86 

Added line #L86 was not covered by tests
</p>
<button
onClick={copyAddress}
Expand Down
11 changes: 3 additions & 8 deletions components/wallet.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import { WalletStatus } from 'cosmos-kit';
import { MdWallet } from 'react-icons/md';
import env from '@/config/env';
import { truncateAddress, truncateString } from '@/utils';

const buttons = {
Disconnected: {
Expand Down Expand Up @@ -130,17 +131,11 @@
className="font-medium text-xl text-center mb-2 truncate"
title={username || 'Connected user'}
>
{username
? username.length > 20
? `${username.slice(0, 20)}...`
: username
: 'Connected User'}
{username ? truncateString(username, 20) : 'Connected User'}

Check warning on line 134 in components/wallet.tsx

View check run for this annotation

Codecov / codecov/patch

components/wallet.tsx#L134 

Added line #L134 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Add comprehensive test coverage for wallet component and utilities

The verification confirms that test coverage is missing for:

  • The wallet component functionality
  • The truncateString utility function
  • Username truncation behavior

Please add test cases covering both the component behavior and the utility function, ensuring various scenarios like:

  • Username truncation with different lengths
  • Empty username handling
  • Edge cases for the truncation function
🔗 Analysis chain

Add test coverage for username truncation.

The username truncation implementation lacks test coverage.

Could you add test cases for the username truncation? Here's a verification script to check current test coverage:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check test coverage for username truncation
rg -l "test.*WalletSection.*username.*truncate" -t ts

Length of output: 55

Script:

#!/bin/bash
# Find all test files related to wallet
echo "=== Wallet-related test files ==="
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts | rg -i "wallet"

# Find truncateString utility tests
echo -e "\n=== TruncateString utility tests ==="
rg -l "truncateString" -t ts | rg "test|spec"

# Find any username truncation tests
echo -e "\n=== Username truncation tests ==="
rg -l "username.*truncate|truncate.*username" -t ts

Length of output: 436

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 134-134: components/wallet.tsx#L134
Added line #L134 was not covered by tests

</p>
<div className="bg-base-100 dark:bg-base-200 rounded-full py-2 px-4 text-center mb-4 flex items-center flex-row justify-between w-full ">
<p className="text-xs truncate flex-grow">
{address
? `${address.slice(0, 12)}...${address.slice(-6)}`
: 'Address not available'}
{address ? truncateAddress(address) : 'Address not available'}

Check warning on line 138 in components/wallet.tsx

View check run for this annotation

Codecov / codecov/patch

components/wallet.tsx#L138 

Added line #L138 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Add test coverage for address truncation utility

The address truncation functionality requires test coverage at two levels:

  • Unit tests for the truncateAddress utility function in the utils directory
  • Integration tests for address display in the WalletSection component

Example test cases should cover:

  • Different address lengths
  • Edge cases (empty string, null, undefined)
  • Correct truncation pattern
🔗 Analysis chain

Add test coverage for address truncation.

The address truncation implementation lacks test coverage.

Could you add test cases for the address truncation? Here's a verification script to check current test coverage:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check test coverage for address truncation
rg -l "test.*WalletSection.*address.*truncate" -t ts

Length of output: 54

Script:

#!/bin/bash
# Find truncateAddress function definition
ast-grep --pattern 'function truncateAddress($_) { $$$ }'

# Find all test files related to wallet
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts . | rg -i "wallet"

# Search for any tests containing "address" string
rg "test.*address" -t ts -t tsx

Length of output: 189

Script:

#!/bin/bash
# Find all wallet-related files to understand structure
fd . -e ts -e tsx | rg -i "wallet"

# Try different patterns for truncateAddress
ast-grep --pattern 'const truncateAddress = $_'
ast-grep --pattern 'export const truncateAddress = $_'
ast-grep --pattern 'export function truncateAddress($_)'

# Search for address-related tests with correct extension
rg "test.*address" -t ts

Length of output: 2538

Script:

#!/bin/bash
# Check for utility files and inline truncate implementation
rg "truncate.*address" -t ts -t tsx

# Look at the wallet component implementation
cat components/wallet.tsx

Length of output: 10539

Script:

#!/bin/bash
# Check for utils tests
fd . -e test.ts -e test.tsx | rg "utils"

# Look at the utils implementation
cat utils/index.ts

Length of output: 400

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 138-138: components/wallet.tsx#L138
Added line #L138 was not covered by tests

</p>
<button
onClick={() => {
Expand Down
16 changes: 10 additions & 6 deletions utils/string.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import { CombinedBalanceInfo } from './types';

export function truncateString(str: string, num: number) {
if (str.length > num) {
return str.slice(0, num) + '...' + str.slice(-6);
} else {
return str;
}
export function truncateString(str: string, prefixLen: number = 6, suffixLen: number = 6): string {
if (str.length <= prefixLen + suffixLen) return str;

return str.slice(0, prefixLen) + '...' + str.slice(-suffixLen);
}

export function truncateAddress(address: string, num: number = 24) {
if (address.length <= num) return address;

return address.slice(0, num) + '...';
}

export function secondsToHumanReadable(seconds: number): string {
Expand Down
Loading