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.

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

Support Sepolia network; Support starknet_chainId endpoint #137

Merged
merged 11 commits into from
Feb 6, 2024

Conversation

DelevoXDG
Copy link
Collaborator

Describe your changes

Linked issues

Closes #131
Closes #132

Breaking changes

  • This issue contains breaking changes
  • All StarknetAccountProtocol signing methods are now async
  • Removed starknetChainId from StarknetProviderProtocol; Use StarknetProviderProtocol.getChainId() instead
  • Removed starknetChainId parameter from all StarknetProvider constructors

Comment on lines 4 to 19
public enum StarknetChainId: String, Codable, Equatable {
case mainnet = "0x534e5f4d41494e"
case goerli = "0x534e5f474f45524c49"
case sepolia_testnet = "0x534e5f5345504f4c4941"
case sepolia_integration = "0x534e5f494e544547524154494f4e5f5345504f4c4941"

public var feltValue: Felt {
switch self {
case .mainnet:
return Felt(fromHex: "0x534e5f4d41494e")!
case .testnet:
return Felt(fromHex: "0x534e5f474f45524c49")!
}
Felt(fromHex: self.rawValue)!
}

enum CodingKeys: String, CodingKey {
case mainnet = "0x534e5f4d41494e"
case goerli = "0x534e5f474f45524c49"
case sepolia_testnet = "0x534e5f5345504f4c4941"
case sepolia_integration = "0x534e5f494e544547524154494f4e5f5345504f4c4941"
}
Copy link
Collaborator Author

@DelevoXDG DelevoXDG Jan 29, 2024

Choose a reason for hiding this comment

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

Not actually sure about having StarknetChainId enum, now that we don't have to pass it to the StarknetProvider constructors?

It's more explicit if someone checks chain id, but I'm not sure there are users that don't know what kind of node they're using 😄
Might as well use Felt instead, and avoid the need to make changes in an unlikely event someone needs to use custom chain ids.

WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Do we need it elsewhere? for coding/decoding or for some verification? I suppose chainId endpoint returns hexes, so if we try to communicate about these ids to users somewhere, we might want to use user-readable names

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we only use those to for signing transactions

Copy link
Member

Choose a reason for hiding this comment

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

If that's the case we could probably remove it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will be addressed in #138

Sources/Starknet/Data/StarknetChainId.swift Outdated Show resolved Hide resolved
case mainnet = "0x534e5f4d41494e"
case goerli = "0x534e5f474f45524c49"
case sepolia_testnet = "0x534e5f5345504f4c4941"
case sepolia_integration = "0x534e5f494e544547524154494f4e5f5345504f4c4941"
Copy link
Member

Choose a reason for hiding this comment

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

not sure we need this tbh.

Copy link
Collaborator Author

@DelevoXDG DelevoXDG Jan 31, 2024

Choose a reason for hiding this comment

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

Probably won't hurt to have it just in case someone needs it?

Copy link
Member

Choose a reason for hiding this comment

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

does goerli integration introduce itself as goerli?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes

Comment on lines 4 to 19
public enum StarknetChainId: String, Codable, Equatable {
case mainnet = "0x534e5f4d41494e"
case goerli = "0x534e5f474f45524c49"
case sepolia_testnet = "0x534e5f5345504f4c4941"
case sepolia_integration = "0x534e5f494e544547524154494f4e5f5345504f4c4941"

public var feltValue: Felt {
switch self {
case .mainnet:
return Felt(fromHex: "0x534e5f4d41494e")!
case .testnet:
return Felt(fromHex: "0x534e5f474f45524c49")!
}
Felt(fromHex: self.rawValue)!
}

enum CodingKeys: String, CodingKey {
case mainnet = "0x534e5f4d41494e"
case goerli = "0x534e5f474f45524c49"
case sepolia_testnet = "0x534e5f5345504f4c4941"
case sepolia_integration = "0x534e5f494e544547524154494f4e5f5345504f4c4941"
}
Copy link
Member

Choose a reason for hiding this comment

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

Do we need it elsewhere? for coding/decoding or for some verification? I suppose chainId endpoint returns hexes, so if we try to communicate about these ids to users somewhere, we might want to use user-readable names

- Change name to `StarknetChainId.name` for chain id `SN_NAME`
@DelevoXDG DelevoXDG mentioned this pull request Feb 1, 2024
1 task
@DelevoXDG DelevoXDG merged commit ad1cd44 into main Feb 6, 2024
1 check passed
@DelevoXDG DelevoXDG deleted the feat/131-support-sepolia branch February 6, 2024 07:56
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.

Support starknet_chainId endpoint Support Sepolia networks
2 participants