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

All callbacks on contract init #467

Closed
ethanfrey opened this issue Jul 10, 2020 · 10 comments
Closed

All callbacks on contract init #467

ethanfrey opened this issue Jul 10, 2020 · 10 comments

Comments

@ethanfrey
Copy link
Member

Use Case

I have a DeFi contract that wants to create two "sub-tokens" (cw20) that it can mint and redeem. Bascially, they would be functionally part of the contract, but they should be independent contracts to get addresses and be able to send and receive with existing tools.

This requires the Creator contract (C) to store the address of A and B. It also requires A and B to store C as the minter. How do we do this?

Currently Best Solution

The token contract for A and B can add an additional field in their InitMsg.

type struct InitMsg {
  // ...
  pub callback: Option<String>,
}

If this field is set to Some(X), after running their init function, they will dispatch a wasm message:

let callback = CallbackMsg {
    id: X,
    created: env.contract.address,
};

let return_msg = WasmMsg::Execute{
    contract_address: env.msg.sender,
    msg: to_binary(&callback)?,
    send: vec![],
}

This way, the original contract can have fields:

type State struct {
   // ...
   pub token_a: Option<CanonicalAddr>,
   pub token_b: Option<CanonicalAddr>,
}

And set them both to None and dispatch the two WasmMsg::Instantiate with a callback set. Later, it will receive a CallbackMsg and if msg.id == "token_a", and state.token_a.is_none() then it will set token_a. Or if msg.id == "token_b", and state.token_b.is_none() then it will set token_b. After the init msg and all redispatched messages is completed, all contracts should have been properly initialized.

Shortcoming

Anyone can call these Callbacks. And there is no guarantee the called contract ever will. If we include callback in the InitMsg, but the receiver doesn't support it, it will silently ignore the extra field and not return the CallbackMsg. Some other person could then register any tokens as token_a and token_b.

This is mainly developer error (as you should check the code you call before you instantiate a contract), but it the token init also calls out to some code (which might be malicious), it might hit the callback before the official version. This may be a bit of an edge case, but if this is a common pattern, we could consider better support for it.

Proposal

We could add first-class support for such a init callback chain. This would be in the wasmd runtime to ensure it works. Something like adding this callback: Option<String> field to WasmMsg::Instatiate. If set, then the runtime would get the new address after Instantiate returns and call back into the original contract with a special callback. It can be over a specified HandleMsg or a new entry point. But we set env.message.sender == env.contract.address, so the contract can be sure this is coming from the runtime (or itself) and cannot be faked by some other contract

Open Questions

  • How should such runtime support look like?
  • Is this overkill?
  • Should I just implement and document the pattern I described as "current solution" as a CWxy spec?
@ethanfrey
Copy link
Member Author

@webmaster128 I would love your feedback here. I think we discussed the use case before. This comes up other places as well - especially in governance contracts (where there are lots of connections between contracts).

If you don't want to change the runtime, maybe you can also help clarify a better pattern for working with existing interfaces. Also, maybe we push any API changes to 2.0?

@webmaster128
Copy link
Member

We could add first-class support for such a init callback chain. This would be in the wasmd runtime to ensure it works. Something like adding this callback: Option<String> field to WasmMsg::Instatiate.

What if – independent of the message type – every entry in

pub messages: Vec<CosmosMsg<T>>,

contains a message and a callback? This callback can return the result data of the message execution.

@ethanfrey
Copy link
Member Author

contains a message and a callback? This callback can return the result data of the message execution.

So this would be like:

type Action<T> struct {
  msg: CosmosMsg<T>,
  callback: Option<String>,
}

// similar for init, migrate, etc... point is the type of messages
type HandleResponse<T> struct {
  actions: Action<T>,
  data: Option<Vec<u8>>,
  log: Vec<Log>,
}

And if callback is set on the Request, then what do we do? Define a standard HandleMsg::Callback(CallbackMsg) as above, and set env.message.sender == env.contract.address? Or another idea?

