feat: coinbase gas manager #1
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update streamlines provider configuration by consolidating connection settings, renaming configuration types (from AlchemyProviderConfig to ProviderConfig), and updating error handling (replacing AlchemyError with ProviderError). Additionally, method signatures and access levels in RPC and client protocols have been revised (e.g. renaming maxPriorityFeePerGas to eth_maxPriorityFeePerGas). New Coinbase integration has been introduced with dedicated protocols, clients, providers, and sponsorship-related structures, while gas management enhancements and hex utility improvements have also been incorporated. Changes
Sequence Diagram(s)sequenceDiagram
participant SA as SmartAccountProvider
participant CCP as CoinbaseClient (via CoinbaseRpcClient)
participant NP as Network Provider
SA->>SA: withCoinbaseGasManager(connectionConfig)
Note right of SA: Gas estimation middleware triggers
alt Sponsorship needed
SA->>CCP: sponsorUserOperation(userOp, entryPoint)
CCP->>NP: Send "pm_sponsorUserOperation" request
NP-->>CCP: Return SponsoredUserOperation
CCP-->>SA: Return paymaster data & sponsorship details
else
SA->>CCP: getPaymasterData(params)
CCP->>NP: Send "pm_getPaymasterStubData" request
NP-->>CCP: Return PaymasterAndData
CCP-->>SA: Return paymaster data
end
SA->>SA: Update user operation with received values
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (14)
Sources/Coinbase/Provider/CoinbaseProvider.swift (1)
12-12: Consider avoiding a static property forrpcUrlto prevent shared global state.Storing mutable data in a static property can lead to potential race conditions and inadvertent side effects if your code runs in a multi-threaded environment. Consider converting it to an instance-level property or passing the URL directly to consumers.
- private static var rpcUrl: String = "" + private var rpcUrl: StringSources/Coinbase/Middleware/GasManager.swift (1)
56-60: Avoid force-casting the returned value toSelf.Force casting can lead to runtime crashes if the actual type isn't what you expect. Prefer a conditional cast or a generic approach.
- return withMiddlewareRpcClient( - rpcClient: try CoinbaseProvider.createRpcClient( - config: ProviderConfig(chain: chain, connectionConfig: connectionConfig) - ) - ) as! Self + guard let typedSelf = withMiddlewareRpcClient( + rpcClient: try CoinbaseProvider.createRpcClient( + config: ProviderConfig(chain: chain, connectionConfig: connectionConfig) + ) + ) as? Self else { + throw ProviderError.invalidTypeCasting("Expected type: \(Self.self)") + } + return typedSelf🧰 Tools
🪛 SwiftLint (0.57.0)
[Error] 60-60: Force casts should be avoided
(force_cast)
Sources/Alchemy/Middleware/GasManager.swift (2)
103-103: Remove the trailing semicolon.Swift doesn't require semicolons except in specific edge cases. Keeping the code consistent without them improves readability.
- }; + }🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 103-103: Lines should not have trailing semicolons
(trailing_semicolon)
107-107: Refactor to avoid force-casting duringwithMiddlewareRpcClient.Similar to line 100-102, prefer a safer cast or updated function signature.
- ) as! Self + ) as? Self ?? self🧰 Tools
🪛 SwiftLint (0.57.0)
[Error] 107-107: Force casts should be avoided
(force_cast)
Sources/Coinbase/Middleware/CoinbaseClient.swift (1)
10-13: Add documentation for protocol methods.The protocol methods would benefit from documentation explaining their purpose, parameters, return values, and possible errors.
Apply this diff to add documentation:
public protocol CoinbaseClient: Erc4337Client { + /// Retrieves paymaster data for a user operation. + /// - Parameter params: Parameters containing the user operation details. + /// - Returns: Paymaster data for the operation. + /// - Throws: Network or validation errors. func getPaymasterData(params: PaymasterDataParams) async throws -> PaymasterAndData + + /// Sponsors a user operation by providing necessary gas fees. + /// - Parameters: + /// - userOp: The user operation to sponsor. + /// - entryPoint: The entry point contract address. + /// - Returns: The sponsored user operation details. + /// - Throws: Network or validation errors. func sponsorUserOperation(userOp: UserOperationRequest, entryPoint: String) async throws -> SponsoredUserOperation }Sources/Coinbase/Middleware/PaymasterDataParams.swift (2)
14-19: Add documentation for the required field order.The
PaymasterDataParamsstruct looks good, but please add documentation explaining why the fields need to be encoded in a specific order. This will help maintainers understand the requirements of the Coinbase API.
17-17: Consider adding chainId validation.The
chainIdis accepted as a String. Consider adding validation to ensure it matches Coinbase's expected format, or consider using a more specific type if possible.Sources/Coinbase/Chain.swift (1)
2-2: Update the copyright year.The copyright year should be 2024 instead of 2025.
-// Copyright (c) 2025 aa-swift +// Copyright (c) 2024 aa-swiftSources/Coinbase/Middleware/CoinbaseRpcClient.swift (2)
12-23: Simplify error handling and type casting.The current implementation has redundant type casting and could benefit from simplified error handling.
Consider this simplified version:
public func getPaymasterData(params: PaymasterDataParams) async throws -> PaymasterAndData { - do { - let data = try await networkProvider.send(method: "pm_getPaymasterStubData", params: params, receive: PaymasterAndData.self) - if let result = data as? PaymasterAndData { - return result - } else { - throw EthereumClientError.unexpectedReturnValue - } - } catch { - throw failureHandler(error) - } + try await networkProvider.send( + method: "pm_getPaymasterStubData", + params: params, + receive: PaymasterAndData.self + ) }The
sendmethod already returns the correct type, making the type cast unnecessary.
25-37: Apply the same simplification to sponsorUserOperation.This method can benefit from the same simplifications as
getPaymasterData.public func sponsorUserOperation(userOp: UserOperationRequest, entryPoint: String) async throws -> SponsoredUserOperation { - do { - let params = SponsorUserOpParams(userOperation: userOp, entryPoint: entryPoint) - let data = try await networkProvider.send(method: "pm_sponsorUserOperation", params: params, receive: SponsoredUserOperation.self) - if let result = data as? SponsoredUserOperation { - return result - } else { - throw EthereumClientError.unexpectedReturnValue - } - } catch { - throw failureHandler(error) - } + let params = SponsorUserOpParams(userOperation: userOp, entryPoint: entryPoint) + try await networkProvider.send( + method: "pm_sponsorUserOperation", + params: params, + receive: SponsoredUserOperation.self + ) }Sources/Alchemy/Provider/AlchemyProvider.swift (1)
16-35: Improve URL construction logic.The URL construction could be simplified and made more robust.
Consider using URL components:
-guard let rpcUrl = config.connectionConfig.rpcUrl ?? (chain.alchemyRpcHttpUrl.map { - let apiKey = config.connectionConfig.apiKey ?? "" - return apiKey.isEmpty ? $0 : "\($0)/\(apiKey)" -}) else { +guard let baseUrl = config.connectionConfig.rpcUrl ?? chain.alchemyRpcHttpUrl, + var components = URLComponents(string: baseUrl) else { throw ProviderError.rpcUrlNotFound("No rpcUrl found for chain \(config.chain.id)") +} + +if let apiKey = config.connectionConfig.apiKey, !apiKey.isEmpty { + components.path = components.path.trimmingCharacters(in: .init(charactersIn: "/")) + components.path += "/\(apiKey)" +} + +guard let rpcUrl = components.string else { + throw ProviderError.rpcUrlNotFound("Invalid URL construction for chain \(config.chain.id)") }Sources/Core/Provider/SmartAccountProvider.swift (3)
23-23: Remove redundant nil initialization.The explicit nil initialization is redundant as optional properties are automatically initialized to nil.
- private var middlewareClient: Erc4337Client? = nil + private var middlewareClient: Erc4337Client?🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 23-23: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
23-23: Remove redundant nil initialization.Optional variables are automatically initialized to nil in Swift.
- private var middlewareClient: Erc4337Client? = nil + private var middlewareClient: Erc4337Client?🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 23-23: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
23-23: Remove redundant nil initialization.The initialization with nil is redundant as optional variables are automatically initialized with nil.
- private var middlewareClient: Erc4337Client? = nil + private var middlewareClient: Erc4337Client?🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 23-23: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
Example/Sources/MainViewModel.swift(1 hunks)Package.swift(2 hunks)Sources/Alchemy/Account/Utils.swift(1 hunks)Sources/Alchemy/AlchemyError.swift(0 hunks)Sources/Alchemy/Chain.swift(1 hunks)Sources/Alchemy/Middleware/AlchemyClient.swift(1 hunks)Sources/Alchemy/Middleware/AlchemyGasAndPaymasterAndData.swift(1 hunks)Sources/Alchemy/Middleware/AlchemyRpcClient.swift(2 hunks)Sources/Alchemy/Middleware/GasManager.swift(6 hunks)Sources/Alchemy/Provider/AlchemyProvider.swift(2 hunks)Sources/Coinbase/Chain.swift(1 hunks)Sources/Coinbase/Middleware/CoinbaseClient.swift(1 hunks)Sources/Coinbase/Middleware/CoinbaseRpcClient.swift(1 hunks)Sources/Coinbase/Middleware/GasManager.swift(1 hunks)Sources/Coinbase/Middleware/PaymasterDataParams.swift(1 hunks)Sources/Coinbase/Middleware/SponsorUserOpParams.swift(1 hunks)Sources/Coinbase/Middleware/SponsoredUserOperation.swift(1 hunks)Sources/Coinbase/Provider/CoinbaseProvider.swift(1 hunks)Sources/Coinbase/Provider/CreateClient.swift(1 hunks)Sources/Core/Client/Erc4337Client.swift(1 hunks)Sources/Core/Client/Erc4337RpcClient.swift(3 hunks)Sources/Core/Client/PaymasterAndData.swift(1 hunks)Sources/Core/Provider/ProviderConfig.swift(1 hunks)Sources/Core/Provider/ProviderError.swift(1 hunks)Sources/Core/Provider/SmartAccountProvider.swift(5 hunks)Sources/Core/Utils/Hex.swift(1 hunks)
💤 Files with no reviewable changes (1)
- Sources/Alchemy/AlchemyError.swift
✅ Files skipped from review due to trivial changes (2)
- Sources/Alchemy/Chain.swift
- Sources/Alchemy/Middleware/AlchemyGasAndPaymasterAndData.swift
👮 Files not reviewed due to content moderation or server errors (3)
- Sources/Core/Client/Erc4337RpcClient.swift
- Example/Sources/MainViewModel.swift
- Sources/Core/Provider/SmartAccountProvider.swift
🧰 Additional context used
🪛 SwiftLint (0.57.0)
Sources/Core/Provider/SmartAccountProvider.swift
[Warning] 23-23: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
Sources/Coinbase/Middleware/GasManager.swift
[Error] 60-60: Force casts should be avoided
(force_cast)
Sources/Alchemy/Middleware/GasManager.swift
[Error] 100-100: Force casts should be avoided
(force_cast)
[Error] 102-102: Force casts should be avoided
(force_cast)
[Error] 107-107: Force casts should be avoided
(force_cast)
[Warning] 103-103: Lines should not have trailing semicolons
(trailing_semicolon)
[Error] 131-131: Force casts should be avoided
(force_cast)
🔇 Additional comments (35)
Sources/Core/Utils/Hex.swift (2)
15-22: LGTM! Great improvement in hex string handling.The new
properHexStringproperty enhances hex string representation by ensuring consistent formatting, especially for zero values. The implementation is clean and handles edge cases appropriately.
28-35: LGTM! Consistent hex string formatting across all numeric fields.The changes improve consistency by using
properHexStringfor all numeric fields while maintaining appropriate fallback values for each field type.Sources/Coinbase/Provider/CoinbaseProvider.swift (1)
38-41: Clarify the purpose of passingnilforrpcUrlin the superclass initializer.You set
CoinbaseProvider.rpcUrlincreateRpcClientbut passnilto thesuper.initcall forrpcUrl. IfSmartAccountProviderrequires this value for functionality (like logging or error messages), you might consider passing the actualrpcUrlinstead ofnilfor clearer intent.Sources/Core/Provider/ProviderError.swift (1)
8-12: LGTM! Well-structured error handling.The error cases are well-defined with descriptive names and associated values for context.
Sources/Alchemy/Middleware/AlchemyClient.swift (1)
13-13: LGTM! Good move towards provider-agnostic types.The change to use
PaymasterAndDatainstead ofAlchemyPaymasterAndDataimproves modularity by making the return type provider-agnostic. This aligns well with supporting multiple providers like Coinbase.Sources/Core/Provider/ProviderConfig.swift (1)
8-8: LGTM! Good abstraction of provider configuration.The rename from
AlchemyProviderConfigtoProviderConfigcreates a more generic configuration structure that can be used across different providers. This improves code reusability and maintains a clean abstraction.Sources/Core/Client/PaymasterAndData.swift (1)
21-21: LGTM! Good standardization of paymaster response type.The rename from
AlchemyPaymasterAndDatatoPaymasterAndDatacreates a standardized response type that can be used across different paymaster implementations. This improves consistency and interoperability between different providers.Sources/Coinbase/Middleware/PaymasterDataParams.swift (2)
10-12: LGTM! Clean Policy structure implementation.The
Policystruct provides a clear and simple representation of Coinbase policy information.
20-26: LGTM! Clean encoding implementation.The custom
encode(to:)implementation ensures the correct field order required by Coinbase's API.Sources/Coinbase/Chain.swift (1)
10-13: LGTM!The supported chains mapping is well-structured and correctly typed.
Sources/Coinbase/Middleware/SponsoredUserOperation.swift (2)
10-18: LGTM!The error structure is well-defined with appropriate protocol conformance.
20-38: Explain access level choices.The structure has inconsistent access levels - some properties are
publicwhile others are internal. Please document the reasoning behind these choices or make them consistent.Sources/Alchemy/Provider/AlchemyProvider.swift (1)
37-41: LGTM!The initialization changes align well with the updated configuration structure.
Sources/Alchemy/Middleware/AlchemyRpcClient.swift (2)
14-27: LGTM! Access level change aligns with protocol update.The change from
overridetopublicaccess level is consistent with the protocol method rename inErc4337Client.swift.
29-40:❓ Verification inconclusive
Verify the impact of return type change.
The return type change from
AlchemyPaymasterAndDatatoPaymasterAndDatalooks good, but let's verify that all callers handle the new type correctly.
🏁 Script executed:
#!/bin/bash # Search for usages of AlchemyPaymasterAndData to ensure they've been updated rg "AlchemyPaymasterAndData" -A 5Length of output: 33
Action Required: Confirm Callers Handle New Return Type
No references to the old type “AlchemyPaymasterAndData” were found, which is a positive sign. However, please manually verify that all callers of the
requestPaymasterAndDatafunction correctly handle the new return typePaymasterAndData. Specifically, ensure that:
- Any instance where the return value is used (or cast) is updated accordingly.
- There are no implicit assumptions in type handling that might inadvertently rely on properties or behaviors specific to the old type.
Once you’ve confirmed that the new type is correctly handled everywhere, this function update can be fully approved.
Package.swift (2)
23-25: LGTM! New library configuration is consistent.The new Coinbase library follows the same pattern as existing libraries.
52-61: LGTM! Target configuration matches existing patterns.The new AASwiftCoinbase target:
- Follows the same dependency pattern as other targets
- Uses consistent path and sources configuration
Sources/Alchemy/Account/Utils.swift (1)
54-54: LGTM! Error type change aligns with provider-agnostic approach.The change from
AlchemyErrortoProviderErroris part of a broader reorganization to make error handling more generic and provider-agnostic.Sources/Core/Client/Erc4337Client.swift (1)
56-56: LGTM! Method rename follows Ethereum JSON-RPC naming convention.The rename from
maxPriorityFeePerGastoeth_maxPriorityFeePerGasaligns with standard Ethereum JSON-RPC method naming patterns.Sources/Core/Client/Erc4337RpcClient.swift (6)
98-111: LGTM! Method name now follows Ethereum JSON-RPC naming convention.The renaming of
maxPriorityFeePerGastoeth_maxPriorityFeePerGasbetter aligns with standard Ethereum JSON-RPC method naming conventions.
114-118: LGTM! Improved handling of baseFeeMultiplier.The changes enhance the handling of
baseFeeMultiplierby:
- Using nil coalescing for a default value of 1.2
- Adding validation to ensure it's greater than 1
98-111: LGTM! Method name now follows Ethereum's JSON-RPC convention.The renaming from
maxPriorityFeePerGastoeth_maxPriorityFeePerGasbetter aligns with Ethereum's JSON-RPC method naming convention.
114-118: LGTM! Improved validation logic for baseFeeMultiplier.The changes enhance the validation logic by:
- Setting a default value of 1.2 when
baseFeeMultiplieris nil- Ensuring the value is greater than 1
98-111: LGTM! Method name now follows Ethereum's JSON-RPC convention.The renaming from
maxPriorityFeePerGastoeth_maxPriorityFeePerGasbetter aligns with Ethereum's JSON-RPC method naming convention.
114-118: LGTM! Improved validation logic for baseFeeMultiplier.The changes enhance the validation logic by:
- Setting a default value of 1.2 when
baseFeeMultiplieris nil- Ensuring the value is greater than 1
Example/Sources/MainViewModel.swift (3)
152-162: LGTM! Improved configuration management.The changes enhance code maintainability by:
- Consolidating connection settings into a shared
connectionConfig- Reusing the same configuration for both provider and gas manager
152-162: LGTM! Improved configuration management.The changes enhance code maintainability by:
- Extracting connection configuration into a reusable variable
- Using the same configuration for both provider and gas manager
152-162: LGTM! Improved configuration management.The changes enhance code maintainability by:
- Consolidating connection settings into a shared
connectionConfigvariable- Using the new
ProviderConfigtype consistentlySources/Core/Provider/SmartAccountProvider.swift (7)
234-237: LGTM! Well-implemented builder pattern.The new method follows the established builder pattern, allowing for fluent method chaining.
253-253: LGTM! Proper fallback handling for middleware client.The implementation correctly uses the middleware client when available, falling back to the RPC client when not set.
234-237: LGTM! Well-implemented builder pattern.The new method follows the builder pattern consistently with other similar methods in the class.
253-253: LGTM! Proper fallback handling for middleware client.The implementation correctly uses nil coalescing to fall back to the regular client when middleware client is not set.
386-387: LGTM! Improved variable handling in chain function.The changes enhance readability by properly storing and updating the intermediate result.
234-237: LGTM! Well-designed middleware client integration.The new method provides a clean way to set an optional middleware client while maintaining a fluent interface.
253-253: LGTM! Proper fallback to default RPC client.The changes correctly use the middleware client when available, falling back to the default RPC client when not set.
| let entryPoint = try provider.getEntryPointAddress() | ||
| let params = PaymasterAndDataParams( | ||
| policyId: config.policyId, | ||
| entryPoint: entryPoint.asString(), | ||
| userOperation: uoStruct.toUserOperationRequest() | ||
| ) | ||
| let alchemyClient = provider.rpcClient as! AlchemyClient | ||
| let alchemyClient = client as! AlchemyClient |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace the force cast to AlchemyClient with a safer alternative or handle error conditions gracefully.
If the client is not of type AlchemyClient, this will result in a runtime crash. Consider throwing an error or returning early if the cast fails.
- let alchemyClient = client as! AlchemyClient
+ guard let alchemyClient = client as? AlchemyClient else {
+ throw ProviderError.invalidClient("Expected AlchemyClient, got \(type(of: client))")
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let alchemyClient = client as! AlchemyClient | |
| guard let alchemyClient = client as? AlchemyClient else { | |
| throw ProviderError.invalidClient("Expected AlchemyClient, got \(type(of: client))") | |
| } |
🧰 Tools
🪛 SwiftLint (0.57.0)
[Error] 131-131: Force casts should be avoided
(force_cast)
| requestPaymasterAndData(provider: self, config: config) as! Self | ||
| } else { | ||
| requestGasAndPaymasterData(provider: self, config: config) as! Self |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid force-casting to Self when returning modified providers.
These force casts may lead to runtime crashes. Consider returning a protocol or base type that encompasses all providers or performing a conditional cast with a fail-safe.
- let provider = if disableGasEstimation {
- requestPaymasterAndData(provider: self, config: config) as! Self
- } else {
- requestGasAndPaymasterData(provider: self, config: config) as! Self
- };
+ let providerBase: SmartAccountProvider = if disableGasEstimation {
+ requestPaymasterAndData(provider: self, config: config)
+ } else {
+ requestGasAndPaymasterData(provider: self, config: config)
+ }
+ guard let typedSelf = providerBase as? Self else {
+ throw ProviderError.invalidTypeCasting("Expected type: \(Self.self)")
+ }Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 SwiftLint (0.57.0)
[Error] 100-100: Force casts should be avoided
(force_cast)
[Error] 102-102: Force casts should be avoided
(force_cast)
| public func createCoinbaseClient( | ||
| url: String, | ||
| chain: Chain, | ||
| headers: [String: String] = [:] | ||
| ) -> CoinbaseClient { | ||
| return CoinbaseRpcClient(url: URL(string: url)!, network: EthereumNetwork.custom(String(describing: chain.id)), headers: headers) | ||
| } |
There was a problem hiding this comment.
Remove force unwrapping of URL to prevent potential crashes.
The current implementation force unwraps the URL, which could crash the app if an invalid URL is provided.
Apply this diff to safely handle invalid URLs:
- return CoinbaseRpcClient(url: URL(string: url)!, network: EthereumNetwork.custom(String(describing: chain.id)), headers: headers)
+ guard let validUrl = URL(string: url) else {
+ throw ProviderError.invalidUrl("Invalid URL format: \(url)")
+ }
+ return CoinbaseRpcClient(
+ url: validUrl,
+ network: EthereumNetwork.custom(chain.id.description),
+ headers: headers
+ )Also, add the following case to ProviderError:
case invalidUrl(String)| struct SponsorUserOpParams: Equatable, Encodable { | ||
| public let userOperation: UserOperationRequest | ||
| public let entryPoint: String | ||
|
|
||
| func encode(to encoder: Encoder) throws { | ||
| var container = encoder.unkeyedContainer() | ||
| try container.encode(userOperation) | ||
| try container.encode(entryPoint) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make the struct public and document the encoding format.
The struct has public properties but is internal, which seems inconsistent. Also, the encoding format using an unkeyed container should be documented.
Apply this diff to improve visibility and documentation:
-struct SponsorUserOpParams: Equatable, Encodable {
+/// Parameters for sponsoring a user operation.
+/// When encoded, produces an array in the format: [userOperation, entryPoint]
+public struct SponsorUserOpParams: Equatable, Encodable {
public let userOperation: UserOperationRequest
public let entryPoint: String
+ /// Encodes the parameters as an array in the format: [userOperation, entryPoint]
func encode(to encoder: Encoder) throws {
var container = encoder.unkeyedContainer()
try container.encode(userOperation)
try container.encode(entryPoint)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| struct SponsorUserOpParams: Equatable, Encodable { | |
| public let userOperation: UserOperationRequest | |
| public let entryPoint: String | |
| func encode(to encoder: Encoder) throws { | |
| var container = encoder.unkeyedContainer() | |
| try container.encode(userOperation) | |
| try container.encode(entryPoint) | |
| } | |
| } | |
| /// Parameters for sponsoring a user operation. | |
| /// When encoded, produces an array in the format: [userOperation, entryPoint] | |
| public struct SponsorUserOpParams: Equatable, Encodable { | |
| public let userOperation: UserOperationRequest | |
| public let entryPoint: String | |
| /// Encodes the parameters as an array in the format: [userOperation, entryPoint] | |
| func encode(to encoder: Encoder) throws { | |
| var container = encoder.unkeyedContainer() | |
| try container.encode(userOperation) | |
| try container.encode(entryPoint) | |
| } | |
| } |
| extension Chain { | ||
| var coinbasePaymasterAndBundlerUrl: String? { | ||
| switch self { | ||
| case Chain.Base: | ||
| return "https://api.developer.coinbase.com/rpc/v1/base" | ||
| case Chain.BaseSepolia: | ||
| return "https://api.developer.coinbase.com/rpc/v1/base-sepolia" | ||
| default: | ||
| return nil | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move URLs to configuration.
The hardcoded URLs should be moved to a configuration file for better maintainability and to support different environments (e.g., staging, production).
Consider creating a configuration struct:
public struct CoinbaseConfig {
let baseUrl: String
let baseSepoliaUrl: String
public static let production = CoinbaseConfig(
baseUrl: "https://api.developer.coinbase.com/rpc/v1/base",
baseSepoliaUrl: "https://api.developer.coinbase.com/rpc/v1/base-sepolia"
)
}Then update the extension to use the configuration:
extension Chain {
- var coinbasePaymasterAndBundlerUrl: String? {
+ func coinbasePaymasterAndBundlerUrl(config: CoinbaseConfig) -> String? {
switch self {
case Chain.Base:
- return "https://api.developer.coinbase.com/rpc/v1/base"
+ return config.baseUrl
case Chain.BaseSepolia:
- return "https://api.developer.coinbase.com/rpc/v1/base-sepolia"
+ return config.baseSepoliaUrl
default:
return nil
}
}
}| public var callGasLimit: BigUInt { | ||
| return BigUInt(hex: callGasLimitStr)! | ||
| } | ||
|
|
||
| public var verificationGasLimit: BigUInt { | ||
| return BigUInt(hex: verificationGasLimitStr)! | ||
| } | ||
|
|
||
| public var preVerificationGas: BigUInt { | ||
| return BigUInt(hex: preVerificationGasStr)! | ||
| } |
There was a problem hiding this comment.
Handle hex conversion failures safely.
The computed properties use force unwrapping (!) which could lead to runtime crashes if the hex strings are invalid.
Consider using optional chaining or throwing an error:
public var callGasLimit: BigUInt {
- return BigUInt(hex: callGasLimitStr)!
+ guard let value = BigUInt(hex: callGasLimitStr) else {
+ // Option 1: Return a default value
+ return BigUInt.zero
+ // Option 2: Throw an error
+ // throw ProviderError.invalidHexString("Invalid hex string for callGasLimit: \(callGasLimitStr)")
+ }
+ return value
}Apply similar changes to verificationGasLimit and preVerificationGas.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public var callGasLimit: BigUInt { | |
| return BigUInt(hex: callGasLimitStr)! | |
| } | |
| public var verificationGasLimit: BigUInt { | |
| return BigUInt(hex: verificationGasLimitStr)! | |
| } | |
| public var preVerificationGas: BigUInt { | |
| return BigUInt(hex: preVerificationGasStr)! | |
| } | |
| public var callGasLimit: BigUInt { | |
| - return BigUInt(hex: callGasLimitStr)! | |
| + guard let value = BigUInt(hex: callGasLimitStr) else { | |
| + // Option 1: Return a default value | |
| + return BigUInt.zero | |
| + // Option 2: Throw an error | |
| + // throw ProviderError.invalidHexString("Invalid hex string for callGasLimit: \(callGasLimitStr)") | |
| + } | |
| + return value | |
| } | |
| public var verificationGasLimit: BigUInt { | |
| - return BigUInt(hex: verificationGasLimitStr)! | |
| + guard let value = BigUInt(hex: verificationGasLimitStr) else { | |
| + // Option 1: Return a default value | |
| + return BigUInt.zero | |
| + // Option 2: Throw an error | |
| + // throw ProviderError.invalidHexString("Invalid hex string for verificationGasLimit: \(verificationGasLimitStr)") | |
| + } | |
| + return value | |
| } | |
| public var preVerificationGas: BigUInt { | |
| - return BigUInt(hex: preVerificationGasStr)! | |
| + guard let value = BigUInt(hex: preVerificationGasStr) else { | |
| + // Option 1: Return a default value | |
| + return BigUInt.zero | |
| + // Option 2: Throw an error | |
| + // throw ProviderError.invalidHexString("Invalid hex string for preVerificationGas: \(preVerificationGasStr)") | |
| + } | |
| + return value | |
| } |
Summary by CodeRabbit
New Features
Refactor