Add some docstrings and adjust formatting

scripts/test_doc/extract_docstrings.go

@@ -101,13 +101,17 @@ func extractDocstrings(filePath string, out *os.File) []string {
 				}
 
 				// Format the description
+
+				// avoid breaking the table format: newlines need to be replaced
+				// for the short description, use spaces
+				shortDescription = strings.ReplaceAll(shortDescription, "\n", " ")
+				// for the long description, use breaks
+				longDescription = strings.ReplaceAll(longDescription, "\n", "<br>")
+
 				description := shortDescription
 				if longDescription != "" {
 					description += fmt.Sprintf("<details><summary>Details</summary>%s</details>", longDescription)
 				}
-				// for formatting the description in markdown table,
-				// replace newlines with spaces
-				description = strings.ReplaceAll(description, "\n", " ")
 
 				fmt.Fprintf(out, "| %s | %s | %s |\n", link, fn.Name.Name, description)
 			} else {

tests/integration/changeover.go

@@ -13,7 +13,7 @@ import (
 // * creates a connection between the two chains
 // * creates a transfer channel between the two chains
 // * transitions the standalone chain to a consumer chain
-// * confirms that no extra transfer channel is created, and instead the existing channel is reused
+// * confirms that no extra transfer channel is created, thus only one transfer channel and one CCV channel exist.
 func (suite *CCVTestSuite) TestRecycleTransferChannel() {
 	consumerKeeper := suite.consumerApp.GetConsumerKeeper()
 

tests/integration/democracy.go

@@ -71,6 +71,11 @@ func (suite *ConsumerDemocracyTestSuite) SetupTest() {
 		suite.consumerApp = suite.setupCallback(&suite.Suite)
 }
 
+// TestDemocracyRewardsDistribution checks that rewards to democracy representatives, community pool, and provider redistribution account are done correctly.
+// @Long Description@
+// * Sets up a democracy consumer chain
+// * Creates a new block
+// * Checks that rewards to democracy representatives, community pool, and provider redistribution account are distributed in the right proportions
 func (s *ConsumerDemocracyTestSuite) TestDemocracyRewardsDistribution() {
 	s.consumerChain.NextBlock()
 	stakingKeeper := s.consumerApp.GetTestStakingKeeper()
@@ -154,7 +159,7 @@ func (s *ConsumerDemocracyTestSuite) TestDemocracyRewardsDistribution() {
 	totalFees := nextConsumerRedistributeBalance.Add(providerDifference)
 	s.Require().Equal(totalFees.Mul(consumerRedistributionFraction), nextConsumerRedistributeBalance)
 
-	// confirm begin blocker changes: democracy module distributes the fees from c onsumer redistribute address to representatives
+	// confirm begin blocker changes: democracy module distributes the fees from consumer redistribute address to representatives
 	// and community fee pool
 	// distribution module got tokens from previous consumer redistribute balance
 	s.Require().Equal(distrModuleDifference, previousConsumerRedistributeBalance)
@@ -176,6 +181,17 @@ func (s *ConsumerDemocracyTestSuite) TestDemocracyRewardsDistribution() {
 	}
 }
 
+// TestDemocracyGovernanceWhitelisting checks that only whitelisted governance proposals
+// can be executed on democracy consumer chains.
+// @Long Description@
+// For context, see the whitelist for proposals in app/consumer-democracy/proposals_whitelisting.go.
+// * Sets up a democracy consumer chain
+// * Submits a proposal containing changes to the auth and mint module parameters
+// * Checks that the proposal is not executed, since the change to the auth module is not whitelisted.
+// * Submits a proposal containing changes *only* to the mint module parameters
+// * Checks that the proposal is executed, since the change to the mint module is whitelisted.
+// * Submits a proposal containing changes *only* to the auth module parameters
+// * Checks that again, the proposal is not executed, since the change to the auth module is not whitelisted.
 func (s *ConsumerDemocracyTestSuite) TestDemocracyGovernanceWhitelisting() {
 	govKeeper := s.consumerApp.GetTestGovKeeper()
 	params, err := govKeeper.Params.Get(s.consumerCtx())
@@ -271,6 +287,11 @@ func (s *ConsumerDemocracyTestSuite) TestDemocracyGovernanceWhitelisting() {
 	s.Assert().Equal(votersOldBalances, getAccountsBalances(s.consumerCtx(), bankKeeper, bondDenom, votingAccounts))
 }
 
+// TestDemocracyMsgUpdateParams checks that the consumer parameters can be updated through a governance proposal.
+// @Long Description@
+// * Sets up a democracy consumer chain
+// * Submits a proposal containing changes to the consumer module parameters
+// * Checks that the proposal is executed, and the parameters are updated
 func (s *ConsumerDemocracyTestSuite) TestDemocracyMsgUpdateParams() {
 	govKeeper := s.consumerApp.GetTestGovKeeper()
 	params, err := govKeeper.Params.Get(s.consumerCtx())
@@ -316,7 +337,6 @@ func (s *ConsumerDemocracyTestSuite) TestDemocracyMsgUpdateParams() {
 
 	// deposit is refunded
 	s.Assert().Equal(votersOldBalances, getAccountsBalances(s.consumerCtx(), bankKeeper, bondDenom, votingAccounts))
-
 }
 
 func submitProposalWithDepositAndVote(govKeeper govkeeper.Keeper, ctx sdk.Context, msgs []sdk.Msg,

tests/integration/distribution.go

@@ -138,8 +138,7 @@ func (s *CCVTestSuite) TestRewardsDistribution() {
 	consuValsRewards := consumerValsOutstandingRewardsFunc(s.providerCtx())
 
 	// increase the block height so validators are eligible for consumer rewards (see `IsEligibleForConsumerRewards`)
-	numberOfBlocksToStartReceivingRewards :=
-		providerKeeper.GetNumberOfEpochsToStartReceivingRewards(s.providerCtx()) * providerKeeper.GetBlocksPerEpoch(s.providerCtx())
+	numberOfBlocksToStartReceivingRewards := providerKeeper.GetNumberOfEpochsToStartReceivingRewards(s.providerCtx()) * providerKeeper.GetBlocksPerEpoch(s.providerCtx())
 
 	for s.providerCtx().BlockHeight() <= numberOfBlocksToStartReceivingRewards {
 		s.providerChain.NextBlock()
@@ -833,6 +832,17 @@ func (s *CCVTestSuite) prepareRewardDist() {
 	s.coordinator.CommitNBlocks(s.consumerChain, uint64(blocksToGo))
 }
 
+// TestAllocateTokensToConsumerValidators tests the allocation of tokens to consumer validators.
+// @Long Description@
+// The test exclusively uses the provider chain.
+// It sets up a current set of consumer validators, then calls the AllocateTokensToConsumerValidators
+// function to allocate a number of tokens to the validators.
+// The test then checks that the expected number of tokens were allocated to the validators.
+// The test covers the following scenarios:
+// - The tokens to be allocated are empty
+// - The consumer validator set is empty
+// - The tokens are allocated to a single validator
+// - The tokens are allocated to multiple validators
 func (s *CCVTestSuite) TestAllocateTokensToConsumerValidators() {
 	providerKeeper := s.providerApp.GetProviderKeeper()
 	distributionKeeper := s.providerApp.GetTestDistributionKeeper()

tests/integration/key_assignment.go

@@ -10,9 +10,26 @@ import (
 	tmprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto"
 
 	providerkeeper "github.com/cosmos/interchain-security/v5/x/ccv/provider/keeper"
+	"github.com/cosmos/interchain-security/v5/x/ccv/provider/types"
 	ccv "github.com/cosmos/interchain-security/v5/x/ccv/types"
 )
 
+// TestKeyAssignment tests key assignments relayed from the provider chain to the consumer chain at different times in the protocol lifecycle.
+// @Long Description@
+// Each test scenarios sets up a provider chain and then assigns a key for a validator.
+// However, the assignment comes at different times in the protocol lifecycle.
+// The test covers the following scenarios:
+// * successfully assign the key before the CCV channel initialization is complete, then check that a VSCPacket is indeed queud
+// * successfully assign the key after the CCV channel initialization is complete
+// * successfully assign the key during an same epoch where the validator power changes
+// * get an error when assigning the same key twice in the same block by different validators
+// * get an error when assigning the same key twice in the same block by the same validator
+// * successfully assign two different keys in the same block by one validator
+// * get an error when assigning the same key twice in different blocks by different validators
+// * get an error when assigning the same key twice in different blocks by the same validator
+// For each scenario where the key assignment does not produce an error,
+// the test also checks that VSCPackets are relayed to the consumer chain and that the clients on
+// the provider and consumer chain can be updated.
 func (s *CCVTestSuite) TestKeyAssignment() {
 	testCases := []struct {
 		name           string
@@ -29,6 +46,9 @@ func (s *CCVTestSuite) TestKeyAssignment() {
 					return err
 				}
 
+				// check that the key was assigned correctly
+				s.CheckKeyAssignment(validator, consumerKey)
+
 				// check that a VSCPacket is queued
 				s.nextEpoch()
 				pendingPackets := pk.GetPendingVSCPackets(s.providerCtx(), s.consumerChain.ChainID)
@@ -51,6 +71,10 @@ func (s *CCVTestSuite) TestKeyAssignment() {
 				if err != nil {
 					return err
 				}
+
+				// check that the key was assigned correctly
+				s.CheckKeyAssignment(validator, consumerKey)
+
 				s.nextEpoch()
 
 				return nil
@@ -73,6 +97,9 @@ func (s *CCVTestSuite) TestKeyAssignment() {
 				delAddr := s.providerChain.SenderAccount.GetAddress()
 				delegate(s, delAddr, bondAmt)
 
+				// check that the key was assigned correctly
+				s.CheckKeyAssignment(validator, consumerKey)
+
 				s.nextEpoch()
 
 				return nil
@@ -90,12 +117,25 @@ func (s *CCVTestSuite) TestKeyAssignment() {
 					return err
 				}
 
+				// check that the key was assigned correctly
+				s.CheckKeyAssignment(validator, consumerKey)
+
 				// same key assignment, but different validator
 				validator2, _ := generateNewConsumerKey(s, 1)
 				err = pk.AssignConsumerKey(s.providerCtx(), s.consumerChain.ChainID, validator2, consumerKey)
+
+				// check that the key was not assigned to the second validator
+				valConsAddr2, getConsAddrErr := validator2.GetConsAddr() // make sure we don't override err, which we are saving for below
+				s.Require().NoError(getConsAddrErr)
+				actualConsumerKey2, found := pk.GetValidatorConsumerPubKey(s.providerCtx(), s.consumerChain.ChainID, types.NewProviderConsAddress(valConsAddr2))
+				s.Require().True(found)
+				// the key for the second validator should *not* be the one we just assigned to the first validator
+				s.Require().NotEqual(consumerKey, actualConsumerKey2)
+
 				if err != nil {
 					return err
 				}
+
 				s.nextEpoch()
 
 				return nil
@@ -113,7 +153,10 @@ func (s *CCVTestSuite) TestKeyAssignment() {
 					return err
 				}
 
-				// same key assignment, but different validator
+				// check that the key was assigned correctly
+				s.CheckKeyAssignment(validator, consumerKey)
+
+				// same key assignment, same validator
 				err = pk.AssignConsumerKey(s.providerCtx(), s.consumerChain.ChainID, validator, consumerKey)
 				if err != nil {
 					return err
@@ -135,12 +178,19 @@ func (s *CCVTestSuite) TestKeyAssignment() {
 					return err
 				}
 
+				// check that the key was assigned correctly
+				s.CheckKeyAssignment(validator, consumerKey)
+
 				// same key assignment
 				validator, consumerKey = generateNewConsumerKey(s, 0)
 				err = pk.AssignConsumerKey(s.providerCtx(), s.consumerChain.ChainID, validator, consumerKey)
 				if err != nil {
 					return err
 				}
+
+				// check that the second key was also assigned correctly
+				s.CheckKeyAssignment(validator, consumerKey)
+
 				s.nextEpoch()
 
 				return nil
@@ -157,6 +207,10 @@ func (s *CCVTestSuite) TestKeyAssignment() {
 				if err != nil {
 					return err
 				}
+
+				// check that the key was assigned correctly
+				s.CheckKeyAssignment(validator, consumerKey)
+
 				s.nextEpoch()
 
 				// same key assignment
@@ -165,6 +219,15 @@ func (s *CCVTestSuite) TestKeyAssignment() {
 				if err != nil {
 					return err
 				}
+
+				// check that the key was not assigned to the second validator
+				valConsAddr2, getConsAddrErr := validator2.GetConsAddr() // make sure we don't override err, which we are saving for below
+				s.Require().NoError(getConsAddrErr)
+				actualConsumerKey2, found := pk.GetValidatorConsumerPubKey(s.providerCtx(), s.consumerChain.ChainID, types.NewProviderConsAddress(valConsAddr2))
+				s.Require().True(found)
+				// the key for the second validator should *not* be the one we just assigned to the first validator
+				s.Require().NotEqual(consumerKey, actualConsumerKey2)
+
 				s.nextEpoch()
 
 				return nil
@@ -181,6 +244,10 @@ func (s *CCVTestSuite) TestKeyAssignment() {
 				if err != nil {
 					return err
 				}
+
+				// check that the key was assigned correctly
+				s.CheckKeyAssignment(validator, consumerKey)
+
 				s.nextEpoch()
 
 				// same key assignment
@@ -204,6 +271,10 @@ func (s *CCVTestSuite) TestKeyAssignment() {
 				if err != nil {
 					return err
 				}
+
+				// check that the key was assigned correctly
+				s.CheckKeyAssignment(validator, consumerKey)
+
 				s.nextEpoch()
 
 				// same key assignment
@@ -212,6 +283,7 @@ func (s *CCVTestSuite) TestKeyAssignment() {
 				if err != nil {
 					return err
 				}
+
 				s.nextEpoch()
 
 				return nil
@@ -286,6 +358,15 @@ func (s *CCVTestSuite) TestKeyAssignment() {
 	}
 }
 
+// CheckKeyAssignmentCorrectly checks if the key was assigned correctly.
+func (s *CCVTestSuite) CheckKeyAssignment(validator stakingtypes.Validator, consumerKey tmprotocrypto.PublicKey) {
+	valConsAddr, err := validator.GetConsAddr()
+	s.Require().NoError(err)
+	actualConsumerKey, found := s.providerApp.GetProviderKeeper().GetValidatorConsumerPubKey(s.providerCtx(), s.consumerChain.ChainID, types.NewProviderConsAddress(valConsAddr))
+	s.Require().True(found)
+	s.Require().Equal(consumerKey, actualConsumerKey)
+}
+
 // generateNewConsumerKey generate new consumer key for the validator with valIndex
 func generateNewConsumerKey(s *CCVTestSuite, valIndex int) (stakingtypes.Validator, tmprotocrypto.PublicKey) {
 	// get validator

tests/integration/misbehaviour.go

@@ -81,6 +81,19 @@ func (s *CCVTestSuite) TestHandleConsumerMisbehaviour() {
 	}
 }
 
+// TestGetByzantineValidators checks the GetByzantineValidators function on various instances of misbehaviour.
+// @Long Description@
+// The test sets up a provider and consumer chain.
+// It creates a header with a subset of the validators on the consumer chain,
+// then creates a second header (in a variety of different ways),
+// and checks which validators are considered Byzantine
+// by calling the GetByzantineValidators function.
+// The test scenarios are:
+// * when one of the headers is empty, the function should return an error
+// * when one of the headers has a corrupted validator set (e.g. by a validator having a different public key), the function should return an error
+// * when the signatures in one of the headers are corrupted, the function should return an error
+// * when the attack is an amnesia attack (i.e. the headers have different block IDs), no validator is considered byzantine
+// * for non-amnesia misbehaviour, all validators that signed both headers are considered byzantine
 func (s *CCVTestSuite) TestGetByzantineValidators() {
 	s.SetupCCVChannel(s.path)
 	// required to have the consumer client revision height greater than 0
@@ -363,6 +376,21 @@ func (s *CCVTestSuite) TestGetByzantineValidators() {
 	}
 }
 
+// TestCheckMisbehaviour tests that the CheckMisbehaviour function correctly checks for misbehaviour.
+// @Long Description@
+// The test sets up a provider and consumer chain.
+// It creates a valid client header and then creates a misbehaviour by creating a second header in a variety of different ways.
+// It then checks that the CheckMisbehaviour function correctly checks for misbehaviour by verifying that
+// it returns an error when the misbehaviour is invalid and no error when the misbehaviour is valid.
+// The test scenarios are:
+// * both headers are identical (returns an error)
+// * the misbehaviour is not for the consumer chain (returns an error)
+// * passing an invalid client id (returns an error)
+// * passing a misbehaviour with different header height (returns an error)
+// * passing a misbehaviour older than the min equivocation evidence height (returns an error)
+// * one header of the misbehaviour has insufficient voting power (returns an error)
+// * passing a valid misbehaviour (no error)
+// It does not test actually submitting the misbehaviour to the chain or or freezing the client.
 func (s *CCVTestSuite) TestCheckMisbehaviour() {
 	s.SetupCCVChannel(s.path)
 	// required to have the consumer client revision height greater than 0
@@ -548,10 +576,6 @@ func (s *CCVTestSuite) TestCheckMisbehaviour() {
 	for _, tc := range testCases {
 		s.Run(tc.name, func() {
 			err := s.providerApp.GetProviderKeeper().CheckMisbehaviour(s.providerCtx(), *tc.misbehaviour)
-			cs, ok := s.providerApp.GetIBCKeeper().ClientKeeper.GetClientState(s.providerCtx(), s.path.EndpointA.ClientID)
-			s.Require().True(ok)
-			// verify that the client wasn't frozen
-			s.Require().Zero(cs.(*ibctmtypes.ClientState).FrozenHeight)
 			if tc.expPass {
 				s.NoError(err)
 			} else {

tests/integration/provider_gov_hooks.go

@@ -48,6 +48,15 @@ func (s *CCVTestSuite) TestAfterPropSubmissionAndVotingPeriodEnded() {
 	s.Require().Empty(providerKeeper.GetProposedConsumerChain(ctx, proposal.Id))
 }
 
+// TestGetConsumerAdditionLegacyPropFromProp manually calls the GetConsumerAdditionLegacyPropFromProp hook on
+// various types of proposals to test the behavior of the hook.
+// @Long Description@
+// The tes case created a provider chain,
+// then submits a Proposal with various different types of content.
+// Then, it tries to get the ConsumerAdditionProposal from the proposal using the hook.
+// Test cases include a proposal with no messages; a proposal with a transfer message; a proposal with an unrelated legacy proposal;
+// a proposal with an invalid legacy proposal; and a proposal with a ConsumerAdditionProposal.
+// In the case of a valid ConsumerAdditionProposal, the test verifies that the proposal is found and returned by the hook.
 func (s *CCVTestSuite) TestGetConsumerAdditionLegacyPropFromProp() {
 	ctx := s.providerChain.GetContext()
 	proposer := s.providerChain.SenderAccount
@@ -121,7 +130,7 @@ func (s *CCVTestSuite) TestGetConsumerAdditionLegacyPropFromProp() {
 				proposal, err = v1.NewProposal([]sdk.Msg{}, 1, time.Now(), time.Now().Add(1*time.Hour), "metadata", "title", "summary", proposer.GetAddress(), false)
 				s.Require().NoError(err)
 			} else {
-				// cover variolus cases where proposal has messages but only some are consumer addition proposals
+				// cover various cases where proposal has messages but only some are consumer addition proposals
 				proposal, err = v1.NewProposal([]sdk.Msg{tc.propMsg}, 1, time.Now(), time.Now().Add(1*time.Hour), "metadata", "title", "summary", proposer.GetAddress(), false)
 				s.Require().NoError(err)
 			}

tests/integration/slashing.go

@@ -255,6 +255,11 @@ func (s *CCVTestSuite) TestRelayAndApplyDoubleSignPacket() {
 	s.Require().NoError(err)
 }
 
+// TestSlashPacketAcknowledgement tests the handling of a slash packet acknowledgement.
+// @Long Description@
+// It sets up a provider and consumer chain, with channel initialization between them performed,
+// then sends a slash packet with randomized fields from the consumer to the provider.
+// The provider processes the packet
 func (s *CCVTestSuite) TestSlashPacketAcknowledgement() {
 	providerKeeper := s.providerApp.GetProviderKeeper()
 	consumerKeeper := s.consumerApp.GetConsumerKeeper()