-
Notifications
You must be signed in to change notification settings - Fork 62
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 spec reader tests #657
base: json-rpc-v0.8.0
Are you sure you want to change the base?
Conversation
… to NotificationData
…nclude pending block as part of result from get_blocks
.for_each(|(_, block_hash)| { | ||
.for_each(|(block_number, block_hash)| { | ||
if *block_number == pending_block_number { | ||
insert_pending_block_in_final_result = false; |
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.
When will it be false? Only when one of the blocks in num_to_hash
has the same number as pending_block? And when should that be the case?
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.
Exactly in this case. num_to_hash
apart from being in the same object as pending_block
has no relation to it.
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.
Creating pending block have to be changed. It should advance the block number, but this is happening not in StarknetBlocks file, but in Starknet. Thats why this case is covered
crates/starknet-devnet-server/src/api/json_rpc/spec_reader/mod.rs
Outdated
Show resolved
Hide resolved
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
fn deserialize_response_to_type_or_panic<T: DeserializeOwned>( |
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.
Why is this method needed? I.e. what does it introduce that isn't supported by serde already? Writing the error to a custom file?
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.
just extracts common logic
- action: CHANGE | ||
what: DEPRECATED_CAIRO_ENTRY_POINT.properties.offset from $ref to type | ||
- action: ADD | ||
what: DoubleDeref tuple maximum 32767 to the integer part |
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.
Why does this need to be done for DoubleDeref
? Why doesn't it need to be done for CellRef
?
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.
It is added to CellRef, but automatically. Check out add:
section of this file.
crates/starknet-devnet-server/test_data/spec/0.8.0/edit_spec_instructions.yaml
Show resolved
Hide resolved
@@ -56,19 +92,11 @@ edit_manually: | |||
what: starknet_getBlockWithTxHashes -> result removed PENDING_BLOCK_WITH_TX_HASHES | |||
- action: ADD | |||
what: execution_status to required fields of the result of starknet_getTransactionStatus | |||
- action: ADD |
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.
Why was the order changed for some of the actions?
Development related changes
CI is failing because libraries used are not updated: starknet-rs, starknet_api, blockifier
Checklist:
./scripts/format.sh
./scripts/clippy_check.sh
./scripts/check_unused_deps.sh
./scripts/check_spelling.sh
./website/README.md