feat: interachain sudo callback refactoring #313
Conversation
pr0n00gler
dismissed
their stale review
I misclicked, it's not approved yet
| @@ -20,118 +20,37 @@ func (k Keeper) HasContractInfo(ctx sdk.Context, contractAddress sdk.AccAddress) | |||
| return k.wasmKeeper.HasContractInfo(ctx, contractAddress) | |||
There was a problem hiding this comment.
probably not in scope of this PR, just want to share my observations:
I think we should remove the HasContractInfo method from the contractmanager.Keeper interface for it's just a proxy, no specific logic inside it. this could 1) make the HasContractInfo calls less ambiguous (why do we ask for a contract info from the contractmanager module, not from the wasm one, given the fact it's the wasm module which owns contracts) and 2) simplify contractmanager.Keeper interface, e.g. make it cleaner, more contractmanager module specialisation-oriented
There was a problem hiding this comment.
another suggestion: I don't see any reasons of SudoKVQueryResult and SudoTxQueryResult in being placed in the contractmanager module. I suggest moving them to the interchainqueries module — to the only place they are used and to the place IMO they belong to in terms of module responsibility and scope
| } | ||
| k.Logger(ctx).Debug("SudoError: contract not found", "senderAddress", senderAddress) | ||
| return nil, fmt.Errorf("%s is not a contract address and not the Transfer module", senderAddress) | ||
| func PrepareOpenAckCallbackMessage(details types.OpenAckDetails) ([]byte, error) { |
There was a problem hiding this comment.
IMO the PrepareOpenAckCallbackMessage func belongs to the interchaintxs module
There was a problem hiding this comment.
the PrepareSudoCallbackMessage func also seems not at the right place for me, but I couldn't come up with a better one since it's used by two modules (not by a single one as PrepareOpenAckCallbackMessage). but since they are quite similar, I think we can either keep the both here or find better places for both of them. a better place for PrepareOpenAckCallbackMessage is obvious — interchaintxs module. any ideas on PrepareSudoCallbackMessage?
https://github.com/neutron-org/neutron-tests/actions/runs/6196055940
related PRs:
neutron-org/neutron-sdk#112
neutron-org/neutron-dev-contracts#27
neutron-org/neutron-integration-tests#204