diff --git a/CHANGELOG.md b/CHANGELOG.md index d682a1b3..0540e4e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ Contains all the PRs that improved the code without changing the behaviors. ### Removed ### Fixed +- [#569](https://github.com/archway-network/archway/pull/569) - Audit remidiations for x/cwerrors and x/cwica ### Improvements - [#567](https://github.com/archway-network/archway/pull/567) - Remove redundant params fetching in SaveCallback diff --git a/app/app.go b/app/app.go index fcb6700a..66a6b3ea 100644 --- a/app/app.go +++ b/app/app.go @@ -786,11 +786,12 @@ func NewArchwayApp( trackingTypes.ModuleName, rewardsTypes.ModuleName, callbackTypes.ModuleName, - cwerrorsTypes.ModuleName, // invariants checks are always the last to run crisistypes.ModuleName, cwfees.ModuleName, // does not have end blocker cwicatypes.ModuleName, + + cwerrorsTypes.ModuleName, // should be after all the other cw modules ) // NOTE: The genutils module must occur after staking so that pools are diff --git a/x/cwerrors/spec/03_end_block.md b/x/cwerrors/spec/03_end_block.md index 8ff51ab5..ba5455e6 100644 --- a/x/cwerrors/spec/03_end_block.md +++ b/x/cwerrors/spec/03_end_block.md @@ -14,4 +14,4 @@ All the contract subscriptions which end in the current block are pruned from st ## Prune old errors -All errors which are marked for deletion for the current block height are pruned from the state. +All errors which haven been associated with the current block height as the expiration height are pruned from the state diff --git a/x/cwerrors/spec/README.md b/x/cwerrors/spec/README.md index c07b02a3..1875d210 100644 --- a/x/cwerrors/spec/README.md +++ b/x/cwerrors/spec/README.md @@ -10,7 +10,7 @@ The module provides two diffferent ways that the error is communicated. ### 1. Errors saved in state (default) -Whenever a contract relevant error is encountered by the protocol, the module stores the error and associates with the contract address. This is stored in the chain state for `x` number of blocks (The `x` value is a module parameter. [See more](./01_state.md)) after which the error is permanently deleted. These stored errors are queryable by the contract using stargate queries. +Whenever a contract relevant error is encountered by the protocol, the module stores the error and associates with the contract address. This is stored in the chain state for `x` number of blocks (The `x` value is a module parameter. [See more](./01_state.md)) after which the error is automatically pruned. These stored errors are queryable by the contract using stargate queries. ### 2. Errors sudo callback @@ -31,6 +31,33 @@ pub enum SudoMsg { The subscriptions are an opt-in feature where the contractadmin/owner has to subscribe to the feature by paying the relevant fees. [See more](01_state.md) The subscription is valid for `x` number of blocks where `x` is decided by the module param. The subscription cannot be cancelled but can be extended by attempting to subscribe again. +When an error is received for a contract with the subscripiton, the module stores the errors in its transient store and executes the Sudo calls at the end block, by reading from the transient store. + +## How to use in another module + +If any module would like to integrate the x/cwerros functionality, add the following snippet into the `x//types/expected_keepers.go` + +```go +// ErrorsKeeper defines the expected interface needed to interact with the cwerrors module. +type ErrorsKeeper interface { + // SetError records a sudo error for a contract + SetError(ctx sdk.Context, sudoErr cwerrortypes.SudoError) error +} +``` + +This keeper would need to be passed in from `app.go` and stored by the module during init. When the module encounters an error it would like to be reported to the x/cwerrors module, it can execute the following snippet. + +```go +sudoerr := cwerrortypes.SudoError { + ModuleName: ModuleName, + ErrorCode: int32(errorCode), + ContractAddress: contractAddr, + InputPayload: inputPayload, + ErrorMessage: errMsg, +} +err := yourmodulekeeper.errorsKeeper.SetError(ctx, sudoerr) +``` + ## Contents 1. [State](./01_state.md) diff --git a/x/cwica/ibc_module.go b/x/cwica/ibc_module.go index 9560e963..e3a7c6a1 100644 --- a/x/cwica/ibc_module.go +++ b/x/cwica/ibc_module.go @@ -39,13 +39,13 @@ func (im IBCModule) OnChanCloseConfirm(ctx sdk.Context, portID string, channelID } // OnAcknowledgementPacket implements the IBCModule interface. -func (im IBCModule) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress) error { - return im.keeper.HandleAcknowledgement(ctx, packet, acknowledgement, relayer) +func (im IBCModule) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, _ sdk.AccAddress) error { + return im.keeper.HandleAcknowledgement(ctx, packet, acknowledgement) } // OnTimeoutPacket implements the IBCModule interface. -func (im IBCModule) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress) error { - return im.keeper.HandleTimeout(ctx, packet, relayer) +func (im IBCModule) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, _ sdk.AccAddress) error { + return im.keeper.HandleTimeout(ctx, packet) } // OnChanOpenTry implements the IBCModule interface. We don't need to implement this handler. diff --git a/x/cwica/keeper/ibc_handlers.go b/x/cwica/keeper/ibc_handlers.go index 7a2ac48c..caeaa013 100644 --- a/x/cwica/keeper/ibc_handlers.go +++ b/x/cwica/keeper/ibc_handlers.go @@ -55,7 +55,7 @@ func (k *Keeper) HandleChanOpenAck( } // HandleAcknowledgement passes the acknowledgement data to the appropriate contract via a sudo call. -func (k *Keeper) HandleAcknowledgement(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress) error { +func (k *Keeper) HandleAcknowledgement(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte) error { icaOwner := types.ICAOwnerFromPort(packet.SourcePort) contractAddress, err := sdk.AccAddressFromBech32(icaOwner) if err != nil { @@ -99,10 +99,10 @@ func (k *Keeper) HandleAcknowledgement(ctx sdk.Context, packet channeltypes.Pack return nil } -// HandleTimeout passes the timeout data to the appropriate contract via a sudo call. +// HandleTimeout creates a new error which can be accessed using the x/cwerrors module. // Since all ICA channels are ORDERED, a single timeout shuts down a channel. // The channel can be reopened by registering the ICA account again. -func (k *Keeper) HandleTimeout(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress) error { +func (k *Keeper) HandleTimeout(ctx sdk.Context, packet channeltypes.Packet) error { icaOwner := types.ICAOwnerFromPort(packet.SourcePort) contractAddress, err := sdk.AccAddressFromBech32(icaOwner) if err != nil { diff --git a/x/cwica/keeper/ibc_handlers_test.go b/x/cwica/keeper/ibc_handlers_test.go index ebda9519..a9979182 100644 --- a/x/cwica/keeper/ibc_handlers_test.go +++ b/x/cwica/keeper/ibc_handlers_test.go @@ -24,10 +24,9 @@ func (s *KeeperTestSuite) TestHandleAcknowledgement() { contractAddress.String(), contractAdminAcc.Address.String(), ) - relayerAddress := s.chain.GetAccount(1).Address // TEST CASE 1: invalid contract address - err := cwicaKeeper.HandleAcknowledgement(ctx, channeltypes.Packet{}, nil, relayerAddress) + err := cwicaKeeper.HandleAcknowledgement(ctx, channeltypes.Packet{}, nil) s.Require().ErrorContains(err, "failed to parse contract address: : invalid address") // TEST CASE 2: invalid packet acknowledgement @@ -36,7 +35,7 @@ func (s *KeeperTestSuite) TestHandleAcknowledgement() { SourcePort: icatypes.ControllerPortPrefix + contractAddress.String(), SourceChannel: "channel-0", } - err = cwicaKeeper.HandleAcknowledgement(ctx, p, nil, relayerAddress) + err = cwicaKeeper.HandleAcknowledgement(ctx, p, nil) s.Require().ErrorContains(err, "cannot unmarshal ICS-27 packet acknowledgement") // TEST CASE 3: success contract SudoResponse @@ -46,12 +45,12 @@ func (s *KeeperTestSuite) TestHandleAcknowledgement() { resAckData, err := channeltypes.SubModuleCdc.MarshalJSON(&resACK) s.Require().NoError(err) - err = cwicaKeeper.HandleAcknowledgement(ctx, p, resAckData, relayerAddress) + err = cwicaKeeper.HandleAcknowledgement(ctx, p, resAckData) s.Require().NoError(err) // TEST CASE 4: contract callback fails - should not return error - because error is swallowed wmKeeper.SetReturnSudoError(errors.New("error sudoResponse")) - err = cwicaKeeper.HandleAcknowledgement(ctx, p, resAckData, relayerAddress) + err = cwicaKeeper.HandleAcknowledgement(ctx, p, resAckData) s.Require().NoError(err) } @@ -71,10 +70,9 @@ func (s *KeeperTestSuite) TestHandleTimeout() { contractAddress.String(), contractAdminAcc.Address.String(), ) - relayerAddress := s.chain.GetAccount(1).Address // TEST CASE 1: invalid contract address - err := cwicaKeeper.HandleTimeout(ctx, channeltypes.Packet{}, relayerAddress) + err := cwicaKeeper.HandleTimeout(ctx, channeltypes.Packet{}) s.Require().ErrorContains(err, "failed to parse contract address: : invalid address") // TEST CASE 2: success contract SudoResponse @@ -84,12 +82,12 @@ func (s *KeeperTestSuite) TestHandleTimeout() { SourceChannel: "channel-0", } - err = cwicaKeeper.HandleTimeout(ctx, p, relayerAddress) + err = cwicaKeeper.HandleTimeout(ctx, p) s.Require().NoError(err) // TEST CASE 3: contract callback fails - should not return error - because error is swallowed wmKeeper.SetReturnSudoError(errors.New("SudoTimeout error")) - err = cwicaKeeper.HandleTimeout(ctx, p, relayerAddress) + err = cwicaKeeper.HandleTimeout(ctx, p) s.Require().NoError(err) } diff --git a/x/cwica/spec/02_messages.md b/x/cwica/spec/02_messages.md index 0b93a898..d43cfa59 100644 --- a/x/cwica/spec/02_messages.md +++ b/x/cwica/spec/02_messages.md @@ -75,7 +75,7 @@ message MsgSendTx { ``` On Success -* An ibc packet is created which will be executed on the counterparty chain +* An ibc packet is created will need to be picked up by the relayer This mesasge is expected to fail if: * There are no msgs to be executed diff --git a/x/cwica/spec/06_ibc.md b/x/cwica/spec/06_ibc.md new file mode 100644 index 00000000..aa6244c3 --- /dev/null +++ b/x/cwica/spec/06_ibc.md @@ -0,0 +1,48 @@ +# IBC Handlers + +Section describes the processing of the various IBC events + +## OnChanOpenInit + +There is no custom logic executed under this handler. + +## OnChanOpenTry + +This handler is not implemented for controller, only host. + +## OnChanOpenAck + +This handler is executed when a channel is successfully created between the chain and the counterparty chain. + +On success ack: +* The handler parses the counterparty version details and extracts the counterparty address and executes a Sudo call to the contract with the details + +## OnChanOpenConfirm + +This handler is not implemented for controller, only host. + +## OnChanCloseInit + +This handler is not implemented for controller, only host. + +## OnChanCloseConfirm + +This handler is currently not implemented by the module + +## OnRecvPacket + +This handler is not implemented for controller, only host. + +## OnAcknowledgementPacket + +This handler is executed when an ibc acknowledgement packet is received for a MsgSendTx operation. + +On success: +* The handler formats the ibc packet and acknowledgement result into a sudo payload and executes a Sudo call to the contract with the details. + +On failure: +* The handler formats the ibc packet and error into a Sudo Error with error code [ERR_EXEC_FAILURE](05_errors.md) and sends it to the x/cwerrors module + +## OnTimeoutPacket + +This handler is executed when an ibc packet times out. It formats the ibc packet int oa Sudo Error with error code [ERR_PACKET_TIMEOUT](05_errors.md) and sends it to the x/cwerrors module \ No newline at end of file diff --git a/x/cwica/spec/README.md b/x/cwica/spec/README.md index 8384336c..701337b8 100644 --- a/x/cwica/spec/README.md +++ b/x/cwica/spec/README.md @@ -66,7 +66,27 @@ let sendtx_stargate_msg = CosmosMsg::Stargate { Ok(Response::new().add_message(sendtx_stargate_msg)) ``` -Once the txs have been submitted, the contract will receive a callback at the Sudo entrypoints. [Here is how to integrate them](../../../proto/archway/cwica/v1/sudo.proto) +Once the txs have been submitted and is successfully executed on the counterparty chain, the contract will receive a callback at the Sudo entrypoints. It can be integrated by implementing the snippet below + +```rust +// msg.rs +pub enum SudoMsg { + Ica { + account_registered: Option, + tx_executed: Option, + }, +} + +#[cw_serde] +pub struct AccountRegistered { + pub counterparty_address: String, +} + +#[cw_serde] +pub struct ICAResponse { + pub data: Binary, +} +``` > **NOTE** @@ -79,4 +99,5 @@ Once the txs have been submitted, the contract will receive a callback at the Su 2. [Messages](./02_messages.md) 3. [Client](./03_client.md) 4. [Wasm Bindings](./04_wasm_bindings.md) -5. [Module Errors](./05_errors.md) \ No newline at end of file +5. [Module Errors](./05_errors.md) +6. [IBC Handlers](./06_ibc.md) \ No newline at end of file diff --git a/x/cwica/types/errors.go b/x/cwica/types/errors.go index 5f2ea071..7170f92c 100644 --- a/x/cwica/types/errors.go +++ b/x/cwica/types/errors.go @@ -8,8 +8,6 @@ import ( // x/cwica module sentinel errors var ( - ErrInvalidAccountAddress = errors.Register(ModuleName, 1101, "invalid account address") - ErrInterchainAccountNotFound = errors.Register(ModuleName, 1102, "interchain account not found") ErrNotContract = errors.Register(ModuleName, 1103, "not a contract") ErrEmptyConnectionID = errors.Register(ModuleName, 1104, "empty connection id") ErrCounterpartyConnectionNotFoundID = errors.Register(ModuleName, 1105, "counterparty connection id not found")