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

Unify rpc #500

Merged
merged 20 commits into from
Jun 21, 2024
Merged

Unify rpc #500

merged 20 commits into from
Jun 21, 2024

Conversation

marioiordanov
Copy link
Contributor

@marioiordanov marioiordanov commented Jun 18, 2024

Usage related changes

Users can use http api endpoints via JSON-RPC api.

Development related changes

  • Endpoints accessible via HTTP endpoints can be invoked via JSON-RPC
  • StarknetRequest -> JsonRpcRequest
  • StarknetResponse -> JsonRpcResponse (StarknetResponse, DevnetResponse, Empty)
  • Most of the http api methods logic is extracted into <http api endpoint>_impl function which is called from JSON-RPC counterpart
  • Updated docs

Checklist:

  • Checked out the contribution guidelines
  • Applied formatting - ./scripts/format.sh
  • No linter errors - ./scripts/clippy_check.sh
  • No unused dependencies - ./scripts/check_unused_deps.sh
  • No spelling errors - ./scripts/check_spelling.sh
  • Performed code self-review
  • Rebased to the latest commit of the target branch (or merged it into my branch)
  • Updated the docs if needed - ./website/README.md
  • Linked the issues resolvable by this PR - linking info
  • Updated the tests if needed; all passing - execution info

@marioiordanov marioiordanov marked this pull request as ready for review June 18, 2024 14:04
@marioiordanov marioiordanov linked an issue Jun 19, 2024 that may be closed by this pull request
Copy link
Contributor

@FabijanC FabijanC left a comment

Choose a reason for hiding this comment

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

  1. Should we eventually document all JSON-RPC methods and parameters in a specification .json file, like Starknet does? We would then not include JSON-RPC examples in these .md documentation files, but would just link to the specification .json.

  2. What about testing? My idea is to convert all tests that relied on HTTP methods to rely on JSON-RPC methods. That would be in line with deprecating these HTTP methods and dropping their support in 2-3 months. For integration tests, it would include changing the utility methods to change the sent requests.

website/docs/balance.md Outdated Show resolved Hide resolved
website/docs/server-config.md Outdated Show resolved Hide resolved
website/docs/predeployed.md Outdated Show resolved Hide resolved
crates/starknet-devnet-server/src/api/json_rpc/mod.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@FabijanC FabijanC left a comment

Choose a reason for hiding this comment

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

Regarding the question from my first review:

Should we eventually document all JSON-RPC methods and parameters in a specification .json file, like Starknet does?

I guess for now it stays as it is.

Thanks for addressing the change requests. Approving.

@marioiordanov
Copy link
Contributor Author

For new JSON-RPC there is an issue created: #508

@marioiordanov marioiordanov merged commit 81ed110 into main Jun 21, 2024
1 check passed
@marioiordanov marioiordanov deleted the unify-rpc branch June 21, 2024 12:37
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.

API unification - more widespread JSON-RPC API
2 participants