refactor: Replaced all instances of 0x${string} with Address.

[PooyaRaki]

Summary

This PR replaces all occurrences of 0x${string} with the Address type.

Changes

• Replaced all occurrences of 0x${string} with the Address type.

[LucieFaire]

💯

[PooyaRaki]

@claude review

[claude]

Claude finished @PooyaRaki's task —— View job

---

Code Review: Type Safety Issues with Address Usage

After reviewing this refactoring, I've identified several critical type safety issues that need to be addressed:

🚨 Major Issues

1. Incorrect Address Type Usage for Non-Address Data

• File: src/domain/common/entities/safe-signature.ts:18-19
• Issue: signature and hash properties are typed as Address but contain non-address data
• Problem: Address in viem represents Ethereum addresses (0x${string} with 20 bytes), but signatures are 65+ bytes and hashes are 32 bytes

2. Invalid Return Types for Signature Components

• File: src/domain/common/entities/safe-signature.ts:36,40
• Issue: get r(): Address and get s(): Address return signature components as Address
• Problem: r and s are 32-byte signature components, not 20-byte addresses

3. Hash Parameter Type Mismatch

• File: src/domain/common/entities/safe-signature.ts:110,115
• Issue: Hash parameters typed as Address in recoverAddress() and recoverPublicKey()
• Problem: Message hashes are 32 bytes, not 20-byte addresses

🔧 Recommended Fixes

1. Use Proper Types:

   • signature: Hex (for signature data)
   • hash: Hex (for message hashes)
   • r, s getters: Hex (for signature components)
   • Keep Address only for actual Ethereum addresses

2. Import Statement:

   import type { Address, Hex } from 'viem';

3. Update Function Signatures:

   function recoverAddress(args: { hash: Hex; signature: Hex }): Address
   function recoverPublicKey(args: { hash: Hex; signature: Hex }): Hex

📋 Files Requiring Review

This pattern appears across multiple files in the PR. Key areas to check:

• All signature-related utilities in src/domain/common/
• Test files using as Address type assertions for non-address data
• Database entities storing signature/hash data

✅ Test Coverage

The test coverage appears adequate, but tests are masking the type issues with as Address assertions. Once types are corrected, these assertions should be removed.

🔒 Security Assessment

No immediate security vulnerabilities, but the type confusion could lead to runtime errors or incorrect data handling in the future.

---

[PooyaRaki]

@claude review

[claude]

Claude finished @PooyaRaki's task —— View job

---

Code Review Complete

🎯 Previously Identified Issues: RESOLVED ✅

The critical type safety issues in safe-signature.ts have been properly fixed:

• signature and hash properties now correctly use Hex type
• r() and s() getters return Hex instead of incorrect Address
• recoverAddress() and recoverPublicKey() functions have proper parameter types

🚨 Remaining Type Safety Issues

1. Incorrect Address Type Assertions in Tests
Multiple test files cast non-address hex data to Address:

• Files: safe-signature.spec.ts:29,30, transaction mappers, staking tests
• Issue: faker.string.hexadecimal() generates arbitrary hex (signatures, hashes, data) but is cast as Address
• Problem: Address in viem is specifically for 20-byte Ethereum addresses, not arbitrary hex data

Examples:

// ❌ Incorrect - signature is not an address
const signature = `${baseSignature}${v}` as Address;
const hash = faker.string.hexadecimal({ length: 66 }) as Address;

// ❌ Incorrect - 64-byte hash cast as address  
const txHash = faker.string.hexadecimal({ length: 64 }) as Address;

// ✅ Correct approach
const signature = `${baseSignature}${v}` as Hex;
const hash = faker.string.hexadecimal({ length: 66 }) as Hex;

2. Type Confusion Pattern

• Transaction hashes (32 bytes) → Should be Hex
• Signatures (65+ bytes) → Should be Hex
• Contract call data → Should be Hex
• Only actual addresses (20 bytes) → Should be Address

⚡ Performance & Code Quality

