Add polling options, struct/enum file refactor, nullable walletAddress, fixed invalid fee option

[andygruening]

Summary

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

Changes

• Adds waitForStatus and statusPolling options to sendTransaction and callContract.
• Introduces TransactionStatusPollingOptions with configurable timeout, fast polling interval/count, and regular polling interval.
• Allows callers to skip polling and return immediately after execute.
• Changes walletAddress from empty-string state to String?, with docs/examples updated to unwrap it.
• Hardens FeeOptionSelector.firstAvailable so malformed raw balance or fee values are treated as unavailable.
• Splits public models into domain folders for auth, sessions, operations, and indexer models.
• Updates README.md and API.md for the new transaction polling options and nullable wallet address behavior.
• Adds tests for skipped polling, zero-delay polling, malformed fee option values, and nullable wallet address expectations.

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

---

PR Review: Polling Options & File Refactor

I've completed my review of this PR focusing on OMS SDK conventions, security, and correctness. Here's my comprehensive analysis:

✅ Positive Findings

Security Compliance

• ✅ Private key material properly protected: No private key material is persisted in session storage. Only signerCredentialId is stored in StorableCredentials.swift:4-5, maintaining the P-256 credential as non-extractable per AGENTS.md requirements
• ✅ Token amount handling: Properly uses string-based amounts and calls formatUnits() at WalletOperations.swift:499, avoiding floating-point math per AGENTS.md line 126

Platform Availability

• ✅ Consistent annotations: All new public model types properly annotated with @available(macOS 12.0, iOS 15.0, *) including:
  • TransactionStatusPollingOptions (TransactionResult.swift:2)
  • FeeOptionWithBalance (FeeOptionsSelector.swift:4)
  • CompleteAuthResult and related Auth models
  • All new Indexer models

Models Organization

• ✅ Excellent refactoring: Models directory properly organized into logical subdirectories:
  • Auth/ - Authentication and wallet selection models
  • Indexer/ - Token balance and metadata models
  • Operations/ - Transaction and fee-related models
  • Sessions/ - Session state management models

Nullable walletAddress Implementation

• ✅ Well-designed: WalletClient.swift:97-107 implements nullable walletAddress with proper thread-safe access and trimming logic
• ✅ Validation: requireActiveWalletAddress() at line 261 properly validates active wallet address

⚠️ Areas of Concern

Missing Platform Availability

• ⚠️ TransactionError: The new TransactionError enum at TransactionError.swift:3 lacks @available(macOS 12.0, iOS 15.0, *) annotation, breaking consistency with other public APIs

Polling Implementation Issues

• ⚠️ Time calculations: WalletOperations.swift:412,437,453 uses Double for time calculations which could introduce precision issues in edge cases. Consider using TimeInterval or integer milliseconds consistently
• ⚠️ Polling logic: The waitForTransactionStatus() method has complex timeout logic that may benefit from unit tests to verify edge cases

Fee Option Logic

• ⚠️ String comparison: FeeOptionsSelector.swift:80-104 implements balance comparison using string manipulation. While avoiding floating-point math (good!), the logic could be brittle with malformed input strings

📋 Test Coverage Assessment

✅ Good Coverage

• Extensive test coverage for wallet auth flows in MockWalletTests.swift
• Tests cover nullable walletAddress scenarios
• Session expiration and persistence testing

⚠️ Missing Tests

• No specific tests for new TransactionStatusPollingOptions parameters
• Fee option selection logic with balance calculations needs test coverage
• Error cases in TransactionError enum not thoroughly tested

🔧 Recommendations

1. Add missing platform availability annotation to TransactionError
2. Consider unit tests for the new polling timeout logic
3. Add tests for fee option balance calculation edge cases
4. Verify CI passes with the new Models structure

📁 File Organization

The Models directory reorganization is excellent and follows clean architecture principles. The separation of concerns between Auth, Indexer, Operations, and Sessions is well thought out.

Overall, this is a solid PR with good security practices and proper adherence to most OMS SDK conventions. The main issues are minor and mostly related to missing annotations and test coverage.

---

[claude]

Claude finished @andygruening's task —— View job

---

PR Review: Polling Options & File Refactor

I've completed my comprehensive review of this PR focusing on OMS SDK conventions, security, and correctness. Here's my analysis:

✅ Positive Findings

Security Compliance

• ✅ Private key material properly protected: No private key material is persisted in session storage. Only signerCredentialId is stored in StorableCredentials.swift:4, maintaining the P-256 credential as non-extractable per AGENTS.md requirements
• ✅ Token amount handling: Properly uses string-based amounts and calls formatUnits() at WalletOperations.swift:499, avoiding floating-point math per AGENTS.md line 126

Platform Availability

• ✅ Consistent annotations: All new public model types properly annotated with @available(macOS 12.0, iOS 15.0, *) including:
  • TransactionStatusPollingOptions (TransactionResult.swift:2)
  • FeeOptionWithBalance (FeeOptionsSelector.swift:4)
  • CompleteAuthResult and related Auth models
  • All new Indexer models

Models Organization

• ✅ Excellent refactoring: Models directory properly organized into logical subdirectories:
  • Auth/ - Authentication and wallet selection models
  • Indexer/ - Token balance and metadata models
  • Operations/ - Transaction and fee-related models
  • Sessions/ - Session state management models

Nullable walletAddress Implementation

• ✅ Well-designed: WalletClient.swift:97-107 implements nullable walletAddress with proper thread-safe access and trimming logic
• ✅ Validation: requireActiveWalletAddress() at line 261 properly validates active wallet address

Shell Commands

• ✅ Proper quoting: All xcodebuild commands in documentation properly quote project and scheme names with spaces

⚠️ Areas of Concern

Missing Platform Availability

• ⚠️ TransactionError: The new TransactionError enum at TransactionError.swift:3 lacks @available(macOS 12.0, iOS 15.0, *) annotation, breaking consistency with other public APIs

Fee Option Logic

• ⚠️ String comparison: FeeOptionsSelector.swift:80-104 implements balance comparison using string manipulation. While avoiding floating-point math (good!), the logic could be brittle with malformed input strings

📋 Test Coverage Assessment

✅ Good Coverage

• Extensive test coverage for wallet auth flows in MockWalletTests.swift
• Tests cover nullable walletAddress scenarios (lines 39-40, 147, 176, etc.)
• Session expiration and persistence testing
• ✅ Polling tests: Found tests for skipped polling (TestWalletSendTransactionCanSkipStatusPolling and TestWalletCallContractCanSkipStatusPolling at lines 1794 and 1825)

✅ Good Test Practices

• Tests properly validate walletAddress == nil in various scenarios
• Comprehensive coverage of auth flows with nullable address expectations

🔧 Recommendations

1. Add missing platform availability annotation to TransactionError enum
2. Consider additional edge case tests for the fee option balance calculation logic

📁 File Organization

The Models directory reorganization is excellent and follows clean architecture principles. The separation of concerns between Auth, Indexer, Operations, and Sessions is well thought out.

📊 Overall Assessment

This is a solid PR with good security practices and proper adherence to most OMS SDK conventions. The main issue is the missing platform availability annotation on TransactionError. The nullable walletAddress implementation is well-designed, the polling options are properly tested, and the file refactoring improves code organization significantly.

Recommendation: Approve after fixing the missing @available annotation on TransactionError.

---