-
Notifications
You must be signed in to change notification settings - Fork 59
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
Conversation
0af3db3
to
220f62d
Compare
```rust | ||
decl_runtime_apis! { | ||
pub trait XcqApi { | ||
fn execute_query(query: Vec<u8>, input: Vec<u8>, weight_limit: u64) -> XcqResult; |
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 by BuyExecution
instruction.
```rust | ||
decl_runtime_apis! { | ||
pub trait XcqApi { | ||
fn execute_query(query: Vec<u8>, input: Vec<u8>, weight_limit: u64) -> XcqResult; |
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.
impl extension_core::ExtensionCore for ExtensionImpl { | ||
type Config = ExtensionImpl; | ||
fn has_extension(id: <Self::Config as extension_core::Config>::ExtensionId) -> bool { | ||
matches!(id, 0 | 1) |
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
text/0126-introduce-xcq.md
Outdated
```rust | ||
ReportQuery { | ||
query: SizeLimitedXcq, | ||
weight_limit: Option<Weight>, |
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.
How can someone get this weight? Is it easy? If not, we could just limit it by the overall fee paid in PayFees
?
We need to have a good way of using this, otherwise everyone will just use Unlimited
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 should modify execute_query
to have it also return used weight so it can be used to estimate weight usage
We could make Unlimited
weight means the weight to be limited by fees. Limited weight will be useful for the case that we need to reserve some fees for other instruction, such as transact
`ErrorCode` is a enum | ||
- `ExceedMaxWeight = 0` | ||
- `MemoryAllocationError = 1` | ||
- `MemoryAccessError = 2` | ||
- `CallError = 3` | ||
- `OtherPVMError = 4` |
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.
This is not what I had in mind nor described in the channel when I introduced XCQ. While it may in and of itself be a reasonable proposal worthy of consideration and support, I think it should be given its own name ("view functions", I suppose, since this is appears to be a write up of that proposed as stated in the prior art section). XCQ is an abstract language similar in nature to XCM. Chains offering XCQ functionality offer an XCQ Translator. This is a small pieces of PVM code which translates an XCQ expression into a) a series of keys whose values must be fetched and b) a means of combining those values and the original expression into a result decodable by SCALE into an XCQ value. Crucially, XCQ is an abstraction. Like XCM it requires no knowledge whatsoever of the underlying chain or what it supports. |
On an early draft of XCQ, it is instruction based similar to XCM. However, in order to execute some expression to combine/transform the values, the instruction set needs to cover arithmetic and some memory management. Then the general feedbacks I got is that we should just use PVM instead and hence the direction we are working so far. The extension design is not too much differ to XCM adapters but generalized and more integrated. I believe the current proposal is able to achieve the goals of the original XCQ idea. XCQ program developer nor XCQ program users need to know the chain specific details (unless they are working with chain specific extensions). |
What is proposed has no obvious abstraction capability. |
The intended usage is that we also define a set of common extensions (e.g. fungibles and non-fungibles) that would be confirmed by all implementation chains. Then people will be able to develop generic XCQ programs with the ability to access this information and it will work with any chains. |
That's definitely not XCQ then. XCQ is meant to be a sister-protocol to XCM for inspection rather than mutating operations. The clue is in the name. The key part of XCM (and thus XCQ) is the abstraction language that averts the need to come to consensus on a commonly comprehensible means of communication. You've totally skipped this and focussed, apparently, only on the mechanism for describing the trie's values to be queried in the target chain. What you've done here is not what I described nor could it be considered "read-only-XCM". Propose it if you like. But don't call it XCQ. |
This is indeed lower level than an abstract language for querying state. But the idea is that we can build such abstract language on top of this work (by defining extensions and compile some DSL or Rust code to PVM). Will try to find another name to better describe this work. |
Looking for feedback.
Summary
This proposal introduces XCQ (Cross Consensus Query), which aims to serve as an intermediary layer between different chain runtime implementations and tools/UIs, to provide a unified interface for cross-chain queries.
Related RFC
A related XCM-format RFC is drafting.
Related Discussions
https://forum.polkadot.network/t/wasm-view-functions/1045
PoC implementations
https://github.com/open-web3-stack/XCQ