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

Update of getBlockWithTxHashes and getStateUpdate #461

Merged
merged 12 commits into from
May 9, 2024
Merged

Conversation

mikiw
Copy link
Contributor

@mikiw mikiw commented May 6, 2024

Usage related changes

  • update of getBlockWithTxHashes and getStateUpdate consistent with starknet-specs-0.7.1

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, including the TODO section
  • Linked the issues resolvable by this PR - linking info
  • Updated the tests if needed; all passing - execution info

@mikiw mikiw marked this pull request as draft May 6, 2024 08:16
@mikiw mikiw linked an issue May 6, 2024 that may be closed by this pull request
1 task
@mikiw mikiw marked this pull request as ready for review May 7, 2024 11:22
@mikiw mikiw changed the title Pending block spec update Update of getBlockWithTxHashes and getStateUpdate May 7, 2024
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.

I'd like @marioiordanov to give his opinion on the type / struct conversations started by @mikiw.

crates/starknet-devnet-core/src/blocks/mod.rs Show resolved Hide resolved
crates/starknet-devnet-types/src/rpc/block.rs Show resolved Hide resolved
crates/starknet-devnet/tests/test_blocks_on_demand.rs Outdated Show resolved Hide resolved
crates/starknet-devnet/tests/common/background_devnet.rs Outdated Show resolved Hide resolved
@marioiordanov
Copy link
Contributor

marioiordanov commented May 8, 2024

In starknet-devnet-server/test_data/spec/0.7.1/<....>.json files, PENDING structs have to be added. Then tests in json_rpc/spec_reader will show if the structs are correct

@mikiw
Copy link
Contributor Author

mikiw commented May 8, 2024

In starknet-devnet-server/test_data/spec/0.7.1/<....>.json files, PENDING structs have to be added. Then tests in json_rpc/spec_reader will show if the structs are correct

fixed in ef745b9

@mikiw mikiw requested a review from FabijanC May 9, 2024 08:02
@mikiw mikiw merged commit a89d827 into main May 9, 2024
1 check passed
@mikiw mikiw deleted the block-spec-update branch May 9, 2024 13:31

// StarknetBlock needs to be mapped to PendingBlock response only in blocks_on_demand mode
// and when block_id is pending
if starknet.config.blocks_on_demand
Copy link
Contributor

Choose a reason for hiding this comment

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

this logic could be moved in Starknet-core project

@@ -24,21 +24,33 @@ impl JsonRpcHandler {

/// starknet_getBlockWithTxHashes
pub async fn get_block_with_tx_hashes(&self, block_id: BlockId) -> StrictRpcResult {
Copy link
Contributor

Choose a reason for hiding this comment

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

what about get_block_with_txs and get_block_with_receipts

}))
// StateUpdate needs to be mapped to PendingStateUpdate response only in blocks_on_demand
// mode and when block_id is pending
if starknet.config.blocks_on_demand
Copy link
Contributor

Choose a reason for hiding this comment

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

the same here, can be moved into Starknet-devnet-core crate.

if starknet.config.blocks_on_demand
&& block_id == ImportedBlockId::Tag(BlockTag::Pending).into()
{
Ok(StarknetResponse::PendingStateUpdate(PendingStateUpdate {
Copy link
Contributor

Choose a reason for hiding this comment

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

StarknetResponse::StateUpdate enum variant can receive an enum as its inner part, similar to StarknetResponse::TransactionTrace

if starknet.config.blocks_on_demand
&& block_id == ImportedBlockId::Tag(BlockTag::Pending).into()
{
Ok(StarknetResponse::PendingBlock(PendingBlock {
Copy link
Contributor

Choose a reason for hiding this comment

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

StarknetResponse::Block can receive an enum as its inner property. For example:

enum StarknetResponse{
Block(Block)
}

enum Block {
Block,
PendingBlock
}

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.

Inconsistent with starknet-spec responses
3 participants