Cross-chain query spec draft #735
Conversation
Amazing! Live implementation before your great changes btw -> https://github.com/defund-labs/defund/tree/main/x/query (relayer -> https://github.com/defund-labs/relayer). Feel free to fork as the starting point for the modularized version |
There was a problem hiding this comment.
Excellent work!! This is exactly what i had in mind. There are just a few minor points on the current spec.
One potential improvement I think is to have an additional message that allows the original caller to prune the result from the store once they retrieve it. This would require storing the caller address in the query result, but I think that's worth doing so that we don't have too many queried information staying in the store forever.
Note that if we do this, and also want to protect against malicious caller modules we may have to use object capabilities which would add complexity.
cc: @dtribble to see if this would be a strict requirement for smart contracts, etc.
|
|
||
| ### Assumptions | ||
|
|
||
| - **Safe chains:** Both the Querying and Queried chains are safe. |
There was a problem hiding this comment.
I have clarified this by adding: This means that, for every chain, the underlying consensus engine satisfies safety (e.g., the chain does not fork) and the execution of the state machine follows the described protocol.
This is probably a good improvement. Let's see what others think. It should not require many changes to the current version, even when using object capabilities. |
There was a problem hiding this comment.
Generally this looks fine to me, modulo a few minor comments. One other question I would have is around "callback" functionality - often, I expect the querying chain will want additional state machine logic to be executed when the query response is received. It seems to me like we could easily allow the bounty already in the query struct to pay for some additional execution - is that worth considering?
One concern I have with this bounty system is that it is vulnerable to front-running - any relayer who actually bothers to query the other chain can be front-run by the block proposer, who doesn't need to bother to query the other chain anymore. Have we considered this issue?
|
|
||
| #### Control Queried Data | ||
|
|
||
| The Querying Chain should have ultimate control over how to handle queried data. Like querying for a certain query form/type. |
There was a problem hiding this comment.
What does this mean, I don't quite follow - querying a specific location in state? What is a query form/type? Can we provide an example?
There was a problem hiding this comment.
I did not write this, it was in the original draft. I am not sure what it means. It may mean that the Querying Chain could pick the format in which the data is fetched, but I am not sure. I lean towards deleting this property.
There was a problem hiding this comment.
What does this mean, I don't quite follow - querying a specific location in state? What is a query form/type? Can we provide an example?
Sorry I wrote this part, To be clear, I am referring to what you are referencing above " I expect the querying chain will want additional state machine logic to be executed when the query response is received". The Querying chain should have full authority and the ability to handle any logic needed on query completion.
| interface CrossChainQuery struct { | ||
| id: Identifier | ||
| path: CommitmentPath | ||
| timeoutHeight: Height |
There was a problem hiding this comment.
should there also be a timeout timestamp, or do we think that isn't useful here?
There was a problem hiding this comment.
What would this be for?
There was a problem hiding this comment.
Query timeouts based on a timestamp on the destination chain, instead of a height. I can imagine this could be useful for use-cases which need to coordinate based on a global clock (e.g. oracles)
There was a problem hiding this comment.
The timeouts here are semantically different than the timeouts in IBC applications. Since there isn't a way for the relayer in this case to prove that they couldn't query a particular key by the timeout height on the destination, this timeout is the timeout on the source chain, at which point the source chain will timeout the query.
Perhaps we should either name it something different to avoid confusion or do away with the concept
There was a problem hiding this comment.
Ah right right, I stand corrected. I still think this could be useful, but it's not that important.
| timeoutHeight: Height | ||
| queryHeight: Height | ||
| clientId: Identifier | ||
| bounty: sdk.Coin |
There was a problem hiding this comment.
Instead of using the sdk.Coin type here, which is specific to Cosmos SDK chains, would it be better to use a generic Fee type, like the one described in ICS-29? Copying here for convenience:
interface Fee {
denom: string,
amount: uint256,
}
There was a problem hiding this comment.
I think it is a good idea. It is kind of weird to have cross-apps dependencies, isn't? We should find a way to define the fee interface somewhere else in the core specs. Since this is a relayer thingy, maybe the relayer should define it. For now I will link to ICS-29, as suggested.
There was a problem hiding this comment.
The spec should not be dependent on SDK types
There was a problem hiding this comment.
The spec uses the Fee interface of ICS29.
I think that if that's the common case, we should consider it.
I am not sure I fully understand the problem. Could you please elaborate a bit more? Is this referring to the fact that the proposer of the Querying Chain may never execute the transaction including the query result because it is not interested, i.e., the relayer never gets "paid"? I thought about this, and that's why one of the assumptions is that the Querying Chain is censorship-resistant, i.e., the Querying Chain cannot selectively omit transactions. |
The spec does not require the existence of a channel between the Querying and Queried chain, so that's not possible. |
No, the proposer can just copy the results of the query from the transaction in the mempool and resubmit the query result in order to claim the fee for themselves, meaning that the relayer never receives it - and thus no relayer, in expectation of the proposer employing this strategy, has any reason to fetch the data at all (so I don't really think this incentive mechanism works, without an alteration such as designating a specific relayer). |
This is correct, and it is true of any permissionless relayer incentivization. ICS29 will ship v1 with effectively the same problem, however the APIs are constructed such that we could easily specify a relayer in the future. Perhaps something similar should be done here. |
|
I'd like to propose two orthogonal but composable features for Interchain Queries to enable IBC PubSub as an application: see the feature summary comment. |
|
For those who didn't get my edits above, I'm attempting to support blockchain-backed applications with many clients all following a state stream, then independently submitting transactions to affect the stream. The only difficulty is in consuming the "next" value from a Queried chain using Interchain Queries. Here are proposed features to enable that application:
Could these be optional fields as part of the query request? Does this warrant a little query language to limit the satisfiability of queries? Implementing these restrictions in relayers should be straightforward, but they can be assisted by standard "on-change" events, such as:
These events would be advisory and for unproved clients; relayers would still request proof of them in the Queried chain's Merkle tree. |
|
Thought people may want to see how this PR relates to streaming: https://github.com/Agoric/agoric-sdk/blob/mfig-vstream/golang/cosmos/x/vstream/spec/01_concepts.md#forward-iteration-and-interchain-queries |
…crosschain-queries/README.md
|
I have addressed all pending concerns (listed in the PR description) as discussed. This is ready for review. |
There was a problem hiding this comment.
Nice work!
I think supporting the streams work @michaelfig is important, but would like to merge this and see any changes for those in a separate PR so we can review them in isolation
| interface CrossChainQuery struct { | ||
| id: Identifier | ||
| path: CommitmentPath | ||
| localTimeoutHeight: Height |
| 1) When the Querying Chain receives a query request, it calls `CrossChainQueryRequest` of the Cross-chain Querying module. This function generates a unique identifier for the query, stores it in its `privateStore` and emits a `sendQuery` event. Query requests can be submitted by other IBC modules as transactions to the Querying Chain or simply executed as part of the `BeginBlock` and `EndBlock` logic. | ||
| 2) A correct relayer listening to `sendQuery` events from the Querying Chain will eventually pick the query request up and execute it at the Queried Chain. The result is then submitted (on-chain) to the Querying Chain. | ||
| 3) When the query result is committed at the Querying Chain, this calls the `CrossChainQueryResult` function of the Cross-chain Querying module. | ||
| 4) The `CrossChainQueryResult` first retrieves the query from the `privateStore` using the query's unique identifier. It then proceeds to verify the result using its local client. If it passes the verification, the function removes the query from the `privateStore` and stores the result in a public path. |
There was a problem hiding this comment.
Why does the result need to be in a public path?
| We pass the relayer address just as in `CrossChainQueryResult` to allow for possible incentivization here as well. | ||
|
|
||
| ```typescript | ||
| function checkQueryTimeout( |
There was a problem hiding this comment.
We should store state in such a way that we can efficiently iterate over existing queries in timeout order (for timestamp and height), though this may be left as details for implementation
There was a problem hiding this comment.
I think this is an implementation detail. I have added the following sentence: "In this case, ongoing queries should be stored indexed by localTimeoutTimestamp and localTimeoutHeight to allow iterating over them more efficiently". In any case, if we think we should make this explicit in the spec, I can do the modification.
|
|
||
| ## Synopsis | ||
|
|
||
| This standard document specifies the data structures and state machine handling logic of the Cross-chain Querying module, which allows for cross-chain querying between IBC enabled chains. |
There was a problem hiding this comment.
nit: The title of the standard is Cross-chain Queries, but throughout Cross-chain Querying is used instead.
There was a problem hiding this comment.
nit: Also, I'd abbreviate Coss-chain Queries to CCQ (similarly to CCV for Cross-Chain Validation).
|
|
||
| ### Motivation | ||
|
|
||
| Interchain Accounts (ICS-27) brings one of the most important features IBC offers, cross-chain transactions (on-chain). Limited in this functionality is the querying of state from one chain, on another chain. Adding cross-chain querying via the Cross-chain Querying module gives unlimited flexibility to chains to build IBC enabled protocols around Interchain Accounts and beyond. |
There was a problem hiding this comment.
Could be useful to expand a bit on the motivation, e.g., what exact problems are solved by cross-chain queries that were not possible to solve with normal queries.
There was a problem hiding this comment.
There is a new motivation now. I think it is better. Thanks for pointing it out :)
|
|
||
| - **Censorship-resistant Querying Chain:** The Querying Chain cannot selectively omit transactions. | ||
|
|
||
| - **Correct relayer:** There is at least one correct relayer between the Querying and Queried chains. This is required for liveness. |
There was a problem hiding this comment.
The correct relayer assumption cannot be defined using correct relayer. In other words, correct relayer should be defined.
|
|
||
| #### Permissionless | ||
|
|
||
| A Querying Chain can query a chain and implement cross-chain querying without any approval from a third party or chain governance. Note that since there is no prior negotiation between chains, the Querying Chain cannot assume that queried data will be in an expected format. |
There was a problem hiding this comment.
Does the Querying Chain need permission from the Queried Chain? If not, we should make this clearer here.
|
|
||
| #### Minimal Queried Chain Work | ||
|
|
||
| A Queried Chain has to do no implementation work or add any module to enable cross-chain querying. By utilizing an RPC client on a relayer, this is possible. |
There was a problem hiding this comment.
I would say here that any chain that provides query support can act as a Queried Chain.
There was a problem hiding this comment.
Are there chains that do not provide query support? What's required to support queries? This is more for my own education :)
There was a problem hiding this comment.
Are there chains that do not provide query support?
Probably not, but just to be sure since it's a prerequisite of Interchain Queries.
Just to make sure, my comment was as a replacement for the entire line. I think that "query support" is the only requirement. Anyway, not that important.
|
|
||
| 1) When the Querying Chain receives a query request, it calls `CrossChainQueryRequest` of the Cross-chain Querying module. This function generates a unique identifier for the query, stores it in its `privateStore` and emits a `sendQuery` event. Query requests can be submitted by other IBC modules as transactions to the Querying Chain or simply executed as part of the `BeginBlock` and `EndBlock` logic. | ||
| 2) A correct relayer listening to `sendQuery` events from the Querying Chain will eventually pick the query request up and execute it at the Queried Chain. The result is then submitted (on-chain) to the Querying Chain. | ||
| 3) When the query result is committed at the Querying Chain, this calls the `CrossChainQueryResult` function of the Cross-chain Querying module. |
There was a problem hiding this comment.
this calls the
CrossChainQueryResultfunction
Name collision with the data structure for results / responses.
| // Sanity-check that localTimeoutHeight is 0 or greater than the current height, otherwise the query will always time out. | ||
| abortTransactionUnless(localTimeoutHeight === 0 || localTimeoutHeight > getCurrentHeight()) | ||
| // Sanity-check that localTimeoutTimestamp is 0 or greater than the current timestamp, otherwise the query will always time out. | ||
| abortTransactionUnless(localTimeoutTimestamp === 0 || localTimeoutTimestamp > currentTimestamp()) |
There was a problem hiding this comment.
localTimeoutTimestamp is missing from function signature.
| abortTransactionUnless(query.localTimeoutTimestamp === 0 || query.localTimeoutTimestamp > currentTimestamp()) | ||
|
|
||
|
|
||
| // Verify query result using the local light client of the Queried Chain. If success, then verify that the data is indeed the value associated with query.path at query.queryHeight at the Queried Chain. Otherwise, verify that query.path does not exist at query.queryHeight at the Queried Chain. |
| The Querying Chain calls the `checkQueryTimeout` function to check whether a specific query has timed out. | ||
|
|
||
| > There are several alternatives on how to handle timeouts. For instance, the relayer could submit on-chain timeout notifications to the Querying Chain. Since the relayer is untrusted, for each of these notifications the Cross-chain Querying module of the Querying Chain MUST call the `checkQueryTimeout` to check if the query has indeed timed out. An alternative could be to make the Cross-chain Querying module responsible for checking | ||
| if any query has timed out by iterating over the ongoing queries at the beginning of a block and calling `checkQueryTimeout`. In this case, ongoing queries should be stored indexed by `localTimeoutTimestamp` and `localTimeoutHeight` to allow iterating over them more efficiently. These are implementation details that this specification does not cover. |
There was a problem hiding this comment.
nit: remove new line (i.e., there are two spaces at the end of the prev line)
|
|
||
| Query requests have associated a `localTimeoutHeight` and a `localTimeoutTimestamp` field that specifies the height and timestamp limit at the Querying Chain after which a query is considered to have failed. | ||
|
|
||
| The Querying Chain calls the `checkQueryTimeout` function to check whether a specific query has timed out. |
There was a problem hiding this comment.
Not sure I understand how the "Querying Chain calls the checkQueryTimeout function". Is this a transaction? The pseudocode seems to contain several abortTransactionUnless.
There was a problem hiding this comment.
It depends. The spec discusses two alternatives: in one checkQueryTimeout is a transaction. I understand the confusion though. Ideally, this function (since it can be both a transaction or simply a function call from the module) would not include any abortTransactionUnless. Any suggestion on how to solve this? I can think of two solutions:
- remove
abortTransactionUnlessfromcheckQueryTimeoutand use if-else conditions. Probably the best solution. - commit to one of the discussed alternatives
There was a problem hiding this comment.
I have opted for the second solution.
|
|
||
| - **Live chains:** Both the Querying and Queried chains MUST be live, i.e., new blocks are eventually added to the chain. | ||
|
|
||
| - **Censorship-resistant Querying Chain:** The Querying Chain cannot selectively omit transactions. |
There was a problem hiding this comment.
what does this mean? not a very formal definition.
There was a problem hiding this comment.
This means that all correctly submitted transactions are eventually committed. Unless one considers live chains to already guarantee this, I think this is required to guarantee that the query result submitted by a correct relayer is eventually processed.
There was a problem hiding this comment.
Correctly submitted to whom? Any nodes can always selectively omit transactions, since you cannot prove that another party received a message. No blockchain can instantiate this property in the general case.
I get what you're going for, though. I agree that we need an assumption in order to guarantee that the query result submitted by a correct relayer is eventually processed - isn't that assumption stronger (processed within a time bound) if timeouts are involved, anyways? I think you could say something like "the relayer must submit and the querying chain must process a "QueryResult" transaction within the specified timeout bound in order for the query protocol to return results to the application" - would that do?
There was a problem hiding this comment.
I think you could say something like "the relayer must submit and the querying chain must process a "QueryResult" transaction within the specified timeout bound in order for the query protocol to return results to the application" - would that do?
Yes, that would work. I believe your property would be Censorship-resistant property + a timely property. We can either have one property covering both things or two separate properties. I am fine with both solutions.
There was a problem hiding this comment.
Correctly submitted to whom?
To the Querying Chain.
Any nodes can always selectively omit transactions, since you cannot prove that another party received a message. No blockchain can instantiate this property in the general case.
I think this is an interesting (open) research question. Non-reconfigurable Byz. consensus protocols are able to satisfy this property. PBFT for instance is censorship-resistant by broadcasting requests, setting timeouts per request, and forcing a view change if a timeout expires. When adding reconfiguration to the equation, things get more complicated, but I believe things may be solved by adding extra assumptions.
There was a problem hiding this comment.
I think this is an interesting (open) research question. Non-reconfigurable Byz. consensus protocols are able to satisfy this property. PBFT for instance is censorship-resistant by broadcasting requests, setting timeouts per request, and forcing a view change if a timeout expires. When adding reconfiguration to the equation, things get more complicated, but I believe things may be solved by adding extra assumptions.
How can a user with a transaction force a view change if a timeout expires? I thought timeouts were for proposals - and block proposers can always choose to omit transactions. Perhaps I don't follow, sorry. Under assumptions of bounded latency message delivery from a user authoring a transaction to a sufficient quorum of validators, and honesty of those validators w.r.t. including all received messages in blocks they propose in a particular order, and limited other transactions competing for block space, I could see some sort of transaction inclusion property holding, but these are far more strict assumptions than IBC in general makes, and should be spelled out in detail imo (if this spec intends to rely on them).
There was a problem hiding this comment.
How can a user with a transaction force a view change if a timeout expires? I thought timeouts were for proposals - and block proposers can always choose to omit transactions. Perhaps I don't follow, sorry.
Let me elaborate a bit. This is roughly how it works. Clients broadcast their requests to all consensus nodes. When a node receives a request, it puts it into its FIFO queue and starts a timer. If the request's timer expires before the request is delivered/committed at a node, it attempts to advance to the next view. Therefore, if the leader of the current view is Byzantine and ignores a request, eventually all correct nodes will attempt to advance and force a leader change (actually we do not need all correct nodes but a majority). Note that if a correct node delivers/commits requests, then all correct ones eventually do. You can find a formal proof at https://arxiv.org/pdf/2202.06679.pdf: Figures 4 and 5 show the pseudocode of a variant of PBFT which includes this mechanism, and Section 5 proves a liveness property that implies censorship-resistance.
There was a problem hiding this comment.
Thanks! Makes sense (and very cool research).
|
|
||
| Cross-chain querying relies on relayers operating between both chains. When a query request is received by the Querying Chain, the Cross-chain Querying module emits a `sendQuery` event. Relayers operating between the Querying and Queried chains must monitor the Querying chain for `sendQuery` events. Eventually, a relayer will retrieve the query request and execute it, i.e., fetch the data and generate the corresponding proofs, at the Queried Chain. The relayer then submits (on-chain) the result at the Querying Chain. The result is finally registered at the Querying Chain by the Cross-chain Querying module. | ||
|
|
||
| A query request includes the height of the Queried Chain at which the query must be executed. The reason is that the keys being queried can have different values at different heights. Thus, a malicious relayer could choose to query a height that has a value that benefits it somehow. By letting the Querying Chain decide the height at which the query is executed, we can prevent relayers from affecting the result data. |
There was a problem hiding this comment.
worth noting, this still creates an opportunity for altering state on the queried chain if the height is in the future in order to change the results of the query
(not a relayer-specific problem, but would be a form of cross-chain MEV)
|
238b6b116f5dc73212019cdecccf564031707f934a7c0012ed8cf1a7bdef9fde.patch |
|
|
||
| - **Censorship-resistant querying chain:** The querying chain cannot selectively omit valid transactions. | ||
|
|
||
| > For example, this means that if a relayer submits a valid transaction to the querying chain, the transaction is guaranteed to be eventually included in a committed block. Note that Tendermint does not currently guarantee this. |
There was a problem hiding this comment.
Not sure whether Tendermint can ever guarantee this.
Cross-chain query spec. This PR supersedes #647.
Taken decisions after discussion:
The bounty system is vulnerable to front-running - any relayer who actually bothers to query the other chain can be front-run by the block proposer. This is true of any permissionless relayer incentivization, e.g., ICS29 will ship v1 with the same problem. However, the APIs are constructed such that we could easily specify a relayer in the future. We have decided to do something similar here. (Cross-chain query spec draft #735 (comment))
Add features to enable Supporting IBC Casting #759. Future work. (Cross-chain query spec draft #735 (comment))
Shall we prevent queryHeight from being in the future to avoid cross-chain MEV? We have decided to add a note to acknowledge the problem. Solving MEV is out of the scope of this spec. (Cross-chain query spec draft #735 (comment))