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

Introduce XCQ #126

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

indirection42
Copy link

@indirection42 indirection42 commented Oct 25, 2024

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

@indirection42 indirection42 changed the title Introduce xcq Introduce XCQ Oct 25, 2024
text/0126-introduce-xcq.md Outdated Show resolved Hide resolved
text/0126-introduce-xcq.md Outdated Show resolved Hide resolved
text/0126-introduce-xcq.md Show resolved Hide resolved
text/0126-introduce-xcq.md Outdated Show resolved Hide resolved
text/0126-introduce-xcq.md Outdated Show resolved Hide resolved
text/0126-introduce-xcq.md Outdated Show resolved Hide resolved
@anaelleltd anaelleltd added the Proposed Is awaiting 3 formal reviews. label Nov 3, 2024
text/0126-introduce-xcq.md Outdated Show resolved Hide resolved
```rust
decl_runtime_apis! {
pub trait XcqApi {
fn execute_query(query: Vec<u8>, input: Vec<u8>, weight_limit: u64) -> XcqResult;
Copy link
Contributor

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?

Copy link
Author

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.

text/0126-introduce-xcq.md Outdated Show resolved Hide resolved
```rust
decl_runtime_apis! {
pub trait XcqApi {
fn execute_query(query: Vec<u8>, input: Vec<u8>, weight_limit: u64) -> XcqResult;
Copy link

@mrshiposha mrshiposha Dec 9, 2024

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?

Copy link
Contributor

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

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)
Copy link
Contributor

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?

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"?

Copy link
Contributor

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 Show resolved Hide resolved
```rust
ReportQuery {
query: SizeLimitedXcq,
weight_limit: Option<Weight>,
Copy link
Contributor

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

Copy link
Contributor

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

Comment on lines +284 to +289
`ErrorCode` is a enum
- `ExceedMaxWeight = 0`
- `MemoryAllocationError = 1`
- `MemoryAccessError = 2`
- `CallError = 3`
- `OtherPVMError = 4`
Copy link
Contributor

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.

Copy link
Contributor

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.

@gavofyork
Copy link
Contributor

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.

@xlc
Copy link
Contributor

xlc commented Dec 18, 2024

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).

@gavofyork
Copy link
Contributor

What is proposed has no obvious abstraction capability.

@xlc
Copy link
Contributor

xlc commented Dec 19, 2024

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.
The common extensions are not included in this RFC because they don't need to. This RFC is about the low level primitives that could be used to build the abstractions. After this is accepted, we could have another RFC to define common extensions.
Polkadot-SDK can offer standard implementations for pallets it provides as well. Basically the XCM adapters and related pallets. But the key things is that chains can just do their own implementation without being blocked by polkadot-sdk or any upstream library.

@gavofyork
Copy link
Contributor

gavofyork commented Jan 2, 2025

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.

@xlc
Copy link
Contributor

xlc commented Jan 3, 2025

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposed Is awaiting 3 formal reviews.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants