Skip to content

Conversation

@dioKaratzas
Copy link
Contributor

@dioKaratzas dioKaratzas commented Aug 1, 2025

https://argent.atlassian.net/browse/IOS-3436

  • Add ThreadSafeBox utility class for thread-safe property wrapper
  • Implement Sendable conformance across all major classes and protocols:
    • Client classes (EthereumHttpClient, EthereumWebSocketClient, etc.)
    • ENS classes (ENSResolver, ENSMultiResolver, EthereumNameService)
    • ERC standard implementations (ERC20, ERC721, ERC1271, ERC165)
    • Account management (EthereumAccount, EthereumKeyLocalStorage)
    • Contract and ABI types
    • Network providers and utilities
  • Convert ENSResolver to use thread-safe property access with ThreadSafeBox
  • Fix ENSMultiResolver thread safety with atomic operations for queries and responses
  • Update EthereumKeyLocalStorage with thread-safe address handling
  • Mark classes as final where appropriate for performance
  • Use @unchecked Sendable for classes with internal synchronization
  • Update test classes to conform to Sendable requirements
  • Ensure all protocols inherit Sendable conformance where needed

This enables safe concurrent usage across the entire web3.swift library
while maintaining backward compatibility and performance.

@dioKaratzas dioKaratzas requested a review from DarthMike August 1, 2025 12:58
@dioKaratzas dioKaratzas changed the title feat: Add comprehensive Sendable conformance and thread safety feat: Add comprehensive Sendable conformance and thread safety [IOS-3436] Aug 1, 2025
@dioKaratzas dioKaratzas force-pushed the tech/sendable branch 6 times, most recently from 5cbf6e5 to e6fa0e6 Compare August 7, 2025 09:27
- https://argent.atlassian.net/browse/IOS-3436

Summary:
- Add ThreadSafeBox utility class for thread-safe property wrapper
- Implement Sendable conformance across all major classes and protocols:
  - Client classes (EthereumHttpClient, EthereumWebSocketClient, etc.)
  - ENS classes (ENSResolver, ENSMultiResolver, EthereumNameService)
  - ERC standard implementations (ERC20, ERC721, ERC1271, ERC165)
  - Account management (EthereumAccount, EthereumKeyLocalStorage)
  - Contract and ABI types
  - Network providers and utilities
- Convert ENSResolver to use thread-safe property access with ThreadSafeBox
- Fix ENSMultiResolver thread safety with atomic operations for queries and responses
- Update EthereumKeyLocalStorage with thread-safe address handling
- Mark classes as final where appropriate for performance
- Use @unchecked Sendable for classes with internal synchronization
- Update test classes to conform to Sendable requirements
- Ensure all protocols inherit Sendable conformance where needed

This enables safe concurrent usage across the entire web3.swift library
while maintaining backward compatibility and performance.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds comprehensive Sendable conformance and thread safety to the web3.swift library to enable safe concurrent usage across Swift 6 strict concurrency checks. It introduces a new ThreadSafeBox utility for thread-safe property access and updates all major classes, protocols, and structures with appropriate Sendable conformance.

  • Add ThreadSafeBox utility class for thread-safe property wrapper
  • Implement Sendable conformance across client classes, ERC standards, ENS services, account management, and contract types
  • Update completion handlers and closures with @sendable annotations for concurrent safety

Reviewed Changes

Copilot reviewed 55 out of 56 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
web3swift/src/Utils/ThreadSafeBox.swift New thread-safe wrapper utility using NSLock for atomic property access
web3swift/src/ENS/ENSResolver.swift Convert to use ThreadSafeBox for thread-safe wildcard support tracking
web3swift/src/ENS/ENSMultiResolver.swift Add thread safety with atomic operations for queries and responses
web3swift/src/Account/EthereumKeyLocalStorage.swift Update address handling with ThreadSafeBox for thread safety
web3swift/src/ERC20/ERC20.swift Add @unchecked Sendable conformance and @sendable completion handlers
web3swift/src/ERC721/ERC721.swift Add @unchecked Sendable conformance and @sendable completion handlers
web3swift/src/Client/HTTP/EthereumHttpClient.swift Add @unchecked Sendable conformance to HTTP client
web3swift/src/Client/WSS/EthereumWebSocketClient.swift Add @unchecked Sendable conformance and @sendable closures
web3swift/src/Utils/KeyUtil.swift Replace deprecated assign with update method
web3swift/src/Contract/Statically Typed/EthereumClient+Static.swift Add debug print statement and EventFilter Sendable conformance

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.


return try parseOrFail(data)
} catch {
print("Error calling function: \(error)")
Copy link

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This debug print statement should be removed from production code or replaced with proper logging using the existing logger infrastructure.

Copilot uses AI. Check for mistakes.
var size = 65
var rv = Data(count: size)
rv.withUnsafeMutableBytes {
_ = rv.withUnsafeMutableBytes {
Copy link

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discarding the return value with '_' is unnecessary here. The withUnsafeMutableBytes method returns Void in this context, so the assignment can be removed.

Suggested change
_ = rv.withUnsafeMutableBytes {
rv.withUnsafeMutableBytes {

Copilot uses AI. Check for mistakes.
@DarthMike DarthMike merged commit 03faffd into develop Sep 9, 2025
2 of 3 checks passed
@DarthMike DarthMike deleted the tech/sendable branch September 9, 2025 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants