Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Introduce XCQ #126
base: main
Are you sure you want to change the base?
Introduce XCQ #126
Changes from all commits
cdf886c
ae88d8d
62174b1
fa97e4a
740f5e0
bff1067
3678eee
220f62d
6c637c3
69e3320
89e098f
9def360
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 guess any user can call this Runtime API and provide their XCQ program and input args? Is it expected that the runtime will have some default gas/weight limit to make it more difficult for users to supply long running/complex programs that DoS the node?
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.
The
weight_limit
argument in this single API restricts the usage of a single execution. The overall resource limit is enforced by Runtime config and RPC rate limiting, which is out of the scope for a single RuntimeAPI. As for XCM usage, the gas/weight limit can be enforced byBuyExecution
instruction.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 the weight limit one-dimensional when two-dimensional weights are in use now? Is this weight somehow different? Or is it depends on the context? If so, can we use a different word to avoid confusion?
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.
Proof size doesn't really matter in the context of runtime API but yeah u64 is ambitious. We could use the Substrate Weight type and ignore proof size
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 we could name it
weight_ref_time_limit
to emphasize that it is intentionally one-dimensional. We could also leave a comment reminding people that the proof size isn't relevant in this context.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 thought extension ids were hashes of the content? The extension decides its id?
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 I read the RFC, I understood the
ExtensionCore
as a "special" "foundational" extension. It seems it will have some generic implementation where the extension IDs will indeed be content-addressed.Is that so? Is this code just an example of "some return value"?
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.
this is just a dummy implementation. we should leave it out in the example
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'm wondering if there's a way to improve these errors. I guess you could run and debug your XCQ program directly on the destination chain.
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.
Yes everything are deterministic so replay with extra logging enabled should be the preferred way for debugging. That's how we debug XCM today.