Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request implements cryptocurrency swap functionality through integration with the Exolix exchange service, enabling users to exchange XTM tokens with other cryptocurrencies within the mobile wallet.
Key Changes
- Exolix Service Integration: Complete API client implementation for the Exolix exchange service with models for currencies, networks, rates, transactions, and error handling
- Swap UI Flows: New SwiftUI screens for swap initiation, deposit confirmation, transaction progress monitoring, and currency selection
- Transaction Fee Refactoring: Simplified transaction fee calculation by removing manual
feePerGramparameter requirements and centralizing fee estimation logic
Reviewed Changes
Copilot reviewed 64 out of 73 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| MobileWallet/Services/Exolix/* | New Exolix API service with models for currencies, networks, rates, transactions, and errors |
| MobileWallet/Screens/Swaps/* | New swap flow screens including main swap view, deposit, progress, confirmation, and currency selection |
| MobileWallet/Screens/Managers/TransactionFeesManager.swift | Refactored fee manager to simplify fee calculation without manual feePerGram |
| MobileWallet/Libraries/TariLib/Core/FFIWalletHandler.swift | Updated wallet FFI methods to calculate feePerGram internally |
| MobileWallet/Libraries/TariLib/Core/Services/* | Updated Tari services to remove feePerGram parameters |
| MobileWallet/SwiftUI/* | New reusable UI components including QRImage, ScrollTrigger, TariButton styles, IconButton enhancements |
| MobileWallet/Common/Extensions/* | New utility extensions for String regex matching and Double conversion |
| MobileWallet/Screens/Home/Home/* | Integration of swap-in-progress indicator on home screen |
| MobileWallet/Assets.xcassets/Assets/Swaps/* | New swap-related image assets |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,52 @@ | |||
| // AppSecrets.swift | |||
There was a problem hiding this comment.
File name in comment is "AppSecrets.swift" (plural) but the actual filename is "AppSecret.swift" (singular). These should match.
| } | ||
|
|
||
| // MARK: - Properties | ||
|
|
There was a problem hiding this comment.
The new property scansAnyText should be documented with a comment explaining its purpose and when it should be set to true.
| /// If set to `true`, the scanner will accept any text (not just valid deeplinks or addresses) as a scan result. | |
| /// Set this to `true` when you want to allow scanning and returning arbitrary text from QR codes, rather than restricting to specific formats. |
| func matches(_ regex: String) -> Bool { | ||
| range(of: regex, options: .regularExpression, range: nil, locale: nil) != nil | ||
| } | ||
| } |
There was a problem hiding this comment.
The matches function should document that it expects a valid regex pattern. Invalid regex patterns will return nil for range, which will be treated as "no match". Consider adding error handling or documentation about this behavior.
| func matches(_ regex: String) -> Bool { | |
| range(of: regex, options: .regularExpression, range: nil, locale: nil) != nil | |
| } | |
| } | |
| /// Returns `true` if the string matches the given regular expression pattern. | |
| /// | |
| /// - Parameter regex: A valid regular expression pattern. | |
| /// - Returns: `true` if the string matches the pattern, `false` otherwise. | |
| /// | |
| /// - Note: If the provided pattern is not a valid regular expression, this function will return `false`. | |
| func matches(_ regex: String) -> Bool { | |
| range(of: regex, options: .regularExpression, range: nil, locale: nil) != nil | |
| } |
| let code: String | ||
| let name: String | ||
| let icon: String | ||
| let addresRegex: String? |
There was a problem hiding this comment.
Typo in property name: addresRegex should be addressRegex (missing 's').
| } | ||
|
|
||
| func validateWithdrawalAddress() { | ||
| guard let addresRegex = externalCurrency?.addresRegex else { return } |
There was a problem hiding this comment.
Typo in variable name: addresRegex should be addressRegex (missing 's'). This matches the property name issue in ExolixCurrency.
| let document: QRCode.Document | ||
|
|
||
| public init(_ value: String, colors: [Color] = [.black, .black], backgroundColor: Color = .white) { | ||
| let document = try! QRCode.Document(utf8String: value) |
There was a problem hiding this comment.
Force-try (try!) should be avoided as it will crash the app if QR code generation fails. Consider using proper error handling and either throwing an error from init or returning an optional/result type.
| document.design.shape.eye = QRCode.EyeShape.Square() | ||
| document.design.shape.onPixels = QRCode.PixelShape.Square() | ||
| let gradientPins = colors.enumerated().map { | ||
| DSFGradient.Pin(UIColor($1).cgColor, CGFloat($0 / (colors.count - 1))) |
There was a problem hiding this comment.
Potential division by zero when colors.count is 1. The expression $0 / (colors.count - 1) will divide by zero. Consider handling the single-color case separately or ensuring colors.count > 1.
No description provided.