-
Notifications
You must be signed in to change notification settings - Fork 334
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
Add "SubCall" to get result from a message #691
Comments
Any feedback on this design? @alpe @webmaster128 I discussed this idea with you before. Input on how to improve the proposal? |
I like the flow, makes much sense. What about avoiding the term "call"/"callback" alltogether to avoid any kind of expectation for Ethereum-like behaviour. What this really is is a message that is emitted one way for which the contract expects a "reply". I would avoid the introduction of fake message types to keep a 1:1 mapping to between #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
pub struct HandleResponse<T = Empty>
where
T: Clone + fmt::Debug + PartialEq + JsonSchema,
{
/// Sub-messages trigger executions that share the same gas budget with the original contract execution.
/// They are processed before all regular messages. Each sub-message causes a reply to be sent to
/// this contract via the `reply` entry point.
pub sub_messages: Vec<SubMsg<T>>,
pub messages: Vec<CosmosMsg<T>>,
/// The attributes that will be emitted as part of a "wasm" event
pub attributes: Vec<Attribute>,
pub data: Option<Binary>,
} with SubMsg<T = Empty> {
id: String,
msg: CosmosMsg<T>,
gas_limit: Option<u64>,
} Also for efficiency, I suggest using I am highly concerned about adding knowledge of gas values into the contract. This requires an immutable gas cost system. Changes in gas cost have broken existing contracts in the past (on Ethereum). Maybe we avoid the |
Separating these out into to different fields makes sense. I assume a minor typo above, where it should be |
Why gas_limit? This can be useful in quite a number of cases and should be provided by the end-user (not the app). We can define it as 'Cosmos SDK Gas' (much used in storage access not Wasm CPU). The reason this is important is that I may want to prevent such sub calls from triggering "out of gas". Use case 1: "Ethereum Alarm Clock" This was a project that was recently requested on discord. Basically, I submit a task and some token reward to an "Alarm Clock" contract, along with an expected time (range) to run it. If someone calls the Alarm Clock with sufficient gas during the time, it will (1) run your requested task and (2) send the pre-paid fees to the person calling the contract (to cover gas costs and more). Now, we can do this right now, but if your requested task returns an error or uses some huge gas amount, it can return an error and abort the payment. If we use a sub_message to isolate it, a failed task can be marked as invalid and still provide a payout. But we have no gas limit we need to pay. If we use a sub_message and the original user provided a gas limit, we can call it with that. If we paid too little gas, it may still OutOfGas and revert the whole transaction. If we paid enough gas to covered the requested gas to the call, even if it needed more gas, we can get out payment (the user requested 200k gas for a call that needed 500k). Use case 2: "Cron job" Imagine a contract that is called every begin block and automatically performs a service like what we describe for the alarm clock (without off-chain caller). It may have some configured gas limit per block, and again, needs to allow the executed tasks to fail (and record that) rather than revert the transaction. It also wants some control over the gas usage of the sub_messages so they don't burn all the available gas of the cron job (which may wish to execute 10 messages). This is similar to above, but since the caller is on chain, it is even more important to provide these limits. We can definitely add a note, that those gas_limits should not be hard-coded inside contracts, but provided from external users. If you really need to dispatch messages with a set value, it should be set in storage on InitMsg, and modifiable by some admin account - never hardcoded in the wasm binary. |
This sounds like a good strategy. It also fits in the vision of multi-chain contracts where one blob runs on different chains, but must expect different prices for things. |
What's your current thinking regarding recursion and order of execution, @ethanfrey? I mean a sub message can emit sub messages itself. |
Good question. Currently, we do a depth-first strategy with The question is do we execute the returned sub-messages or returned messages first. Executing the sub-message depth-first includes executing the callback to this contract. Assume contract C returns submessage S and message M, and they don't dispatch others, we have two choices:
I am not certain which is better, but I am tending towards the first - sub-messages before messages. This means, we get and process the results of these points where we want feedback before dispatching the generic "fire and forget" type messges. I am open to any reasons why one ordering is better than the other. (This is the only downside of the new design, before the ordering was explicit by the contract dev) |
Closes #292 (concrete solution to that issue)
We can add a new variant to
CosmosMsg
, like:CosmosMsg::SubCall
would have the following semantics:SubCall.msg
would be executed in a sub-transaction, with an optional gas limit (so it cannot use up all the gas of the caller)SubCall.msg
would be reverted and an error returned to the caller (see below)SubCall.msg
would be committed and the result returned to the callerSubCall.msg == CosmosMsg::SubCall
is illegal and returns an error (aborting the transaction, not returned)SubCall.msg
is executed, if the transaction has not aborted (illegal values in CosmosMsg::SubCall or out of gas), then the calling contract is guaranteed to get a callback in the same atomic transaction.callback
, which can be wrapped likehandle
, but exposed like this in a contractTo keep all the guarantees the much of the cosmos sdk modules depend on, we need to revert any partial state changes from the subcall if it errors.
By returning the result and not aborting, we allow the caller to handle it. Maybe a module triggers an optional listener with a gas limit. If it fails, we can record it as failed and keep processing the main work. By returning a
Result
type here, we make it easy to simply "abort on error in subcall" with a line or two, but allow the caller to also handle that case if they wish.Note the
callback
entry point is not required on modules. However, if they returnCosmosMsg::SubCall
and do not export that entry point, this will abort the transaction.The text was updated successfully, but these errors were encountered: