From 11849eb63391094b89bcd39c1009077fa4c4f12e Mon Sep 17 00:00:00 2001 From: katspaugh Date: Tue, 28 Oct 2025 15:29:45 +0100 Subject: [PATCH 1/3] feat: Add EIP-3770 chain-prefixed address support with validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add comprehensive support for EIP-3770 chain-prefixed addresses across all address input fields. Users can now use either plain addresses (0x...) or chain-prefixed addresses (eth:0x..., matic:0x..., etc.). When a chain prefix is provided, the system validates that it matches the current Safe's chain and errors out if there's a mismatch. Changes: - Add validateAddressWithChain() and assertAddressWithChain() methods to ValidationService for EIP-3770 support and chain validation - Update transaction creation 'To' field to support EIP-3770 format - Update TransactionBuilder to validate address-type parameters with chain prefix support - Update account management commands (open, create, add-owner) to accept chain-prefixed addresses - Add 17 comprehensive tests for EIP-3770 validation - Maintain backward compatibility: plain addresses work as before All address inputs now show helpful placeholders like "0x... or eth:0x..." and provide clear error messages on chain mismatch (e.g., "Chain mismatch: address is for Polygon (matic:) but current Safe is on Ethereum"). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/commands/account/add-owner.ts | 30 +++-- src/commands/account/create.ts | 27 +++- src/commands/account/open.ts | 21 +-- src/commands/tx/create.ts | 12 +- src/services/transaction-builder.ts | 42 +++++- src/services/validation-service.ts | 97 ++++++++++++++ .../integration-transaction-builder.test.ts | 2 +- .../unit/services/transaction-builder.test.ts | 15 ++- .../unit/services/validation-service.test.ts | 122 +++++++++++++++++- 9 files changed, 330 insertions(+), 38 deletions(-) diff --git a/src/commands/account/add-owner.ts b/src/commands/account/add-owner.ts index 8f5629d..6da2911 100644 --- a/src/commands/account/add-owner.ts +++ b/src/commands/account/add-owner.ts @@ -1,14 +1,14 @@ import * as p from '@clack/prompts' import pc from 'picocolors' -import { isAddress, type Address } from 'viem' +import { type Address } from 'viem' import { getConfigStore } from '../../storage/config-store.js' import { getSafeStorage } from '../../storage/safe-store.js' import { getWalletStorage } from '../../storage/wallet-store.js' import { getTransactionStore } from '../../storage/transaction-store.js' import { TransactionService } from '../../services/transaction-service.js' +import { getValidationService } from '../../services/validation-service.js' import { SafeCLIError } from '../../utils/errors.js' import { parseSafeAddress, formatSafeAddress } from '../../utils/eip3770.js' -import { validateAndChecksumAddress } from '../../utils/validation.js' import { renderScreen } from '../../ui/render.js' import { OwnerAddSuccessScreen } from '../../ui/screens/index.js' @@ -125,13 +125,20 @@ export async function addOwner(account?: string) { } // Get new owner address + const validator = getValidationService() const newOwnerInput = await p.text({ - message: 'New owner address:', - placeholder: '0x...', + message: 'New owner address (supports EIP-3770 format: shortName:address):', + placeholder: '0x... or eth:0x...', validate: (value) => { - if (!value) return 'Address is required' - if (!isAddress(value)) return 'Invalid Ethereum address' - if (currentOwners.some((o) => o.toLowerCase() === value.toLowerCase())) { + const addressError = validator.validateAddressWithChain(value, chainId, chains) + if (addressError) return addressError + const checksummed = validator.assertAddressWithChain( + value as string, + chainId, + chains, + 'Owner address' + ) + if (currentOwners.some((o) => o.toLowerCase() === checksummed.toLowerCase())) { return 'Address is already an owner' } return undefined @@ -143,10 +150,15 @@ export async function addOwner(account?: string) { return } - // Checksum the address immediately + // Checksum the address (strips EIP-3770 prefix if present) let newOwner: Address try { - newOwner = validateAndChecksumAddress(newOwnerInput as string) + newOwner = validator.assertAddressWithChain( + newOwnerInput as string, + chainId, + chains, + 'Owner address' + ) } catch (error) { p.log.error(error instanceof Error ? error.message : 'Invalid address') p.outro('Failed') diff --git a/src/commands/account/create.ts b/src/commands/account/create.ts index 369ca24..faf6472 100644 --- a/src/commands/account/create.ts +++ b/src/commands/account/create.ts @@ -46,6 +46,7 @@ export async function createSafe() { } const chain = configStore.getChain(chainId as string)! + const chainsConfig = configStore.getAllChains() // Configure owners const owners: Address[] = [] @@ -86,13 +87,22 @@ export async function createSafe() { } const ownerAddress = await p.text({ - message: 'Owner address:', - placeholder: '0x...', + message: 'Owner address (supports EIP-3770 format: shortName:address):', + placeholder: '0x... or eth:0x...', validate: (value) => { - const addressError = validator.validateAddress(value) + const addressError = validator.validateAddressWithChain( + value, + chainId as string, + chainsConfig + ) if (addressError) return addressError - const checksummed = checksumAddress(value as string) - if (owners.includes(checksummed as Address)) return 'Owner already added' + const checksummed = validator.assertAddressWithChain( + value as string, + chainId as string, + chainsConfig, + 'Owner address' + ) + if (owners.includes(checksummed)) return 'Owner already added' return undefined }, }) @@ -102,7 +112,12 @@ export async function createSafe() { return } - const checksummed = checksumAddress(ownerAddress as string) + const checksummed = validator.assertAddressWithChain( + ownerAddress as string, + chainId as string, + chainsConfig, + 'Owner address' + ) owners.push(checksummed) console.log(`✓ Added ${shortenAddress(checksummed)}`) } diff --git a/src/commands/account/open.ts b/src/commands/account/open.ts index 3c595fe..78921a5 100644 --- a/src/commands/account/open.ts +++ b/src/commands/account/open.ts @@ -4,7 +4,7 @@ import { type Address, isAddress } from 'viem' import { getConfigStore } from '../../storage/config-store.js' import { getSafeStorage } from '../../storage/safe-store.js' import { SafeService } from '../../services/safe-service.js' -import { isValidAddress } from '../../utils/validation.js' +import { getValidationService } from '../../services/validation-service.js' import { checksumAddress, shortenAddress } from '../../utils/ethereum.js' import { logError } from '../../ui/messages.js' import { renderScreen } from '../../ui/render.js' @@ -82,14 +82,11 @@ export async function openSafe(addressArg?: string) { chainId = selectedChainId as string // Get Safe address + const validator = getValidationService() const address = await p.text({ - message: 'Safe address:', - placeholder: '0x...', - validate: (value) => { - if (!value) return 'Address is required' - if (!isValidAddress(value)) return 'Invalid Ethereum address' - return undefined - }, + message: 'Safe address (supports EIP-3770 format: shortName:address):', + placeholder: '0x... or eth:0x...', + validate: (value) => validator.validateAddressWithChain(value, chainId, chains), }) if (p.isCancel(address)) { @@ -97,7 +94,13 @@ export async function openSafe(addressArg?: string) { return } - safeAddress = checksumAddress(address as string) as Address + // Strip EIP-3770 prefix if present + safeAddress = validator.assertAddressWithChain( + address as string, + chainId, + chains, + 'Safe address' + ) } const chain = configStore.getChain(chainId)! diff --git a/src/commands/tx/create.ts b/src/commands/tx/create.ts index 6a0125f..188a331 100644 --- a/src/commands/tx/create.ts +++ b/src/commands/tx/create.ts @@ -104,9 +104,9 @@ export async function createTransaction() { // Get transaction details const toInput = await p.text({ - message: 'To address', - placeholder: '0x...', - validate: (value) => validator.validateAddress(value), + message: 'To address (supports EIP-3770 format: shortName:address)', + placeholder: '0x... or eth:0x...', + validate: (value) => validator.validateAddressWithChain(value, chainId, chains), }) if (p.isCancel(toInput)) { @@ -114,8 +114,8 @@ export async function createTransaction() { return } - // Checksum the address - const to = validator.assertAddress(toInput as string, 'To address') + // Checksum the address (strips EIP-3770 prefix if present) + const to = validator.assertAddressWithChain(toInput as string, chainId, chains, 'To address') // Check if address is a contract const contractService = new ContractService(chain) @@ -275,7 +275,7 @@ export async function createTransaction() { } // Build transaction using interactive builder - const builder = new TransactionBuilder(abi) + const builder = new TransactionBuilder(abi, chainId, chains) const result = await builder.buildFunctionCall(func) value = result.value diff --git a/src/services/transaction-builder.ts b/src/services/transaction-builder.ts index b79a757..8833e3a 100644 --- a/src/services/transaction-builder.ts +++ b/src/services/transaction-builder.ts @@ -1,7 +1,9 @@ import * as p from '@clack/prompts' import { encodeFunctionData, parseEther, type Address } from 'viem' import type { ABIFunction, ABI } from './abi-service.js' +import type { ChainConfig } from '../types/config.js' import { SafeCLIError } from '../utils/errors.js' +import { isEIP3770, parseEIP3770, getChainIdFromShortName } from '../utils/eip3770.js' export interface TransactionBuilderResult { data: `0x${string}` @@ -13,9 +15,13 @@ export interface TransactionBuilderResult { */ export class TransactionBuilder { private abi: ABI + private chainId: string + private chains: Record - constructor(abi: ABI) { + constructor(abi: ABI, chainId: string, chains: Record) { this.abi = abi + this.chainId = chainId + this.chains = chains } /** @@ -102,7 +108,7 @@ export class TransactionBuilder { * Get placeholder text for parameter type */ private getPlaceholder(type: string): string { - if (type === 'address') return '0x...' + if (type === 'address') return '0x... or eth:0x...' if (type.startsWith('uint') || type.startsWith('int')) return '123' if (type === 'bool') return 'true or false' if (type === 'string') return 'your text here' @@ -127,8 +133,38 @@ export class TransactionBuilder { * Parse parameter value based on type */ private parseParameter(value: string, type: string): unknown { - // Address + // Address (with EIP-3770 support) if (type === 'address') { + // Check if EIP-3770 format + if (isEIP3770(value)) { + try { + const { shortName, address } = parseEIP3770(value) + + // Resolve the chainId from the shortName + const chainId = getChainIdFromShortName(shortName, this.chains) + + // Check if it matches the expected chain + if (chainId !== this.chainId) { + const expectedChain = this.chains[this.chainId] + const providedChain = this.chains[chainId] + const expectedName = expectedChain?.name || this.chainId + const providedName = providedChain?.name || chainId + + throw new Error( + `Chain mismatch: address is for ${providedName} (${shortName}:) but current Safe is on ${expectedName}` + ) + } + + return address + } catch (error) { + if (error instanceof Error) { + throw error + } + throw new Error('Invalid EIP-3770 address format') + } + } + + // Plain address format if (!/^0x[a-fA-F0-9]{40}$/.test(value)) { throw new Error('Invalid address format') } diff --git a/src/services/validation-service.ts b/src/services/validation-service.ts index 3774bed..2e2f7f4 100644 --- a/src/services/validation-service.ts +++ b/src/services/validation-service.ts @@ -1,5 +1,7 @@ import { isAddress, isHex, getAddress, type Address } from 'viem' import { ValidationError } from '../utils/errors.js' +import { isEIP3770, parseEIP3770, getChainIdFromShortName } from '../utils/eip3770.js' +import type { ChainConfig } from '../types/config.js' /** * Centralized validation service for all input validation across the CLI. @@ -23,6 +25,61 @@ export class ValidationService { return undefined } + /** + * Validates an Ethereum address with EIP-3770 support and chain verification + * Accepts both plain addresses (0x...) and EIP-3770 format (shortName:0x...) + * If EIP-3770 format is provided, validates that the chain prefix matches expectedChainId + * + * @param value - Address to validate (plain or EIP-3770 format) + * @param expectedChainId - The chain ID that the address should be for + * @param chains - Chain configurations to resolve shortNames + * @returns Error message or undefined if valid + */ + validateAddressWithChain( + value: unknown, + expectedChainId: string, + chains: Record + ): string | undefined { + if (!value || typeof value !== 'string') { + return 'Address is required' + } + + // Check if it's EIP-3770 format + if (isEIP3770(value)) { + try { + const { shortName, address } = parseEIP3770(value) + + // Resolve the chainId from the shortName + const chainId = getChainIdFromShortName(shortName, chains) + + // Check if it matches the expected chain + if (chainId !== expectedChainId) { + const expectedChain = chains[expectedChainId] + const providedChain = chains[chainId] + const expectedName = expectedChain?.name || expectedChainId + const providedName = providedChain?.name || chainId + + return `Chain mismatch: address is for ${providedName} (${shortName}:) but current Safe is on ${expectedName}` + } + + // Validate the address part + if (!isAddress(address)) { + return 'Invalid Ethereum address' + } + + return undefined + } catch (error) { + if (error instanceof Error) { + return error.message + } + return 'Invalid EIP-3770 address format' + } + } + + // Plain address format - validate normally + return this.validateAddress(value) + } + /** * Asserts an Ethereum address is valid and returns checksummed version * @throws ValidationError if invalid @@ -41,6 +98,46 @@ export class ValidationService { } } + /** + * Asserts an Ethereum address is valid (with EIP-3770 support) and returns checksummed version + * Strips the EIP-3770 prefix if present and validates chain match + * + * @param value - Address to validate (plain or EIP-3770 format) + * @param expectedChainId - The chain ID that the address should be for + * @param chains - Chain configurations to resolve shortNames + * @param fieldName - Field name for error messages + * @returns Checksummed address (without EIP-3770 prefix) + * @throws ValidationError if invalid + */ + assertAddressWithChain( + value: string, + expectedChainId: string, + chains: Record, + fieldName = 'Address' + ): Address { + const error = this.validateAddressWithChain(value, expectedChainId, chains) + if (error) { + throw new ValidationError(`${fieldName}: ${error}`) + } + + // If EIP-3770 format, extract the address part + let address: string + if (isEIP3770(value)) { + const parsed = parseEIP3770(value) + address = parsed.address + } else { + address = value + } + + try { + return getAddress(address) + } catch (error) { + throw new ValidationError( + `${fieldName}: Invalid address checksum - ${error instanceof Error ? error.message : 'Unknown error'}` + ) + } + } + /** * Validates a private key (64 hex characters with optional 0x prefix) * @returns Error message or undefined if valid diff --git a/src/tests/integration/integration-transaction-builder.test.ts b/src/tests/integration/integration-transaction-builder.test.ts index 5f6ddcb..8f49a52 100644 --- a/src/tests/integration/integration-transaction-builder.test.ts +++ b/src/tests/integration/integration-transaction-builder.test.ts @@ -191,7 +191,7 @@ describe('E2E Transaction Builder Test', () => { // 5. Build Approval Transaction // ============================================ console.log('\n[E2E] Step 5: Build ERC20 approval transaction') - const transactionBuilder = new TransactionBuilder(abi) + const transactionBuilder = new TransactionBuilder(abi, SEPOLIA_CHAIN_ID, DEFAULT_CHAINS) // Build the transaction data for approving 100 DAI to the Safe itself // (This is safe since we control the Safe) diff --git a/src/tests/unit/services/transaction-builder.test.ts b/src/tests/unit/services/transaction-builder.test.ts index 558c80e..fa37f60 100644 --- a/src/tests/unit/services/transaction-builder.test.ts +++ b/src/tests/unit/services/transaction-builder.test.ts @@ -1,7 +1,9 @@ import { describe, it, expect, beforeEach, vi } from 'vitest' import { TransactionBuilder } from '../../../services/transaction-builder.js' import type { ABI, ABIFunction } from '../../../services/abi-service.js' +import type { ChainConfig } from '../../../types/config.js' import * as p from '@clack/prompts' +import { TEST_CHAINS } from '../../fixtures/chains.js' // Mock @clack/prompts vi.mock('@clack/prompts', () => ({ @@ -25,6 +27,13 @@ vi.mock('viem', async () => { describe('TransactionBuilder', () => { let builder: TransactionBuilder + const chainId = '1' + const chains: Record = { + '1': TEST_CHAINS.ethereum, + '11155111': TEST_CHAINS.sepolia, + '137': TEST_CHAINS.polygon, + } + const mockABI: ABI = [ { type: 'function', @@ -47,13 +56,13 @@ describe('TransactionBuilder', () => { beforeEach(() => { vi.clearAllMocks() - builder = new TransactionBuilder(mockABI) + builder = new TransactionBuilder(mockABI, chainId, chains) vi.mocked(p.isCancel).mockReturnValue(false) }) describe('constructor', () => { - it('should create builder with ABI', () => { - const b = new TransactionBuilder(mockABI) + it('should create builder with ABI, chainId, and chains', () => { + const b = new TransactionBuilder(mockABI, chainId, chains) expect(b).toBeInstanceOf(TransactionBuilder) }) }) diff --git a/src/tests/unit/services/validation-service.test.ts b/src/tests/unit/services/validation-service.test.ts index d09d79d..e2adaf2 100644 --- a/src/tests/unit/services/validation-service.test.ts +++ b/src/tests/unit/services/validation-service.test.ts @@ -1,7 +1,8 @@ import { describe, it, expect, beforeEach } from 'vitest' import { ValidationService } from '../../../services/validation-service.js' import { ValidationError } from '../../../utils/errors.js' -import { TEST_ADDRESSES, TEST_PRIVATE_KEYS } from '../../fixtures/index.js' +import { TEST_ADDRESSES, TEST_PRIVATE_KEYS, TEST_CHAINS } from '../../fixtures/index.js' +import type { ChainConfig } from '../../../types/config.js' describe('ValidationService', () => { let service: ValidationService @@ -994,4 +995,123 @@ describe('ValidationService', () => { }) }) }) + + describe('validateAddressWithChain / assertAddressWithChain', () => { + const chains: Record = { + '1': TEST_CHAINS.ethereum, + '11155111': TEST_CHAINS.sepolia, + '137': TEST_CHAINS.polygon, + '42161': TEST_CHAINS.arbitrum, + } + + describe('plain addresses (without EIP-3770 prefix)', () => { + it('should accept valid plain address', () => { + const result = service.validateAddressWithChain(TEST_ADDRESSES.owner1, '1', chains) + expect(result).toBeUndefined() + }) + + it('should accept lowercase plain address', () => { + const lowercase = TEST_ADDRESSES.owner1.toLowerCase() + const result = service.validateAddressWithChain(lowercase, '1', chains) + expect(result).toBeUndefined() + }) + + it('should reject invalid plain address', () => { + const result = service.validateAddressWithChain(TEST_ADDRESSES.invalidShort, '1', chains) + expect(result).toBe('Invalid Ethereum address') + }) + }) + + describe('EIP-3770 addresses (with chain prefix)', () => { + it('should accept EIP-3770 address matching the expected chain', () => { + const eip3770Address = `eth:${TEST_ADDRESSES.owner1}` + const result = service.validateAddressWithChain(eip3770Address, '1', chains) + expect(result).toBeUndefined() + }) + + it('should accept EIP-3770 address with different chain when it matches', () => { + const eip3770Address = `sep:${TEST_ADDRESSES.owner1}` + const result = service.validateAddressWithChain(eip3770Address, '11155111', chains) + expect(result).toBeUndefined() + }) + + it('should reject EIP-3770 address when chain prefix does not match expected chain', () => { + const eip3770Address = `matic:${TEST_ADDRESSES.owner1}` // Polygon + const result = service.validateAddressWithChain(eip3770Address, '1', chains) // Expecting Ethereum + expect(result).toContain('Chain mismatch') + expect(result).toContain('Polygon') + expect(result).toContain('Ethereum') + }) + + it('should reject EIP-3770 address with unknown chain prefix', () => { + const eip3770Address = `unknown:${TEST_ADDRESSES.owner1}` + const result = service.validateAddressWithChain(eip3770Address, '1', chains) + expect(result).toContain('Chain with shortName "unknown" not found') + }) + + it('should reject EIP-3770 address with invalid address part', () => { + const eip3770Address = 'eth:0xinvalid' + const result = service.validateAddressWithChain(eip3770Address, '1', chains) + expect(result).toContain('Invalid Ethereum address') + }) + + it('should show chain names in error message when chains are configured', () => { + const eip3770Address = `arb1:${TEST_ADDRESSES.owner1}` // Arbitrum + const result = service.validateAddressWithChain(eip3770Address, '137', chains) // Expecting Polygon + expect(result).toContain('Arbitrum') + expect(result).toContain('Polygon') + }) + + it('should show chain IDs when chain not in config', () => { + const eip3770Address = `eth:${TEST_ADDRESSES.owner1}` + const result = service.validateAddressWithChain(eip3770Address, '999999', chains) + expect(result).toContain('Chain mismatch') + }) + }) + + describe('assertAddressWithChain', () => { + it('should return checksummed address for plain address', () => { + const lowercase = TEST_ADDRESSES.owner1.toLowerCase() + const result = service.assertAddressWithChain(lowercase, '1', chains) + expect(result).toBe(TEST_ADDRESSES.owner1) + }) + + it('should return checksummed address and strip EIP-3770 prefix', () => { + const eip3770Address = `eth:${TEST_ADDRESSES.owner1.toLowerCase()}` + const result = service.assertAddressWithChain(eip3770Address, '1', chains) + expect(result).toBe(TEST_ADDRESSES.owner1) + expect(result).not.toContain(':') + }) + + it('should throw ValidationError for chain mismatch', () => { + const eip3770Address = `matic:${TEST_ADDRESSES.owner1}` + expect(() => + service.assertAddressWithChain(eip3770Address, '1', chains, 'To address') + ).toThrow(ValidationError) + expect(() => + service.assertAddressWithChain(eip3770Address, '1', chains, 'To address') + ).toThrow('To address:') + }) + + it('should throw ValidationError for invalid address', () => { + expect(() => + service.assertAddressWithChain(TEST_ADDRESSES.invalidShort, '1', chains, 'Owner address') + ).toThrow(ValidationError) + }) + + it('should throw ValidationError for unknown chain prefix', () => { + const eip3770Address = `unknown:${TEST_ADDRESSES.owner1}` + expect(() => service.assertAddressWithChain(eip3770Address, '1', chains)).toThrow( + ValidationError + ) + }) + + it('should include custom field name in error', () => { + const eip3770Address = `matic:${TEST_ADDRESSES.owner1}` + expect(() => + service.assertAddressWithChain(eip3770Address, '1', chains, 'Destination address') + ).toThrow('Destination address:') + }) + }) + }) }) From 3f7c4ce091994aa40655a3180750bec21137a4e3 Mon Sep 17 00:00:00 2001 From: katspaugh Date: Tue, 28 Oct 2025 15:33:46 +0100 Subject: [PATCH 2/3] fix: Wrap assertAddressWithChain in try-catch in validation functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validation functions should only return error strings, not throw errors. The assertAddressWithChain call was duplicated - once inside the validate function and once after the prompt. This fix wraps the assertion in a try-catch block inside the validate function to properly handle errors as return values. Changes: - Wrap assertAddressWithChain in try-catch within validation functions - Add comments to clarify the duplicate check logic - Keep single assertion after prompt for final processing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/commands/account/add-owner.ts | 24 ++++++++++++++++-------- src/commands/account/create.ts | 22 +++++++++++++++------- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/src/commands/account/add-owner.ts b/src/commands/account/add-owner.ts index 6da2911..0dc41c9 100644 --- a/src/commands/account/add-owner.ts +++ b/src/commands/account/add-owner.ts @@ -132,15 +132,23 @@ export async function addOwner(account?: string) { validate: (value) => { const addressError = validator.validateAddressWithChain(value, chainId, chains) if (addressError) return addressError - const checksummed = validator.assertAddressWithChain( - value as string, - chainId, - chains, - 'Owner address' - ) - if (currentOwners.some((o) => o.toLowerCase() === checksummed.toLowerCase())) { - return 'Address is already an owner' + + // Check for duplicates - need to get checksummed version + try { + const checksummed = validator.assertAddressWithChain( + value as string, + chainId, + chains, + 'Owner address' + ) + if (currentOwners.some((o) => o.toLowerCase() === checksummed.toLowerCase())) { + return 'Address is already an owner' + } + } catch (error) { + // Should not happen since validateAddressWithChain already passed + return error instanceof Error ? error.message : 'Invalid address' } + return undefined }, }) diff --git a/src/commands/account/create.ts b/src/commands/account/create.ts index faf6472..a6b011e 100644 --- a/src/commands/account/create.ts +++ b/src/commands/account/create.ts @@ -96,13 +96,21 @@ export async function createSafe() { chainsConfig ) if (addressError) return addressError - const checksummed = validator.assertAddressWithChain( - value as string, - chainId as string, - chainsConfig, - 'Owner address' - ) - if (owners.includes(checksummed)) return 'Owner already added' + + // Check for duplicates - need to get checksummed version + try { + const checksummed = validator.assertAddressWithChain( + value as string, + chainId as string, + chainsConfig, + 'Owner address' + ) + if (owners.includes(checksummed)) return 'Owner already added' + } catch (error) { + // Should not happen since validateAddressWithChain already passed + return error instanceof Error ? error.message : 'Invalid address' + } + return undefined }, }) From fe7bc80c6d4d4434a38a9faff76c95aead8c5be5 Mon Sep 17 00:00:00 2001 From: katspaugh Date: Tue, 28 Oct 2025 15:35:38 +0100 Subject: [PATCH 3/3] refactor: Use ValidationService for address validation in TransactionBuilder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace duplicated EIP-3770 validation logic in TransactionBuilder with calls to ValidationService methods. This eliminates code duplication and ensures consistent validation behavior across the codebase. Changes: - Add ValidationService instance to TransactionBuilder - Replace parseParameter address logic with assertAddressWithChain() - Use validateAddressWithChain() directly in validateParameter() - Remove unused Address import - Add comments explaining the refactoring Benefits: - Single source of truth for address validation - Reduced maintenance burden - Consistent error messages - Better code organization 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/services/transaction-builder.ts | 49 +++++++---------------------- 1 file changed, 12 insertions(+), 37 deletions(-) diff --git a/src/services/transaction-builder.ts b/src/services/transaction-builder.ts index 8833e3a..12748bd 100644 --- a/src/services/transaction-builder.ts +++ b/src/services/transaction-builder.ts @@ -1,9 +1,9 @@ import * as p from '@clack/prompts' -import { encodeFunctionData, parseEther, type Address } from 'viem' +import { encodeFunctionData, parseEther } from 'viem' import type { ABIFunction, ABI } from './abi-service.js' import type { ChainConfig } from '../types/config.js' +import { getValidationService } from './validation-service.js' import { SafeCLIError } from '../utils/errors.js' -import { isEIP3770, parseEIP3770, getChainIdFromShortName } from '../utils/eip3770.js' export interface TransactionBuilderResult { data: `0x${string}` @@ -17,6 +17,7 @@ export class TransactionBuilder { private abi: ABI private chainId: string private chains: Record + private validator = getValidationService() constructor(abi: ABI, chainId: string, chains: Record) { this.abi = abi @@ -121,6 +122,12 @@ export class TransactionBuilder { * Validate parameter input */ private validateParameter(value: string, type: string): string | undefined { + // For addresses, use ValidationService directly to avoid try-catch overhead + if (type === 'address') { + return this.validator.validateAddressWithChain(value, this.chainId, this.chains) + } + + // For other types, parse and catch errors try { this.parseParameter(value, type) return undefined @@ -133,42 +140,10 @@ export class TransactionBuilder { * Parse parameter value based on type */ private parseParameter(value: string, type: string): unknown { - // Address (with EIP-3770 support) + // Address (with EIP-3770 support using ValidationService) if (type === 'address') { - // Check if EIP-3770 format - if (isEIP3770(value)) { - try { - const { shortName, address } = parseEIP3770(value) - - // Resolve the chainId from the shortName - const chainId = getChainIdFromShortName(shortName, this.chains) - - // Check if it matches the expected chain - if (chainId !== this.chainId) { - const expectedChain = this.chains[this.chainId] - const providedChain = this.chains[chainId] - const expectedName = expectedChain?.name || this.chainId - const providedName = providedChain?.name || chainId - - throw new Error( - `Chain mismatch: address is for ${providedName} (${shortName}:) but current Safe is on ${expectedName}` - ) - } - - return address - } catch (error) { - if (error instanceof Error) { - throw error - } - throw new Error('Invalid EIP-3770 address format') - } - } - - // Plain address format - if (!/^0x[a-fA-F0-9]{40}$/.test(value)) { - throw new Error('Invalid address format') - } - return value as Address + // Use ValidationService for consistent address validation and chain checking + return this.validator.assertAddressWithChain(value, this.chainId, this.chains, 'Parameter') } // Boolean