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

Jump to bottom

JSON-RPC methods returning extra properties not in the spec #248

Closed
marioiordanov opened this issue Nov 14, 2023 · 5 comments · Fixed by #335
Closed

JSON-RPC methods returning extra properties not in the spec #248

marioiordanov opened this issue Nov 14, 2023 · 5 comments · Fixed by #335
Assignees
@marioiordanov

Comments

@marioiordanov
Copy link
Contributor

The following methods slightly diverge from the spec, due to returning more fields than those fields in the JSON-RPC specification.
"starknet_getBlockWithTxs", "starknet_getTransactionByBlockIdAndIndex", "starknet_getTransactionByHash", "starknet_getTransactionReceipt"
@marioiordanov
Copy link
Contributor Author

marioiordanov commented Nov 14, 2023 •
edited
Loading

example:

  • DeclareTransactionV1 -> contract_class in starknet_getTransactionByHash
  • DeployAccountTransation -> contract_address in starknet_getBlockWithTxns

@marioiordanov marioiordanov mentioned this issue Nov 14, 2023
Merged
10 tasks
@FabijanC FabijanC mentioned this issue Nov 15, 2023
Closed
2 tasks
@FabijanC FabijanC changed the title Methods diverge from the spec JSON-RPC methods returning extra properties not in the spec Nov 15, 2023
marioiordanov added a commit that referenced this issue Nov 17, 2023
@marioiordanov
added forgotten method mentioned in issue #248
956acf2
marioiordanov added a commit that referenced this issue Nov 21, 2023
@marioiordanov @FabijanC
0.5.0 spec (#230)
2c984aa 
* modified contract error in ApiError

* remove pending transactions endpoint

* add spec_version

* added config file where the version of the rpc_spec is specified

* add get_transaction_execution_and_finality_status, make TransactionFinalityStatus non optional, remove create_rejected method

* added starknet_version and l1_gas_price to block header and entry to config file

* added constants for builtins names

* execution resources struct

* map transaction execution info to execution resources

* use builtins constants in vm_resource_fee_cost setting

* use txn execution info when generating a receipt

* add check for empty string passed as abi when deserializing sierra class

* removed empty file

* test functionality for reading json-rpc spec and testing it against existing in/out structures

* remove not needed test files

* anotate request inputs with deny_unknown_fields so if there are any additions to the requests in the spec, the deserialization will fail

* move models/state to types/rpc/state

* improve algorithm for generating request responses of spec

* extract ThinStateDiff logic generation into converter

* replace PatriciaKeyHex with PatriciaKey

* add state diff to transaction trace

* add instructions to modify the spec

* modify tests for simulated transactions

* remove println

* generate only required fields

* tests for generated requests/responses from the spec

* edit the spec

* comments about extra fields not present in 0.5.0 spec

* add execution info to get_receipt params

* add StarknetResponse - enum for covering all return values from json rpc methods

* return StarknetResponse from each method of json-rpc

* changes to match 0.5.0 spec

* dependencies

* clippy + fmt

* lock file

* clippy fix

* marked spec 0.5.0

* Update crates/starknet-server/tests/test_simulate_transactions.rs

Co-authored-by: FabijanC <[email protected]>

* remove comment

* fmt

* edit starknet_version

* update starknet_version in config

* added ordered event and ordered message to l1

* use thread_rng().gen_bool in do_for_boolean_primitive

* improved generated_combined_schema readability

* not needed serde attribute

* fmt

* added comment

* added forgotten method mentioned in issue  #248

* fix test

* clippy

* fmt

---------

Co-authored-by: FabijanC <[email protected]>
@DelevoXDG
Copy link

DelevoXDG commented Nov 23, 2023 •
edited
Loading

Hi, I think this should be a part of 0.5.0 tag (once it's ready), it might be confusing that the tagged version is not fully compatible with the spec 😅

@DelevoXDG DelevoXDG mentioned this issue Nov 29, 2023
Closed
@marioiordanov
Copy link
Contributor Author

starknet_getTransactionReceipt will be fixed in #273

@marioiordanov marioiordanov mentioned this issue Dec 11, 2023
Merged
10 tasks
@DelevoXDG
Copy link

@marioiordanov

Any updates on this?

@marioiordanov
Copy link
Contributor Author

Hey @DelevoXDG unfortunately we didn't do any work on this issue, recently. Couple of refactoring PRs took place, so we will reevaluate it.

@marioiordanov marioiordanov linked a pull request Feb 6, 2024 that will close this issue
Remove additional fields returned compared to RPC spec #335
Merged
10 tasks
@marioiordanov marioiordanov self-assigned this Feb 6, 2024
@franciszekjob franciszekjob mentioned this issue May 20, 2024
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@marioiordanov marioiordanov

Labels
None yet
Projects
starknet-devnet-rs
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove additional fields returned compared to RPC spec 0xSpaceShard/starknet-devnet-rs
2 participants
@DelevoXDG @marioiordanov