-
Notifications
You must be signed in to change notification settings - Fork 81
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
Support RPC 0.7.0 #1307
Support RPC 0.7.0 #1307
Conversation
Note: Some tests on network will fail, because the testnet networks don't support Starknet |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## development #1307 +/- ##
===============================================
- Coverage 98.01% 98.00% -0.02%
===============================================
Files 93 93
Lines 4592 4658 +66
===============================================
+ Hits 4501 4565 +64
- Misses 91 93 +2
|
class PendingBlockHeaderSchema(Schema): | ||
parent_hash = Felt(data_key="parent_hash", required=True) | ||
timestamp = fields.Integer(data_key="timestamp", required=True) | ||
sequencer_address = Felt(data_key="sequencer_address", required=True) | ||
transactions = fields.List( | ||
fields.Nested(TypesOfTransactionsSchema(unknown=EXCLUDE)), | ||
data_key="transactions", | ||
required=True, | ||
l1_gas_price = fields.Nested( | ||
ResourcePriceSchema(), data_key="l1_gas_price", required=True | ||
) | ||
l1_data_gas_price = fields.Nested( | ||
ResourcePriceSchema(), data_key="l1_data_gas_price", required=True | ||
) | ||
l1_da_mode = L1DAModeField(data_key="l1_da_mode", required=True) | ||
starknet_version = fields.String(data_key="starknet_version", required=True) | ||
|
||
|
||
class BlockHeaderSchema(Schema): | ||
block_hash = Felt(data_key="block_hash", required=True) | ||
parent_hash = Felt(data_key="parent_hash", required=True) | ||
block_number = fields.Integer(data_key="block_number", required=True) | ||
new_root = Felt(data_key="new_root", required=True) | ||
timestamp = fields.Integer(data_key="timestamp", required=True) | ||
sequencer_address = Felt(data_key="sequencer_address", required=True) | ||
l1_gas_price = fields.Nested( | ||
ResourcePriceSchema(), data_key="l1_gas_price", required=True | ||
) | ||
l1_data_gas_price = fields.Nested( | ||
ResourcePriceSchema(), data_key="l1_data_gas_price", required=True | ||
) | ||
l1_da_mode = L1DAModeField(data_key="l1_da_mode", required=True) | ||
starknet_version = fields.String(data_key="starknet_version", required=True) |
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.
Nit: Would it be reasonable to add an intermediate class to avoid some code duplication?
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.
On the other hand, it's "copied" from starknet-spec. IMO it's cleaner when we keep this basic spec class implement explicit
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.
We can consider introducing an additional class, such as BlockHeaderCommon
in #1231
STARKNET_VERSION: "0.13.0" | ||
RPC_SPEC_VERSION: "0.6.0" | ||
DEVNET_SHA: "1bd447d" | ||
DEVNET_SHA: "c6ffb99" |
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.
shouldn't we use devnet version: 230b74c? https://github.com/0xSpaceShard/starknet-devnet-rs/releases/tag/v0.0.3
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 doesn't make any difference, as there is no relevant changes in the commits following c6ffb99
(only changes to CI and package version).
@@ -1,3 +1,5 @@ | |||
# pylint: disable=too-many-lines |
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.
we could think about splitting this file into the smallest. but in separate task
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.
Co-authored-by: Maksim Zdobnikau <43750648+DelevoXDG@users.noreply.github.com>
Closes #1281
Initial PR #1282
Introduced changes
StarknetBlockCommon
withBlockHeader
and addPendingBlockHeader
to align with the RPC specificationparent_block_hash
,root
->parent_hash
,new_root
BlockHeader
inStarknetBlock
,StarknetBlockWithTxHashes
andStarknetBlockWithReceipts
, the same for respective pending classesAccount
classFullNodeClient.get_block_with_receipts
max_fee
/l1_resource_bounds
for devnet tests