-
Notifications
You must be signed in to change notification settings - Fork 582
feat: add starknet support to getProvider()
and multicall()
and to networks.json
#1155
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
feat: add starknet support to getProvider()
and multicall()
and to networks.json
#1155
Conversation
There was a problem hiding this 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 introduces Starknet network support by updating provider functions, adding corresponding tests, and adding new entries to the networks list. Key changes include:
- Enhancing provider functions (getProvider and getBatchedProvider) to support Starknet.
- Adding new test cases for Starknet providers.
- Updating network-related schemas and the networks configuration with Starknet entries.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
test/e2e/utils/providers.spec.ts | Added tests for both EVM and Starknet providers. |
src/utils/provider.ts | Implemented provider support for Starknet networks alongside EVM. |
src/utils.ts | Removed AJV custom keyword for Starknet networks validation. |
src/schemas/space.json | Modified network validation schema by removing Starknet support. |
src/networks.json | Added new configuration entries for Starknet mainnet and testnet. |
Comments suppressed due to low confidence (1)
src/utils.ts:175
- Removing the custom AJV keyword 'starknetNetwork' alters the validation behavior for networks. If this change is intentional, update the relevant documentation and tests to reflect that Starknet networks are validated differently.
ajv.addKeyword({ keyword: 'starknetNetwork', ... });
49edcae
to
af0dc60
Compare
af0dc60
to
a56e313
Compare
There was a problem hiding this 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 integrates Starknet support by adding Starknet networks to the networks list and providing corresponding provider implementations. Key changes include:
- Adding Starknet network and strategy examples for testing.
- Updating provider utilities (getProvider and getBatchedProvider) to support Starknet networks.
- Adjusting JSON schemas and network definitions to include Starknet information.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
test/schema.spec.ts | Added tests for new Starknet network and strategy examples |
test/examples/space-starknet-strategies.json | Introduced a Starknet-specific strategy example |
test/examples/space-starknet-network.json | Introduced a Starknet-specific network example |
test/e2e/utils/providers.spec.ts | Added end-to-end tests for Starknet provider functionality |
src/utils/provider.ts | Updated provider implementations to support Starknet networks |
src/utils.ts | Simplified network schema validation by removing Starknet-specific keywords |
src/schemas/space.json | Adjusted network property validations in the space schema |
src/networks.json | Added Starknet network definitions with proper chainIds |
Comments suppressed due to low confidence (1)
src/utils/provider.ts:16
- The constant name DEFAULT_BROVIDER_URL appears to be a typo. Consider renaming it to DEFAULT_PROVIDER_URL for clarity.
const DEFAULT_BROVIDER_URL = 'https://rpc.snapshot.org';
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds full Starknet support by registering Starknet networks, updating provider logic, extending multicall utilities, and adding schema examples and tests.
- Registers Starknet mainnet/testnet in
networks.json
and routes requests inprovider.ts
- Updates
multicall
andMulticaller
to handle custom options and Starknet flows - Adds new example JSON schemas and integration/E2E tests for Starknet
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
src/networks.json | Added entries for Starknet mainnet and testnet |
src/utils/provider.ts | Updated getProvider /getBatchedProvider to handle Starknet vs EVM |
src/utils.ts | Removed deprecated schema keywords and adjusted multicall option cleanup |
src/utils/multicaller.ts | Typed paths and refined call signature |
src/schemas/space.json | Removed outdated maxLength and anyOf constraints |
test/schema.spec.ts | Added schema tests for Starknet network and strategies |
test/examples/space-starknet-*.json | Added example JSON for Starknet configurations |
test/integration/**/*.spec.ts | Extended multicaller and multicall integration tests |
test/e2e/utils/providers.spec.ts | Added E2E tests for Starknet provider behavior |
Comments suppressed due to low confidence (2)
src/utils/provider.ts:46
- The parameter name
broviderUrl
appears to be a typo and may be confusing; consider renaming it toproviderUrl
for clarity.
broviderUrl = DEFAULT_BROVIDER_URL,
src/utils/provider.ts:95
- Similarly, rename
broviderUrl
toproviderUrl
in the batched provider function signature to maintain consistency.
broviderUrl = DEFAULT_PROVIDER_URL,
getProvider()
and multicall()
and to networks.json
getProvider()
and multicall()
and to networks.jsongetProvider()
and multicall()
and to networks.json
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this 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 Starknet network support across snapshot’s provider and multicall functionalities by extending network configurations, updating memoization logic, and integrating new multicall implementations for Starknet. Key changes include:
- Extending tests and schemas to support Starknet networks
- Updating provider memoization and multicall functions to distinguish between EVM and Starknet protocols
- Adding new examples and network entries for Starknet mainnet and testnet
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
test/schema.spec.ts | Added new schema examples for Starknet delegation, network, and strategies |
test/integration/utils/providers.spec.ts | Extended provider tests to cover Starknet and memoization scenarios |
test/integration/multicall/* | Added comprehensive integration tests for multicall and multicaller with Starknet support |
src/utils/provider.ts | Updated provider functions to support Starknet alongside EVM using memoization |
src/networks.json | Introduced starknet network configurations (mainnet and testnet) |
src/multicall/* | Added new Starknet-specific multicall logic and updated multicaller for dual protocol support |
src/schemas/space.json | Revised schema validations for network fields to accommodate new Starknet chain ID formats |
src/utils.ts | Removed deprecated custom Starknet network keyword validations |
Comments suppressed due to low confidence (2)
src/schemas/space.json:131
- The removal of the maxLength constraint for the 'network' property (and similar changes in delegated network validation) may allow unexpected network values; please ensure this relaxation in validation is intentional and well documented for both EVM and Starknet networks.
"maxLength": 12,
src/utils/provider.ts:9
- [nitpick] The variable name 'broviderUrl' appears unconventional; if this naming is intentional, consider adding documentation or comments to clarify its purpose and avoid confusion with the standard 'providerUrl'.
readonly broviderUrl?: string;
…ps://github.com/snapshot-labs/snapshot.js into feat-add-starknet-to-network-list-and-providers
Failing tests are coming from After some digging, it seems to also fail on master, looks like our provider on brovider side may not support it (it was working 10 days ago) |
4362cfd
to
ec499c2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tAck
Toward https://github.com/snapshot-labs/workflow/issues/562
This PR adds the starknet mainnet and testnet to the networks list, allowing:
This also adds support for starknet to the
multicall
function exported in the utils (inspired by https://github.com/argentlabs/x-multicall), as well as theMulticaller
class, andgetProvider