• Large-scale refactoring is consistent and well-executed
• No performance concerns identified
• Good adherence to existing code patterns

🔒 Security Assessment

• No immediate security vulnerabilities
• Type confusion could lead to runtime errors in edge cases
• Tests may pass incorrectly due to lenient type assertions

📋 Recommendations

1. Fix test type assertions: Replace as Address with as Hex for non-address data
2. Use proper viem types: Address for addresses only, Hex for all other hex data
3. Pattern consistency: Apply this distinction across all 100 modified files

✅ Overall Assessment

Good refactoring foundation with the main type safety issues resolved. The remaining issues are primarily in test files and don't affect runtime functionality, but should be fixed for type correctness and maintainability.

---

[LucieFaire]

oh wow, the Claude did a very solid review, i have missed those nuances

[PooyaRaki]

@claude review

[claude]

Claude finished @PooyaRaki's task —— View job

---

Code Review: Address Type Refactoring

Todo List:

• Check current state of safe-signature.ts (previously identified issues)
• Review test files for incorrect Address type assertions
• Scan key files for remaining type safety issues
• Provide final assessment and recommendations

---

🎯 Major Issues from Previous Review: RESOLVED ✅

The critical type safety issues in safe-signature.ts have been properly fixed:

• Properties now use correct Hex type for signatures and hashes
• Getters return appropriate types (Hex vs Address)
• Function parameters use proper viem types

🚨 Remaining Type Safety Issues

1. Incorrect Test Type Assertions
Multiple test files still cast non-address data as Address:

// ❌ Issues found in ~35+ files:
const signature = faker.string.hexadecimal() as Address;           // Should be Hex
const data = faker.string.hexadecimal({ length: 32 }) as Address; // Should be Hex
const transactionId = faker.string.hexadecimal({ length: 64 }) as Address; // Should be Hex

Examples:

• src/domain/swaps/entities/__tests__/order.builder.ts:64 - signature as Address
• src/routes/transactions/mappers/common/bridge-transaction.mapper.spec.ts - transaction IDs and data as Address
• src/domain/notifications/v2/notifications.repository.spec.ts - 32-byte hex as Address

2. Pattern Analysis
The incorrect pattern occurs in:

• Test builders: Generating non-address hex data but casting as Address
• Mock data: Using faker.string.hexadecimal() for signatures, hashes, and arbitrary data
• Transaction mappers: Test data for transaction IDs and call data

✅ What's Working Well

• Core domain logic properly uses Address vs Hex types
• Main safe-signature.ts implementation is type-safe
• Runtime functionality unaffected (tests pass with type assertions)

📋 Recommendations

High Priority (Type Safety):

1. Replace incorrect as Address assertions in test files:
   • Signatures → as Hex
   • Transaction hashes → as Hex
   • Call data → as Hex
   • Only actual Ethereum addresses → as Address

Medium Priority (Code Quality):
2. Use proper test builders: Create typed faker functions for different hex data types
3. Pattern consistency: Apply Address only for 20-byte Ethereum addresses

🔒 Security & Performance

• No security issues identified
• No performance concerns
• Type confusion limited to test files, doesn't affect runtime

📊 Impact Assessment

• Positive: Major type safety improvements in core domain logic
• Minimal Risk: Remaining issues are test-only and don't affect production
• Code Quality: Good foundation, needs test cleanup for full type correctness

Overall: Strong refactoring with core issues resolved. Test file cleanup needed for complete type safety.

---

[PooyaRaki]

@claude review

[claude]

Claude finished @PooyaRaki's task —— View job

---

Code Review: Address Type Refactoring

Todo List:

• Check current state of safe-signature.ts (previously had critical issues)
• Review test files for incorrect Address type assertions
• Sample key domain files for proper type usage
• Provide final assessment and recommendations

---

✅ Major Issues from Previous Reviews: RESOLVED

The critical type safety issues in safe-signature.ts have been properly fixed:

• Properties now correctly use Hex type for signatures and hashes
• Getters return appropriate types (Hex vs Address)
• Function parameters use proper viem types

