Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor StarknetProvider and StarknetRequest #217

Merged
merged 18 commits into from
Sep 2, 2024
Merged

Refactor StarknetProvider and StarknetRequest #217

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
16658eb
Refactor `StarknetProvider` and `StarknetRequest`
franciszekjob Aug 29, 2024
5dd956e
Remove unused `StarknetBatchRequest`
franciszekjob Aug 29, 2024
2def07c
Update methods descriptions
franciszekjob Aug 29, 2024
e3f1254
Update methods descriptions
franciszekjob Aug 29, 2024
cdc3c24
Add missing `orderRpcResults` function
franciszekjob Aug 29, 2024
f8d0bc7
Add public init to `StarknetRequest`
franciszekjob Aug 29, 2024
59eed4d
Format
franciszekjob Aug 29, 2024
a1e6722
Restore `emptyBatchRequestError`
franciszekjob Aug 30, 2024
86d90e9
Correct comment phrasing
franciszekjob Aug 30, 2024
e5798bb
Remove public `init` from `StarknetRequest`
franciszekjob Aug 30, 2024
e30d9d9
Keep `JsonRpcMethod` internal
franciszekjob Aug 30, 2024
9be66ea
Change `StarknetRequest` extension to `RequestBuilder` enum
franciszekjob Aug 30, 2024
a909a1d
Remove generic type from returned `StarknetRequest` in `RequestBuilder`
franciszekjob Aug 30, 2024
ed08337
Remove unnecessarily added `public` modifiers
franciszekjob Aug 30, 2024
5615492
Format DocC
franciszekjob Aug 30, 2024
e0cb012
Remove `provider` from `StarknetAccountProtocol`
franciszekjob Aug 30, 2024
cb1caf1
Keep `provider` private in `StarknetAccount`
franciszekjob Aug 30, 2024
3f8581b
Change methods in `RequestBuilder` to be public
franciszekjob Aug 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 19 additions & 19 deletions Sources/Starknet/Accounts/StarknetAccount.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public class StarknetAccount: StarknetAccountProtocol {
public let chainId: StarknetChainId

private let signer: StarknetSignerProtocol
private let provider: StarknetProviderProtocol
public let provider: StarknetProviderProtocol
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

note: provider needs to be changed to public, because it's added in StarknetAccountProtocol here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmmm, I understand that it's used in the extensions, but I wonder if provider should be included in the account protocol, as it's making the protocol reliant on component that is more implementation-specific (rpc communication), therefore making the account protocol less abstract

Copy link
Collaborator Author

@franciszekjob franciszekjob Aug 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

as it's making the protocol reliant on component that is more implementation-specific (rpc communication), therefore making the account protocol less abstract

I agree 👍 Just one remark - not adding provider in StarknetAccountProtocol enforces to add extra provider param in estimateFeeV1 and estimateFeeV3. Hope this one is not too cumbersome while using these methods.

Wdyt guys @DelevoXDG @mluisbrown?

Copy link

Choose a reason for hiding this comment

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

We would only need to pass in the provider in the estimate methods that don't take nonce param, right? That seems ok to me.

Copy link

Choose a reason for hiding this comment

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

We would keep the provider property in StarknetAccount though, right? It doesn't need to be in the protocol but it needs to be a public property in StarknetAccount.

Copy link
Collaborator Author

@franciszekjob franciszekjob Aug 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Ofc, we will keep it in StarknetAccount . I added provider param to estimateFeeV1/3 which don't have nonce param 👍 .

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't we just make the methods abstract and define them in StarknetAccount instead?


public init(address: Felt, signer: StarknetSignerProtocol, provider: StarknetProviderProtocol, chainId: StarknetChainId, cairoVersion: CairoVersion) {
self.address = address
Expand Down Expand Up @@ -93,20 +93,20 @@ public class StarknetAccount: StarknetAccountProtocol {
if let paramsNonce = params.nonce {
nonce = paramsNonce
} else {
nonce = try await getNonce().send()
nonce = try await provider.send(request: getNonce())
}

if let paramsMaxFee = params.maxFee {
maxFee = paramsMaxFee
} else {
let feeEstimate = try await estimateFeeV1(calls: calls, nonce: nonce).send()[0]
let feeEstimate = try await provider.send(request: estimateFeeV1(calls: calls, nonce: nonce))[0]
maxFee = feeEstimate.toMaxFee()
}

let params = StarknetInvokeParamsV1(nonce: nonce, maxFee: maxFee)
let signedTransaction = try signV1(calls: calls, params: params, forFeeEstimation: false)

return provider.addInvokeTransaction(signedTransaction)
return StarknetRequest<StarknetInvokeTransactionResponse>.addInvokeTransaction(signedTransaction)
}

public func executeV3(calls: [StarknetCall], params: StarknetOptionalInvokeParamsV3) async throws -> StarknetRequest<StarknetInvokeTransactionResponse> {
Expand All @@ -116,70 +116,70 @@ public class StarknetAccount: StarknetAccountProtocol {
if let paramsNonce = params.nonce {
nonce = paramsNonce
} else {
nonce = try await getNonce().send()
nonce = try await provider.send(request: getNonce())
}

if let paramsResourceBounds = params.resourceBounds {
resourceBounds = paramsResourceBounds
} else {
let feeEstimate = try await estimateFeeV3(calls: calls, nonce: nonce).send()[0]
let feeEstimate = try await provider.send(request: estimateFeeV3(calls: calls, nonce: nonce))[0]
resourceBounds = feeEstimate.toResourceBounds()
}

let params = StarknetInvokeParamsV3(nonce: nonce, l1ResourceBounds: resourceBounds.l1Gas)
let signedTransaction = try signV3(calls: calls, params: params, forFeeEstimation: false)

return provider.addInvokeTransaction(signedTransaction)
return StarknetRequest<StarknetInvokeTransactionResponse>.addInvokeTransaction(signedTransaction)
}

public func executeV1(calls: [StarknetCall], estimateFeeMultiplier: Double) async throws -> StarknetRequest<StarknetInvokeTransactionResponse> {
let nonce = try await getNonce().send()
let feeEstimate = try await estimateFeeV1(calls: calls, nonce: nonce).send()[0]
let nonce = try await provider.send(request: getNonce())
let feeEstimate = try await provider.send(request: estimateFeeV1(calls: calls, nonce: nonce))[0]
let maxFee = feeEstimate.toMaxFee(multiplier: estimateFeeMultiplier)

let params = StarknetInvokeParamsV1(nonce: nonce, maxFee: maxFee)
let signedTransaction = try signV1(calls: calls, params: params, forFeeEstimation: false)

return provider.addInvokeTransaction(signedTransaction)
return StarknetRequest<StarknetInvokeTransactionResponse>.addInvokeTransaction(signedTransaction)
}

public func executeV3(calls: [StarknetCall], estimateAmountMultiplier: Double, estimateUnitPriceMultiplier: Double) async throws -> StarknetRequest<StarknetInvokeTransactionResponse> {
let nonce = try await getNonce().send()
let feeEstimate = try await estimateFeeV3(calls: calls, nonce: nonce).send()[0]
let nonce = try await provider.send(request: getNonce())
let feeEstimate = try await provider.send(request: estimateFeeV3(calls: calls, nonce: nonce))[0]
let resourceBounds = feeEstimate.toResourceBounds(amountMultiplier: estimateAmountMultiplier, unitPriceMultiplier: estimateUnitPriceMultiplier)

let params = StarknetInvokeParamsV3(nonce: nonce, l1ResourceBounds: resourceBounds.l1Gas)
let signedTransaction = try signV3(calls: calls, params: params, forFeeEstimation: false)

return provider.addInvokeTransaction(signedTransaction)
return StarknetRequest<StarknetInvokeTransactionResponse>.addInvokeTransaction(signedTransaction)
}

public func estimateFeeV1(calls: [StarknetCall], nonce: Felt, skipValidate: Bool) async throws -> StarknetRequest<[StarknetFeeEstimate]> {
let params = StarknetInvokeParamsV1(nonce: nonce, maxFee: .zero)
let signedTransaction = try signV1(calls: calls, params: params, forFeeEstimation: true)

return provider.estimateFee(for: signedTransaction, simulationFlags: skipValidate ? [.skipValidate] : [])
return StarknetRequest<StarknetFeeEstimate>.estimateFee(for: signedTransaction, simulationFlags: skipValidate ? [.skipValidate] : [])
}

public func estimateFeeV3(calls: [StarknetCall], nonce: Felt, skipValidate: Bool) async throws -> StarknetRequest<[StarknetFeeEstimate]> {
let params = StarknetInvokeParamsV3(nonce: nonce, l1ResourceBounds: .zero)
let signedTransaction = try signV3(calls: calls, params: params, forFeeEstimation: true)

return provider.estimateFee(for: signedTransaction, simulationFlags: skipValidate ? [.skipValidate] : [])
return StarknetRequest<StarknetFeeEstimate>.estimateFee(for: signedTransaction, simulationFlags: skipValidate ? [.skipValidate] : [])
}

public func estimateDeployAccountFeeV1(classHash: Felt, calldata: StarknetCalldata, salt: Felt, nonce: Felt, skipValidate: Bool) async throws -> StarknetRequest<[StarknetFeeEstimate]> {
let params = StarknetDeployAccountParamsV1(nonce: nonce, maxFee: 0)
let signedTransaction = try signDeployAccountV1(classHash: classHash, calldata: calldata, salt: salt, params: params, forFeeEstimation: true)

return provider.estimateFee(for: signedTransaction, simulationFlags: skipValidate ? [.skipValidate] : [])
return StarknetRequest<StarknetFeeEstimate>.estimateFee(for: signedTransaction, simulationFlags: skipValidate ? [.skipValidate] : [])
}

public func estimateDeployAccountFeeV3(classHash: Felt, calldata: StarknetCalldata, salt: Felt, nonce: Felt, skipValidate: Bool) async throws -> StarknetRequest<[StarknetFeeEstimate]> {
let params = StarknetDeployAccountParamsV3(nonce: nonce, l1ResourceBounds: .zero)
let signedTransaction = try signDeployAccountV3(classHash: classHash, calldata: calldata, salt: salt, params: params, forFeeEstimation: true)

return provider.estimateFee(for: signedTransaction, simulationFlags: skipValidate ? [.skipValidate] : [])
return StarknetRequest<StarknetFeeEstimate>.estimateFee(for: signedTransaction, simulationFlags: skipValidate ? [.skipValidate] : [])
franciszekjob marked this conversation as resolved.
Show resolved Hide resolved
}

public func sign(typedData: StarknetTypedData) throws -> StarknetSignature {
Expand All @@ -197,7 +197,7 @@ public class StarknetAccount: StarknetAccountProtocol {
)

do {
let result = try await provider.callContract(call).send()
let result = try await provider.send(request: StarknetRequest<[Felt]>.callContract(call))

guard result.count == 1 else {
throw StarknetAccountError.invalidResponse
Expand All @@ -220,6 +220,6 @@ public class StarknetAccount: StarknetAccountProtocol {
}

public func getNonce() async throws -> StarknetRequest<Felt> {
provider.getNonce(of: address)
StarknetRequest<Felt>.getNonce(of: address)
}
}
6 changes: 4 additions & 2 deletions Sources/Starknet/Accounts/StarknetAccountProtocol.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ public protocol StarknetAccountProtocol {
var address: Felt { get }
/// Chain id of the Starknet provider.
var chainId: StarknetChainId { get }
// Starknet provider.
var provider: StarknetProviderProtocol { get }

/// Sign list of calls as invoke transaction v1
///
Expand Down Expand Up @@ -361,7 +363,7 @@ public extension StarknetAccountProtocol {

/// - Returns: struct containing fee estimate
func estimateFeeV1(calls: [StarknetCall], skipValidate: Bool = false) async throws -> StarknetRequest<[StarknetFeeEstimate]> {
let nonce = try await getNonce().send()
let nonce = try await provider.send(request: getNonce())
return try await estimateFeeV1(calls: calls, nonce: nonce, skipValidate: skipValidate)
}

Expand All @@ -373,7 +375,7 @@ public extension StarknetAccountProtocol {
///
/// - Returns: struct containing fee estimate
func estimateFeeV3(calls: [StarknetCall], skipValidate: Bool = false) async throws -> StarknetRequest<[StarknetFeeEstimate]> {
let nonce = try await getNonce().send()
let nonce = try await provider.send(request: getNonce())
return try await estimateFeeV3(calls: calls, nonce: nonce, skipValidate: skipValidate)
}

Expand Down
30 changes: 0 additions & 30 deletions Sources/Starknet/Network/StarknetBatchRequest.swift
View file
Open in desktop
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

note: Deleting as it's not used anymore.

This file was deleted.

Loading
Loading