I start wondering where it makes sense to add required structure to HandleMsg and to what extent we should just add more entry points (like migrate). Part of my question is the syntactic difference. This is a general question that will come up again once we start integrating IBC packets into CosmWasm as well.

@ethanfrey
Copy link
Member Author

Also, does this make sense for 0.10 / 1.0? Or punt to 2.0?

@webmaster128
Copy link
Member

So this would be like

Exactly. Since we still don't call directly but enqueue, naming would be along the lines of struct MessageEmission<T> { message: CosmosMsg<T>, callback: Option<???> }.

then what do we do? Define a standard HandleMsg::Callback(CallbackMsg) as above, and set env.message.sender == env.contract.address? Or another idea?

Good question.

I start wondering where it makes sense to add required structure to HandleMsg and to what extent we should just add more entry points (like migrate).

I think this primarily depends on the the question wether or not we want to make those callbacks usable from the client. In Ethereum you don't differentiate. But there you also don't need callbacks due to synchonous return values. So we might want to have an entry point that can only be used by the callback framework.

Independent of how we call back: do we need a connection between the original message execution and the callback (some kind of ID)? If this the framework's or the contract's business?

does this make sense for 0.10 / 1.0? Or punt to 2.0?

I don't see this stabilizing before the end of this month (0.10). Other than that, I think it depends on users' requirements.

@ethanfrey
Copy link
Member Author

Independent of how we call back: do we need a connection between the original message execution and the callback (some kind of ID)? If this the framework's or the contract's business?

What you call ID, is what I was calling String in callback: Option<String>. Just some ID to use in the callback (so the caller can register for it). Maybe we can extend this to callback: Option<struct {id: String, ...}>. But I don't know what else needs to go there?

I think this primarily depends on the the question wether or not we want to make those callbacks usable from the client.

Yeah, I was thinking this should be framework internal (in fact likely tx internal), and not exposed to clinet

I don't see this stabilizing before the end of this month (0.10). Other than that, I think it depends on users' requirements.

I guess I can make do with current tools for the contracts we develop in the next month, and then refine this approach. I would like to get it into 1.0 if we can, so as to avoid breaking changes between 1.0 and 2.0. However, this callback mechanism is somehow similar to what IBC will need for returning receipts/errors from the remote side, so it makes sense to have a design for both before stabilizing this one

@assafmo
Copy link
Contributor

assafmo commented Sep 21, 2020

In the Secret Network we enforce a unique label across the entire chain, so if you instantiate a new contract and the tx passed, you can be sure the label you used to instantiate the contract is unique. You can call a contract by address or by label (and we pull the address behind the scenes).
After you have this, all that's left to add is support for messaging a contract by its label or by its address.

@ethanfrey
Copy link
Member Author

I am partial to the idea, but I feel a bit like it is mixing concerns. It solved the special case of two unique issues, but not the general one.

One is a general way to get a result from a message to use later. By passing a unique label in the construct method that can later be resolved to the contract, we avoid the name here, but are there other places besides init where this matters? Or maybe this is the best way to solve the issue for init and init is the cause of 99% of such cases.

The second is a more flexible way to reference contracts and accounts by name. I started thinking about this a while ago and happy for some comments there. Your solution does give nice names to contracts, but not to pubkey accounts.

I do remember NEAR protocol using such nice names (Alice, bob, etc) as account aliases and this was the core idea of my work at IOV in 2018 as well. So I really think that usage of DIDs should be done quite well, not just as a side effect of another field.

I would love to hear more opinions on this. Maybe I am just over complicating everything

@assafmo
Copy link
Contributor

assafmo commented Sep 22, 2020

You're right, there's also the case where A messages B and wants B's output. Right now B's output is lost even to the tx sender.

@ethanfrey
Copy link
Member Author

Closed by CosmWasm/cw-plus#152 which documents a pattern we use to set this up.
Circular init inside the contract itself is just too much trouble (not to support, but to do safely)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants