Remove ibc callbacks for accounts.

[Kayanski]

This PR aims at removing IBC callbacks on Abstract Accounts.
This will be replaced in another PR by execution and queries between abstract modules (#259)

Checklist

• CI is green.
• Changelog updated.

[cloudflare-workers-and-pages]

Deploying abstract-docs with    Cloudflare Pages

Latest commit:	012cad4
Status:	⚡️  Build in progress...

View logs

[Buckram123]

Looks good, but would really like to see test, where it's called by "attacker"

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 70.37037% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 83.8%. Comparing base (21e55ad) to head (cbfc938).
Report is 45 commits behind head on main.

Additional details and impacted files

Files	Coverage Δ
...mework/contracts/native/ibc-client/src/commands.rs	98.5% <100.0%> (-0.3%)	⬇️
...mework/contracts/native/ibc-client/src/contract.rs	98.6% <100.0%> (-0.2%)	⬇️
framework/contracts/native/ibc-client/src/ibc.rs	100.0% <ø> (ø)
framework/packages/abstract-adapter/src/state.rs	96.0% <100.0%> (ø)
framework/packages/abstract-app/src/state.rs	91.7% <100.0%> (+3.0%)	⬆️
framework/packages/abstract-core/src/native/ibc.rs	66.6% <ø> (-5.6%)	⬇️
...rk/packages/abstract-core/src/native/ibc_client.rs	84.0% <100.0%> (+4.0%)	⬆️
...packages/abstract-interface/src/account/manager.rs	57.4% <ø> (+1.5%)	⬆️
framework/packages/abstract-sdk/src/apis/ibc.rs	51.7% <100.0%> (-10.1%)	⬇️
...rk/packages/abstract-sdk/src/base/contract_base.rs	92.4% <100.0%> (-0.8%)	⬇️
... and 5 more

... and 133 files with indirect coverage changes

[Buckram123]

Nice!

[CyberHoward]

Why would we want to support a module on another account doing an IBC callback on a module? Do you have an application that needs that?

Can we keep IBC callbacks to same-account modules? I think that's the sensible default.

[CyberHoward]

Convention over Configuration.

Our convention is that inter-account calls (shielded calls) are the default security parameter. If we ever need to loosen those boundaries for a use-case that should be a configurable setting.

[Kayanski]

Why would we want to support a module on another account doing an IBC callback on a module? Do you have an application that needs that?

Can we keep IBC callbacks to same-account modules? I think that's the sensible default.

It's not really a question of support, it's a question of callback definiton. Our callback definition has a receiver address that can be set by the sender to anything. We can change that but to do that we need to redefine what the callback API looks like and what callback we want to allow. Remember, it's the user proxy that sends the callback

[CyberHoward]

Why would we want to support a module on another account doing an IBC callback on a module? Do you have an application that needs that?
Can we keep IBC callbacks to same-account modules? I think that's the sensible default.

Remember, it's the user proxy that sends the callback

It's the ibc-client that sends it:

IbcClientResponse::action("user_specific_callback")
                .add_message(callback.into_cosmos_msg(callback_info.receiver)?)

But yeah you have a point on the permissionless nature of the ibc callbacks! I'm just not sure if the AccountId is really the sender metric to use here. What if we use ModuleInfo as the sender identification instead and, like we're discussing in the Notion, the client asserts that this sender information is correct?

[Kayanski]

Why would we want to support a module on another account doing an IBC callback on a module? Do you have an application that needs that?
Can we keep IBC callbacks to same-account modules? I think that's the sensible default.

Remember, it's the user proxy that sends the callback

It's the ibc-client that sends it:

IbcClientResponse::action("user_specific_callback")
                .add_message(callback.into_cosmos_msg(callback_info.receiver)?)

But yeah you have a point on the permissionless nature of the ibc callbacks! I'm just not sure if the AccountId is really the sender metric to use here. What if we use ModuleInfo as the sender identification instead and, like we're discussing in the Notion, the client asserts that this sender information is correct?

Sorry, I meant the callback is registered in the ibc-client by the proxy.
Yeah but you may want to send a callback without a sending module. And would the sending module set this field themselves ?
Again here, we have no way today of verifying which module is sending the message because it's the proxy that sends the message that registers the callback

[CyberHoward]

Why would we want to support a module on another account doing an IBC callback on a module? Do you have an application that needs that?
Can we keep IBC callbacks to same-account modules? I think that's the sensible default.

Remember, it's the user proxy that sends the callback

It's the ibc-client that sends it:

IbcClientResponse::action("user_specific_callback")
                .add_message(callback.into_cosmos_msg(callback_info.receiver)?)

But yeah you have a point on the permissionless nature of the ibc callbacks! I'm just not sure if the AccountId is really the sender metric to use here. What if we use ModuleInfo as the sender identification instead and, like we're discussing in the Notion, the client asserts that this sender information is correct?

Sorry, I meant the callback is registered in the ibc-client by the proxy. Yeah but you may want to send a callback without a sending module. And would the sending module set this field themselves ? Again here, we have no way today of verifying which module is sending the message because it's the proxy that sends the message that registers the callback

Is it not safe to assume that if the sending module is registered on the account, the call is allowed to pass? I think we could assume that for now until we have direct module calls. Only case that would go wrong is with a maleficent module installed.

[CyberHoward]

Does this PR need to be merged before #259 ?

[Kayanski]

Does this PR need to be merged before #259 ?

Yes