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

0.6 spec #273

Merged
merged 78 commits into from
Dec 11, 2023
Merged

0.6 spec #273

merged 78 commits into from
Dec 11, 2023

Conversation

marioiordanov
Copy link
Contributor

@marioiordanov marioiordanov commented Dec 6, 2023

Usage related changes

0.6.0 RPC spec

Development related changes

Checklist:

  • Applied formatting - ./scripts/format.sh
  • No linter errors - ./scripts/clippy_check.sh
  • No unused dependencies - ./scripts/check_unused_deps.sh
  • Performed code self-review
  • Rebased to the last commit of the target branch (or merged it into my branch)
  • Documented the changes
  • Linked the issues which this PR resolves
  • Checked the TODO section in README.md if this PR resolves it
  • Updated the tests
  • All tests are passing - cargo test

renamed and refactored create_erc20 to create_erc20_at_address
renamed and refactored initialize_erc20 to intialize_erc20_at_address
…dress. get_balance method accepts an enum to return corresponding balance. adapted tests
… letters, but starknet_api use serde_rename with uppercase
@marioiordanov
Copy link
Contributor Author

Hi @marioiordanov, can you please add FEE_PAYMENT along with its usages?
@marioiordanov There are few more issues I came across so far:

  1. Missing execution_resources in FUNCTION_INVOCATION
  2. price_in_fri of l1_gas_price is not always returned e.g. when making a starknet_getBlockWithTxs or starknet_getBlockWithTxHashes request

@DelevoXDG the commits after your comments address the requested changes

@DelevoXDG
Copy link

Thanks!

@marioiordanov
Copy link
Contributor Author

Thanks!

the next commit will address the execution_resources in FUNCTION_INVOCATION

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.

Can the two old comments with this content:

// TODO: in spec RPC response the contract class is missing

be resolved now?

For hash calculation logic, I stopped carefully reviewing it after I realized it's all tested against real transactions and is producing expected hash values.

crates/types/src/rpc/transactions.rs Outdated Show resolved Hide resolved
crates/starknet-server/src/main.rs Outdated Show resolved Hide resolved
crates/starknet/src/starknet/mod.rs Show resolved Hide resolved
crates/starknet/src/starknet/estimations.rs Outdated Show resolved Hide resolved
crates/starknet/src/transactions.rs Outdated Show resolved Hide resolved
crates/types/src/rpc/transaction_receipt.rs Outdated Show resolved Hide resolved
crates/starknet/src/starknet/add_invoke_transaction.rs Outdated Show resolved Hide resolved
@marioiordanov
Copy link
Contributor Author

Can the two old comments with this content:

// TODO: in spec RPC response the contract class is missing

be resolved now?

For hash calculation logic, I stopped carefully reviewing it after I realized it's all tested against real transactions and is producing expected hash values.

No, because this is the reason for this issue #248 . Dump functionality needs refactor

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.

3 participants