osmo CL lper + base account callbacks

[bekauz]

No description provided.

[keyleu]

Just looked at the callback handling for now, interesting, I wasn't thinking in using events but I guess it does the trick. The way I was thinking it was doing an ExecuteMsg to the original service in the reply of the base account and define a standard entry point for that in the ExecuteMsg of the service (we could add it with a macro or something).

Now I see the advantage of using events because it's quite straightforward and doesn't require an extra API entry point but I feel it can be dangerous because events might not be stable (?). Quoted from Simon a few days ago: "Events are never carefully designed. Tho whole stack from CosmWasm through Cosmos SDK to Comet BFT is an unstructured mess when it comes to events."

What if they decide to modify the events json structure in the future or they decide to modify the key like "wasm" into different kind of "wasm-". Or some chain out there running a fork and messing with these events, not sure just speculating. Then we wouldn't have it working everywhere (would work for specific wasmd module versions)

Just speculating at this point not requiring any changes but prob worth a discussion

[bekauz]

Just looked at the callback handling for now, interesting, I wasn't thinking in using events but I guess it does the trick. The way I was thinking it was doing an ExecuteMsg to the original service in the reply of the base account and define a standard entry point for that in the ExecuteMsg of the service (we could add it with a macro or something).

Yeah, I just felt like that would complicate things on the account side a bit.

Now I see the advantage of using events because it's quite straightforward and doesn't require an extra API entry point but I feel it can be dangerous because events might not be stable (?). Quoted from Simon a few days ago: "Events are never carefully designed. Tho whole stack from CosmWasm through Cosmos SDK to Comet BFT is an unstructured mess when it comes to events."

100%, that's exactly the kind of worry I have and would love to hear more thoughts from others on this.

At least right now I feel like the way it's done is safe because I can't think of a situation where someone may "spoof" or mess with the expected event keys.
Account execution is gated by the approved services, so the account reply handler could be triggered iff the service initiates the submsg delegation.
On the service level, they await for that callback from the account. This feels like a closed loop where we control 2/3 (account + service, but not the target contract/module). I suppose the target address called by the underlying message we delegate to the account could do something malicious. Do you have any thoughts on this @keyleu ?

What if they decide to modify the events json structure in the future or they decide to modify the key like "wasm" into different kind of "wasm-". Or some chain out there running a fork and messing with these events, not sure just speculating. Then we wouldn't have it working everywhere (would work for specific wasmd module versions)

Yeah, that's a good one. Ideally, imho, we would take advantage of the new SubMsg design (from cosmwasm 2.0) which has the payload: Binary field. But given that it's not ready, I think transitioning to it will be easier from this attributes-based approach rather than from a dedicated execute msg?

[bekauz]

One more thought on that semi-closed loop point i made where we do not control the integration, but control the rest.

I think it's a totally valid concern but I am not sure how far does it make sense to take it. We are already making a choice of trusting those integrations. In theory, there is nothing stopping some dApp from migrating their contracts into something malicious where the api would remain the same, but instead of performing the expected action, they would just take the input coins and send it to themselves?

[keyleu]

100%, that's exactly the kind of worry I have and would love to hear more thoughts from others on this.

At least right now I feel like the way it's done is safe because I can't think of a situation where someone may "spoof" or mess with the expected event keys. Account execution is gated by the approved services, so the account reply handler could be triggered iff the service initiates the submsg delegation. On the service level, they await for that callback from the account. This feels like a closed loop where we control 2/3 (account + service, but not the target contract/module). I suppose the target address called by the underlying message we delegate to the account could do something malicious. Do you have any thoughts on this @keyleu ?

Yeah I'm not worried about chains messing with event keys but more of Cosmwasm transitioning to different even structures, but that's also very unlikely.

Yeah the account could get migrated and do malicious things but that can happen regardless with anything, not particular to this, if the accounts are created with an admin and admin rights given to someone different than us.

This loop is closed so should be fine.

Yeah, that's a good one. Ideally, imho, we would take advantage of the new SubMsg design (from cosmwasm 2.0) which has the payload: Binary field. But given that it's not ready, I think transitioning to it will be easier from this attributes-based approach rather than from a dedicated execute msg?

Not sure how the payload would fix/change anything here tho or maybe I'm just not seeing it. The payload could be used if we added an entry point to the contract like I suggested then we could get the data from the payload instead of the API msg.

E.g. We could have in the service:
ValenceCallback {} and take it from the payload instead of ValenceCallback{ callback_msg: Binary }.

Or maybe I'm just not seeing it.

[bekauz]

Yeah, that's a good one. Ideally, imho, we would take advantage of the new SubMsg design (from cosmwasm 2.0) which has the payload: Binary field. But given that it's not ready, I think transitioning to it will be easier from this attributes-based approach rather than from a dedicated execute msg?

Not sure how the payload would fix/change anything here tho or maybe I'm just not seeing it. The payload could be used if we added an entry point to the contract like I suggested then we could get the data from the payload instead of the API msg.

I was thinking something along the lines of receiving the callback in the account, taking the reply, wrapping it in this payload, and firing it back to the calling service. Then service gets the reply and ignores everything except the payload, because that payload should be indistinguishable from as if the service fired that message directly. Msg and the reply id fired by the account are configured by the service, so the service can just treat it as if the account didn't exist.

[keyleu]

firing it back to the calling service.

Fire it back how?, only see the option of an ExecuteMsg so we're back to needing an entry point. Because the Reply we're getting on the service will only have the events of the wasm execution (what you have now)