Skip to content
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
40 changes: 30 additions & 10 deletions src/commands/account/add-owner.ts
Original file line number Diff line number Diff line change
@@ -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'

Expand Down Expand Up @@ -125,15 +125,30 @@ 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())) {
return 'Address is already an owner'
const addressError = validator.validateAddressWithChain(value, chainId, chains)
if (addressError) return addressError

// 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
},
})
Expand All @@ -143,10 +158,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')
Expand Down
35 changes: 29 additions & 6 deletions src/commands/account/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export async function createSafe() {
}

const chain = configStore.getChain(chainId as string)!
const chainsConfig = configStore.getAllChains()

// Configure owners
const owners: Address[] = []
Expand Down Expand Up @@ -86,13 +87,30 @@ 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'

// 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
},
})
Expand All @@ -102,7 +120,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)}`)
}
Expand Down
21 changes: 12 additions & 9 deletions src/commands/account/open.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -82,22 +82,25 @@ 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)) {
p.cancel('Operation cancelled')
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)!
Expand Down
12 changes: 6 additions & 6 deletions src/commands/tx/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,18 +104,18 @@ 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)) {
p.cancel('Operation cancelled')
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)
Expand Down Expand Up @@ -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
Expand Down
27 changes: 19 additions & 8 deletions src/services/transaction-builder.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
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'

export interface TransactionBuilderResult {
Expand All @@ -13,9 +15,14 @@ export interface TransactionBuilderResult {
*/
export class TransactionBuilder {
private abi: ABI
private chainId: string
private chains: Record<string, ChainConfig>
private validator = getValidationService()

constructor(abi: ABI) {
constructor(abi: ABI, chainId: string, chains: Record<string, ChainConfig>) {
this.abi = abi
this.chainId = chainId
this.chains = chains
}

/**
Expand Down Expand Up @@ -102,7 +109,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'
Expand All @@ -115,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
Expand All @@ -127,12 +140,10 @@ export class TransactionBuilder {
* Parse parameter value based on type
*/
private parseParameter(value: string, type: string): unknown {
// Address
// Address (with EIP-3770 support using ValidationService)
if (type === 'address') {
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
Expand Down
97 changes: 97 additions & 0 deletions src/services/validation-service.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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, ChainConfig>
): 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
Expand All @@ -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<string, ChainConfig>,
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading