fix(delayedack): fix missing validation of channel id when validating…

… rollapp packet (#796)

Co-authored-by: Daniel T <[email protected]>

ibctesting/delayed_ack_test.go

@@ -98,6 +98,7 @@ func (suite *DelayedAckTestSuite) TestTransferRollappToHubNotFinalized() {
 
 	suite.CreateRollapp()
 	suite.RegisterSequencer()
+	suite.GenesisEvent(path.EndpointA.ChannelID)
 	suite.UpdateRollappState(uint64(suite.rollappChain.GetContext().BlockHeight()))
 
 	timeoutHeight := clienttypes.NewHeight(100, 110)
@@ -132,6 +133,7 @@ func (suite *DelayedAckTestSuite) TestTransferRollappToHubFinalization() {
 
 	suite.CreateRollapp()
 	suite.RegisterSequencer()
+	suite.GenesisEvent(path.EndpointA.ChannelID)
 
 	// Upate rollapp state
 	currentRollappBlockHeight := uint64(suite.rollappChain.GetContext().BlockHeight())
@@ -182,6 +184,7 @@ func (suite *DelayedAckTestSuite) TestHubToRollappTimeout() {
 	// Create rollapp and update its initial state
 	suite.CreateRollapp()
 	suite.RegisterSequencer()
+	suite.GenesisEvent(path.EndpointA.ChannelID)
 	suite.UpdateRollappState(uint64(suite.rollappChain.GetContext().BlockHeight()))
 	// Set the timeout height
 	timeoutHeight := clienttypes.GetSelfHeight(suite.rollappChain.GetContext())

ibctesting/denom_metadata_test.go

@@ -35,7 +35,7 @@ func (suite *DenomMetaDataTestSuite) TestDenomRegistationRollappToHub() {
 	app := ConvertToApp(suite.hubChain)
 
 	// invoke genesis event, in order to register denoms
-	suite.GenesisEvent(path.EndpointB.Chain.ChainID, path.EndpointA.ChannelID)
+	suite.GenesisEvent(path.EndpointA.ChannelID)
 
 	// Finalize the rollapp 100 blocks later so all packets are received immediately
 	currentRollappBlockHeight := uint64(suite.rollappChain.GetContext().BlockHeight())

ibctesting/eibc_test.go

@@ -48,11 +48,13 @@ func (suite *EIBCTestSuite) TestEIBCDemandOrderCreation() {
 	suite.CreateRollapp()
 	// Register sequencer
 	suite.RegisterSequencer()
-	// adding state for the rollapp
-	suite.UpdateRollappState(uint64(suite.rollappChain.GetContext().BlockHeight()))
 	// Create path so we'll be using the same channel
 	path := suite.NewTransferPath(suite.hubChain, suite.rollappChain)
 	suite.coordinator.Setup(path)
+	// Trigger the genesis event to register the denoms
+	suite.GenesisEvent(path.EndpointA.ChannelID)
+	// adding state for the rollapp
+	suite.UpdateRollappState(uint64(suite.rollappChain.GetContext().BlockHeight()))
 	// Setup globals for the test cases
 	IBCSenderAccount := suite.rollappChain.SenderAccount.GetAddress().String()
 	// Create cases
@@ -172,6 +174,8 @@ func (suite *EIBCTestSuite) TestEIBCDemandOrderFulfillment() {
 	suite.RegisterSequencer()
 	path := suite.NewTransferPath(suite.hubChain, suite.rollappChain)
 	suite.coordinator.Setup(path)
+	// Trigger the genesis event to register the denoms
+	suite.GenesisEvent(path.EndpointA.ChannelID)
 	// Setup globals for the test
 	totalDemandOrdersCreated := 0
 	eibcKeeper := ConvertToApp(suite.hubChain).EIBCKeeper
@@ -332,6 +336,8 @@ func (suite *EIBCTestSuite) TestTimeoutEIBCDemandOrderFulfillment() {
 	// Create rollapp and update its initial state
 	suite.CreateRollapp()
 	suite.RegisterSequencer()
+	// Trigger the genesis event to register the denoms
+	suite.GenesisEvent(path.EndpointA.ChannelID)
 	suite.UpdateRollappState(uint64(suite.rollappChain.GetContext().BlockHeight()))
 
 	type TC struct {

ibctesting/rollapp_genesis_token_test.go

@@ -230,6 +230,12 @@ func (suite *RollappGenesisTokenTestSuite) TestTriggerGenesisEvent() {
 			_, err := suite.msgServer.TriggerGenesisEvent(ctx, tc.msg)
 			suite.hubChain.NextBlock()
 			suite.Require().ErrorIs(err, tc.expErr)
+
+			if tc.expErr == nil {
+				rollapp, _ = rollappKeeper.GetRollapp(suite.ctx, suite.rollappChain.ChainID)
+				suite.Require().NotEmpty(rollapp.ChannelId)
+			}
+
 			// Validate no tokens are in the module account
 			accountKeeper := ConvertToApp(suite.hubChain).AccountKeeper
 			bankKeeper := ConvertToApp(suite.hubChain).BankKeeper

ibctesting/utils_test.go

@@ -99,19 +99,13 @@ func (suite *IBCTestUtilSuite) CreateRollapp() {
 	suite.Require().NoError(err) // message committed
 }
 
-func (suite *IBCTestUtilSuite) GenesisEvent(chainID, channelID string) {
+func (suite *IBCTestUtilSuite) GenesisEvent(channelID string) {
 	// add sender to deployer whitelist
 	app := ConvertToApp(suite.hubChain)
 	params := app.RollappKeeper.GetParams(suite.hubChain.GetContext())
 	params.DeployerWhitelist = []rollapptypes.DeployerParams{{Address: suite.hubChain.SenderAccount.GetAddress().String()}}
 	app.RollappKeeper.SetParams(suite.hubChain.GetContext(), params)
 
-	// add genesis state to rollapp
-	rollapp, found := app.RollappKeeper.GetRollapp(suite.hubChain.GetContext(), chainID)
-	suite.Require().True(found)
-	rollapp.GenesisState = rollapptypes.RollappGenesisState{}
-	app.RollappKeeper.SetRollapp(suite.hubChain.GetContext(), rollapp)
-
 	msgGenesisEvent := rollapptypes.NewMsgRollappGenesisEvent(
 		suite.hubChain.SenderAccount.GetAddress().String(),
 		channelID,

x/delayedack/ibc_middleware.go

@@ -306,10 +306,20 @@ func (im IBCMiddleware) ExtractRollappIDAndTransferPacket(ctx sdk.Context, packe
 	if err != nil {
 		return "", &data, err
 	}
-	_, found := im.keeper.GetRollapp(ctx, chainID)
+	rollapp, found := im.keeper.GetRollapp(ctx, chainID)
 	if !found {
 		return "", &data, nil
 	}
+	if rollapp.ChannelId == "" {
+		return "", &data, errorsmod.Wrapf(rollapptypes.ErrGenesisEventNotTriggered, "empty channel id: rollap id: %s", chainID)
+	}
+	// check if the channelID matches the rollappID's channelID
+	if rollapp.ChannelId != packet.GetDestChannel() {
+		return "", &data, errorsmod.Wrapf(
+			rollapptypes.ErrMismatchedChannelID,
+			"channel id mismatch: expect: %s: got: %s", rollapp.ChannelId, packet.GetDestChannel(),
+		)
+	}
 
 	return chainID, &data, nil
 }

x/delayedack/keeper/keeper.go

@@ -186,20 +186,19 @@ func (k *Keeper) LookupModuleByChannel(ctx sdk.Context, portID, channelID string
 	return k.channelKeeper.LookupModuleByChannel(ctx, portID, channelID)
 }
 
-// ValidateRollappId checks that the rollappid from the ibc connection matches the rollapp checking the sequencer registered with the consensus state validator set
-func (k *Keeper) ValidateRollappId(ctx sdk.Context, rollapp, portID, channelID string) error {
+// ValidateRollappId checks that the rollapp id from the ibc connection matches the rollapp, checking the sequencer registered with the consensus state validator set
+func (k *Keeper) ValidateRollappId(ctx sdk.Context, rollappID, portID, channelID string) error {
 	// Get the sequencer from the latest state info update and check the validator set hash
-	// from the headers match with the sequencer for the rollapp
+	// from the headers match with the sequencer for the rollappID
 	// As the assumption the sequencer is honest we don't check the packet proof height.
-	latestStateIndex, found := k.rollappKeeper.GetLatestStateInfoIndex(ctx, rollapp)
+	latestStateIndex, found := k.rollappKeeper.GetLatestStateInfoIndex(ctx, rollappID)
 	if !found {
-		return errorsmod.Wrapf(rollapptypes.ErrUnknownRollappID, "state index not found for the rollapp: %s", rollapp)
+		return errorsmod.Wrapf(rollapptypes.ErrUnknownRollappID, "state index not found for the rollappID: %s", rollappID)
 	}
-	stateInfo, found := k.rollappKeeper.GetStateInfo(ctx, rollapp, latestStateIndex.Index)
+	stateInfo, found := k.rollappKeeper.GetStateInfo(ctx, rollappID, latestStateIndex.Index)
 	if !found {
-		return errorsmod.Wrapf(rollapptypes.ErrUnknownRollappID, "state info not found for the rollapp: %s with index: %d", rollapp, latestStateIndex.Index)
+		return errorsmod.Wrapf(rollapptypes.ErrUnknownRollappID, "state info not found for the rollappID: %s with index: %d", rollappID, latestStateIndex.Index)
 	}
-
 	// Compare the validators set hash of the consensus state to the sequencer hash.
 	// TODO (srene): We compare the validator set of the last consensus height, because it fails to  get consensus for a different height,
 	// but we should compare the validator set at the height of the last state info, because sequencer may have changed after that.
@@ -213,7 +212,7 @@ func (k *Keeper) ValidateRollappId(ctx sdk.Context, rollapp, portID, channelID s
 	// Gets sequencer information from the sequencer address found in the latest state info
 	sequencer, found := k.sequencerKeeper.GetSequencer(ctx, stateInfo.Sequencer)
 	if !found {
-		return sdkerrors.Wrapf(sequencertypes.ErrUnknownSequencer, "sequencer %s not found for the rollapp %s", stateInfo.Sequencer, rollapp)
+		return sdkerrors.Wrapf(sequencertypes.ErrUnknownSequencer, "sequencer %s not found for the rollappID %s", stateInfo.Sequencer, rollappID)
 	}
 
 	// Gets the validator set hash made out of the pub key for the sequencer
@@ -222,9 +221,9 @@ func (k *Keeper) ValidateRollappId(ctx sdk.Context, rollapp, portID, channelID s
 		return err
 	}
 
-	// It compares the validator set hash from the consensus state with the one we recreated from the sequencer. If its true it means the chain corresponds to the rollapp chain
+	// It compares the validator set hash from the consensus state with the one we recreated from the sequencer. If its true it means the chain corresponds to the rollappID chain
 	if !bytes.Equal(tmConsensusState.NextValidatorsHash, seqPubKeyHash) {
-		errMsg := fmt.Sprintf("consensus state does not match: consensus state validators %x, rollapp sequencer %x",
+		errMsg := fmt.Sprintf("consensus state does not match: consensus state validators %x, rollappID sequencer %x",
 			tmConsensusState.NextValidatorsHash, stateInfo.Sequencer)
 		return sdkerrors.Wrap(types.ErrMismatchedSequencer, errMsg)
 	}

x/rollapp/types/errors.go

@@ -36,12 +36,13 @@ var (
 	ErrInvalidClientState                  = errorsmod.Register(ModuleName, 1025, "invalid client state")
 	ErrInvalidSequencer                    = errorsmod.Register(ModuleName, 1026, "invalid sequencer")
 	ErrInvalidGenesisChannelId             = errorsmod.Register(ModuleName, 1027, "invalid genesis channel id")
-	ErrGenesisEventNotDefined              = errorsmod.Register(ModuleName, 1028, "genesis event not defined")
+	ErrGenesisEventNotTriggered            = errorsmod.Register(ModuleName, 1028, "genesis event not triggered yet")
 	ErrGenesisEventAlreadyTriggered        = errorsmod.Register(ModuleName, 1029, "genesis event already triggered")
 	ErrTooManyPermissionedAddresses        = errorsmod.Register(ModuleName, 1030, "invalid number of permissioned addresses")
 	ErrInvalidGenesisAccount               = errorsmod.Register(ModuleName, 1031, "invalid genesis account")
 	ErrMintTokensFailed                    = errorsmod.Register(ModuleName, 1032, "failed to mint tokens")
 	ErrRegisterDenomMetadataFailed         = errorsmod.Register(ModuleName, 1033, "failed to register denom metadata")
+	ErrMismatchedChannelID                 = errorsmod.Register(ModuleName, 1034, "mismatched channel id")
 	/* ------------------------------ fraud related ----------------------------- */
 	ErrDisputeAlreadyFinalized = errorsmod.Register(ModuleName, 2000, "disputed height already finalized")
 	ErrDisputeAlreadyReverted  = errorsmod.Register(ModuleName, 2001, "disputed height already reverted")