-
Notifications
You must be signed in to change notification settings - Fork 87
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.8.0 #1510
base: development
Are you sure you want to change the base?
Support RPC 0.8.0 #1510
Conversation
- Starknet - `0.13.3 <https://docs.starknet.io/documentation/starknet_versions/version_notes/#version0.13.3>`_ | ||
- RPC - `0.8.0 <https://github.com/starkware-libs/starknet-specs/releases/tag/v0.8.0>`_ | ||
|
||
TODO (#1498): List changes |
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.
note: Migration guide will be updated once we confirm that all introduced changes are correct.
note: Tests will be addressed later, once devnet fully implements RPC 0.8.0. |
starknet_py/net/client_models.py
Outdated
Calculates L1 max amount as `l1_max_amount` = `overall_fee` / `l1_gas_price`, unless `l1_gas_price` is 0, | ||
then L1 max amount is 0. Calculates `l1_max_price_per_unit` as `l1_max_price_per_unit` = `l1_gas_price`. | ||
|
||
Then multiplies `max_amount` by `amount_multiplier` and `max_price_per_unit` by `unit_price_multiplier`. | ||
Calculates L2 max amount as `l2_max_amount` = `overall_fee` / `l2_gas_price`, unless `l2_gas_price` is 0, | ||
then L2 max amount is 0. Calculates `l2_max_price_per_unit` as `l2_max_price_per_unit` = `l2_gas_price`. | ||
|
||
Then multiplies L1 max amount and L2 max amount by `amount_multiplier` and `l1_max_price_per_unit` | ||
and `l2_max_price_per_unit` by `unit_price_multiplier`. |
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 looks we no longer need to use this hacky method, as we now have all the necessary fields in EstimatedFee
.
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.
I've also added todo for decreasing amount and unit price multipliers.
@@ -231,7 +237,7 @@ async def deploy_v3( | |||
unique: bool = True, | |||
constructor_args: Optional[Union[List, Dict]] = None, | |||
nonce: Optional[int] = None, | |||
l1_resource_bounds: Optional[ResourceBounds] = None, | |||
resource_bounds: Optional[ResourceBoundsMapping] = None, |
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.
Maybe having 2 parameters l1_resource_bounds
and l2_resource_bounds
would be more convenient?
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.
Tbh I don't think it's good to pass all fields of data class separately.
Maybe we can set l1_gas
and l2_gas
as optional in ResourceBoundsMapping
(if not passed they will be zero)?
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.
Let's leave a parameter only for ResourceBoundsMapping
as it is now, but please add a method in ResourceBoundsMapping
that allows to specify only l1_gas
and sets the rest of the fields to 0
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.
Added init_with_l1_gas_only
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.
Please re-request once there are tests added
@@ -231,7 +237,7 @@ async def deploy_v3( | |||
unique: bool = True, | |||
constructor_args: Optional[Union[List, Dict]] = None, | |||
nonce: Optional[int] = None, | |||
l1_resource_bounds: Optional[ResourceBounds] = None, | |||
resource_bounds: Optional[ResourceBoundsMapping] = None, |
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.
Let's leave a parameter only for ResourceBoundsMapping
as it is now, but please add a method in ResourceBoundsMapping
that allows to specify only l1_gas
and sets the rest of the fields to 0
Addresses #1498
Introduced changes
Support RPC 0.8.0.
starknet_getStorageProof
StorageProofResponse
,GlobalRoots
,ContractsProof
,ContractLeafData
,NodeHashToNodeMappingItem
,BinaryNode
,EdgeNode
,ContractStorageKeys
data classesget_storage_proof
method inFullNodeClient
andClient
starknet_getMessagesStatus
MessageStatus
data classget_messages_status
method inFullNodeClient
andClient
l1_resource_bounds
param toresource_bounds
in tx-related methods and classes constructorsEstimatedFee
:gas_consumed
tol1_gas_consumed
gas_price
tol1_gas_price
data_gas_price
tol1_data_gas_price
data_gas_consumed
tol1_data_gas_consumed
l2_gas_consumed
,l2_gas_price
fieldsComputationResources
and updateExecutionResources
DataResources
toInnerCallExecutionResources
computation_resources
toexecution_resources
inFunctionInvocation
and change its type toInnerCallExecutionResources
instead ofComputationResources
failure_reason
toTransactionStatusResponse
starknet_getCompiledCasm
starknet_getCompiledCasm
#1514resource_bounds
instead ofl1_resource_bounds
paramComputationResources
has been removed;ExecutionResources
andStarknetFeeEstimate
have been updated