chore: beautify x/alerts (#93)

* init

* format

* types

* fix new issue

* strat

* keeper

* cli

---------

Co-authored-by: David Terpay <[email protected]>

x/alerts/client/tx.go

@@ -31,10 +31,10 @@ func AlertTxCmd() *cobra.Command {
 		Short: "Create a new alert",
 		Long: `
 Create a new alert with the specified height, sender, and currency-pair.
-	Example: "slinkycli tx alerts alert cosmos1deadbeef 1 BTC/USD"
-	Structure: "slinkycli tx alerts alert <sender> <height> <currency-pair>
+	Example: "slinkyd tx alerts alert cosmos... 1 BTC/USD"
+	Structure: "slinkyd tx alerts alert <sender> <height> <currency-pair>
 `,
-		Example: "slinkycli tx alerts alert cosmos1deadbeef 1 BTC/USD",
+		Example: "slinkyd tx alerts alert cosmos... 1 BTC/USD",
 		Args:    cobra.ExactArgs(3),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			clientCtx, err := client.GetClientTxContext(cmd)

x/alerts/keeper/abci.go

@@ -14,8 +14,8 @@ import (
 //	It is used to determine which Alerts are to be purged, and if they should be purged, the alerts will be removed from state.
 //
 // If the AlertStatus of the Alert is concluded, nothing will be done. If the AlertStatus
-// is Unconcluded, the alert will be Concluded positively (i.e the bond will be returned to the bond-address).
-func (k Keeper) EndBlocker(goCtx context.Context) ([]cmtabci.ValidatorUpdate, error) {
+// is Unconcluded, the alert will be Concluded positively (i.e, the bond will be returned to the bond-address).
+func (k *Keeper) EndBlocker(goCtx context.Context) ([]cmtabci.ValidatorUpdate, error) {
 	// unwrap the context
 	ctx := sdk.UnwrapSDKContext(goCtx)
 

x/alerts/keeper/abci_test.go

@@ -15,7 +15,7 @@ func (s *KeeperTestSuite) TestEndBlocker() {
 	// set context
 	s.ctx = s.ctx.WithBlockHeight(10)
 
-	// set three alerts (this shld be purged first)
+	// set three alerts (this should be purged first)
 	alert1 := types.NewAlertWithStatus(
 		types.NewAlert(1, sdk.AccAddress("abc1"), slinkytypes.NewCurrencyPair("BTC", "USD")),
 		types.NewAlertStatus(10, 10, time.Time{}, types.Concluded),

x/alerts/keeper/conclusions.go

@@ -18,7 +18,7 @@ const (
 // ConcludeAlert takes an Alert and status. This method returns the Alert's bond to the Alert's owner if the Alert
 // is concluded with positive status, if the alert is concluded with negative status, the bond is burned.
 // Finally, the alert's status is set to Concluded, and it's purge height is set to alert.Height + AlertParams.MaxBlockAge.
-func (k Keeper) ConcludeAlert(ctx sdk.Context, alertToConclude types.Alert, status ConclusionStatus) error {
+func (k *Keeper) ConcludeAlert(ctx sdk.Context, alertToConclude types.Alert, status ConclusionStatus) error {
 	// check that the alert is valid
 	if err := alertToConclude.ValidateBasic(); err != nil {
 		return err
@@ -61,7 +61,7 @@ func (k Keeper) ConcludeAlert(ctx sdk.Context, alertToConclude types.Alert, stat
 }
 
 // unescrowBond sends the bond at the module account back to the alert's signer.
-func (k Keeper) unescrowBond(ctx sdk.Context, a types.Alert, bond sdk.Coin) error {
+func (k *Keeper) unescrowBond(ctx sdk.Context, a types.Alert, bond sdk.Coin) error {
 	alertSigner, err := sdk.AccAddressFromBech32(a.Signer)
 	if err != nil {
 		return err
@@ -77,7 +77,7 @@ func (k Keeper) unescrowBond(ctx sdk.Context, a types.Alert, bond sdk.Coin) erro
 }
 
 // burnBond burns the bond stored at the module account's address.
-func (k Keeper) burnBond(ctx sdk.Context, bond sdk.Coin) error {
+func (k *Keeper) burnBond(ctx sdk.Context, bond sdk.Coin) error {
 	// burn the coins
 	return k.bankKeeper.BurnCoins(
 		ctx,

x/alerts/keeper/conclusions_test.go

@@ -58,13 +58,13 @@ func (s *KeeperTestSuite) TestConcludeAlert() {
 			keeper.Negative,
 			func(ctx sdk.Context) {
 				// set the alert with concluded AlertStatus
-				s.alertKeeper.SetAlert(
+				s.Require().NoError(s.alertKeeper.SetAlert(
 					ctx,
 					types.NewAlertWithStatus(
 						types.NewAlert(1, sdk.AccAddress("abc"), slinkytypes.NewCurrencyPair("BASE", "QUOTE")),
 						types.NewAlertStatus(1, 1, time.Now(), types.Concluded),
 					),
-				)
+				))
 			},
 			fmt.Errorf("alert already concluded"),
 			types.AlertWithStatus{},
@@ -75,13 +75,13 @@ func (s *KeeperTestSuite) TestConcludeAlert() {
 			keeper.ConclusionStatus(3),
 			func(ctx sdk.Context) {
 				// set the alert with concluded AlertStatus
-				s.alertKeeper.SetAlert(
+				s.Require().NoError(s.alertKeeper.SetAlert(
 					ctx,
 					types.NewAlertWithStatus(
 						types.NewAlert(1, sdk.AccAddress("abc"), slinkytypes.NewCurrencyPair("BASE", "QUOTE")),
 						types.NewAlertStatus(1, 1, time.Now(), types.Unconcluded),
 					),
-				)
+				))
 			},
 			fmt.Errorf("invalid status: 3"),
 			types.AlertWithStatus{},

x/alerts/keeper/genesis.go

@@ -8,7 +8,7 @@ import (
 
 // InitGenesis initializes the module state from a GenesisState object. Specifically, this method sets the
 // params to state and adds all alerts from the genesis state to state.
-func (k Keeper) InitGenesis(ctx sdk.Context, gs types.GenesisState) {
+func (k *Keeper) InitGenesis(ctx sdk.Context, gs types.GenesisState) {
 	// validate the genesis state
 	if err := gs.ValidateBasic(); err != nil {
 		panic(err)
@@ -29,7 +29,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, gs types.GenesisState) {
 
 // ExportGenesis returns a GenesisState object containing the current module state. Specifically, this method
 // returns the current params and all alerts in state.
-func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
+func (k *Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
 	// get the params
 	params := k.GetParams(ctx)
 

x/alerts/keeper/keeper.go

@@ -70,7 +70,7 @@ func NewKeeper(
 	return k
 }
 
-func (k Keeper) Logger(ctx sdk.Context) log.Logger {
+func (k *Keeper) Logger(ctx sdk.Context) log.Logger {
 	return ctx.Logger()
 }
 
@@ -100,7 +100,7 @@ func (k *Keeper) GetAllAlerts(ctx sdk.Context) ([]types.AlertWithStatus, error)
 }
 
 // GetAllAlertsWithCondition returns all alerts for which the Condition evaluates to true.
-func (k Keeper) GetAllAlertsWithCondition(ctx sdk.Context, c Condition) ([]types.AlertWithStatus, error) {
+func (k *Keeper) GetAllAlertsWithCondition(ctx sdk.Context, c Condition) ([]types.AlertWithStatus, error) {
 	if c == nil {
 		return nil, fmt.Errorf("condition cannot be nil")
 	}

x/alerts/keeper/keeper_test.go

@@ -7,7 +7,7 @@ import (
 	storetypes "cosmossdk.io/store/types"
 	cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
 	cmttime "github.com/cometbft/cometbft/types/time"
-	secp256k1 "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
+	"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
 	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
 	"github.com/cosmos/cosmos-sdk/runtime"
 	"github.com/cosmos/cosmos-sdk/testutil"

x/alerts/keeper/msg_server.go

@@ -88,7 +88,7 @@ func (m msgServer) Alert(goCtx context.Context, req *types.MsgAlert) (*types.Msg
 }
 
 // escrowBondAmount is a helper function that will escrow the bond amount at the module address.
-func (k Keeper) escrowBondAmount(ctx sdk.Context, signer string, bondAmount sdk.Coin) error {
+func (k *Keeper) escrowBondAmount(ctx sdk.Context, signer string, bondAmount sdk.Coin) error {
 	// get the sdk address for the signer
 	addr, err := sdk.AccAddressFromBech32(signer)
 	if err != nil {
@@ -157,9 +157,10 @@ func (m msgServer) Conclusion(goCtx context.Context, req *types.MsgConclusion) (
 
 		incentives := make([]incentivetypes.Incentive, 0)
 
-		// determine whether or not to issue an incentive to each validator who signed a vote in the Commit referenced
+		// determine whether to issue an incentive to each validator who signed a vote in the Commit referenced
 		for _, vote := range extCommit.Votes {
-			m.k.Logger(ctx).Info("issuing incentive to validator", "validator", sdk.ConsAddress(vote.Validator.Address).String(), "alert", fmt.Sprintf("%X", conclusion.GetAlert().UID()))
+			alert := conclusion.GetAlert()
+			m.k.Logger(ctx).Info("issuing incentive to validator", "validator", sdk.ConsAddress(vote.Validator.Address).String(), "alert", fmt.Sprintf("%X", alert.UID()))
 
 			// execute the ValidatorIncentiveHandler to determine if validator should be issued an incentive
 			incentive, err := m.k.validatorIncentiveHandler(vote, conclusion.GetPriceBound(), conclusion.GetAlert(), conclusion.GetCurrencyPairID())
@@ -169,7 +170,8 @@ func (m msgServer) Conclusion(goCtx context.Context, req *types.MsgConclusion) (
 
 			// if the incentive is non-nil, then add it to the list of incentives to issue
 			if incentive != nil {
-				m.k.Logger(ctx).Info("incentive issued to validator", "validator", vote.Validator.Address, "incentive", incentive.String(), "alert", fmt.Sprintf("%X", conclusion.GetAlert().UID()))
+				getAlert := conclusion.GetAlert()
+				m.k.Logger(ctx).Info("incentive issued to validator", "validator", vote.Validator.Address, "incentive", incentive.String(), "alert", fmt.Sprintf("%X", getAlert.UID()))
 				incentives = append(incentives, incentive)
 			}
 		}

x/alerts/keeper/msg_server_test.go

@@ -591,6 +591,10 @@ func (s *KeeperTestSuite) TestConclusion() {
 }
 
 func (s *KeeperTestSuite) TestUpdateParams() {
+	invalidParams := types.MsgUpdateParams{
+		Authority: "invalid",
+	}
+
 	cases := []struct {
 		name      string
 		msg       *types.MsgUpdateParams
@@ -606,9 +610,7 @@ func (s *KeeperTestSuite) TestUpdateParams() {
 			&types.MsgUpdateParams{
 				Authority: "invalid",
 			},
-			fmt.Errorf("message validation failed: %w", types.MsgUpdateParams{
-				Authority: "invalid",
-			}.ValidateBasic()),
+			fmt.Errorf("message validation failed: %w", invalidParams.ValidateBasic()),
 		},
 		{
 			"signer is not the authority - fail",

x/alerts/types/alert.go

@@ -43,8 +43,8 @@ func NewAlert(height uint64, signer sdk.AccAddress, cp slinkytypes.CurrencyPair)
 	}
 }
 
-// ValidateBasic performs stateless validation on the Claim, i.e checks that the signer and the currency-pair are valid.
-func (a Alert) ValidateBasic() error {
+// ValidateBasic performs stateless validation on the Claim, i.e. checks that the signer and the currency-pair are valid.
+func (a *Alert) ValidateBasic() error {
 	// validate the currency-pair
 	if err := a.CurrencyPair.ValidateBasic(); err != nil {
 		return fmt.Errorf("invalid alert: %w", err)
@@ -62,7 +62,7 @@ func (a Alert) ValidateBasic() error {
 // this will be used to derive the key under which this Claim will be stored. This method appends
 // the Height, signer, currency-pair strings into a byte-array, and returns the first 20-bytes of
 // the hash of that array.
-func (a Alert) UID() []byte {
+func (a *Alert) UID() []byte {
 	heightBz := []byte(fmt.Sprintf("%d", a.Height))
 	signerBz := []byte(a.Signer)
 	currencyPairBz := []byte(a.CurrencyPair.String())
@@ -79,9 +79,9 @@ func NewAlertStatus(submissionHeight uint64, purgeHeight uint64, blockTimestamp
 	}
 }
 
-// ValidateBasic performs a basic validation of the ConclusionStatus, i.e that the submissionHeight
+// ValidateBasic performs a basic validation of the ConclusionStatus, i.e. that the submissionHeight
 // is before the purgeHeight, and that the status is either Unconcluded or Concluded.
-func (a AlertStatus) ValidateBasic() error {
+func (a *AlertStatus) ValidateBasic() error {
 	if a.SubmissionHeight >= a.PurgeHeight {
 		return fmt.Errorf("invalid alert status: submission height must be before purge height")
 	}
@@ -102,7 +102,7 @@ func NewAlertWithStatus(alert Alert, status AlertStatus) AlertWithStatus {
 }
 
 // ValidateBasic validates that both the alert status and the alert are valid.
-func (aws AlertWithStatus) ValidateBasic() error {
+func (aws *AlertWithStatus) ValidateBasic() error {
 	if err := aws.Alert.ValidateBasic(); err != nil {
 		return err
 	}

x/alerts/types/conclusion.go

@@ -7,11 +7,11 @@ import (
 
 // Conclusion defines the basic meta-data necessary for the alerts module to resolve an alert. At a minimum, this
 // message must contain the OracleData referenced in the conclusion, the Alert that the Conclusion corresponds to,
-// metadata necessary for verification (i.e signatures), and the status of the conclusion itself.
+// metadata necessary for verification (i.e. signatures), and the status of the conclusion itself.
 type Conclusion interface {
 	proto.Message
 
-	// performs stateless validation on the Claim
+	// ValidateBasic performs stateless validation on the Claim
 	ValidateBasic() error
 
 	// Marshal the Claim to bytes (this is what will be stored in state)
@@ -26,13 +26,13 @@ type Conclusion interface {
 	// GetExtendedCommitInfo returns the ExtendedCommitInfo referenced in the conclusion.
 	GetExtendedCommitInfo() cmtabci.ExtendedCommitInfo
 
-	// Alert returns the Alert that the Conclusion corresponds to.
+	// GetAlert returns the Alert that the Conclusion corresponds to.
 	GetAlert() Alert
 
-	// Status returns the status of the conclusion.
+	// GetStatus returns the status of the conclusion.
 	GetStatus() bool
 
-	// PriceBounds returns the price-bounds of the conclusion.
+	// GetPriceBound returns the price-bounds of the conclusion.
 	GetPriceBound() PriceBound
 
 	// GetCurrencyPairID returns the ID of the CurrencyPair for which the alert is filed
@@ -43,7 +43,7 @@ type Conclusion interface {
 type ConclusionVerificationParams interface {
 	proto.Message
 
-	// performs stateless validation on the Claim
+	// ValidateBasic performs stateless validation on the Claim
 	ValidateBasic() error
 
 	// Marshal the Claim to bytes (this is what will be stored in state)

x/alerts/types/genesis.go

@@ -23,7 +23,7 @@ func NewGenesisState(params Params, alerts []AlertWithStatus) GenesisState {
 
 // ValidateBasic performs stateless validation on the GenesisState, specifically it
 // validates that the Params are valid, and that each of the alerts is also valid.
-func (gs GenesisState) ValidateBasic() error {
+func (gs *GenesisState) ValidateBasic() error {
 	// validate the genesis-state
 	if err := gs.Params.Validate(); err != nil {
 		return err

x/alerts/types/genesis_test.go

@@ -21,22 +21,21 @@ func TestGenesisValidation(t *testing.T) {
 
 	cases := []testCase{
 		{
-			"genesis with invalid params - fail",
-			types.GenesisState{
-				Params: types.NewParams(types.AlertParams{false, sdk.NewCoin("test", math.NewInt(1000000)), 0}, nil, types.PruningParams{}),
+			name: "genesis with invalid params - fail",
+			genesis: types.GenesisState{
+				Params: types.NewParams(types.AlertParams{BondAmount: sdk.NewCoin("test", math.NewInt(1000000))}, nil, types.PruningParams{}),
 			},
-			false,
 		},
 		{
-			"genesis with valid params - pass",
-			types.GenesisState{
-				Params: types.NewParams(types.AlertParams{false, sdk.NewCoin("test", math.NewInt(0)), 0}, nil, types.PruningParams{}),
+			name: "genesis with valid params - pass",
+			genesis: types.GenesisState{
+				Params: types.NewParams(types.AlertParams{BondAmount: sdk.NewCoin("test", math.NewInt(0))}, nil, types.PruningParams{}),
 			},
-			true,
+			valid: true,
 		},
 		{
-			"genesis with an invalid alert - fail",
-			types.NewGenesisState(types.NewParams(types.AlertParams{true, sdk.NewCoin("test", math.NewInt(1000000)), 1}, nil, types.PruningParams{}), []types.AlertWithStatus{
+			name: "genesis with an invalid alert - fail",
+			genesis: types.NewGenesisState(types.NewParams(types.AlertParams{Enabled: true, BondAmount: sdk.NewCoin("test", math.NewInt(1000000)), MaxBlockAge: 1}, nil, types.PruningParams{}), []types.AlertWithStatus{
 				types.NewAlertWithStatus(
 					types.Alert{
 						Height: 1,
@@ -49,11 +48,10 @@ func TestGenesisValidation(t *testing.T) {
 					types.NewAlertStatus(1, 2, time.Now(), 1),
 				),
 			}),
-			false,
 		},
 		{
-			"genesis with duplicate alerts - fail",
-			types.NewGenesisState(types.NewParams(types.AlertParams{true, sdk.NewCoin("test", math.NewInt(1000000)), 1}, nil, types.PruningParams{}), []types.AlertWithStatus{
+			name: "genesis with duplicate alerts - fail",
+			genesis: types.NewGenesisState(types.NewParams(types.AlertParams{Enabled: true, BondAmount: sdk.NewCoin("test", math.NewInt(1000000)), MaxBlockAge: 1}, nil, types.PruningParams{}), []types.AlertWithStatus{
 				types.NewAlertWithStatus(
 					types.NewAlert(1, sdk.AccAddress("test"), slinkytypes.NewCurrencyPair("BASE", "QUOTE")),
 					types.NewAlertStatus(1, 2, time.Now(), 1),
@@ -63,11 +61,10 @@ func TestGenesisValidation(t *testing.T) {
 					types.NewAlertStatus(1, 2, time.Now(), 0),
 				),
 			}),
-			false,
 		},
 		{
-			"genesis with valid non-duplicatge alerts - pass",
-			types.NewGenesisState(types.NewParams(types.AlertParams{true, sdk.NewCoin("test", math.NewInt(1000000)), 1}, nil, types.PruningParams{}), []types.AlertWithStatus{
+			name: "genesis with valid non-duplicate alerts - pass",
+			genesis: types.NewGenesisState(types.NewParams(types.AlertParams{Enabled: true, BondAmount: sdk.NewCoin("test", math.NewInt(1000000)), MaxBlockAge: 1}, nil, types.PruningParams{}), []types.AlertWithStatus{
 				types.NewAlertWithStatus(
 					types.NewAlert(1, sdk.AccAddress("test"), slinkytypes.NewCurrencyPair("BASE", "QUOTE")),
 					types.NewAlertStatus(1, 2, time.Now(), 1),
@@ -77,7 +74,7 @@ func TestGenesisValidation(t *testing.T) {
 					types.NewAlertStatus(1, 2, time.Now(), 0),
 				),
 			}),
-			true,
+			valid: true,
 		},
 	}
 

x/alerts/types/msgs.go

@@ -39,15 +39,15 @@ func NewMsgAlert(a Alert) *MsgAlert {
 	}
 }
 
-// GetSigners gets the expected signers for the MsgAlert, i.e the signer of the
+// GetSigners gets the expected signers for the MsgAlert, i.e. the signer of the
 // underlying alert.
-func (msg MsgAlert) GetSigners() []sdk.AccAddress {
+func (msg *MsgAlert) GetSigners() []sdk.AccAddress {
 	signer := sdk.MustAccAddressFromBech32(msg.Alert.Signer)
 	return []sdk.AccAddress{signer}
 }
 
-// ValidateBasic performs basic validation of the MsgAlert fields, i.e on the underlying Alert.
-func (msg MsgAlert) ValidateBasic() error {
+// ValidateBasic performs basic validation of the MsgAlert fields, i.e. on the underlying Alert.
+func (msg *MsgAlert) ValidateBasic() error {
 	return msg.Alert.ValidateBasic()
 }
 
@@ -71,14 +71,14 @@ func NewMsgConclusion(c Conclusion, signer sdk.AccAddress) *MsgConclusion {
 }
 
 // GetSigners gets the expected signers for the MsgConclusion.
-func (msg MsgConclusion) GetSigners() []sdk.AccAddress {
+func (msg *MsgConclusion) GetSigners() []sdk.AccAddress {
 	signer := sdk.MustAccAddressFromBech32(msg.Signer)
 	return []sdk.AccAddress{signer}
 }
 
-// ValidateBasic performs basic validation of the MsgConclusion fields, i.e that the signer address
+// ValidateBasic performs basic validation of the MsgConclusion fields, i.e. that the signer address
 // is valid, and the alert is non-nil.
-func (msg MsgConclusion) ValidateBasic() error {
+func (msg *MsgConclusion) ValidateBasic() error {
 	// check signer validity
 	if _, err := sdk.AccAddressFromBech32(msg.Signer); err != nil {
 		return err
@@ -109,7 +109,7 @@ func NewMsgUpdateParams(params Params, authority sdk.AccAddress) *MsgUpdateParam
 
 // ValidateBasic checks that the params in the msg are valid, and that the authority address is valid
 // if either check fails the method will error.
-func (m MsgUpdateParams) ValidateBasic() error {
+func (m *MsgUpdateParams) ValidateBasic() error {
 	// check authority address
 	if _, err := sdk.AccAddressFromBech32(m.Authority); err != nil {
 		return err