encapsulated WalletClient into WalletAuth, WalletSessions and WalletOperations

[andygruening]

Summary

PR title must follow Conventional Commits, e.g. feat(scope): short description.
See: https://www.conventionalcommits.org

Changes

• Encapsulated WalletClient into WalletAuth, WalletSessions and WalletOperations extension classes

Testing

• swift build passes
• swift test passes
• SDK demo app builds (xcodebuild -project Examples/sdk-demo/oms-sdk-demo.xcodeproj -scheme oms-sdk-demo build CODE_SIGNING_ALLOWED=NO)
• Trails Actions demo app builds (xcodebuild -project Examples/trails-actions/trails-actions.xcodeproj -scheme trails-actions build CODE_SIGNING_ALLOWED=NO)

Related

[claude]

Claude finished @andygruening's task —— View job

---

Code Review: WalletClient Encapsulation

Reviewing the encapsulation of WalletClient into WalletAuth, WalletSessions, and WalletOperations.

Review Checklist

• Analyze the architectural changes and file structure
• Security review: P-256 credential handling and no private key material in session storage
• Check for floating-point math in token amount handling
• Verify platform availability annotations (@available) on new public surfaces
• Assess test coverage implications
• Check for proper handling of paths with spaces
• Provide comprehensive feedback

Summary: APPROVED ✅

This is an excellent refactoring that improves code organization without introducing security or functional regressions.

Detailed Findings

✅ Architecture & Organization

Excellent modular design: The split creates clear separation of concerns:

• WalletAuth.swift (622 lines): Authentication flows, OIDC, OTP, wallet selection
• WalletSessions.swift (372 lines): Session lifecycle, persistence, access management
• WalletOperations.swift (452 lines): Transaction signing, contract calls, fee handling
• WalletClient.swift (377 lines): Core initialization and shared utilities

All extensions maintain access to private members appropriately while exposing a clean public API.

✅ Security Review

P-256 Credential Handling: All cryptographic operations correctly use credentialSession.signer which maintains the non-extractable P-256 credential (WalletClient.swift:314, WalletAuth.swift:309). No direct private key material handling found.

Session Storage: Only metadata persisted (wallet IDs, addresses, session expiration, login types) - no cryptographic material stored in session state.

✅ Token Amount Handling

No Floating-Point Math: All financial calculations use string-based formatUnits() (WalletOperations.swift:451). The single Double usage is for nanosecond time conversion (WalletSessions.swift:142), not token amounts.

✅ Platform Availability

All extension files properly use @available(macOS 12.0, iOS 15.0, *), maintaining SDK platform requirements consistency.

✅ Test Coverage

Existing test structure in MockWalletTests.swift covers all major WalletClient functionality through the public API. Since this is purely a code organization change (functionality moved to extensions), tests should continue passing without modification.

✅ Paths with Spaces

No shell command examples or path handling changes in this refactoring.

Recommendations

1. Consider Adding Internal Access Control

Some methods could be marked internal instead of private for better testability:

// WalletAuth.swift:485
internal func signOutOnFailure<T>(_ operation: () async throws -> T) async throws -> T

2. Documentation Consistency

Consider adding file-level documentation comments to each extension explaining their responsibilities:

/// WalletAuth.swift - Authentication flows including OTP, OIDC, and wallet selection
@available(macOS 12.0, iOS 15.0, *)
extension WalletClient {

Minor Observations

• Thread safety properly maintained with sessionLock across all extensions
• Error handling patterns consistent throughout
• Public API surface unchanged, maintaining backward compatibility
• Method organization follows logical grouping

This refactoring significantly improves maintainability while preserving all security properties and functional behavior. Great work!

[claude]

Claude finished @andygruening's task —— View job

---

Code Review: WalletClient Encapsulation

Reviewing the encapsulation of WalletClient into WalletAuth, WalletSessions, and WalletOperations.

Review Checklist

• Analyze the architectural changes and file structure
• Security review: P-256 credential handling and no private key material in session storage
• Check for floating-point math in token amount handling
• Verify platform availability annotations (@available) on new public surfaces
• Assess test coverage implications
• Check for proper handling of paths with spaces
• Provide comprehensive feedback

Summary: APPROVED ✅

This is an excellent refactoring that improves code organization without introducing security or functional regressions.

Detailed Findings

✅ Architecture & Organization

Excellent modular design: The split creates clear separation of concerns:

• WalletAuth.swift (622 lines): Authentication flows, OIDC, OTP, wallet selection
• WalletSessions.swift (372 lines): Session lifecycle, persistence, access management
• WalletOperations.swift (452 lines): Transaction signing, contract calls, fee handling
• WalletClient.swift (377 lines): Core initialization and shared utilities

All extensions maintain access to private members appropriately while exposing a clean public API.

✅ Security Review

P-256 Credential Handling: All cryptographic operations correctly use credentialSession.signer which maintains the non-extractable P-256 credential (WalletClient.swift:314, WalletAuth.swift:309). No direct private key material handling found.

Session Storage: Only metadata persisted (wallet IDs, addresses, session expiration, login types) - no cryptographic material stored in session state.

✅ Token Amount Handling

No Floating-Point Math: All financial calculations use string-based formatUnits() (WalletOperations.swift:451). The single Double usage is for nanosecond time conversion (WalletSessions.swift:142), not token amounts.

✅ Platform Availability

All extension files properly use @available(macOS 12.0, iOS 15.0, *), maintaining SDK platform requirements consistency.

✅ Test Coverage

Existing test structure in MockWalletTests.swift covers all major WalletClient functionality through the public API. Since this is purely a code organization change (functionality moved to extensions), tests should continue passing without modification.

✅ Paths with Spaces

No shell command examples or path handling changes in this refactoring.

Recommendations

1. Consider Adding Internal Access Control

Some methods could be marked internal instead of private for better testability:

// WalletAuth.swift:485
internal func signOutOnFailure<T>(_ operation: () async throws -> T) async throws -> T

2. Documentation Consistency

Consider adding file-level documentation comments to each extension explaining their responsibilities:

/// WalletAuth.swift - Authentication flows including OTP, OIDC, and wallet selection
@available(macOS 12.0, iOS 15.0, *)
extension WalletClient {

Minor Observations

• Thread safety properly maintained with sessionLock across all extensions
• Error handling patterns consistent throughout
• Public API surface unchanged, maintaining backward compatibility
• Method organization follows logical grouping

This refactoring significantly improves maintainability while preserving all security properties and functional behavior. Great work!

---