🚨 Remaining Type Safety Issues

Incorrect Test Type Assertions: Multiple test files cast non-address data as Address:

Examples of Issues:

// ❌ 32-byte hex (not 20-byte address)
safeAddress: faker.string.hexadecimal({ length: 32 }) as Address

// ❌ Arbitrary hex data cast as address  
.with('data', faker.string.hexadecimal() as Address)
.with('signature', faker.string.hexadecimal() as Address)

// ❌ Transaction hashes cast as addresses
faker.string.hexadecimal({ length: HASH_LENGTH }) as Address

Key Files with Issues:

• src/domain/notifications/v2/notifications.repository.spec.ts - 32-byte hex as Address
• src/domain/swaps/entities/order.entity.spec.ts - signature as Address
• src/domain/safe/entities/__tests__/multisig-transaction.builder.ts - transaction data/hashes as Address
• src/datasources/data-decoder-api/data-decoder-api.service.spec.ts - call data as Address

📋 Recommendations

Fix Type Assertions: Replace incorrect as Address with as Hex for:

• Signatures (65+ bytes) → as Hex
• Transaction hashes (32 bytes) → as Hex
• Call data (arbitrary length) → as Hex
• Only 40-char hex strings (20 bytes) → as Address

✅ What's Working Well

• Core domain logic properly distinguishes Address vs Hex types
• Main implementation files use correct types
• Runtime functionality unaffected by test assertions

🔒 Security & Performance

• No security issues identified
• No performance concerns
• Type confusion limited to test files only

Overall: Excellent refactoring with major type safety improvements. Test cleanup needed for complete type correctness, but production code is solid.

[PooyaRaki]

@claude review

[claude]

Claude finished @PooyaRaki's task —— View job

---

Code Review: Address Type Refactoring

Todo List:

• Check current state of safe-signature.ts (previously had critical issues)
• Review test files for incorrect Address type assertions
• Sample key domain files for proper type usage
• Provide final assessment and recommendations

---

✅ Major Issues from Previous Reviews: RESOLVED

The critical type safety issues in safe-signature.ts have been properly fixed:

• Properties now correctly use Hex type for signatures and hashes
• Getters return appropriate types (Hex vs Address)
• Function parameters use proper viem types

🚨 Remaining Type Safety Issues

Incorrect Test Type Assertions: Multiple test files cast non-address data as Address:

Examples of Issues:

// ❌ 32-byte hex (not 20-byte address)
safeAddress: faker.string.hexadecimal({ length: 32 }) as Address

// ❌ Arbitrary hex data cast as address  
.with('data', faker.string.hexadecimal() as Address)
.with('signature', faker.string.hexadecimal() as Address)

// ❌ Transaction hashes cast as addresses
faker.string.hexadecimal({ length: HASH_LENGTH }) as Address

Key Files with Issues:

• src/domain/notifications/v2/notifications.repository.spec.ts - 32-byte hex as Address
• src/domain/swaps/entities/order.entity.spec.ts - signature as Address
• src/domain/safe/entities/__tests__/multisig-transaction.builder.ts - transaction data/hashes as Address
• src/datasources/data-decoder-api/data-decoder-api.service.spec.ts - call data as Address

📋 Recommendations

Fix Type Assertions: Replace incorrect as Address with as Hex for:

• Signatures (65+ bytes) → as Hex
• Transaction hashes (32 bytes) → as Hex
• Call data (arbitrary length) → as Hex
• Only 40-char hex strings (20 bytes) → as Address

✅ What's Working Well

• Core domain logic properly distinguishes Address vs Hex types
• Main implementation files use correct types
• Runtime functionality unaffected by test assertions

🔒 Security & Performance

• No security issues identified
• No performance concerns
• Type confusion limited to test files only

Overall: Excellent refactoring with major type safety improvements. Test cleanup needed for complete type correctness, but production code is solid.

[PooyaRaki]

It looks good now, All the review comments are fixed.

[LucieFaire]

👍