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

Support RPC 0.8.0 #530

Draft
wants to merge 77 commits into
base: main
Choose a base branch
from
Draft

Support RPC 0.8.0 #530

wants to merge 77 commits into from

Conversation

franciszekjob
Copy link
Collaborator

@franciszekjob franciszekjob commented Oct 22, 2024

Describe your changes

Support RPC 0.8.0.

  • starknet_getStorageProof
    • Add StorageProof, GlobalRoots, ContractsProof, ContractLeafData, NodeHashToNodeMappingItem, BinaryNode, EdgeNode, ContractStorageKey data classes
    • Add getStorageProof method in Provider and JsonRpcProvider
  • starknet_getMessagesStatus
    • Add MessageStatusList, MessageStatus data classes
    • Add getMessagesStatus method in Provider and JsonRpcProvider
  • Adapt execution resources to new receipt
    • Account, StandardAccount:
      • signDeployAccountV3 and executeV3 have now resourceBounds param instead of l1ResourceBounds
    • InvokeParamsV3, DeclareParamsV3, DeployAccountParamsV3:
      • Change l1ResourceBounds param to resourceBounds in constructor
    • EstimateFeeResponse :
      • rename gasConsumed to l1GasConsumed
      • rename gasPrice to l1GasPrice
      • rename dataGasPrice to l1DataGasPrice
      • rename dataGasConsumed to l1DataGasConsumed
      • add l2GasConsumed, l2GasPrice fields
    • Remove ComputationResources and update ExecutionResources
    • Add InnerCallExecutionResources
    • Remove constructor accepting only l1Gas in ResourceBoundsMapping
    • Rename computationResources to executionResources in FunctionInvocation and change its type to InnerCallExecutionResources instead of ComputationResources
    • Add ResourceBoundsMapping.ZERO
  • Add failureReason to GetTransactionStatusResponse

Linked issues

Closes #519

Breaking changes

  • This issue contains breaking changes
  • Not compatible with JSON-RPC 0.7.0 spec
  • Use resourceBounds instead of l1ResourceBounds param
  • ComputationResources has been removed; ExecutionResources and EstimateFeeResponse have been updated

@franciszekjob franciszekjob changed the title Feat/519 rpc 0.8.0 Support RPC 0.8.0 Oct 22, 2024
Comment on lines 665 to 677
/**
* Get merkle paths in one of the state tries.
*
* Get merkle paths in one of the state tries: global state, classes, individual contract.
*
* @param classHashes list of class hashes for which we want to prove membership
* @param contractAddresses list of contract addresses for which we want to prove membership
* @param contractsStorageKeys list of contract address and storage keys pairs
*
* @throws RequestFailedException
*/
fun getStorageProof(classHashes: List<Felt>? = null, contractAddresses: List<Felt>? = null, contractsStorageKeys: List<ContractStorageKey>? = null): Request<StorageProof>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

note: It's a bit exceptional situation, due to the fact that creating overloads would cause a conflict, e.g. for

fun getStorageProof(classHashes: List<Felt>, contractsStorageKeys: List<ContractStorageKey>): Request<StorageProof>

and

fun getStorageProof(contractAddresses: List<Felt>, contractsStorageKeys: List<ContractStorageKey>): Request<StorageProof>

(compiler won't know which method to use, contractAddresses and classHashes are of the same type). So unfortunately we need to add default values here.

@franciszekjob
Copy link
Collaborator Author

franciszekjob commented Oct 23, 2024

note: This PR is still wip.

  1. Tests will be addressed once devnet is not a blocker anymore.
  2. Full description about included changes will be added later.

Copy link
Contributor

@DelevoXDG DelevoXDG left a comment

Choose a reason for hiding this comment

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

I like where this is going 👍🏻 Waiting for the tests

Comment on lines +10 to +21
override fun selectDeserializer(element: JsonElement): DeserializationStrategy<NodeHashToNodeMappingItem.MerkleNode> {
val jsonElement = element.jsonObject
val binaryNodeKeys = NodeHashToNodeMappingItem.BinaryNode.serializer().descriptor.elementNames.toSet()
val edgeNodeKeys = NodeHashToNodeMappingItem.EdgeNode.serializer().descriptor.elementNames.toSet()

return when (jsonElement.keys) {
binaryNodeKeys -> NodeHashToNodeMappingItem.BinaryNode.serializer()
edgeNodeKeys -> NodeHashToNodeMappingItem.EdgeNode.serializer()
else -> throw IllegalArgumentException("Invalid MerkleNode JSON object: $jsonElement")
}
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

note: We could potentially give an error with more meaningful message (saying which fields are missing and if it's binary/edge node). However, in case of json with keys that belong both to binary and edge node
e.g.

{
    "left": ...,
    "path": ...
}

we can't clearly decide which type it is intended to be (binary/edge). I'm not fully sure if such logic is worth introducing.

A simple improvement would be to display required keys for binary/edge nodes.

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.

Support JSON-RPC v0.8.0
2 participants