From f086a2b377bd345c0991820786d0f113ffa14679 Mon Sep 17 00:00:00 2001 From: Jacob Gadikian Date: Sat, 17 Feb 2024 04:57:08 +0530 Subject: [PATCH 01/18] Update .golangci.yml --- .golangci.yml | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 0baf9541a..7fa3ff336 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -18,6 +18,7 @@ linters: - gosimple - govet - ineffassign + - maintidx - misspell - nakedret - staticcheck @@ -62,10 +63,6 @@ linters-settings: # Do NOT whine about the following, full explanation found in: # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#description-of-available-rules rules: - - name: use-any - disabled: true - - name: if-return - disabled: true - name: max-public-structs disabled: true - name: unchecked-type-assertion @@ -102,12 +99,8 @@ linters-settings: disabled: true - name: confusing-results disabled: true - - name: unused-parameter - disabled: true - name: modifies-value-receiver disabled: true - - name: early-return - disabled: true - name: confusing-naming disabled: true - name: defer From 421d82d905e4987425708888705cd620348b5c22 Mon Sep 17 00:00:00 2001 From: Jacob Gadikian Date: Thu, 22 Feb 2024 16:45:35 +0200 Subject: [PATCH 02/18] Update .golangci.yml --- .golangci.yml | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 7fa3ff336..e542c5704 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -18,18 +18,19 @@ linters: - gosimple - govet - ineffassign - - maintidx - misspell - nakedret + - nestif - staticcheck - thelper - typecheck - stylecheck - revive + - testifylint - typecheck - tenv - unconvert - - unparam # Prefer unparam over revive's unused param. It is more thorough in its checking. + - unparam - unused - misspell @@ -91,8 +92,6 @@ linters-settings: disabled: true - name: empty-lines disabled: true - - name: unused-receiver # remove later - disabled: true - name: banned-characters disabled: true - name: deep-exit @@ -105,9 +104,7 @@ linters-settings: disabled: true - name: defer disabled: true - - name: unused-parameter # Disabled in favour of unparam. - disabled: true - - name: unhandled-error # enable later, currently covered by errcheck + - name: unhandled-error disabled: true arguments: - 'b.WriteString' From 807f47de620de1c73cb9683ae947d7efe0459408 Mon Sep 17 00:00:00 2001 From: Jacob Gadikian Date: Thu, 22 Feb 2024 17:49:24 +0200 Subject: [PATCH 03/18] enable nestif linter --- .golangci.yml | 4 +- app/test_helpers.go | 2 +- app/tps_counter_test.go | 2 +- cmd/quicksilverd/root.go | 2 +- icq-relayer/pkg/runner/run.go | 2 +- server/config/config.go | 2 +- test/simulation/simtypes/rand.go | 2 +- .../gamm/pool-models/balancer/pool_asset.go | 2 +- .../osmosis-types/osmomath/decimal.go | 2 +- .../osmosis-types/osmomath/int.go | 2 +- .../osmosis-types/osmoutils/cache_ctx.go | 2 +- .../osmosis-types/osmoutils/slice_helper.go | 2 +- .../umee-types/leverage/params.go | 10 +-- utils/sort.go | 4 +- wasmbinding/query_plugin_test.go | 4 +- wasmbinding/test/custom_query_test.go | 2 +- x/airdrop/keeper/claim_handler.go | 84 +++++++++---------- x/airdrop/keeper/zonedrop.go | 8 +- x/airdrop/module.go | 12 +-- x/airdrop/types/airdrop.go | 84 +++++++++---------- x/airdrop/types/msgs.go | 8 +- x/airdrop/types/params.go | 4 +- x/airdrop/types/proposals.go | 4 +- x/claimsmanager/keeper/abci.go | 2 +- x/claimsmanager/keeper/hooks.go | 2 +- x/claimsmanager/keeper/keeper.go | 2 +- x/claimsmanager/keeper/keeper_test.go | 2 +- x/claimsmanager/module.go | 10 +-- x/claimsmanager/types/params.go | 4 +- x/epochs/types/identifier.go | 2 +- x/interchainquery/genesis_test.go | 2 +- x/interchainquery/keeper/keeper.go | 2 +- x/interchainquery/keeper/keeper_test.go | 2 +- x/interchainquery/keeper/queries.go | 2 +- x/interchainquery/types/callbacks.go | 2 +- x/interchainstaking/ibc_module.go | 6 +- x/interchainstaking/keeper/callbacks.go | 2 +- .../keeper/ibc_packet_handlers.go | 54 ++++++------ x/interchainstaking/keeper/keeper_test.go | 2 +- x/interchainstaking/types/delegation.go | 16 +--- x/interchainstaking/types/params.go | 6 +- x/interchainstaking/types/proposals_test.go | 4 +- x/lsmtypes/msg.go | 36 ++++---- x/mint/types/params.go | 12 +-- x/participationrewards/keeper/callbacks.go | 2 +- x/participationrewards/keeper/distribution.go | 74 +++++++--------- x/participationrewards/types/params.go | 4 +- .../types/proposals_test.go | 4 +- x/supply/keeper/keeper.go | 2 +- x/tokenfactory/types/params.go | 2 +- 50 files changed, 235 insertions(+), 275 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index e542c5704..8adb4a000 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -26,7 +26,7 @@ linters: - typecheck - stylecheck - revive - - testifylint +# - testifylint - typecheck - tenv - unconvert @@ -104,6 +104,8 @@ linters-settings: disabled: true - name: defer disabled: true + - name: unused-parameter + disabled: true - name: unhandled-error disabled: true arguments: diff --git a/app/test_helpers.go b/app/test_helpers.go index 3c3edf6b0..4e69809a5 100644 --- a/app/test_helpers.go +++ b/app/test_helpers.go @@ -32,7 +32,7 @@ import ( type EmptyAppOptions struct{} // Get implements AppOptions. -func (EmptyAppOptions) Get(_ string) interface{} { +func (EmptyAppOptions) Get(_ string) any { return nil } diff --git a/app/tps_counter_test.go b/app/tps_counter_test.go index 1690c8101..adc752b13 100644 --- a/app/tps_counter_test.go +++ b/app/tps_counter_test.go @@ -69,7 +69,7 @@ type writerLogger struct { var _ log.Logger = (*writerLogger)(nil) -func (wl *writerLogger) Info(msg string, keyVals ...interface{}) { +func (wl *writerLogger) Info(msg string, keyVals ...any) { wl.mu.Lock() defer wl.mu.Unlock() diff --git a/cmd/quicksilverd/root.go b/cmd/quicksilverd/root.go index dc1067bb7..17112cd9c 100644 --- a/cmd/quicksilverd/root.go +++ b/cmd/quicksilverd/root.go @@ -201,7 +201,7 @@ func txCommand() *cobra.Command { // initAppConfig helps to override default appConfig template and configs. // return "", nil if no custom configuration is required for the application. -func initAppConfig() (string, interface{}) { +func initAppConfig() (string, any) { customAppTemplate, customAppConfig := servercfg.AppConfig(quicksilverconfig.BaseDenom) srvCfg, ok := customAppConfig.(servercfg.Config) diff --git a/icq-relayer/pkg/runner/run.go b/icq-relayer/pkg/runner/run.go index cac84fa7d..c7cde2861 100644 --- a/icq-relayer/pkg/runner/run.go +++ b/icq-relayer/pkg/runner/run.go @@ -545,7 +545,7 @@ func doRequest(query Query, logger log.Logger, metrics prommetrics.Metrics) { // does not contain the Tx object (that we don't use, because the TxProof already contains a byte representation of tx anyway!) // Note: this function is compatible with 0.34 and 0.37 representations of transactions. func Tx(client *lensclient.ChainClient, hash []byte) (tmtypes.TxProof, int64, error) { - params := map[string]interface{}{ + params := map[string]any{ "hash": hash, "prove": true, } diff --git a/server/config/config.go b/server/config/config.go index a0bd84d6f..21966425a 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -40,7 +40,7 @@ type SupplyConfig struct { // AppConfig helps to override default appConfig template and configs. // return "", nil if no custom configuration is required for the application. -func AppConfig(denom string) (customAppTemplate string, customAppConfig interface{}) { +func AppConfig(denom string) (customAppTemplate string, customAppConfig any) { // Optionally allow the chain developer to overwrite the SDK's default // server config. srvCfg := config.DefaultConfig() diff --git a/test/simulation/simtypes/rand.go b/test/simulation/simtypes/rand.go index 82dc01e87..b9fd21936 100644 --- a/test/simulation/simtypes/rand.go +++ b/test/simulation/simtypes/rand.go @@ -188,7 +188,7 @@ func RandLTEBound[T constraints.Integer](r *rand.Rand, upperbound T) T { return t } -func RandSelect[T interface{}](r *rand.Rand, args ...T) T { +func RandSelect[T any](r *rand.Rand, args ...T) T { choice := RandLTBound(r, len(args)) return args[choice] } diff --git a/third-party-chains/osmosis-types/gamm/pool-models/balancer/pool_asset.go b/third-party-chains/osmosis-types/gamm/pool-models/balancer/pool_asset.go index 91f64a9d0..080ad6274 100644 --- a/third-party-chains/osmosis-types/gamm/pool-models/balancer/pool_asset.go +++ b/third-party-chains/osmosis-types/gamm/pool-models/balancer/pool_asset.go @@ -41,7 +41,7 @@ func (pa PoolAsset) prettify() poolAssetPretty { // MarshalYAML returns the YAML representation of a PoolAsset. // This is assumed to not be called on a stand-alone instance, so it removes the first marshalled line. -func (pa PoolAsset) MarshalYAML() (interface{}, error) { +func (pa PoolAsset) MarshalYAML() (any, error) { bz, err := yaml.Marshal(pa.prettify()) if err != nil { return nil, err diff --git a/third-party-chains/osmosis-types/osmomath/decimal.go b/third-party-chains/osmosis-types/osmomath/decimal.go index b6aa442ad..6ac67a6fa 100644 --- a/third-party-chains/osmosis-types/osmomath/decimal.go +++ b/third-party-chains/osmosis-types/osmomath/decimal.go @@ -700,7 +700,7 @@ func (d *BigDec) UnmarshalJSON(bz []byte) error { } // MarshalYAML returns the YAML representation. -func (d BigDec) MarshalYAML() (interface{}, error) { +func (d BigDec) MarshalYAML() (any, error) { return d.String(), nil } diff --git a/third-party-chains/osmosis-types/osmomath/int.go b/third-party-chains/osmosis-types/osmomath/int.go index 1ad5d796a..e887999f3 100644 --- a/third-party-chains/osmosis-types/osmomath/int.go +++ b/third-party-chains/osmosis-types/osmomath/int.go @@ -371,7 +371,7 @@ func unmarshalJSON(i *big.Int, bz []byte) error { } // MarshalYAML returns the YAML representation. -func (i BigInt) MarshalYAML() (interface{}, error) { +func (i BigInt) MarshalYAML() (any, error) { return i.String(), nil } diff --git a/third-party-chains/osmosis-types/osmoutils/cache_ctx.go b/third-party-chains/osmosis-types/osmoutils/cache_ctx.go index 60b382236..ae765dee7 100644 --- a/third-party-chains/osmosis-types/osmoutils/cache_ctx.go +++ b/third-party-chains/osmosis-types/osmoutils/cache_ctx.go @@ -37,7 +37,7 @@ func ApplyFuncIfNoError(ctx sdk.Context, f func(ctx sdk.Context) error) (err err // PrintPanicRecoveryError error logs the recoveryError, along with the stacktrace, if it can be parsed. // If not emits them to stdout. -func PrintPanicRecoveryError(ctx sdk.Context, recoveryError interface{}) { +func PrintPanicRecoveryError(ctx sdk.Context, recoveryError any) { errStackTrace := string(debug.Stack()) switch e := recoveryError.(type) { case string: diff --git a/third-party-chains/osmosis-types/osmoutils/slice_helper.go b/third-party-chains/osmosis-types/osmoutils/slice_helper.go index f4c14a7b9..7883c7a57 100644 --- a/third-party-chains/osmosis-types/osmoutils/slice_helper.go +++ b/third-party-chains/osmosis-types/osmoutils/slice_helper.go @@ -14,7 +14,7 @@ func SortSlice[T constraints.Ordered](s []T) { }) } -func Filter[T interface{}](filter func(T) bool, s []T) []T { +func Filter[T any](filter func(T) bool, s []T) []T { filteredSlice := []T{} for _, s := range s { if filter(s) { diff --git a/third-party-chains/umee-types/leverage/params.go b/third-party-chains/umee-types/leverage/params.go index 8dd60fbd5..d91a5dff1 100644 --- a/third-party-chains/umee-types/leverage/params.go +++ b/third-party-chains/umee-types/leverage/params.go @@ -91,7 +91,7 @@ func (p Params) Validate() error { return validateDirectLiquidationFee(p.DirectLiquidationFee) } -func validateLiquidationThreshold(i interface{}) error { +func validateLiquidationThreshold(i any) error { v, ok := i.(sdk.Dec) if !ok { return fmt.Errorf("invalid parameter type: %T", i) @@ -108,7 +108,7 @@ func validateLiquidationThreshold(i interface{}) error { return nil } -func validateMinimumCloseFactor(i interface{}) error { +func validateMinimumCloseFactor(i any) error { v, ok := i.(sdk.Dec) if !ok { return fmt.Errorf("invalid parameter type: %T", i) @@ -124,7 +124,7 @@ func validateMinimumCloseFactor(i interface{}) error { return nil } -func validateOracleRewardFactor(i interface{}) error { +func validateOracleRewardFactor(i any) error { v, ok := i.(sdk.Dec) if !ok { return fmt.Errorf("invalid parameter type: %T", i) @@ -140,7 +140,7 @@ func validateOracleRewardFactor(i interface{}) error { return nil } -func validateSmallLiquidationSize(i interface{}) error { +func validateSmallLiquidationSize(i any) error { v, ok := i.(sdk.Dec) if !ok { return fmt.Errorf("invalid parameter type: %T", i) @@ -153,7 +153,7 @@ func validateSmallLiquidationSize(i interface{}) error { return nil } -func validateDirectLiquidationFee(i interface{}) error { +func validateDirectLiquidationFee(i any) error { v, ok := i.(sdk.Dec) if !ok { return fmt.Errorf("invalid parameter type: %T", i) diff --git a/utils/sort.go b/utils/sort.go index d5549ac4b..ab10acbda 100644 --- a/utils/sort.go +++ b/utils/sort.go @@ -7,7 +7,7 @@ import ( "golang.org/x/exp/constraints" ) -func Keys[V interface{}](in map[string]V) []string { +func Keys[V any](in map[string]V) []string { out := make([]string, 0) for k := range in { @@ -27,7 +27,7 @@ func SortSlice[T constraints.Ordered](s []T) { }) } -func Unique[V interface{}](in []V) []V { +func Unique[V any](in []V) []V { keys := make(map[string]struct{}, len(in)) list := []V{} for _, entry := range in { diff --git a/wasmbinding/query_plugin_test.go b/wasmbinding/query_plugin_test.go index a322564c7..f20b043c0 100644 --- a/wasmbinding/query_plugin_test.go +++ b/wasmbinding/query_plugin_test.go @@ -45,7 +45,7 @@ func (s *StargateTestSuite) TestStargateQuerier() { testSetup func() path string requestData func() []byte - responseProtoStruct interface{} + responseProtoStruct any expectedQuerierError bool expectedUnMarshalError bool resendRequest bool @@ -224,7 +224,7 @@ func (s *StargateTestSuite) TestDeterministicJsonMarshal() { originalResponsebz []byte updatedResponsebz []byte queryPath string - responseProtoStruct interface{} + responseProtoStruct any expectedProto func() proto.Message }{ /* diff --git a/wasmbinding/test/custom_query_test.go b/wasmbinding/test/custom_query_test.go index 5f2edeb12..3e1546a97 100644 --- a/wasmbinding/test/custom_query_test.go +++ b/wasmbinding/test/custom_query_test.go @@ -76,7 +76,7 @@ type ChainResponse struct { Data []byte `json:"data"` } -func queryCustom(t *testing.T, ctx sdk.Context, quicksilver *app.Quicksilver, contract sdk.AccAddress, request bindings.QuickSilverQuery, response interface{}) { +func queryCustom(t *testing.T, ctx sdk.Context, quicksilver *app.Quicksilver, contract sdk.AccAddress, request bindings.QuickSilverQuery, response any) { t.Helper() msgBz, err := json.Marshal(request) diff --git a/x/airdrop/keeper/claim_handler.go b/x/airdrop/keeper/claim_handler.go index 1170bceb0..2d1969fe4 100644 --- a/x/airdrop/keeper/claim_handler.go +++ b/x/airdrop/keeper/claim_handler.go @@ -337,71 +337,63 @@ func (k *Keeper) completeClaim(ctx sdk.Context, cr *types.ClaimRecord, action ty // after updating the relevant claim record. func (k *Keeper) getClaimAmountAndUpdateRecord(ctx sdk.Context, cr *types.ClaimRecord, action types.Action) (uint64, error) { var claimAmount uint64 + var err error // Declare err here - // check and initialize ActionsCompleted map if cr.ActionsCompleted == nil { cr.ActionsCompleted = make(map[int32]*types.CompletedAction) } - // The concept here is to intuitively claim all outstanding deposit tiers - // that are below the current deposit claim (improved user experience). - // - // ActionDepositT5: t5amount - // ActionDepositT4: t4amount - // ActionDepositT3: t3amount <-- eg. for T3 - // ActionDepositT2: t2amount <-- add to claimAmount if not CompletedAction - // ActionDepositT1: t1amount <-- add to claimAmount if not CompletedAction - // - // For any given deposit action above ActionDepositT1, sum the claimable - // amounts of non completed deposit actions and mark them as complete. - // Then, if no errors occurred, update the ClaimRecord state. - - // check for summable ActionDeposit (T2-T5, T1 has nothing below it to sum) if action > types.ActionDepositT1 && action <= types.ActionDepositT5 { - // check ActionDeposits from T1 to the target tier - // this also ensures that for any completed ActionDeposit tier, all - // tiers below are guaranteed to be completed as well. - for a := types.ActionDepositT1; a <= action; a++ { - if _, exists := cr.ActionsCompleted[int32(a)]; !exists { - // obtain claimable amount per deposit action - claimable, err := k.GetClaimableAmountForAction(ctx, cr.ChainId, cr.Address, a) - if err != nil { - return 0, err - } - - // update claim record (transient, not yet written to state) - cr.ActionsCompleted[int32(a)] = &types.CompletedAction{ - CompleteTime: ctx.BlockTime(), - ClaimAmount: claimable, - } - - // sum total claimable - claimAmount += claimable - } + claimAmount, err = k.processSummableDeposits(ctx, cr, action) // Use '=' instead of ':=' + if err != nil { + return 0, err } } else { - // obtain claimable amount - claimable, err := k.GetClaimableAmountForAction(ctx, cr.ChainId, cr.Address, action) + claimAmount, err = k.processSingleAction(ctx, cr, action) // Use '=' instead of ':=' if err != nil { return 0, err } + } + + if err := k.SetClaimRecord(ctx, *cr); err != nil { // This now checks the correct 'err' + return 0, err + } + + return claimAmount, nil +} - // set claim amount - claimAmount = claimable +func (k *Keeper) processSummableDeposits(ctx sdk.Context, cr *types.ClaimRecord, action types.Action) (uint64, error) { + var claimAmount uint64 + for a := types.ActionDepositT1; a <= action; a++ { + if _, exists := cr.ActionsCompleted[int32(a)]; !exists { + claimable, err := k.GetClaimableAmountForAction(ctx, cr.ChainId, cr.Address, a) + if err != nil { + return 0, err + } - // update claim record - cr.ActionsCompleted[int32(action)] = &types.CompletedAction{ - CompleteTime: ctx.BlockTime(), - ClaimAmount: claimAmount, + cr.ActionsCompleted[int32(a)] = &types.CompletedAction{ + CompleteTime: ctx.BlockTime(), + ClaimAmount: claimable, + } + + claimAmount += claimable } } + return claimAmount, nil +} - // set claim record (persistent) - if err := k.SetClaimRecord(ctx, *cr); err != nil { +func (k *Keeper) processSingleAction(ctx sdk.Context, cr *types.ClaimRecord, action types.Action) (uint64, error) { + claimable, err := k.GetClaimableAmountForAction(ctx, cr.ChainId, cr.Address, action) + if err != nil { return 0, err } - return claimAmount, nil + cr.ActionsCompleted[int32(action)] = &types.CompletedAction{ + CompleteTime: ctx.BlockTime(), + ClaimAmount: claimable, + } + + return claimable, nil } func (k *Keeper) sendCoins(ctx sdk.Context, cr types.ClaimRecord, amount uint64) (sdk.Coins, error) { diff --git a/x/airdrop/keeper/zonedrop.go b/x/airdrop/keeper/zonedrop.go index 40db951f7..3ff0518b5 100644 --- a/x/airdrop/keeper/zonedrop.go +++ b/x/airdrop/keeper/zonedrop.go @@ -8,7 +8,7 @@ import ( ) // GetZoneDropAccountAddress returns the zone airdrop account address. -func (k *Keeper) GetZoneDropAccountAddress(chainID string) sdk.AccAddress { +func (*Keeper) GetZoneDropAccountAddress(chainID string) sdk.AccAddress { name := types.ModuleName + "." + chainID return authtypes.NewModuleAddress(name) } @@ -99,7 +99,7 @@ func (k *Keeper) AllActiveZoneDrops(ctx sdk.Context) []types.ZoneDrop { // // isActive: s <= timenow <= s+D+d // notActive: timenow < s || timenow > s+D+d. -func (k *Keeper) IsActiveZoneDrop(ctx sdk.Context, zd types.ZoneDrop) bool { +func (*Keeper) IsActiveZoneDrop(ctx sdk.Context, zd types.ZoneDrop) bool { bt := ctx.BlockTime() // negated checks here ensure inclusive range @@ -129,7 +129,7 @@ func (k *Keeper) AllFutureZoneDrops(ctx sdk.Context) []types.ZoneDrop { // // isFuture: timenow < s // notFuture: s <= timenow. -func (k *Keeper) IsFutureZoneDrop(ctx sdk.Context, zd types.ZoneDrop) bool { +func (*Keeper) IsFutureZoneDrop(ctx sdk.Context, zd types.ZoneDrop) bool { bt := ctx.BlockTime() return bt.Before(zd.StartTime) @@ -158,7 +158,7 @@ func (k *Keeper) AllExpiredZoneDrops(ctx sdk.Context) []types.ZoneDrop { // // isExpired: timenow > s+D+d // notExpired: timenow < s+D+d. -func (k *Keeper) IsExpiredZoneDrop(ctx sdk.Context, zd types.ZoneDrop) bool { +func (*Keeper) IsExpiredZoneDrop(ctx sdk.Context, zd types.ZoneDrop) bool { bt := ctx.BlockTime() return bt.After(zd.StartTime.Add(zd.Duration).Add(zd.Decay)) diff --git a/x/airdrop/module.go b/x/airdrop/module.go index 7857ba90a..59ac8a458 100644 --- a/x/airdrop/module.go +++ b/x/airdrop/module.go @@ -49,7 +49,7 @@ func (AppModuleBasic) RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) { } // RegisterInterfaces registers the module's interface types. -func (a AppModuleBasic) RegisterInterfaces(reg cdctypes.InterfaceRegistry) { +func (AppModuleBasic) RegisterInterfaces(reg cdctypes.InterfaceRegistry) { // RegisterInterfaces registers interfaces and implementations of the bank module. types.RegisterInterfaces(reg) } @@ -82,7 +82,7 @@ func (AppModuleBasic) RegisterGRPCGatewayRoutes(clientCtx client.Context, m *run } // GetTxCmd returns the airdrop module's root tx command. -func (a AppModuleBasic) GetTxCmd() *cobra.Command { +func (AppModuleBasic) GetTxCmd() *cobra.Command { return cli.GetTxCmd() } @@ -117,7 +117,7 @@ func (am AppModule) RegisterInvariants(ir sdk.InvariantRegistry) { } // Route returns the message routing key for the airdrop module. -func (am AppModule) Route() sdk.Route { +func (AppModule) Route() sdk.Route { return sdk.Route{} } @@ -127,7 +127,7 @@ func (AppModule) QuerierRoute() string { } // LegacyQuerierHandler returns the x/airdrop module's sdk.Querier. -func (am AppModule) LegacyQuerierHandler(_ *codec.LegacyAmino) sdk.Querier { +func (AppModule) LegacyQuerierHandler(_ *codec.LegacyAmino) sdk.Querier { return func(sdk.Context, []string, abci.RequestQuery) ([]byte, error) { return nil, fmt.Errorf("legacy querier not supported for the x/%s module", types.ModuleName) } @@ -158,7 +158,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw } // BeginBlock executes all ABCI BeginBlock logic respective to the airdrop module. -func (am AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) { +func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) { } // EndBlock executes all ABCI EndBlock logic respective to the airdrop module. It @@ -190,7 +190,7 @@ func (AppModule) RandomizedParams(_ *rand.Rand) []simtypes.ParamChange { } // RegisterStoreDecoder registers a decoder for mint module's types. -func (am AppModule) RegisterStoreDecoder(_ sdk.StoreDecoderRegistry) { +func (AppModule) RegisterStoreDecoder(_ sdk.StoreDecoderRegistry) { } // WeightedOperations doesn't return any mint module operation. diff --git a/x/airdrop/types/airdrop.go b/x/airdrop/types/airdrop.go index f4b44b1f7..05f6d04fb 100644 --- a/x/airdrop/types/airdrop.go +++ b/x/airdrop/types/airdrop.go @@ -12,73 +12,71 @@ import ( func (zd *ZoneDrop) ValidateBasic() error { errs := make(map[string]error) - // must be defined + validateChainID(zd, errs) + validateDuration(zd, errs) + validateDecay(zd, errs) + validateSumDurationDecay(zd, errs) + validateAllocation(zd, errs) + validateActions(zd, errs) + + if len(errs) > 0 { + return multierror.New(errs) + } + + return nil +} + +func validateChainID(zd *ZoneDrop, errs map[string]error) { if zd.ChainId == "" { errs["ChainID"] = ErrUndefinedAttribute } +} - // must be greater or equal to 0 - // - // - equal will result in immediate decay; - // - greater specifies period of full claim; - // - // specific bounds can be applied via proposal process +func validateDuration(zd *ZoneDrop, errs map[string]error) { if zd.Duration.Microseconds() < 0 { errs["Duration"] = fmt.Errorf("%w, must not be negative", ErrInvalidDuration) } +} - // must be greater or equal to 0 - // - // - equal will result in a full airdrop reward with immediate cut off on - // expiry; - // - greater will result in a proportionally discounted airdrop reward over - // the duration of decay; - // - // specific bounds can be applied via proposal process +func validateDecay(zd *ZoneDrop, errs map[string]error) { if zd.Decay.Microseconds() < 0 { errs["Decay"] = fmt.Errorf("%w, must not be negative", ErrInvalidDuration) } +} - // sum of Duration and Decay may not be zero as this will result in - // immediate expiry of the zone airdrop +func validateSumDurationDecay(zd *ZoneDrop, errs map[string]error) { if zd.Duration.Microseconds()+zd.Decay.Microseconds() == 0 { - if _, exists := errs["Duration"]; !exists { - errs["Duration"] = fmt.Errorf("%w, sum of Duration and Decay must not be zero", ErrInvalidDuration) - } - if _, exists := errs["Decay"]; !exists { - errs["Decay"] = fmt.Errorf("%w, sum of Duration and Decay must not be zero", ErrInvalidDuration) - } + errs["Duration"] = fmt.Errorf("%w, sum of Duration and Decay must not be zero", ErrInvalidDuration) + errs["Decay"] = fmt.Errorf("%w, sum of Duration and Decay must not be zero", ErrInvalidDuration) } +} - // must be positive value +func validateAllocation(zd *ZoneDrop, errs map[string]error) { if zd.Allocation == 0 { errs["Allocation"] = ErrUndefinedAttribute } +} - // must have at least one defined +func validateActions(zd *ZoneDrop, errs map[string]error) { if len(zd.Actions) == 0 { errs["Actions"] = ErrUndefinedAttribute - } else { - // may not exceed defined types.Action bounds - // * (-1) to account for enum: 0 = undefined (protobuf3 spec) - if len(zd.Actions) > len(Action_name)-1 { - errs["Action"] = fmt.Errorf("exceeds number of defined actions (%d)", len(Action_name)-1) - } else { - weightSum := sdk.ZeroDec() - for _, aw := range zd.Actions { - weightSum = weightSum.Add(aw) - } - if !weightSum.Equal(sdk.OneDec()) { - errs["Actions"] = fmt.Errorf("%w, got %s", ErrActionWeights, weightSum) - } - } + return } - - if len(errs) > 0 { - return multierror.New(errs) + if len(zd.Actions) > len(Action_name)-1 { + errs["Action"] = fmt.Errorf("exceeds number of defined actions (%d)", len(Action_name)-1) + return } + validateActionWeights(zd, errs) +} - return nil +func validateActionWeights(zd *ZoneDrop, errs map[string]error) { + weightSum := sdk.ZeroDec() + for _, aw := range zd.Actions { + weightSum = weightSum.Add(aw) + } + if !weightSum.Equal(sdk.OneDec()) { + errs["Actions"] = fmt.Errorf("%w, got %s", ErrActionWeights, weightSum) + } } func (cr *ClaimRecord) ValidateBasic() error { diff --git a/x/airdrop/types/msgs.go b/x/airdrop/types/msgs.go index 795057229..e94911057 100644 --- a/x/airdrop/types/msgs.go +++ b/x/airdrop/types/msgs.go @@ -30,10 +30,10 @@ func NewMsgClaim(chainID string, action int64, fromAddress sdk.Address) *MsgClai } // Route implements Msg. -func (msg MsgClaim) Route() string { return RouterKey } +func (MsgClaim) Route() string { return RouterKey } // Type implements Msg. -func (msg MsgClaim) Type() string { return TypeMsgClaim } +func (MsgClaim) Type() string { return TypeMsgClaim } // ValidateBasic implements Msg. func (msg MsgClaim) ValidateBasic() error { @@ -99,10 +99,10 @@ func NewMsgIncentivePoolSpend(authority, toAddress sdk.Address, amt sdk.Coins) * } // Route implements Msg. -func (msg MsgIncentivePoolSpend) Route() string { return RouterKey } +func (MsgIncentivePoolSpend) Route() string { return RouterKey } // Type implements Msg. -func (msg MsgIncentivePoolSpend) Type() string { return TypeMsgClaim } +func (MsgIncentivePoolSpend) Type() string { return TypeMsgClaim } // ValidateBasic implements Msg. func (msg MsgIncentivePoolSpend) ValidateBasic() error { diff --git a/x/airdrop/types/params.go b/x/airdrop/types/params.go index e17076579..4e581cd78 100644 --- a/x/airdrop/types/params.go +++ b/x/airdrop/types/params.go @@ -22,14 +22,14 @@ func DefaultParams() Params { } // ParamSetPairs implements params.ParamSet. -func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs { +func (*Params) ParamSetPairs() paramtypes.ParamSetPairs { return paramtypes.ParamSetPairs{ // paramtypes.NewParamSetPair(Key, &p.Attribute, validateAttrib), } } // Validate validates params. -func (p *Params) Validate() error { +func (*Params) Validate() error { return nil } diff --git a/x/airdrop/types/proposals.go b/x/airdrop/types/proposals.go index cad1fb3fc..75d430190 100644 --- a/x/airdrop/types/proposals.go +++ b/x/airdrop/types/proposals.go @@ -16,8 +16,8 @@ var _ govv1beta1.Content = &RegisterZoneDropProposal{} func (m *RegisterZoneDropProposal) GetDescription() string { return m.Description } func (m *RegisterZoneDropProposal) GetTitle() string { return m.Title } -func (m *RegisterZoneDropProposal) ProposalRoute() string { return RouterKey } -func (m *RegisterZoneDropProposal) ProposalType() string { return ProposalTypeRegisterZoneDrop } +func (*RegisterZoneDropProposal) ProposalRoute() string { return RouterKey } +func (*RegisterZoneDropProposal) ProposalType() string { return ProposalTypeRegisterZoneDrop } // ValidateBasic runs basic stateless validity checks. // diff --git a/x/claimsmanager/keeper/abci.go b/x/claimsmanager/keeper/abci.go index 1854d7dbb..7d1951f0d 100644 --- a/x/claimsmanager/keeper/abci.go +++ b/x/claimsmanager/keeper/abci.go @@ -5,5 +5,5 @@ import ( ) // BeginBlocker of claimsmanager module. -func (k Keeper) BeginBlocker(_ sdk.Context) { +func (Keeper) BeginBlocker(_ sdk.Context) { } diff --git a/x/claimsmanager/keeper/hooks.go b/x/claimsmanager/keeper/hooks.go index 174b55fab..5ec8e8170 100644 --- a/x/claimsmanager/keeper/hooks.go +++ b/x/claimsmanager/keeper/hooks.go @@ -16,7 +16,7 @@ func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochN return nil } -func (k Keeper) AfterEpochEnd(_ sdk.Context, _ string, _ int64) error { +func (Keeper) AfterEpochEnd(_ sdk.Context, _ string, _ int64) error { return nil } diff --git a/x/claimsmanager/keeper/keeper.go b/x/claimsmanager/keeper/keeper.go index 763dedda7..3d9c347a9 100644 --- a/x/claimsmanager/keeper/keeper.go +++ b/x/claimsmanager/keeper/keeper.go @@ -43,7 +43,7 @@ func NewKeeper( } // Logger returns a module-specific logger. -func (k Keeper) Logger(ctx sdk.Context) log.Logger { +func (Keeper) Logger(ctx sdk.Context) log.Logger { return ctx.Logger().With("module", fmt.Sprintf("x/%s", types.ModuleName)) } diff --git a/x/claimsmanager/keeper/keeper_test.go b/x/claimsmanager/keeper/keeper_test.go index d9f11bbb6..39b47d8cd 100644 --- a/x/claimsmanager/keeper/keeper_test.go +++ b/x/claimsmanager/keeper/keeper_test.go @@ -44,7 +44,7 @@ type KeeperTestSuite struct { path *ibctesting.Path } -func (suite *KeeperTestSuite) GetQuicksilverApp(chain *ibctesting.TestChain) *app.Quicksilver { +func (*KeeperTestSuite) GetQuicksilverApp(chain *ibctesting.TestChain) *app.Quicksilver { quicksilver, ok := chain.App.(*app.Quicksilver) if !ok { panic("not quicksilver app") diff --git a/x/claimsmanager/module.go b/x/claimsmanager/module.go index a0e1d1854..fc0fb36f5 100644 --- a/x/claimsmanager/module.go +++ b/x/claimsmanager/module.go @@ -48,7 +48,7 @@ func (AppModuleBasic) RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) { } // RegisterInterfaces registers the module's interface types. -func (a AppModuleBasic) RegisterInterfaces(reg cdctypes.InterfaceRegistry) { +func (AppModuleBasic) RegisterInterfaces(reg cdctypes.InterfaceRegistry) { // RegisterInterfaces registers interfaces and implementations of the interchainstaking module. // (see types/codec.go) types.RegisterInterfaces(reg) @@ -78,7 +78,7 @@ func (AppModuleBasic) RegisterGRPCGatewayRoutes(clientCtx client.Context, mux *r } // GetTxCmd returns the claimsmanager module's root tx command. -func (a AppModuleBasic) GetTxCmd() *cobra.Command { +func (AppModuleBasic) GetTxCmd() *cobra.Command { return cli.GetTxCmd() } @@ -108,10 +108,10 @@ func NewAppModule(cdc codec.Codec, k keeper.Keeper) AppModule { } // RegisterInvariants registers the claimsmanager module invariants. -func (am AppModule) RegisterInvariants(_ sdk.InvariantRegistry) {} +func (AppModule) RegisterInvariants(_ sdk.InvariantRegistry) {} // Route returns the message routing key for the claimsmanager module. -func (am AppModule) Route() sdk.Route { +func (AppModule) Route() sdk.Route { return sdk.Route{} } @@ -121,7 +121,7 @@ func (AppModule) QuerierRoute() string { } // LegacyQuerierHandler returns the x/claimsmanager module's sdk.Querier. -func (am AppModule) LegacyQuerierHandler(_ *codec.LegacyAmino) sdk.Querier { +func (AppModule) LegacyQuerierHandler(_ *codec.LegacyAmino) sdk.Querier { return func(sdk.Context, []string, abci.RequestQuery) ([]byte, error) { return nil, fmt.Errorf("legacy querier not supported for the x/%s module", types.ModuleName) } diff --git a/x/claimsmanager/types/params.go b/x/claimsmanager/types/params.go index 6ec90222a..a465f8522 100644 --- a/x/claimsmanager/types/params.go +++ b/x/claimsmanager/types/params.go @@ -22,14 +22,14 @@ func DefaultParams() Params { } // ParamSetPairs implements params.ParamSet. -func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs { +func (*Params) ParamSetPairs() paramtypes.ParamSetPairs { return paramtypes.ParamSetPairs{ // paramtypes.NewParamSetPair(Key, &p.Attribute, validateAttrib), } } // Validate validates params. -func (p *Params) Validate() error { +func (*Params) Validate() error { return nil } diff --git a/x/epochs/types/identifier.go b/x/epochs/types/identifier.go index cea719b4a..f2b82209c 100644 --- a/x/epochs/types/identifier.go +++ b/x/epochs/types/identifier.go @@ -5,7 +5,7 @@ import ( "strings" ) -func ValidateEpochIdentifierInterface(i interface{}) error { +func ValidateEpochIdentifierInterface(i any) error { v, ok := i.(string) if !ok { return fmt.Errorf("invalid parameter type: %T", i) diff --git a/x/interchainquery/genesis_test.go b/x/interchainquery/genesis_test.go index 6819b1f82..8bd60e9f3 100644 --- a/x/interchainquery/genesis_test.go +++ b/x/interchainquery/genesis_test.go @@ -34,7 +34,7 @@ type InterChainQueryTestSuite struct { path *ibctesting.Path } -func (s *InterChainQueryTestSuite) GetSimApp(chain *ibctesting.TestChain) *app.Quicksilver { +func (*InterChainQueryTestSuite) GetSimApp(chain *ibctesting.TestChain) *app.Quicksilver { quicksilver, ok := chain.App.(*app.Quicksilver) if !ok { panic("not quicksilver app") diff --git a/x/interchainquery/keeper/keeper.go b/x/interchainquery/keeper/keeper.go index 60ec541d9..33e00c3cb 100644 --- a/x/interchainquery/keeper/keeper.go +++ b/x/interchainquery/keeper/keeper.go @@ -50,7 +50,7 @@ func (k *Keeper) SetCallbackHandler(module string, handler types.QueryCallbacks) } // Logger returns a module-specific logger. -func (k Keeper) Logger(ctx sdk.Context) log.Logger { +func (Keeper) Logger(ctx sdk.Context) log.Logger { return ctx.Logger().With("module", fmt.Sprintf("x/%s", types.ModuleName)) } diff --git a/x/interchainquery/keeper/keeper_test.go b/x/interchainquery/keeper/keeper_test.go index 19d215bca..0593ba290 100644 --- a/x/interchainquery/keeper/keeper_test.go +++ b/x/interchainquery/keeper/keeper_test.go @@ -36,7 +36,7 @@ type KeeperTestSuite struct { path *ibctesting.Path } -func (suite *KeeperTestSuite) GetSimApp(chain *ibctesting.TestChain) *app.Quicksilver { +func (*KeeperTestSuite) GetSimApp(chain *ibctesting.TestChain) *app.Quicksilver { quicksilver, ok := chain.App.(*app.Quicksilver) if !ok { panic("not quicksilver app") diff --git a/x/interchainquery/keeper/queries.go b/x/interchainquery/keeper/queries.go index 61c645d2e..af6460994 100644 --- a/x/interchainquery/keeper/queries.go +++ b/x/interchainquery/keeper/queries.go @@ -19,7 +19,7 @@ func GenerateQueryHash(connectionID, chainID, queryType string, request []byte, // ---------------------------------------------------------------- -func (k Keeper) NewQuery( +func (Keeper) NewQuery( module, connectionID, chainID, diff --git a/x/interchainquery/types/callbacks.go b/x/interchainquery/types/callbacks.go index 799d1981b..14929369f 100644 --- a/x/interchainquery/types/callbacks.go +++ b/x/interchainquery/types/callbacks.go @@ -5,7 +5,7 @@ import ( ) type QueryCallbacks interface { - AddCallback(id string, fn interface{}) QueryCallbacks + AddCallback(id string, fn any) QueryCallbacks RegisterCallbacks() QueryCallbacks Call(ctx sdk.Context, id string, args []byte, query Query) error Has(id string) bool diff --git a/x/interchainstaking/ibc_module.go b/x/interchainstaking/ibc_module.go index 2ba20bef2..05af3051e 100644 --- a/x/interchainstaking/ibc_module.go +++ b/x/interchainstaking/ibc_module.go @@ -76,7 +76,7 @@ func (im IBCModule) OnChanOpenAck( } // OnChanOpenConfirm implements the IBCModule interface. -func (im IBCModule) OnChanOpenConfirm( +func (IBCModule) OnChanOpenConfirm( _ sdk.Context, _, _ string, @@ -85,7 +85,7 @@ func (im IBCModule) OnChanOpenConfirm( } // OnChanCloseInit implements the IBCModule interface. -func (im IBCModule) OnChanCloseInit( +func (IBCModule) OnChanCloseInit( _ sdk.Context, _, _ string, @@ -105,7 +105,7 @@ func (IBCModule) OnChanCloseConfirm( // OnRecvPacket implements the IBCModule interface. A successful acknowledgement // is returned if the packet data is successfully decoded and the receive application // logic returns without error. -func (im IBCModule) OnRecvPacket( +func (IBCModule) OnRecvPacket( _ sdk.Context, _ channeltypes.Packet, _ sdk.AccAddress, diff --git a/x/interchainstaking/keeper/callbacks.go b/x/interchainstaking/keeper/callbacks.go index 9518cca66..9586e47ce 100644 --- a/x/interchainstaking/keeper/callbacks.go +++ b/x/interchainstaking/keeper/callbacks.go @@ -62,7 +62,7 @@ func (c Callbacks) Has(id string) bool { return found } -func (c Callbacks) AddCallback(id string, fn interface{}) icqtypes.QueryCallbacks { +func (c Callbacks) AddCallback(id string, fn any) icqtypes.QueryCallbacks { c.callbacks[id], _ = fn.(Callback) return c } diff --git a/x/interchainstaking/keeper/ibc_packet_handlers.go b/x/interchainstaking/keeper/ibc_packet_handlers.go index 58cb7decd..963165dbd 100644 --- a/x/interchainstaking/keeper/ibc_packet_handlers.go +++ b/x/interchainstaking/keeper/ibc_packet_handlers.go @@ -560,7 +560,7 @@ func (k *Keeper) HandleBeginRedelegate(ctx sdk.Context, msg sdk.Msg, completion } k.Logger(ctx).Info("Received MsgBeginRedelegate acknowledgement") - // first, type assertion. we should have stakingtypes.MsgBeginRedelegate + redelegateMsg, ok := msg.(*stakingtypes.MsgBeginRedelegate) if !ok { return errors.New("unable to unmarshal MsgBeginRedelegate") @@ -572,36 +572,33 @@ func (k *Keeper) HandleBeginRedelegate(ctx sdk.Context, msg sdk.Msg, completion } if completion.IsZero() { - // a zero completion time can only happen when the validator is unbonded; this means the redelegation has _already_ completed and can be removed. k.DeleteRedelegationRecord(ctx, zone.ChainId, redelegateMsg.ValidatorSrcAddress, redelegateMsg.ValidatorDstAddress, epochNumber) - } else { - record, found := k.GetRedelegationRecord(ctx, zone.ChainId, redelegateMsg.ValidatorSrcAddress, redelegateMsg.ValidatorDstAddress, epochNumber) - if !found { - // it is possible that the record was cleaned up if there was a long delay in processing acknowledgements. - // just create a new one - record = types.RedelegationRecord{ - ChainId: zone.ChainId, - EpochNumber: epochNumber, - Source: redelegateMsg.ValidatorSrcAddress, - Destination: redelegateMsg.ValidatorDstAddress, - Amount: redelegateMsg.Amount.Amount.Int64(), - CompletionTime: completion, - } - } + return nil + } - k.Logger(ctx).Info("updating redelegation record with completion time", "completion", completion) - record.CompletionTime = completion - k.SetRedelegationRecord(ctx, record) + record, found := k.GetRedelegationRecord(ctx, zone.ChainId, redelegateMsg.ValidatorSrcAddress, redelegateMsg.ValidatorDstAddress, epochNumber) + if !found { + record = types.RedelegationRecord{ + ChainId: zone.ChainId, + EpochNumber: epochNumber, + Source: redelegateMsg.ValidatorSrcAddress, + Destination: redelegateMsg.ValidatorDstAddress, + Amount: redelegateMsg.Amount.Amount.Int64(), + CompletionTime: completion, + } } + k.Logger(ctx).Info("updating redelegation record with completion time", "completion", completion) + record.CompletionTime = completion + k.SetRedelegationRecord(ctx, record) + tgtDelegation, found := k.GetDelegation(ctx, zone.ChainId, redelegateMsg.DelegatorAddress, redelegateMsg.ValidatorDstAddress) if !found { tgtDelegation = types.NewDelegation(redelegateMsg.DelegatorAddress, redelegateMsg.ValidatorDstAddress, redelegateMsg.Amount) } else { tgtDelegation.Amount = tgtDelegation.Amount.Add(redelegateMsg.Amount) } - // RedelegationEnd is used to determine whether the delegation is 'locked' for transient redelegations. - tgtDelegation.RedelegationEnd = completion.Unix() // this field should be a timestamp, but let's avoid unnecessary state changes. + tgtDelegation.RedelegationEnd = completion.Unix() k.SetDelegation(ctx, zone.ChainId, tgtDelegation) delAddr, err := addressutils.AccAddressFromBech32(redelegateMsg.DelegatorAddress, zone.AccountPrefix) @@ -614,7 +611,6 @@ func (k *Keeper) HandleBeginRedelegate(ctx sdk.Context, msg sdk.Msg, completion } data := stakingtypes.GetDelegationKey(delAddr, valAddr) - // send request to update delegation record for target del/val tuple. k.ICQKeeper.MakeRequest( ctx, zone.ConnectionId, @@ -632,26 +628,23 @@ func (k *Keeper) HandleBeginRedelegate(ctx sdk.Context, msg sdk.Msg, completion k.Logger(ctx).Error("unable to find delegation record", "chain", zone.ChainId, "source", redelegateMsg.ValidatorSrcAddress, "dst", redelegateMsg.ValidatorDstAddress, "epoch_number", epochNumber) return fmt.Errorf("unable to find delegation record for chain %s, src: %s, dst: %s, at epoch %d", zone.ChainId, redelegateMsg.ValidatorSrcAddress, redelegateMsg.ValidatorDstAddress, epochNumber) } + srcDelegation.Amount, err = srcDelegation.Amount.SafeSub(redelegateMsg.Amount) if err != nil { - if strings.Contains(err.Error(), "negative coin amount") { - // we received a negative srcDelegation. Obviously this cannot happen, but we can get a crossed re/un/delegation, all which fetch absolute values. - k.Logger(ctx).Error("possible race condition; unable to sub redelegation amount. requerying delegation anyway") - } else { - // we got some other, unrecoverable err + if !strings.Contains(err.Error(), "negative coin amount") { return err } - } else { - k.SetDelegation(ctx, zone.ChainId, srcDelegation) + k.Logger(ctx).Error("possible race condition; unable to sub redelegation amount. requerying delegation anyway") } + k.SetDelegation(ctx, zone.ChainId, srcDelegation) + valAddr, err = addressutils.ValAddressFromBech32(redelegateMsg.ValidatorDstAddress, zone.AccountPrefix+"valoper") if err != nil { return err } data = stakingtypes.GetDelegationKey(delAddr, valAddr) - // send request to update delegation record for src del/val tuple. k.ICQKeeper.MakeRequest( ctx, zone.ConnectionId, @@ -663,6 +656,7 @@ func (k *Keeper) HandleBeginRedelegate(ctx sdk.Context, msg sdk.Msg, completion "delegation", 0, ) + return nil } diff --git a/x/interchainstaking/keeper/keeper_test.go b/x/interchainstaking/keeper/keeper_test.go index a56647f5d..d81566a5e 100644 --- a/x/interchainstaking/keeper/keeper_test.go +++ b/x/interchainstaking/keeper/keeper_test.go @@ -59,7 +59,7 @@ type KeeperTestSuite struct { path *ibctesting.Path } -func (suite *KeeperTestSuite) GetQuicksilverApp(chain *ibctesting.TestChain) *app.Quicksilver { +func (*KeeperTestSuite) GetQuicksilverApp(chain *ibctesting.TestChain) *app.Quicksilver { quicksilver, ok := chain.App.(*app.Quicksilver) if !ok { panic("not quicksilver app") diff --git a/x/interchainstaking/types/delegation.go b/x/interchainstaking/types/delegation.go index b0fcb6cf4..57e9103d8 100644 --- a/x/interchainstaking/types/delegation.go +++ b/x/interchainstaking/types/delegation.go @@ -96,22 +96,14 @@ func (vi ValidatorIntents) GetForValoper(valoper string) (*ValidatorIntent, bool } func (vi ValidatorIntents) SetForValoper(valoper string, intent *ValidatorIntent) ValidatorIntents { - idx := -1 // the index of the valoper if found for i, v := range vi { - // Search for the valoper. if v.ValoperAddress == valoper { - idx = i - break + vi[i] = intent + return vi } } - - if idx >= 0 { // We found the valoper so just replace it - vi[idx] = intent - } else { - vi = append(vi, intent) - return vi.Sort() - } - return vi + vi = append(vi, intent) + return vi.Sort() } func (vi ValidatorIntents) MustGetForValoper(valoper string) *ValidatorIntent { diff --git a/x/interchainstaking/types/params.go b/x/interchainstaking/types/params.go index 2d391fa29..c49bfd37d 100644 --- a/x/interchainstaking/types/params.go +++ b/x/interchainstaking/types/params.go @@ -125,7 +125,7 @@ func (p ParamsV1) String() string { return string(out) } -func validateBoolean(i interface{}) error { +func validateBoolean(i any) error { _, ok := i.(bool) if !ok { return fmt.Errorf("invalid parameter type: %T", i) @@ -134,7 +134,7 @@ func validateBoolean(i interface{}) error { return nil } -func validatePositiveInt(i interface{}) error { +func validatePositiveInt(i any) error { intval, ok := i.(uint64) if !ok { return fmt.Errorf("invalid parameter type: %T", i) @@ -146,7 +146,7 @@ func validatePositiveInt(i interface{}) error { return nil } -func validateNonNegativeDec(i interface{}) error { +func validateNonNegativeDec(i any) error { dec, ok := i.(sdk.Dec) if !ok { return fmt.Errorf("invalid parameter type: %T", i) diff --git a/x/interchainstaking/types/proposals_test.go b/x/interchainstaking/types/proposals_test.go index 766ce96eb..d660bf783 100644 --- a/x/interchainstaking/types/proposals_test.go +++ b/x/interchainstaking/types/proposals_test.go @@ -231,7 +231,7 @@ func TestRegisterZoneProposal_ValidateBasic(t *testing.T) { } } -var sink interface{} +var sink any func BenchmarkUpdateZoneProposalString(b *testing.B) { uzp := &types.UpdateZoneProposal{ @@ -256,5 +256,5 @@ func BenchmarkUpdateZoneProposalString(b *testing.B) { if sink == nil { b.Fatal("Benchmark did not run") } - sink = interface{}(nil) + sink = any(nil) } diff --git a/x/lsmtypes/msg.go b/x/lsmtypes/msg.go index 9a48a78c3..979d08837 100644 --- a/x/lsmtypes/msg.go +++ b/x/lsmtypes/msg.go @@ -49,10 +49,10 @@ func NewMsgUnbondValidator(valAddr sdk.ValAddress) *MsgUnbondValidator { } // Route implements the sdk.Msg interface. -func (msg MsgUnbondValidator) Route() string { return RouterKey } +func (MsgUnbondValidator) Route() string { return RouterKey } // Type implements the sdk.Msg interface. -func (msg MsgUnbondValidator) Type() string { return TypeMsgUnbondValidator } +func (MsgUnbondValidator) Type() string { return TypeMsgUnbondValidator } // GetSigners implements the sdk.Msg interface. func (msg MsgUnbondValidator) GetSigners() []sdk.AccAddress { @@ -91,10 +91,10 @@ func NewMsgTokenizeShares(delAddr sdk.AccAddress, valAddr sdk.ValAddress, amount } // Route implements the sdk.Msg interface. -func (msg MsgTokenizeShares) Route() string { return RouterKey } +func (MsgTokenizeShares) Route() string { return RouterKey } // Type implements the sdk.Msg interface. -func (msg MsgTokenizeShares) Type() string { return TypeMsgTokenizeShares } +func (MsgTokenizeShares) Type() string { return TypeMsgTokenizeShares } // GetSigners implements the sdk.Msg interface. func (msg MsgTokenizeShares) GetSigners() []sdk.AccAddress { @@ -144,10 +144,10 @@ func NewMsgRedeemTokensForShares(delAddr sdk.AccAddress, amount sdk.Coin) *MsgRe } // Route implements the sdk.Msg interface. -func (msg MsgRedeemTokensForShares) Route() string { return RouterKey } +func (MsgRedeemTokensForShares) Route() string { return RouterKey } // Type implements the sdk.Msg interface. -func (msg MsgRedeemTokensForShares) Type() string { return TypeMsgRedeemTokensForShares } +func (MsgRedeemTokensForShares) Type() string { return TypeMsgRedeemTokensForShares } // GetSigners implements the sdk.Msg interface. func (msg MsgRedeemTokensForShares) GetSigners() []sdk.AccAddress { @@ -192,10 +192,10 @@ func NewMsgTransferTokenizeShareRecord(recordID uint64, sender, newOwner sdk.Acc } // Route implements the sdk.Msg interface. -func (msg MsgTransferTokenizeShareRecord) Route() string { return RouterKey } +func (MsgTransferTokenizeShareRecord) Route() string { return RouterKey } // Type implements the sdk.Msg interface. -func (msg MsgTransferTokenizeShareRecord) Type() string { return TypeMsgTransferTokenizeShareRecord } +func (MsgTransferTokenizeShareRecord) Type() string { return TypeMsgTransferTokenizeShareRecord } // GetSigners implements the sdk.Msg interface. func (msg MsgTransferTokenizeShareRecord) GetSigners() []sdk.AccAddress { @@ -234,10 +234,10 @@ func NewMsgDisableTokenizeShares(delAddr sdk.AccAddress) *MsgDisableTokenizeShar } // Route implements the sdk.Msg interface. -func (msg MsgDisableTokenizeShares) Route() string { return RouterKey } +func (MsgDisableTokenizeShares) Route() string { return RouterKey } // Type implements the sdk.Msg interface. -func (msg MsgDisableTokenizeShares) Type() string { return TypeMsgDisableTokenizeShares } +func (MsgDisableTokenizeShares) Type() string { return TypeMsgDisableTokenizeShares } // GetSigners implements the sdk.Msg interface. func (msg MsgDisableTokenizeShares) GetSigners() []sdk.AccAddress { @@ -273,10 +273,10 @@ func NewMsgEnableTokenizeShares(delAddr sdk.AccAddress) *MsgEnableTokenizeShares } // Route implements the sdk.Msg interface. -func (msg MsgEnableTokenizeShares) Route() string { return RouterKey } +func (MsgEnableTokenizeShares) Route() string { return RouterKey } // Type implements the sdk.Msg interface. -func (msg MsgEnableTokenizeShares) Type() string { return TypeMsgEnableTokenizeShares } +func (MsgEnableTokenizeShares) Type() string { return TypeMsgEnableTokenizeShares } // GetSigners implements the sdk.Msg interface. func (msg MsgEnableTokenizeShares) GetSigners() []sdk.AccAddress { @@ -313,10 +313,10 @@ func NewMsgValidatorBond(delAddr sdk.AccAddress, valAddr sdk.ValAddress) *MsgVal } // Route implements the sdk.Msg interface. -func (msg MsgValidatorBond) Route() string { return RouterKey } +func (MsgValidatorBond) Route() string { return RouterKey } // Type implements the sdk.Msg interface. -func (msg MsgValidatorBond) Type() string { return TypeMsgValidatorBond } +func (MsgValidatorBond) Type() string { return TypeMsgValidatorBond } // GetSigners implements the sdk.Msg interface. func (msg MsgValidatorBond) GetSigners() []sdk.AccAddress { @@ -352,8 +352,8 @@ func NewMsgWithdrawTokenizeShareRecordReward(ownerAddr sdk.AccAddress, recordID } } -func (msg MsgWithdrawTokenizeShareRecordReward) Route() string { return ModuleName } -func (msg MsgWithdrawTokenizeShareRecordReward) Type() string { +func (MsgWithdrawTokenizeShareRecordReward) Route() string { return ModuleName } +func (MsgWithdrawTokenizeShareRecordReward) Type() string { return TypeMsgWithdrawTokenizeShareRecordReward } @@ -386,9 +386,9 @@ func NewMsgWithdrawAllTokenizeShareRecordReward(ownerAddr sdk.AccAddress) *MsgWi } } -func (msg MsgWithdrawAllTokenizeShareRecordReward) Route() string { return ModuleName } +func (MsgWithdrawAllTokenizeShareRecordReward) Route() string { return ModuleName } -func (msg MsgWithdrawAllTokenizeShareRecordReward) Type() string { +func (MsgWithdrawAllTokenizeShareRecordReward) Type() string { return TypeMsgWithdrawAllTokenizeShareRecordReward } diff --git a/x/mint/types/params.go b/x/mint/types/params.go index a9e96249b..2e0ca6c0a 100644 --- a/x/mint/types/params.go +++ b/x/mint/types/params.go @@ -105,7 +105,7 @@ func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs { } } -func validateMintDenom(i interface{}) error { +func validateMintDenom(i any) error { v, ok := i.(string) if !ok { return fmt.Errorf("invalid parameter type: %T", i) @@ -117,7 +117,7 @@ func validateMintDenom(i interface{}) error { return sdk.ValidateDenom(v) } -func validateGenesisEpochProvisions(i interface{}) error { +func validateGenesisEpochProvisions(i any) error { v, ok := i.(sdk.Dec) if !ok { return fmt.Errorf("invalid parameter type: %T", i) @@ -130,7 +130,7 @@ func validateGenesisEpochProvisions(i interface{}) error { return nil } -func validateReductionPeriodInEpochs(i interface{}) error { +func validateReductionPeriodInEpochs(i any) error { v, ok := i.(int64) if !ok { return fmt.Errorf("invalid parameter type: %T", i) @@ -143,7 +143,7 @@ func validateReductionPeriodInEpochs(i interface{}) error { return nil } -func validateReductionFactor(i interface{}) error { +func validateReductionFactor(i any) error { v, ok := i.(sdk.Dec) if !ok { return fmt.Errorf("invalid parameter type: %T", i) @@ -160,7 +160,7 @@ func validateReductionFactor(i interface{}) error { return nil } -func validateDistributionProportions(i interface{}) error { +func validateDistributionProportions(i any) error { v, ok := i.(DistributionProportions) if !ok { return fmt.Errorf("invalid parameter type: %T", i) @@ -190,7 +190,7 @@ func validateDistributionProportions(i interface{}) error { return nil } -func validateMintingRewardsDistributionStartEpoch(i interface{}) error { +func validateMintingRewardsDistributionStartEpoch(i any) error { v, ok := i.(int64) if !ok { return fmt.Errorf("invalid parameter type: %T", i) diff --git a/x/participationrewards/keeper/callbacks.go b/x/participationrewards/keeper/callbacks.go index bfb3d842c..f3506c3c5 100644 --- a/x/participationrewards/keeper/callbacks.go +++ b/x/participationrewards/keeper/callbacks.go @@ -58,7 +58,7 @@ func (c Callbacks) Has(id string) bool { return found } -func (c Callbacks) AddCallback(id string, fn interface{}) icqtypes.QueryCallbacks { +func (c Callbacks) AddCallback(id string, fn any) icqtypes.QueryCallbacks { c.callbacks[id], _ = fn.(Callback) return c } diff --git a/x/participationrewards/keeper/distribution.go b/x/participationrewards/keeper/distribution.go index 6424ad8b0..a3572801f 100644 --- a/x/participationrewards/keeper/distribution.go +++ b/x/participationrewards/keeper/distribution.go @@ -21,22 +21,19 @@ func (k *Keeper) CalcTokenValues(ctx sdk.Context) (TokenValues, error) { data, found := k.GetProtocolData(ctx, types.ProtocolDataTypeOsmosisParams, "osmosisparams") if !found { - return TokenValues{}, errors.New("could not find osmosisparams protocol data") + return nil, errors.New("could not find osmosisparams protocol data") } osmoParams, err := types.UnmarshalProtocolData(types.ProtocolDataTypeOsmosisParams, data.Data) if err != nil { - return TokenValues{}, err + return nil, err } baseDenom := osmoParams.(*types.OsmosisParamsProtocolData).BaseDenom baseChain := osmoParams.(*types.OsmosisParamsProtocolData).BaseChain - tvs := make(map[string]sdk.Dec) - - // add base value + tvs := make(TokenValues) tvs[baseDenom] = sdk.OneDec() - // capture errors from iteratora errs := make(map[string]error) k.IteratePrefixedProtocolDatas(ctx, types.GetPrefixProtocolDataKey(types.ProtocolDataTypeOsmosisPool), func(idx int64, _ []byte, data types.ProtocolData) bool { idxLabel := fmt.Sprintf("index[%d]", idx) @@ -45,18 +42,11 @@ func (k *Keeper) CalcTokenValues(ctx sdk.Context) (TokenValues, error) { errs[idxLabel] = err return true } - pool, _ := ipool.(*types.OsmosisPoolProtocolData) - - // pool must be a base pair - if len(pool.Denoms) != 2 { - // not a pair: skip - return false + pool, ok := ipool.(*types.OsmosisPoolProtocolData) + if !ok || len(pool.Denoms) != 2 { + return true } - // values to be captured and used - // - baseIBCDenom -> the cosmos IBC denom in this pair - // - queryIBCDenom -> the target IBC denom in this pair - // - valueDenom -> the target zone.BaseDenom var baseIBCDenom, queryIBCDenom, valueDenom string isBasePair := false @@ -64,43 +54,35 @@ func (k *Keeper) CalcTokenValues(ctx sdk.Context) (TokenValues, error) { if pool.Denoms[ibcDenom].ChainID == baseChain { isBasePair = true baseIBCDenom = ibcDenom - } else { - zone, ok := k.icsKeeper.GetZone(ctx, pool.Denoms[ibcDenom].ChainID) - if !ok { - // errs[idxLabel] = fmt.Errorf("zone not found, %s", denom.ChainId) - return false - } - - if pool.Denoms[ibcDenom].Denom == zone.BaseDenom { - queryIBCDenom = ibcDenom - valueDenom = zone.BaseDenom - } else { - return false - } + continue } - } - - if isBasePair { - if pool.PoolData == nil { - errs[idxLabel] = fmt.Errorf("pool data is nil, awaiting OsmosisPoolUpdateCallback") - return true - } - gammPool, err := pool.GetPool() - if err != nil { - errs[idxLabel] = err + zone, ok := k.icsKeeper.GetZone(ctx, pool.Denoms[ibcDenom].ChainID) + if !ok || pool.Denoms[ibcDenom].Denom != zone.BaseDenom { return true } + queryIBCDenom = ibcDenom + valueDenom = zone.BaseDenom + } - value, err := gammPool.SpotPrice(ctx, baseIBCDenom, queryIBCDenom) - if err != nil { - errs[idxLabel] = err - return true - } + if !isBasePair || pool.PoolData == nil { + errs[idxLabel] = fmt.Errorf("invalid base pair or pool data is nil, awaiting OsmosisPoolUpdateCallback") + return true + } - tvs[valueDenom] = value + gammPool, err := pool.GetPool() + if err != nil { + errs[idxLabel] = err + return true } - return false + value, err := gammPool.SpotPrice(ctx, baseIBCDenom, queryIBCDenom) + if err != nil { + errs[idxLabel] = err + return true + } + + tvs[valueDenom] = value + return true }) if len(errs) > 0 { diff --git a/x/participationrewards/types/params.go b/x/participationrewards/types/params.go index 2f5d595eb..768a63bf9 100644 --- a/x/participationrewards/types/params.go +++ b/x/participationrewards/types/params.go @@ -59,7 +59,7 @@ func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs { } } -func validateDistributionProportions(i interface{}) error { +func validateDistributionProportions(i any) error { dp, ok := i.(DistributionProportions) if !ok { return fmt.Errorf("invalid parameter type: %T", i) @@ -68,7 +68,7 @@ func validateDistributionProportions(i interface{}) error { return dp.ValidateBasic() } -func validateBoolean(i interface{}) error { +func validateBoolean(i any) error { _, ok := i.(bool) if !ok { return fmt.Errorf("invalid parameter type: %T", i) diff --git a/x/participationrewards/types/proposals_test.go b/x/participationrewards/types/proposals_test.go index 7e28b08b0..4e2a75a79 100644 --- a/x/participationrewards/types/proposals_test.go +++ b/x/participationrewards/types/proposals_test.go @@ -157,7 +157,7 @@ Data: { }) } -var sink interface{} +var sink any func BenchmarkUpdateZoneProposalString(b *testing.B) { adp := &types.AddProtocolDataProposal{ @@ -185,5 +185,5 @@ func BenchmarkUpdateZoneProposalString(b *testing.B) { if sink == nil { b.Fatal("Benchmark did not run") } - sink = interface{}(nil) + sink = any(nil) } diff --git a/x/supply/keeper/keeper.go b/x/supply/keeper/keeper.go index e7e8e7ab0..63a62cd08 100644 --- a/x/supply/keeper/keeper.go +++ b/x/supply/keeper/keeper.go @@ -49,7 +49,7 @@ func NewKeeper( // _____________________________________________________________________ // Logger returns a module-specific logger. -func (k Keeper) Logger(ctx sdk.Context) log.Logger { +func (Keeper) Logger(ctx sdk.Context) log.Logger { return ctx.Logger().With("module", "x/"+types.ModuleName) } diff --git a/x/tokenfactory/types/params.go b/x/tokenfactory/types/params.go index 1e0dcb63d..a18c53187 100644 --- a/x/tokenfactory/types/params.go +++ b/x/tokenfactory/types/params.go @@ -39,7 +39,7 @@ func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs { } } -func validateDenomCreationFee(i interface{}) error { +func validateDenomCreationFee(i any) error { v, ok := i.(sdk.Coins) if !ok { return fmt.Errorf("invalid parameter type: %T", i) From 3e91bb5a2369e8178ba2ec7b2d6f74b67958785a Mon Sep 17 00:00:00 2001 From: Jacob Gadikian Date: Thu, 22 Feb 2024 17:52:05 +0200 Subject: [PATCH 04/18] revert changes that led to test failures --- .../keeper/ibc_packet_handlers.go | 54 ++++++++++--------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/x/interchainstaking/keeper/ibc_packet_handlers.go b/x/interchainstaking/keeper/ibc_packet_handlers.go index 963165dbd..58cb7decd 100644 --- a/x/interchainstaking/keeper/ibc_packet_handlers.go +++ b/x/interchainstaking/keeper/ibc_packet_handlers.go @@ -560,7 +560,7 @@ func (k *Keeper) HandleBeginRedelegate(ctx sdk.Context, msg sdk.Msg, completion } k.Logger(ctx).Info("Received MsgBeginRedelegate acknowledgement") - + // first, type assertion. we should have stakingtypes.MsgBeginRedelegate redelegateMsg, ok := msg.(*stakingtypes.MsgBeginRedelegate) if !ok { return errors.New("unable to unmarshal MsgBeginRedelegate") @@ -572,25 +572,27 @@ func (k *Keeper) HandleBeginRedelegate(ctx sdk.Context, msg sdk.Msg, completion } if completion.IsZero() { + // a zero completion time can only happen when the validator is unbonded; this means the redelegation has _already_ completed and can be removed. k.DeleteRedelegationRecord(ctx, zone.ChainId, redelegateMsg.ValidatorSrcAddress, redelegateMsg.ValidatorDstAddress, epochNumber) - return nil - } - - record, found := k.GetRedelegationRecord(ctx, zone.ChainId, redelegateMsg.ValidatorSrcAddress, redelegateMsg.ValidatorDstAddress, epochNumber) - if !found { - record = types.RedelegationRecord{ - ChainId: zone.ChainId, - EpochNumber: epochNumber, - Source: redelegateMsg.ValidatorSrcAddress, - Destination: redelegateMsg.ValidatorDstAddress, - Amount: redelegateMsg.Amount.Amount.Int64(), - CompletionTime: completion, + } else { + record, found := k.GetRedelegationRecord(ctx, zone.ChainId, redelegateMsg.ValidatorSrcAddress, redelegateMsg.ValidatorDstAddress, epochNumber) + if !found { + // it is possible that the record was cleaned up if there was a long delay in processing acknowledgements. + // just create a new one + record = types.RedelegationRecord{ + ChainId: zone.ChainId, + EpochNumber: epochNumber, + Source: redelegateMsg.ValidatorSrcAddress, + Destination: redelegateMsg.ValidatorDstAddress, + Amount: redelegateMsg.Amount.Amount.Int64(), + CompletionTime: completion, + } } - } - k.Logger(ctx).Info("updating redelegation record with completion time", "completion", completion) - record.CompletionTime = completion - k.SetRedelegationRecord(ctx, record) + k.Logger(ctx).Info("updating redelegation record with completion time", "completion", completion) + record.CompletionTime = completion + k.SetRedelegationRecord(ctx, record) + } tgtDelegation, found := k.GetDelegation(ctx, zone.ChainId, redelegateMsg.DelegatorAddress, redelegateMsg.ValidatorDstAddress) if !found { @@ -598,7 +600,8 @@ func (k *Keeper) HandleBeginRedelegate(ctx sdk.Context, msg sdk.Msg, completion } else { tgtDelegation.Amount = tgtDelegation.Amount.Add(redelegateMsg.Amount) } - tgtDelegation.RedelegationEnd = completion.Unix() + // RedelegationEnd is used to determine whether the delegation is 'locked' for transient redelegations. + tgtDelegation.RedelegationEnd = completion.Unix() // this field should be a timestamp, but let's avoid unnecessary state changes. k.SetDelegation(ctx, zone.ChainId, tgtDelegation) delAddr, err := addressutils.AccAddressFromBech32(redelegateMsg.DelegatorAddress, zone.AccountPrefix) @@ -611,6 +614,7 @@ func (k *Keeper) HandleBeginRedelegate(ctx sdk.Context, msg sdk.Msg, completion } data := stakingtypes.GetDelegationKey(delAddr, valAddr) + // send request to update delegation record for target del/val tuple. k.ICQKeeper.MakeRequest( ctx, zone.ConnectionId, @@ -628,23 +632,26 @@ func (k *Keeper) HandleBeginRedelegate(ctx sdk.Context, msg sdk.Msg, completion k.Logger(ctx).Error("unable to find delegation record", "chain", zone.ChainId, "source", redelegateMsg.ValidatorSrcAddress, "dst", redelegateMsg.ValidatorDstAddress, "epoch_number", epochNumber) return fmt.Errorf("unable to find delegation record for chain %s, src: %s, dst: %s, at epoch %d", zone.ChainId, redelegateMsg.ValidatorSrcAddress, redelegateMsg.ValidatorDstAddress, epochNumber) } - srcDelegation.Amount, err = srcDelegation.Amount.SafeSub(redelegateMsg.Amount) if err != nil { - if !strings.Contains(err.Error(), "negative coin amount") { + if strings.Contains(err.Error(), "negative coin amount") { + // we received a negative srcDelegation. Obviously this cannot happen, but we can get a crossed re/un/delegation, all which fetch absolute values. + k.Logger(ctx).Error("possible race condition; unable to sub redelegation amount. requerying delegation anyway") + } else { + // we got some other, unrecoverable err return err } - k.Logger(ctx).Error("possible race condition; unable to sub redelegation amount. requerying delegation anyway") + } else { + k.SetDelegation(ctx, zone.ChainId, srcDelegation) } - k.SetDelegation(ctx, zone.ChainId, srcDelegation) - valAddr, err = addressutils.ValAddressFromBech32(redelegateMsg.ValidatorDstAddress, zone.AccountPrefix+"valoper") if err != nil { return err } data = stakingtypes.GetDelegationKey(delAddr, valAddr) + // send request to update delegation record for src del/val tuple. k.ICQKeeper.MakeRequest( ctx, zone.ConnectionId, @@ -656,7 +663,6 @@ func (k *Keeper) HandleBeginRedelegate(ctx sdk.Context, msg sdk.Msg, completion "delegation", 0, ) - return nil } From 47bb0994e9767d9db93c5ff32785e075a555c69d Mon Sep 17 00:00:00 2001 From: Jacob Gadikian Date: Thu, 22 Feb 2024 19:19:22 +0200 Subject: [PATCH 05/18] simplify nested if's --- wasmbinding/message_plugin.go | 15 +++--- x/interchainquery/keeper/keeper.go | 43 +++++++++------ x/interchainstaking/keeper/callbacks.go | 53 +++++++++++-------- .../keeper/ibc_packet_handlers.go | 51 +++++++++++------- x/interchainstaking/transfer_middleware.go | 28 ++++++---- 5 files changed, 111 insertions(+), 79 deletions(-) diff --git a/wasmbinding/message_plugin.go b/wasmbinding/message_plugin.go index fcaa90afd..149e4d06c 100644 --- a/wasmbinding/message_plugin.go +++ b/wasmbinding/message_plugin.go @@ -38,22 +38,19 @@ var _ wasmkeeper.Messenger = (*CustomMessenger)(nil) // DispatchMsg executes on the contractMsg. func (m *CustomMessenger) DispatchMsg(ctx sdk.Context, contractAddr sdk.AccAddress, contractIBCPortID string, msg wasmvmtypes.CosmosMsg) ([]sdk.Event, [][]byte, error) { if msg.Custom != nil { - // only handle the happy path where this is really creating / minting / swapping ... - // leave everything else for the wrapped version var contractMsg bindings.TokenFactoryMsg if err := json.Unmarshal(msg.Custom, &contractMsg); err != nil { return nil, nil, sdkioerrors.Wrap(err, "TokenFactoryMsg msg") } - if contractMsg.CreateDenom != nil { + + switch { + case contractMsg.CreateDenom != nil: return m.createDenom(ctx, contractAddr, contractMsg.CreateDenom) - } - if contractMsg.MintTokens != nil { + case contractMsg.MintTokens != nil: return m.mintTokens(ctx, contractAddr, contractMsg.MintTokens) - } - if contractMsg.ChangeAdmin != nil { + case contractMsg.ChangeAdmin != nil: return m.changeAdmin(ctx, contractAddr, contractMsg.ChangeAdmin) - } - if contractMsg.BurnTokens != nil { + case contractMsg.BurnTokens != nil: return m.burnTokens(ctx, contractAddr, contractMsg.BurnTokens) } } diff --git a/x/interchainquery/keeper/keeper.go b/x/interchainquery/keeper/keeper.go index 33e00c3cb..00f1034a5 100644 --- a/x/interchainquery/keeper/keeper.go +++ b/x/interchainquery/keeper/keeper.go @@ -142,29 +142,38 @@ func (k *Keeper) MakeRequest( "callback", callbackID, "ttl", ttl, ) + key := GenerateQueryHash(connectionID, chainID, queryType, request, module) existingQuery, found := k.GetQuery(ctx, key) - if !found { - if module != "" && callbackID != "" { - if _, exists := k.callbacks[module]; !exists { - err := fmt.Errorf("no callback handler registered for module %s", module) - k.Logger(ctx).Error(err.Error()) - panic(err) - } - if exists := k.callbacks[module].Has(callbackID); !exists { - err := fmt.Errorf("no callback %s registered for module %s", callbackID, module) - k.Logger(ctx).Error(err.Error()) - panic(err) - } - } - newQuery := k.NewQuery(module, connectionID, chainID, queryType, request, period, callbackID, ttl) - k.SetQuery(ctx, *newQuery) - } else { - // a re-request of an existing query triggers resetting of height to trigger immediately. + + if found { + // Handle re-request of existing query k.Logger(ctx).Debug("re-request", "LastHeight", existingQuery.LastHeight) existingQuery.LastHeight = sdk.ZeroInt() k.SetQuery(ctx, existingQuery) + return } + + // Handle creation of new query + if err := k.validateCallbacks(module, callbackID); err != nil { + k.Logger(ctx).Error(err.Error()) + panic(err) + } + + newQuery := k.NewQuery(module, connectionID, chainID, queryType, request, period, callbackID, ttl) + k.SetQuery(ctx, *newQuery) +} + +func (k *Keeper) validateCallbacks(module, callbackID string) error { + if module != "" && callbackID != "" { + if _, exists := k.callbacks[module]; !exists { + return fmt.Errorf("no callback handler registered for module %s", module) + } + if !k.callbacks[module].Has(callbackID) { + return fmt.Errorf("no callback %s registered for module %s", callbackID, module) + } + } + return nil } // Heights diff --git a/x/interchainstaking/keeper/callbacks.go b/x/interchainstaking/keeper/callbacks.go index 9586e47ce..982ca7a65 100644 --- a/x/interchainstaking/keeper/callbacks.go +++ b/x/interchainstaking/keeper/callbacks.go @@ -162,37 +162,46 @@ func DelegationCallback(k *Keeper, ctx sdk.Context, args []byte, query icqtypes. } delegation := stakingtypes.Delegation{} - // delegations _can_ legitimately be nil here, so explicitly DON'T guard against this. err := k.cdc.Unmarshal(args, &delegation) if err != nil { return err } - k.Logger(ctx).Debug("Delegation callback", "delegation", delegation, "chain", zone.ChainId) + if shouldRemoveDelegation(delegation) { + return handleDelegationRemoval(k, ctx, &zone, query) + } - if delegation.Shares.IsNil() || delegation.Shares.IsZero() { - // delegation never gets removed, even with zero shares. - delegator, validator, err := types.ParseStakingDelegationKey(query.Request) - if err != nil { - return err - } - validatorAddress, err := addressutils.EncodeAddressToBech32(zone.GetValoperPrefix(), validator) - if err != nil { - return err - } - delegatorAddress, err := addressutils.EncodeAddressToBech32(zone.GetAccountPrefix(), delegator) + return processDelegation(k, ctx, delegation, &zone) +} + +func shouldRemoveDelegation(delegation stakingtypes.Delegation) bool { + return delegation.Shares.IsNil() || delegation.Shares.IsZero() +} + +func handleDelegationRemoval(k *Keeper, ctx sdk.Context, zone *types.Zone, query icqtypes.Query) error { + delegator, validator, err := types.ParseStakingDelegationKey(query.Request) + if err != nil { + return err + } + validatorAddress, err := addressutils.EncodeAddressToBech32(zone.GetValoperPrefix(), validator) + if err != nil { + return err + } + delegatorAddress, err := addressutils.EncodeAddressToBech32(zone.GetAccountPrefix(), delegator) + if err != nil { + return err + } + + if delegation, ok := k.GetDelegation(ctx, zone.ChainId, delegatorAddress, validatorAddress); ok { + err := k.RemoveDelegation(ctx, zone.ChainId, delegation) if err != nil { return err } - - if delegation, ok := k.GetDelegation(ctx, zone.ChainId, delegatorAddress, validatorAddress); ok { - err := k.RemoveDelegation(ctx, zone.ChainId, delegation) - if err != nil { - return err - } - } - return nil } + return nil +} + +func processDelegation(k *Keeper, ctx sdk.Context, delegation stakingtypes.Delegation, zone *types.Zone) error { valAddrBytes, err := addressutils.ValAddressFromBech32(delegation.ValidatorAddress, zone.GetValoperPrefix()) if err != nil { return err @@ -204,7 +213,7 @@ func DelegationCallback(k *Keeper, ctx sdk.Context, args []byte, query icqtypes. return err } - return k.UpdateDelegationRecordForAddress(ctx, delegation.DelegatorAddress, delegation.ValidatorAddress, sdk.NewCoin(zone.BaseDenom, val.SharesToTokens(delegation.Shares)), &zone, true) + return k.UpdateDelegationRecordForAddress(ctx, delegation.DelegatorAddress, delegation.ValidatorAddress, sdk.NewCoin(zone.BaseDenom, val.SharesToTokens(delegation.Shares)), zone, true) } func PerfBalanceCallback(k *Keeper, ctx sdk.Context, response []byte, query icqtypes.Query) error { diff --git a/x/interchainstaking/keeper/ibc_packet_handlers.go b/x/interchainstaking/keeper/ibc_packet_handlers.go index 58cb7decd..2d98c0c2e 100644 --- a/x/interchainstaking/keeper/ibc_packet_handlers.go +++ b/x/interchainstaking/keeper/ibc_packet_handlers.go @@ -424,7 +424,7 @@ func (k *Keeper) HandleWithdrawForUser(ctx sdk.Context, zone *types.Zone, msg *b // case 1: total amount - native unbonding // this statement is ridiculous, but currently calling coins.Equals against coins with different denoms panics; which is pretty useless. - if len(withdrawalRecord.Amount) == 1 && len(msg.Amount) == 1 && msg.Amount[0].Denom == withdrawalRecord.Amount[0].Denom && withdrawalRecord.Amount.IsEqual(msg.Amount) { + if k.isMatchingWithdrawal(withdrawalRecord, msg) { k.Logger(ctx).Info("found matching withdrawal; marking as completed") k.UpdateWithdrawalRecordStatus(ctx, &withdrawalRecord, types.WithdrawStatusCompleted) if err := k.BankKeeper.BurnCoins(ctx, types.EscrowModuleAccount, sdk.NewCoins(withdrawalRecord.BurnAmount)); err != nil { @@ -473,6 +473,16 @@ func (k *Keeper) HandleWithdrawForUser(ctx sdk.Context, zone *types.Zone, msg *b return k.EmitValSetQuery(ctx, zone.ConnectionId, zone.ChainId, query, sdkmath.NewInt(period)) } +func (*Keeper) isMatchingWithdrawal(withdrawalRecord types.WithdrawalRecord, msg *banktypes.MsgSend) bool { + if len(withdrawalRecord.Amount) != 1 || len(msg.Amount) != 1 { + return false + } + if msg.Amount[0].Denom != withdrawalRecord.Amount[0].Denom { + return false + } + return withdrawalRecord.Amount.IsEqual(msg.Amount) +} + func (k *Keeper) GCCompletedRedelegations(ctx sdk.Context) error { var err error @@ -634,13 +644,12 @@ func (k *Keeper) HandleBeginRedelegate(ctx sdk.Context, msg sdk.Msg, completion } srcDelegation.Amount, err = srcDelegation.Amount.SafeSub(redelegateMsg.Amount) if err != nil { - if strings.Contains(err.Error(), "negative coin amount") { - // we received a negative srcDelegation. Obviously this cannot happen, but we can get a crossed re/un/delegation, all which fetch absolute values. - k.Logger(ctx).Error("possible race condition; unable to sub redelegation amount. requerying delegation anyway") - } else { - // we got some other, unrecoverable err + if !strings.Contains(err.Error(), "negative coin amount") { + // If the error is not due to a negative coin amount, return the error immediately. return err } + // Log the error for a negative coin amount but continue execution. + k.Logger(ctx).Error("possible race condition; unable to sub redelegation amount. requerying delegation anyway") } else { k.SetDelegation(ctx, zone.ChainId, srcDelegation) } @@ -1068,34 +1077,36 @@ func (k *Keeper) HandleFailedDelegate(ctx sdk.Context, msg sdk.Msg, memo string) func (k *Keeper) HandleUpdatedWithdrawAddress(ctx sdk.Context, msg sdk.Msg) error { k.Logger(ctx).Info("Received MsgSetWithdrawAddress acknowledgement") - // first, type assertion. we should have distrtypes.MsgSetWithdrawAddress + + // First, type assertion. We should have distrtypes.MsgSetWithdrawAddress original, ok := msg.(*distrtypes.MsgSetWithdrawAddress) if !ok { k.Logger(ctx).Error("unable to cast source message to MsgSetWithdrawAddress") return errors.New("unable to cast source message to MsgSetWithdrawAddress") } + + var err error zone, found := k.GetZoneForDelegateAccount(ctx, original.DelegatorAddress) - if !found { + if found { + err = zone.DelegationAddress.SetWithdrawalAddress(original.WithdrawAddress) + } else { zone, found = k.GetZoneForPerformanceAccount(ctx, original.DelegatorAddress) - if !found { + if found { + err = zone.PerformanceAddress.SetWithdrawalAddress(original.WithdrawAddress) + } else { zone, found = k.GetZoneForDepositAccount(ctx, original.DelegatorAddress) if !found { return errors.New("unable to find zone") } - if err := zone.DepositAddress.SetWithdrawalAddress(original.WithdrawAddress); err != nil { - return err - } - } - if err := zone.PerformanceAddress.SetWithdrawalAddress(original.WithdrawAddress); err != nil { - return err - } - } else { - if err := zone.DelegationAddress.SetWithdrawalAddress(original.WithdrawAddress); err != nil { - return err + err = zone.DepositAddress.SetWithdrawalAddress(original.WithdrawAddress) } } - k.SetZone(ctx, zone) + if err != nil { + return err + } + + k.SetZone(ctx, zone) return nil } diff --git a/x/interchainstaking/transfer_middleware.go b/x/interchainstaking/transfer_middleware.go index 772251ea5..38ab8b0a3 100644 --- a/x/interchainstaking/transfer_middleware.go +++ b/x/interchainstaking/transfer_middleware.go @@ -94,17 +94,23 @@ func (im TransferMiddleware) OnRecvPacket( } ack := im.app.OnRecvPacket(ctx, packet, relayer) - if ack.Success() { - _, found := im.keeper.GetZoneForWithdrawalAccount(ctx, data.Sender) - if found { - if data.Receiver == im.keeper.AccountKeeper.GetModuleAddress(types.ModuleName).String() { - im.keeper.Logger(ctx).Info("MsgTransfer to ics module account from withdrawal address") - err := im.keeper.HandleMsgTransfer(ctx, data, utils.DeriveIbcDenom(packet.DestinationPort, packet.DestinationChannel, packet.SourcePort, packet.SourceChannel, data.Denom)) - if err != nil { - im.keeper.Logger(ctx).Error("unable to disperse rewards", "error", err.Error()) - } - } - } + if !ack.Success() { + return ack + } + + _, found := im.keeper.GetZoneForWithdrawalAccount(ctx, data.Sender) + if !found { + return ack + } + + if data.Receiver != im.keeper.AccountKeeper.GetModuleAddress(types.ModuleName).String() { + return ack + } + + im.keeper.Logger(ctx).Info("MsgTransfer to ics module account from withdrawal address") + err := im.keeper.HandleMsgTransfer(ctx, data, utils.DeriveIbcDenom(packet.DestinationPort, packet.DestinationChannel, packet.SourcePort, packet.SourceChannel, data.Denom)) + if err != nil { + im.keeper.Logger(ctx).Error("unable to disperse rewards", "error", err.Error()) } return ack From 48c466f9a0df47fbf4eba9951356965e3bd9e7cc Mon Sep 17 00:00:00 2001 From: Jacob Gadikian Date: Thu, 22 Feb 2024 19:21:52 +0200 Subject: [PATCH 06/18] refactor SigningInfoCallback --- x/interchainstaking/keeper/callbacks.go | 60 +++++++++++++------------ 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/x/interchainstaking/keeper/callbacks.go b/x/interchainstaking/keeper/callbacks.go index 982ca7a65..6fab72919 100644 --- a/x/interchainstaking/keeper/callbacks.go +++ b/x/interchainstaking/keeper/callbacks.go @@ -291,41 +291,45 @@ func SigningInfoCallback(k *Keeper, ctx sdk.Context, args []byte, query icqtypes return err } if valSigningInfo.Tombstoned { - consAddr, err := addressutils.AddressFromBech32(valSigningInfo.Address, "") - if err != nil { - return err - } - valAddr, found := k.GetValidatorAddrByConsAddr(ctx, zone.ChainId, consAddr) - if !found { - return fmt.Errorf("can not get validator address from consensus address: %s", valSigningInfo.Address) - } + return handleTombstonedValidator(k, ctx, &zone, valSigningInfo) + } + return nil +} + +func handleTombstonedValidator(k *Keeper, ctx sdk.Context, zone *types.Zone, valSigningInfo slashingtypes.ValidatorSigningInfo) error { + consAddr, err := addressutils.AddressFromBech32(valSigningInfo.Address, "") + if err != nil { + return err + } + valAddr, found := k.GetValidatorAddrByConsAddr(ctx, zone.ChainId, consAddr) + if !found { + return fmt.Errorf("can not get validator address from consensus address: %s", valSigningInfo.Address) + } - k.Logger(ctx).Info("tombstoned validator found", "valoper", valAddr) + k.Logger(ctx).Info("tombstoned validator found", "valoper", valAddr) - valAddrBytes, err := addressutils.ValAddressFromBech32(valAddr, zone.GetValoperPrefix()) + valAddrBytes, err := addressutils.ValAddressFromBech32(valAddr, zone.GetValoperPrefix()) + if err != nil { + return err + } + val, found := k.GetValidator(ctx, zone.ChainId, valAddrBytes) + if !found { + err := k.SetValidator(ctx, zone.ChainId, types.Validator{ + ValoperAddress: valAddr, + Jailed: true, + Tombstoned: true, + }) if err != nil { return err } - val, found := k.GetValidator(ctx, zone.ChainId, valAddrBytes) - // NOTE: this shouldn't be reachable, but keeping here as it doesn't do any harm. - if !found { - err := k.SetValidator(ctx, zone.ChainId, types.Validator{ - ValoperAddress: valAddr, - Jailed: true, - Tombstoned: true, - }) - if err != nil { - return err - } - } else { - val.Tombstoned = true - if err = k.SetValidator(ctx, zone.ChainId, val); err != nil { - return err - } + } else { + val.Tombstoned = true + if err = k.SetValidator(ctx, zone.ChainId, val); err != nil { + return err } - k.Logger(ctx).Info(fmt.Sprintf("%q on chainID: %q was found to already have been tombstoned, added information", val.ValoperAddress, zone.ChainId)) - } + k.Logger(ctx).Info(fmt.Sprintf("%q on chainID: %q was found to already have been tombstoned, added information", val.ValoperAddress, zone.ChainId)) + return nil } From 088e385fcc44ce19535f9dd157c3ae1560bc11e8 Mon Sep 17 00:00:00 2001 From: Jacob Gadikian Date: Thu, 22 Feb 2024 19:32:12 +0200 Subject: [PATCH 07/18] refactor ValidateClaim --- x/interchainstaking/keeper/ibc_packet_handlers.go | 10 +++------- x/participationrewards/keeper/submodule_osmosis.go | 13 ++++++++----- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/x/interchainstaking/keeper/ibc_packet_handlers.go b/x/interchainstaking/keeper/ibc_packet_handlers.go index 2d98c0c2e..84b3aa6f1 100644 --- a/x/interchainstaking/keeper/ibc_packet_handlers.go +++ b/x/interchainstaking/keeper/ibc_packet_handlers.go @@ -1087,21 +1087,17 @@ func (k *Keeper) HandleUpdatedWithdrawAddress(ctx sdk.Context, msg sdk.Msg) erro var err error zone, found := k.GetZoneForDelegateAccount(ctx, original.DelegatorAddress) - if found { - err = zone.DelegationAddress.SetWithdrawalAddress(original.WithdrawAddress) - } else { + if !found { zone, found = k.GetZoneForPerformanceAccount(ctx, original.DelegatorAddress) - if found { - err = zone.PerformanceAddress.SetWithdrawalAddress(original.WithdrawAddress) - } else { + if !found { zone, found = k.GetZoneForDepositAccount(ctx, original.DelegatorAddress) if !found { return errors.New("unable to find zone") } - err = zone.DepositAddress.SetWithdrawalAddress(original.WithdrawAddress) } } + err = zone.DelegationAddress.SetWithdrawalAddress(original.WithdrawAddress) if err != nil { return err } diff --git a/x/participationrewards/keeper/submodule_osmosis.go b/x/participationrewards/keeper/submodule_osmosis.go index 09991e802..1af19a228 100644 --- a/x/participationrewards/keeper/submodule_osmosis.go +++ b/x/participationrewards/keeper/submodule_osmosis.go @@ -73,9 +73,12 @@ func (m *OsmosisModule) Hooks(ctx sdk.Context, k *Keeper) { func (*OsmosisModule) ValidateClaim(ctx sdk.Context, k *Keeper, msg *types.MsgSubmitClaim) (uint64, error) { var amount uint64 - var lock osmolockup.PeriodLock for _, proof := range msg.Proofs { - if proof.ProofType == types.ProofTypeBank { + var lock osmolockup.PeriodLock + var err error + + switch proof.ProofType { + case types.ProofTypeBank: addr, poolDenom, err := banktypes.AddressAndDenomFromBalancesStore(proof.Key[1:]) if err != nil { return 0, err @@ -89,9 +92,8 @@ func (*OsmosisModule) ValidateClaim(ctx sdk.Context, k *Keeper, msg *types.MsgSu return 0, err } lock = osmolockup.NewPeriodLock(uint64(poolID), addr, time.Hour, time.Time{}, sdk.NewCoins(coin)) - } else { - lock = osmolockup.PeriodLock{} - err := k.cdc.Unmarshal(proof.Data, &lock) + default: + err = k.cdc.Unmarshal(proof.Data, &lock) if err != nil { return 0, err } @@ -105,6 +107,7 @@ func (*OsmosisModule) ValidateClaim(ctx sdk.Context, k *Keeper, msg *types.MsgSu return 0, errors.New("not a valid proof for submitting user") } } + sdkAmount, err := osmosistypes.DetermineApplicableTokensInPool(ctx, k, lock, msg.Zone) if err != nil { return 0, err From cce08f2af593f231a80c4c9e60710eb0f2f19a2c Mon Sep 17 00:00:00 2001 From: Jacob Gadikian Date: Fri, 23 Feb 2024 11:37:08 +0200 Subject: [PATCH 08/18] simplify genaccounts.go --- cmd/quicksilverd/genaccounts.go | 45 ++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/cmd/quicksilverd/genaccounts.go b/cmd/quicksilverd/genaccounts.go index 3f897e274..664521dab 100644 --- a/cmd/quicksilverd/genaccounts.go +++ b/cmd/quicksilverd/genaccounts.go @@ -45,28 +45,9 @@ contain valid denominations. Accounts may optionally be supplied with vesting pa config.SetRoot(clientCtx.HomeDir) - var kr keyring.Keyring addr, err := sdk.AccAddressFromBech32(args[0]) if err != nil { - inBuf := bufio.NewReader(cmd.InOrStdin()) - keyringBackend, _ := cmd.Flags().GetString(flags.FlagKeyringBackend) - - if keyringBackend != "" && clientCtx.Keyring == nil { - var err error - kr, err = keyring.New(sdk.KeyringServiceName(), keyringBackend, clientCtx.HomeDir, inBuf, clientCtx.Codec) - if err != nil { - return err - } - } else { - kr = clientCtx.Keyring - } - - k, err := kr.Key(args[0]) - if err != nil { - return fmt.Errorf("failed to get address from Keyring: %w", err) - } - - addr, err = k.GetAddress() + addr, err = getAddressFromKeyring(args[0], cmd, clientCtx) if err != nil { return err } @@ -223,3 +204,27 @@ contain valid denominations. Accounts may optionally be supplied with vesting pa return cmd } + +// New function to encapsulate the keyring logic +func getAddressFromKeyring(arg string, cmd *cobra.Command, clientCtx client.Context) (sdk.AccAddress, error) { + inBuf := bufio.NewReader(cmd.InOrStdin()) + keyringBackend, _ := cmd.Flags().GetString(flags.FlagKeyringBackend) + + var kr keyring.Keyring + var err error + if keyringBackend != "" && clientCtx.Keyring == nil { + kr, err = keyring.New(sdk.KeyringServiceName(), keyringBackend, clientCtx.HomeDir, inBuf, clientCtx.Codec) + if err != nil { + return nil, err + } + } else { + kr = clientCtx.Keyring + } + + k, err := kr.Key(arg) + if err != nil { + return nil, fmt.Errorf("failed to get address from Keyring: %w", err) + } + + return k.GetAddress() +} From 3a1890aa5a8271eccacd7a49b9c1ce3263ad3818 Mon Sep 17 00:00:00 2001 From: Jacob Gadikian Date: Fri, 23 Feb 2024 12:27:49 +0200 Subject: [PATCH 09/18] refactor handlewithdrawforuser --- .../keeper/ibc_packet_handlers.go | 69 +++++++++++-------- 1 file changed, 39 insertions(+), 30 deletions(-) diff --git a/x/interchainstaking/keeper/ibc_packet_handlers.go b/x/interchainstaking/keeper/ibc_packet_handlers.go index 84b3aa6f1..93267cfd4 100644 --- a/x/interchainstaking/keeper/ibc_packet_handlers.go +++ b/x/interchainstaking/keeper/ibc_packet_handlers.go @@ -433,44 +433,53 @@ func (k *Keeper) HandleWithdrawForUser(ctx sdk.Context, zone *types.Zone, msg *b } k.Logger(ctx).Info("burned coins post-withdrawal", "coins", withdrawalRecord.BurnAmount) } else { - // case 2: per validator amounts - LSM unbonding - - dlist := make(map[int]struct{}) - for i, dist := range withdrawalRecord.Distribution { - if msg.Amount[0].Amount.Equal(sdk.NewIntFromUint64(dist.Amount)) { // check valoper here too? - dlist[i] = struct{}{} - // matched amount - if len(withdrawalRecord.Distribution) == len(dlist) { - // we just removed the last element - k.Logger(ctx).Info("found matching withdrawal; marking as completed") - k.UpdateWithdrawalRecordStatus(ctx, &withdrawalRecord, types.WithdrawStatusCompleted) - if err := k.BankKeeper.BurnCoins(ctx, types.EscrowModuleAccount, sdk.NewCoins(withdrawalRecord.BurnAmount)); err != nil { - // if we can't burn the coins, fail. - return err - } - k.Logger(ctx).Info("burned coins post-withdrawal", "coins", withdrawalRecord.BurnAmount) - } - break - } + if err := k.processPerValidatorWithdrawal(ctx, &withdrawalRecord, msg); err != nil { + return err } + } - if len(dlist) > 0 { - newDist := make([]*types.Distribution, 0) - for idx := range withdrawalRecord.Distribution { - if _, remove := dlist[idx]; !remove { - newDist = append(newDist, withdrawalRecord.Distribution[idx]) + period := int64(k.GetParam(ctx, types.KeyValidatorSetInterval)) + query := stakingtypes.QueryValidatorsRequest{} + return k.EmitValSetQuery(ctx, zone.ConnectionId, zone.ChainId, query, sdkmath.NewInt(period)) +} + +// New method to process per-validator withdrawals +func (k Keeper) processPerValidatorWithdrawal(ctx sdk.Context, withdrawalRecord *types.WithdrawalRecord, msg *banktypes.MsgSend) error { + dlist := make(map[int]struct{}) + for i, dist := range withdrawalRecord.Distribution { + if msg.Amount[0].Amount.Equal(sdk.NewIntFromUint64(dist.Amount)) { + dlist[i] = struct{}{} + if len(withdrawalRecord.Distribution) == len(dlist) { + k.Logger(ctx).Info("found matching withdrawal; marking as completed") + k.UpdateWithdrawalRecordStatus(ctx, withdrawalRecord, types.WithdrawStatusCompleted) + if err := k.BankKeeper.BurnCoins(ctx, types.EscrowModuleAccount, sdk.NewCoins(withdrawalRecord.BurnAmount)); err != nil { + return err } + k.Logger(ctx).Info("burned coins post-withdrawal", "coins", withdrawalRecord.BurnAmount) + return nil } - k.Logger(ctx).Info("found matching withdrawal; awaiting additional messages") - withdrawalRecord.Distribution = newDist - k.SetWithdrawalRecord(ctx, withdrawalRecord) + break } } - period := int64(k.GetParam(ctx, types.KeyValidatorSetInterval)) - query := stakingtypes.QueryValidatorsRequest{} - return k.EmitValSetQuery(ctx, zone.ConnectionId, zone.ChainId, query, sdkmath.NewInt(period)) + if len(dlist) > 0 { + withdrawalRecord.Distribution = k.removeMatchedDistributions(withdrawalRecord.Distribution, dlist) + k.Logger(ctx).Info("found matching withdrawal; awaiting additional messages") + k.SetWithdrawalRecord(ctx, *withdrawalRecord) + } + return nil +} + +// Helper method to remove matched distributions +func (Keeper) removeMatchedDistributions(distributions []*types.Distribution, dlist map[int]struct{}) []*types.Distribution { + newDist := make([]*types.Distribution, 0) + for idx, dist := range distributions { + if _, remove := dlist[idx]; !remove { + newDist = append(newDist, dist) + } + } + return newDist } func (*Keeper) isMatchingWithdrawal(withdrawalRecord types.WithdrawalRecord, msg *banktypes.MsgSend) bool { From 1731968ac9ddf25115d6544b3089c652978c1823 Mon Sep 17 00:00:00 2001 From: Jacob Gadikian Date: Fri, 23 Feb 2024 12:39:32 +0200 Subject: [PATCH 10/18] matching withdrawal --- .../keeper/ibc_packet_handlers.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/x/interchainstaking/keeper/ibc_packet_handlers.go b/x/interchainstaking/keeper/ibc_packet_handlers.go index 93267cfd4..aed9ef469 100644 --- a/x/interchainstaking/keeper/ibc_packet_handlers.go +++ b/x/interchainstaking/keeper/ibc_packet_handlers.go @@ -483,13 +483,22 @@ func (Keeper) removeMatchedDistributions(distributions []*types.Distribution, dl } func (*Keeper) isMatchingWithdrawal(withdrawalRecord types.WithdrawalRecord, msg *banktypes.MsgSend) bool { - if len(withdrawalRecord.Amount) != 1 || len(msg.Amount) != 1 { + if len(withdrawalRecord.Amount) != len(msg.Amount) { return false } - if msg.Amount[0].Denom != withdrawalRecord.Amount[0].Denom { - return false + for _, wrCoin := range withdrawalRecord.Amount { + matchFound := false + for _, msgCoin := range msg.Amount { + if wrCoin.Denom == msgCoin.Denom && wrCoin.Amount.Equal(msgCoin.Amount) { + matchFound = true + break + } + } + if !matchFound { + return false + } } - return withdrawalRecord.Amount.IsEqual(msg.Amount) + return true } func (k *Keeper) GCCompletedRedelegations(ctx sdk.Context) error { From 68291dc9afd75ac8a3e08eb6bca46ce954700579 Mon Sep 17 00:00:00 2001 From: Jacob Gadikian Date: Fri, 8 Mar 2024 12:55:20 +0200 Subject: [PATCH 11/18] refactor SetValidatorsForZone --- .golangci.yml | 2 + x/interchainstaking/keeper/callbacks.go | 2 +- x/interchainstaking/keeper/keeper.go | 229 +++++++++++++----------- 3 files changed, 126 insertions(+), 107 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 8adb4a000..12c5bf7ad 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -12,7 +12,9 @@ linters: - errcheck - gci - goconst + - gocognit - gocritic + - gocyclo - gofumpt - gosec - gosimple diff --git a/x/interchainstaking/keeper/callbacks.go b/x/interchainstaking/keeper/callbacks.go index ddd9e03d7..c2e1148f4 100644 --- a/x/interchainstaking/keeper/callbacks.go +++ b/x/interchainstaking/keeper/callbacks.go @@ -62,7 +62,7 @@ func (c Callbacks) Has(id string) bool { return found } -func (c Callbacks) AddCallback(id string, fn interface{}) icqtypes.QueryCallbacks { +func (c Callbacks) AddCallback(id string, fn any) icqtypes.QueryCallbacks { c.callbacks[id], _ = fn.(Callback) return c } diff --git a/x/interchainstaking/keeper/keeper.go b/x/interchainstaking/keeper/keeper.go index c81db1681..f2a619aed 100644 --- a/x/interchainstaking/keeper/keeper.go +++ b/x/interchainstaking/keeper/keeper.go @@ -325,11 +325,9 @@ func (k *Keeper) SetValidatorsForZone(ctx sdk.Context, data []byte, icqQuery icq func (k *Keeper) SetValidatorForZone(ctx sdk.Context, zone *types.Zone, data []byte) error { if data == nil { k.Logger(ctx).Error("expected validator state, got nil") - // return nil here, as if we receive nil we fail to unmarshal (as nil validators are invalid), - // so we can never hope to resolve this query. Possibly received a valset update from a - // different chain. return nil } + validator, err := k.UnmarshalValidator(data) if err != nil { k.Logger(ctx).Error("unable to unmarshal validator info for zone", "zone", zone.ChainId, "err", err) @@ -340,138 +338,157 @@ func (k *Keeper) SetValidatorForZone(ctx sdk.Context, zone *types.Zone, data []b if err != nil { return err } + val, found := k.GetValidator(ctx, zone.ChainId, valAddrBytes) if !found { - k.Logger(ctx).Debug("Unable to find validator - adding...", "valoper", validator.OperatorAddress) + return k.handleNewValidator(ctx, zone, validator) + } - jailTime := time.Time{} - if validator.IsJailed() { - var pk cryptotypes.PubKey - err := k.cdc.UnpackAny(validator.ConsensusPubkey, &pk) - if err != nil { - return err - } - consAddr := sdk.ConsAddress(pk.Address().Bytes()) - k.SetValidatorAddrByConsAddr(ctx, zone.ChainId, validator.OperatorAddress, consAddr) - jailTime = ctx.BlockTime() + if val.Tombstoned { + k.Logger(ctx).Debug(fmt.Sprintf("%q on chainID: %q was found to already have been tombstoned; not updating state.", validator.OperatorAddress, zone.ChainId)) + return nil + } - err = k.EmitSigningInfoQuery(ctx, zone.ConnectionId, zone.ChainId, validator) - if err != nil { - return err - } + if err := k.handleJailStatusTransition(ctx, zone, val, validator); err != nil { + return err + } + + if err := k.updateValidatorFields(ctx, zone, val, validator); err != nil { + return err + } + + if err := k.SetValidator(ctx, zone.ChainId, val); err != nil { + return err + } + + if _, found := k.GetPerformanceDelegation(ctx, zone.ChainId, zone.PerformanceAddress, validator.OperatorAddress); !found { + if err := k.MakePerformanceDelegation(ctx, zone, validator.OperatorAddress); err != nil { + return err } + } + + return nil +} + +func (k *Keeper) handleNewValidator(ctx sdk.Context, zone *types.Zone, validator lsmstakingtypes.Validator) error { + k.Logger(ctx).Debug("Unable to find validator - adding...", "valoper", validator.OperatorAddress) - if err := k.SetValidator(ctx, zone.ChainId, types.Validator{ - ValoperAddress: validator.OperatorAddress, - CommissionRate: validator.GetCommission(), - VotingPower: validator.Tokens, - DelegatorShares: validator.DelegatorShares, - Score: sdk.ZeroDec(), - Status: validator.Status.String(), - Jailed: validator.IsJailed(), - JailedSince: jailTime, - }); err != nil { + jailTime := time.Time{} + if validator.IsJailed() { + var pk cryptotypes.PubKey + err := k.cdc.UnpackAny(validator.ConsensusPubkey, &pk) + if err != nil { return err } + consAddr := sdk.ConsAddress(pk.Address().Bytes()) + k.SetValidatorAddrByConsAddr(ctx, zone.ChainId, validator.OperatorAddress, consAddr) + jailTime = ctx.BlockTime() - if err := k.MakePerformanceDelegation(ctx, zone, validator.OperatorAddress); err != nil { + err = k.EmitSigningInfoQuery(ctx, zone.ConnectionId, zone.ChainId, validator) + if err != nil { return err } - } else { - if val.Tombstoned { - k.Logger(ctx).Debug(fmt.Sprintf("%q on chainID: %q was found to already have been tombstoned; not updating state.", validator.OperatorAddress, zone.ChainId)) - return nil + } + + if err := k.SetValidator(ctx, zone.ChainId, types.Validator{ + ValoperAddress: validator.OperatorAddress, + CommissionRate: validator.GetCommission(), + VotingPower: validator.Tokens, + DelegatorShares: validator.DelegatorShares, + Score: sdk.ZeroDec(), + Status: validator.Status.String(), + Jailed: validator.IsJailed(), + JailedSince: jailTime, + }); err != nil { + return err + } + + if err := k.MakePerformanceDelegation(ctx, zone, validator.OperatorAddress); err != nil { + return err + } + + return nil +} + +func (k *Keeper) handleJailStatusTransition(ctx sdk.Context, zone *types.Zone, val types.Validator, validator lsmstakingtypes.Validator) error { + if !val.Jailed && validator.IsJailed() { + k.Logger(ctx).Info("Transitioning validator to jailed state", "valoper", validator.OperatorAddress, "old_vp", val.VotingPower, "new_vp", validator.Tokens, "new_shares", validator.DelegatorShares, "old_shares", val.DelegatorShares) + + var pk cryptotypes.PubKey + err := k.cdc.UnpackAny(validator.ConsensusPubkey, &pk) + if err != nil { + return err + } + consAddr := sdk.ConsAddress(pk.Address().Bytes()) + k.SetValidatorAddrByConsAddr(ctx, zone.ChainId, validator.OperatorAddress, consAddr) + + err = k.EmitSigningInfoQuery(ctx, zone.ConnectionId, zone.ChainId, validator) + if err != nil { + return err } - if !val.Jailed && validator.IsJailed() { - k.Logger(ctx).Info("Transitioning validator to jailed state", "valoper", validator.OperatorAddress, "old_vp", val.VotingPower, "new_vp", validator.Tokens, "new_shares", validator.DelegatorShares, "old_shares", val.DelegatorShares) + val.Jailed = true + val.JailedSince = ctx.BlockTime() - var pk cryptotypes.PubKey - err := k.cdc.UnpackAny(validator.ConsensusPubkey, &pk) + if !val.VotingPower.IsPositive() { + return fmt.Errorf("existing voting power must be greater than zero, received %s", val.VotingPower) + } + if validator.Tokens.IsNegative() { + return fmt.Errorf("incoming voting power must not be negative, received %s", validator.Tokens) + } + if validator.Tokens.IsZero() { + err = k.UpdateWithdrawalRecordsForSlash(ctx, zone, val.ValoperAddress, sdk.ZeroDec()) if err != nil { return err } - consAddr := sdk.ConsAddress(pk.Address().Bytes()) - k.SetValidatorAddrByConsAddr(ctx, zone.ChainId, validator.OperatorAddress, consAddr) - - err = k.EmitSigningInfoQuery(ctx, zone.ConnectionId, zone.ChainId, validator) + } else { + prevRatio := val.DelegatorShares.Quo(sdk.NewDecFromInt(val.VotingPower)) + newRatio := validator.DelegatorShares.Quo(sdk.NewDecFromInt(validator.Tokens)) + delta := newRatio.Quo(prevRatio) + err = k.UpdateWithdrawalRecordsForSlash(ctx, zone, val.ValoperAddress, delta) if err != nil { return err } - - val.Jailed = true - val.JailedSince = ctx.BlockTime() - - // be defensive, so we don't get division weirdness! - if !val.VotingPower.IsPositive() { - return fmt.Errorf("existing voting power must be greater than zero, received %s", val.VotingPower) - } - if validator.Tokens.IsNegative() { - return fmt.Errorf("incoming voting power must not be negative, received %s", validator.Tokens) - } - if validator.Tokens.IsZero() { - // edge case: if validator tokens is now zero, val was slashed to zero. - err = k.UpdateWithdrawalRecordsForSlash(ctx, zone, val.ValoperAddress, sdk.ZeroDec()) - if err != nil { - return err - } - } else { - // determine difference between previous vp/shares ratio and new ratio. - prevRatio := val.DelegatorShares.Quo(sdk.NewDecFromInt(val.VotingPower)) - newRatio := validator.DelegatorShares.Quo(sdk.NewDecFromInt(validator.Tokens)) - delta := newRatio.Quo(prevRatio) - err = k.UpdateWithdrawalRecordsForSlash(ctx, zone, val.ValoperAddress, delta) - if err != nil { - return err - } - } - } else if val.Jailed && !validator.IsJailed() { - k.Logger(ctx).Debug("Transitioning validator to unjailed state", "valoper", validator.OperatorAddress) - - val.Jailed = false - val.JailedSince = time.Time{} } + } else if val.Jailed && !validator.IsJailed() { + k.Logger(ctx).Debug("Transitioning validator to unjailed state", "valoper", validator.OperatorAddress) - if !val.CommissionRate.Equal(validator.GetCommission()) { - k.Logger(ctx).Debug("Validator commission rate change; updating...", "valoper", validator.OperatorAddress, "oldRate", val.CommissionRate, "newRate", validator.GetCommission()) - val.CommissionRate = validator.GetCommission() - } - - if !val.VotingPower.Equal(validator.Tokens) { - k.Logger(ctx).Debug("Validator voting power change; updating", "valoper", validator.OperatorAddress, "oldPower", val.VotingPower, "newPower", validator.Tokens) - val.VotingPower = validator.Tokens - } + val.Jailed = false + val.JailedSince = time.Time{} + } - if !val.DelegatorShares.Equal(validator.DelegatorShares) { - k.Logger(ctx).Debug("Validator delegator shares change; updating", "valoper", validator.OperatorAddress, "oldShares", val.DelegatorShares, "newShares", validator.DelegatorShares) - val.DelegatorShares = validator.DelegatorShares - } + return nil +} - if val.Status != validator.Status.String() { - k.Logger(ctx).Debug("Transitioning validator status", "valoper", validator.OperatorAddress, "previous", val.Status, "current", validator.Status.String()) +func (k *Keeper) updateValidatorFields(ctx sdk.Context, zone *types.Zone, val types.Validator, validator lsmstakingtypes.Validator) error { + if !val.CommissionRate.Equal(validator.GetCommission()) { + k.Logger(ctx).Debug("Validator commission rate change; updating...", "valoper", validator.OperatorAddress, "oldRate", val.CommissionRate, "newRate", validator.GetCommission()) + val.CommissionRate = validator.GetCommission() + } - val.Status = validator.Status.String() - } + if !val.VotingPower.Equal(validator.Tokens) { + k.Logger(ctx).Debug("Validator voting power change; updating", "valoper", validator.OperatorAddress, "oldPower", val.VotingPower, "newPower", validator.Tokens) + val.VotingPower = validator.Tokens + } - if !validator.ValidatorBondShares.IsNil() && !val.ValidatorBondShares.Equal(validator.ValidatorBondShares) { - k.Logger(ctx).Info("Validator bonded shares change; updating", "valoper", validator.OperatorAddress, "oldShares", val.ValidatorBondShares, "newShares", validator.ValidatorBondShares) - val.ValidatorBondShares = validator.ValidatorBondShares - } + if !val.DelegatorShares.Equal(validator.DelegatorShares) { + k.Logger(ctx).Debug("Validator delegator shares change; updating", "valoper", validator.OperatorAddress, "oldShares", val.DelegatorShares, "newShares", validator.DelegatorShares) + val.DelegatorShares = validator.DelegatorShares + } - if !validator.LiquidShares.IsNil() && !val.LiquidShares.Equal(validator.LiquidShares) { - k.Logger(ctx).Info("Validator liquid shares change; updating", "valoper", validator.OperatorAddress, "oldShares", val.LiquidShares, "newShares", validator.LiquidShares) - val.LiquidShares = validator.LiquidShares - } + if val.Status != validator.Status.String() { + k.Logger(ctx).Debug("Transitioning validator status", "valoper", validator.OperatorAddress, "previous", val.Status, "current", validator.Status.String()) + val.Status = validator.Status.String() + } - if err := k.SetValidator(ctx, zone.ChainId, val); err != nil { - return err - } + if !validator.ValidatorBondShares.IsNil() && !val.ValidatorBondShares.Equal(validator.ValidatorBondShares) { + k.Logger(ctx).Info("Validator bonded shares change; updating", "valoper", validator.OperatorAddress, "oldShares", val.ValidatorBondShares, "newShares", validator.ValidatorBondShares) + val.ValidatorBondShares = validator.ValidatorBondShares + } - if _, found := k.GetPerformanceDelegation(ctx, zone.ChainId, zone.PerformanceAddress, validator.OperatorAddress); !found { - if err := k.MakePerformanceDelegation(ctx, zone, validator.OperatorAddress); err != nil { - return err - } - } + if !validator.LiquidShares.IsNil() && !val.LiquidShares.Equal(validator.LiquidShares) { + k.Logger(ctx).Info("Validator liquid shares change; updating", "valoper", validator.OperatorAddress, "oldShares", val.LiquidShares, "newShares", validator.LiquidShares) + val.LiquidShares = validator.LiquidShares } return nil From 280d3c4a4913a0a5c8ae4a5f20d6bff7021080e9 Mon Sep 17 00:00:00 2001 From: Jacob Gadikian Date: Fri, 8 Mar 2024 13:28:46 +0200 Subject: [PATCH 12/18] more simplification --- x/interchainstaking/keeper/callbacks.go | 146 +++++--- .../keeper/ibc_packet_handlers.go | 322 ++++++++++-------- x/interchainstaking/keeper/keeper.go | 10 +- 3 files changed, 265 insertions(+), 213 deletions(-) diff --git a/x/interchainstaking/keeper/callbacks.go b/x/interchainstaking/keeper/callbacks.go index c2e1148f4..4b236d4f9 100644 --- a/x/interchainstaking/keeper/callbacks.go +++ b/x/interchainstaking/keeper/callbacks.go @@ -180,7 +180,6 @@ func delegationCallback(k *Keeper, ctx sdk.Context, args []byte, query icqtypes. } delegation := stakingtypes.Delegation{} - // delegations _can_ legitimately be nil here, so explicitly DON'T guard against this. err := k.cdc.Unmarshal(args, &delegation) if err != nil { return err @@ -189,32 +188,41 @@ func delegationCallback(k *Keeper, ctx sdk.Context, args []byte, query icqtypes. k.Logger(ctx).Debug("Delegation callback", "delegation", delegation, "chain", zone.ChainId) if delegation.Shares.IsNil() || delegation.Shares.IsZero() { - // delegation never gets removed, even with zero shares. - delegator, validator, err := types.ParseStakingDelegationKey(query.Request) - if err != nil { - return err - } - validatorAddress, err := addressutils.EncodeAddressToBech32(zone.GetValoperPrefix(), validator) - if err != nil { - return err - } - delegatorAddress, err := addressutils.EncodeAddressToBech32(zone.GetAccountPrefix(), delegator) - if err != nil { - return err - } + return handleZeroDelegation(k, ctx, query, &zone) + } - if delegation, ok := k.GetDelegation(ctx, zone.ChainId, delegatorAddress, validatorAddress); ok { - err := k.RemoveDelegation(ctx, zone.ChainId, delegation) - if err != nil { - return err - } - } - return nil + return updateDelegationRecord(k, ctx, delegation, &zone, isEpoch) +} + +func handleZeroDelegation(k *Keeper, ctx sdk.Context, query icqtypes.Query, zone *types.Zone) error { + delegator, validator, err := types.ParseStakingDelegationKey(query.Request) + if err != nil { + return err + } + + validatorAddress, err := addressutils.EncodeAddressToBech32(zone.GetValoperPrefix(), validator) + if err != nil { + return err + } + + delegatorAddress, err := addressutils.EncodeAddressToBech32(zone.GetAccountPrefix(), delegator) + if err != nil { + return err } + + if delegation, ok := k.GetDelegation(ctx, zone.ChainId, delegatorAddress, validatorAddress); ok { + return k.RemoveDelegation(ctx, zone.ChainId, delegation) + } + + return nil +} + +func updateDelegationRecord(k *Keeper, ctx sdk.Context, delegation stakingtypes.Delegation, zone *types.Zone, isEpoch bool) error { valAddrBytes, err := addressutils.ValAddressFromBech32(delegation.ValidatorAddress, zone.GetValoperPrefix()) if err != nil { return err } + val, found := k.GetValidator(ctx, zone.ChainId, valAddrBytes) if !found { err := fmt.Errorf("unable to get validator: %s", delegation.ValidatorAddress) @@ -222,7 +230,7 @@ func delegationCallback(k *Keeper, ctx sdk.Context, args []byte, query icqtypes. return err } - return k.UpdateDelegationRecordForAddress(ctx, delegation.DelegatorAddress, delegation.ValidatorAddress, sdk.NewCoin(zone.BaseDenom, val.SharesToTokens(delegation.Shares)), &zone, true, isEpoch) + return k.UpdateDelegationRecordForAddress(ctx, delegation.DelegatorAddress, delegation.ValidatorAddress, sdk.NewCoin(zone.BaseDenom, val.SharesToTokens(delegation.Shares)), zone, true, isEpoch) } func PerfBalanceCallback(k *Keeper, ctx sdk.Context, response []byte, query icqtypes.Query) error { @@ -290,51 +298,77 @@ func SigningInfoCallback(k *Keeper, ctx sdk.Context, args []byte, query icqtypes k.Logger(ctx).Debug("Validator signing info callback", "zone", zone.ChainId) - valSigningInfo := slashingtypes.ValidatorSigningInfo{} + valSigningInfo, err := getValidatorSigningInfo(k, ctx, args, query) + if err != nil { + return err + } + + if !valSigningInfo.Tombstoned { + return nil + } + + valAddr, err := getValidatorAddressByConsensusAddress(k, ctx, &zone, valSigningInfo.Address) + if err != nil { + return err + } + + return handleTombstonedValidator(k, ctx, &zone, valAddr) +} + +func getValidatorSigningInfo(k *Keeper, ctx sdk.Context, args []byte, query icqtypes.Query) (*slashingtypes.ValidatorSigningInfo, error) { if len(args) == 0 { k.Logger(ctx).Error("unable to find signing info for validator", "query", query.Request) - return nil + return nil, nil } + + valSigningInfo := slashingtypes.ValidatorSigningInfo{} err := k.cdc.Unmarshal(args, &valSigningInfo) if err != nil { - return err + return nil, err } - if valSigningInfo.Tombstoned { - consAddr, err := addressutils.AddressFromBech32(valSigningInfo.Address, "") - if err != nil { - return err - } - valAddr, found := k.GetValidatorAddrByConsAddr(ctx, zone.ChainId, consAddr) - if !found { - return fmt.Errorf("can not get validator address from consensus address: %s", valSigningInfo.Address) - } - k.Logger(ctx).Info("tombstoned validator found", "valoper", valAddr) + return &valSigningInfo, nil +} - valAddrBytes, err := addressutils.ValAddressFromBech32(valAddr, zone.GetValoperPrefix()) - if err != nil { - return err - } - val, found := k.GetValidator(ctx, zone.ChainId, valAddrBytes) - // NOTE: this shouldn't be reachable, but keeping here as it doesn't do any harm. - if !found { - err := k.SetValidator(ctx, zone.ChainId, types.Validator{ - ValoperAddress: valAddr, - Jailed: true, - Tombstoned: true, - }) - if err != nil { - return err - } - } else { - val.Tombstoned = true - if err = k.SetValidator(ctx, zone.ChainId, val); err != nil { - return err - } +func getValidatorAddressByConsensusAddress(k *Keeper, ctx sdk.Context, zone *types.Zone, consAddr string) (string, error) { + consensusAddr, err := addressutils.AddressFromBech32(consAddr, "") + if err != nil { + return "", err + } + + valAddr, found := k.GetValidatorAddrByConsAddr(ctx, zone.ChainId, consensusAddr) + if !found { + return "", fmt.Errorf("can not get validator address from consensus address: %s", consAddr) + } + + return valAddr, nil +} + +func handleTombstonedValidator(k *Keeper, ctx sdk.Context, zone *types.Zone, valAddr string) error { + k.Logger(ctx).Info("tombstoned validator found", "valoper", valAddr) + + valAddrBytes, err := addressutils.ValAddressFromBech32(valAddr, zone.GetValoperPrefix()) + if err != nil { + return err + } + + val, found := k.GetValidator(ctx, zone.ChainId, valAddrBytes) + if !found { + val = types.Validator{ + ValoperAddress: valAddr, + Jailed: true, + Tombstoned: true, } - k.Logger(ctx).Info(fmt.Sprintf("%q on chainID: %q was found to already have been tombstoned, added information", val.ValoperAddress, zone.ChainId)) + } else { + val.Tombstoned = true + } + if err := k.SetValidator(ctx, zone.ChainId, val); err != nil { + return err } + + k.Logger(ctx).Info(fmt.Sprintf("%q on chainID: %q was found to already have been tombstoned, added information", val.ValoperAddress, zone.ChainId)) + return nil } diff --git a/x/interchainstaking/keeper/ibc_packet_handlers.go b/x/interchainstaking/keeper/ibc_packet_handlers.go index 9b94635fa..04494a413 100644 --- a/x/interchainstaking/keeper/ibc_packet_handlers.go +++ b/x/interchainstaking/keeper/ibc_packet_handlers.go @@ -107,11 +107,8 @@ func (k *Keeper) HandleAcknowledgement(ctx sdk.Context, packet channeltypes.Pack } for msgIndex, msg := range msgs { - // use msgData for v0.45 and below and msgResponse for v0.46+ - //nolint:staticcheck // SA1019 ignore this! var msgResponse []byte - // check that the msgResponses slice is at least the length of the current index. switch { case !success: // no-op - there is no msgresponse for a AckErr @@ -123,165 +120,190 @@ func (k *Keeper) HandleAcknowledgement(ctx sdk.Context, packet channeltypes.Pack return fmt.Errorf("could not find msgresponse for index %d", msgIndex) } - switch msg.Type { - case "/cosmos.distribution.v1beta1.MsgWithdrawDelegatorReward": - if !success { - withdrawalMsg, ok := msg.Msg.(*distrtypes.MsgWithdrawDelegatorReward) - if !ok { - return errors.New("unable to unmarshal MsgWithdrawDelegatorReward") - } - k.Logger(ctx).Error("failed to withdraw rewards; will try again next epoch", "validator", withdrawalMsg.ValidatorAddress) - return nil - } - k.Logger(ctx).Info("Rewards withdrawn") - if err := k.HandleWithdrawRewards(ctx, msg.Msg, connectionID); err != nil { - return err - } - continue - case "/cosmos.staking.v1beta1.MsgRedeemTokensForShares": - if !success { - if err := k.HandleFailedRedeemTokens(ctx, msg.Msg, packetData.Memo); err != nil { - return err - } - continue - } - response := lsmstakingtypes.MsgRedeemTokensForSharesResponse{} - - err = proto.Unmarshal(msgResponse, &response) - if err != nil { - k.Logger(ctx).Error("unable to unmarshal MsgRedeemTokensForShares response", "error", err) - return err - } - - k.Logger(ctx).Info("Tokens redeemed for shares", "response", response) - // we should update delegation records here. - if err := k.HandleRedeemTokens(ctx, msg.Msg, response.Amount, packetData.Memo, connectionID); err != nil { - return err - } - continue - case "/cosmos.staking.v1beta1.MsgTokenizeShares": - if !success { - // We can safely ignore this, as this can reasonably fail, and we cater for this in the flush logic. - return nil - } - response := lsmstakingtypes.MsgTokenizeSharesResponse{} + if err := handleMsgType(k, ctx, msg, msgResponse, success, packetData, connectionID); err != nil { + return err + } + } - err = proto.Unmarshal(msgResponse, &response) - if err != nil { - k.Logger(ctx).Error("unable to unpack MsgTokenizeShares response", "error", err) - return err - } + return nil +} - k.Logger(ctx).Info("Shares tokenized", "response", response) - if err := k.HandleTokenizedShares(ctx, msg.Msg, response.Amount, packetData.Memo); err != nil { - return err - } - continue - case "/cosmos.staking.v1beta1.MsgDelegate": - if !success { - if err := k.HandleFailedDelegate(ctx, msg.Msg, packetData.Memo); err != nil { - return err - } - continue - } - response := stakingtypes.MsgDelegateResponse{} - err = proto.Unmarshal(msgResponse, &response) - if err != nil { - k.Logger(ctx).Error("unable to unpack MsgDelegate response", "error", err) - return err - } +func handleMsgType(k *Keeper, ctx sdk.Context, msg TypedMsg, msgResponse []byte, success bool, packetData icatypes.InterchainAccountPacketData, connectionID string) error { + switch msg.Type { + case "/cosmos.distribution.v1beta1.MsgWithdrawDelegatorReward": + return handleMsgWithdrawDelegatorReward(k, ctx, msg.Msg, success, connectionID) + case "/cosmos.staking.v1beta1.MsgRedeemTokensForShares": + return handleMsgRedeemTokensForShares(k, ctx, msg.Msg, msgResponse, success, packetData.Memo, connectionID) + case "/cosmos.staking.v1beta1.MsgTokenizeShares": + return handleMsgTokenizeShares(k, ctx, msg.Msg, msgResponse, success, packetData.Memo) + case "/cosmos.staking.v1beta1.MsgDelegate": + return handleMsgDelegate(k, ctx, msg.Msg, msgResponse, success, packetData.Memo) + case "/cosmos.staking.v1beta1.MsgBeginRedelegate": + return handleMsgBeginRedelegate(k, ctx, msg.Msg, msgResponse, success, packetData.Memo) + case "/cosmos.staking.v1beta1.MsgUndelegate": + return handleMsgUndelegate(k, ctx, msg.Msg, msgResponse, success, packetData.Memo) + case "/cosmos.bank.v1beta1.MsgSend": + return handleMsgSend(k, ctx, msg.Msg, msgResponse, success, packetData.Memo, connectionID) + case "/cosmos.distribution.v1beta1.MsgSetWithdrawAddress": + return handleMsgSetWithdrawAddress(k, ctx, msg.Msg, msgResponse, success) + case "/ibc.applications.transfer.v1.MsgTransfer": + k.Logger(ctx).Debug("Received MsgTransfer acknowledgement; no action") + return nil + default: + k.Logger(ctx).Error("unhandled acknowledgement packet", "type", reflect.TypeOf(msg.Msg).Name()) + return nil + } +} - k.Logger(ctx).Info("Delegated", "response", response) - // we should update delegation records here. - if err := k.HandleDelegate(ctx, msg.Msg, packetData.Memo); err != nil { - return err - } - continue - case "/cosmos.staking.v1beta1.MsgBeginRedelegate": - if success { - response := stakingtypes.MsgBeginRedelegateResponse{} - err = proto.Unmarshal(msgResponse, &response) - k.Logger(ctx).Info("unmarshalling msgResponse", "response", response) - if err != nil { - k.Logger(ctx).Error("unable to unpack MsgBeginRedelegate response", "error", err) - return err - } +func handleMsgWithdrawDelegatorReward(k *Keeper, ctx sdk.Context, msg sdk.Msg, success bool, connectionID string) error { + if !success { + withdrawalMsg, ok := msg.(*distrtypes.MsgWithdrawDelegatorReward) + if !ok { + return errors.New("unable to unmarshal MsgWithdrawDelegatorReward") + } + k.Logger(ctx).Error("failed to withdraw rewards; will try again next epoch", "validator", withdrawalMsg.ValidatorAddress) + return nil + } + k.Logger(ctx).Info("Rewards withdrawn") + if err := k.HandleWithdrawRewards(ctx, msg, connectionID); err != nil { + return err + } + return nil +} - k.Logger(ctx).Info("Redelegation initiated", "response", response) - if err := k.HandleBeginRedelegate(ctx, msg.Msg, response.CompletionTime, packetData.Memo); err != nil { - return err - } - } else { - if err := k.HandleFailedBeginRedelegate(ctx, msg.Msg, packetData.Memo); err != nil { - return err - } - } - continue - case "/cosmos.staking.v1beta1.MsgUndelegate": - if success { - response := stakingtypes.MsgUndelegateResponse{} - err = proto.Unmarshal(msgResponse, &response) - if err != nil { - k.Logger(ctx).Error("unable to unpack MsgUndelegate response", "error", err) - return err - } +func handleMsgRedeemTokensForShares(k *Keeper, ctx sdk.Context, msg sdk.Msg, msgResponse []byte, success bool, memo string, connectionID string) error { + if !success { + if err := k.HandleFailedRedeemTokens(ctx, msg, memo); err != nil { + return err + } + return nil + } + response := lsmstakingtypes.MsgRedeemTokensForSharesResponse{} + err := proto.Unmarshal(msgResponse, &response) + if err != nil { + k.Logger(ctx).Error("unable to unmarshal MsgRedeemTokensForShares response", "error", err) + return err + } + k.Logger(ctx).Info("Tokens redeemed for shares", "response", response) + if err := k.HandleRedeemTokens(ctx, msg, response.Amount, memo, connectionID); err != nil { + return err + } + return nil +} - k.Logger(ctx).Info("Undelegation started", "response", response) - if err := k.HandleUndelegate(ctx, msg.Msg, response.CompletionTime, packetData.Memo); err != nil { - return err - } - } else { - if err := k.HandleFailedUndelegate(ctx, msg.Msg, packetData.Memo); err != nil { - return err - } - } - continue +func handleMsgTokenizeShares(k *Keeper, ctx sdk.Context, msg sdk.Msg, msgResponse []byte, success bool, memo string) error { + if !success { + return nil + } + response := lsmstakingtypes.MsgTokenizeSharesResponse{} + err := proto.Unmarshal(msgResponse, &response) + if err != nil { + k.Logger(ctx).Error("unable to unpack MsgTokenizeShares response", "error", err) + return err + } + k.Logger(ctx).Info("Shares tokenized", "response", response) + if err := k.HandleTokenizedShares(ctx, msg, response.Amount, memo); err != nil { + return err + } + return nil +} - case "/cosmos.bank.v1beta1.MsgSend": - if !success { - if err := k.HandleFailedBankSend(ctx, msg.Msg, packetData.Memo, connectionID); err != nil { - k.Logger(ctx).Error("unable to handle failed MsgSend", "error", err) - return err - } - continue - } - response := banktypes.MsgSendResponse{} - err = proto.Unmarshal(msgResponse, &response) - if err != nil { - k.Logger(ctx).Error("unable to unpack MsgSend response", "error", err) - return err - } +func handleMsgDelegate(k *Keeper, ctx sdk.Context, msg sdk.Msg, msgResponse []byte, success bool, memo string) error { + if !success { + if err := k.HandleFailedDelegate(ctx, msg, memo); err != nil { + return err + } + return nil + } + response := stakingtypes.MsgDelegateResponse{} + err := proto.Unmarshal(msgResponse, &response) + if err != nil { + k.Logger(ctx).Error("unable to unpack MsgDelegate response", "error", err) + return err + } + k.Logger(ctx).Info("Delegated", "response", response) + if err := k.HandleDelegate(ctx, msg, memo); err != nil { + return err + } + return nil +} - k.Logger(ctx).Info("Funds Transferred", "response", response) - // check tokenTransfers - if end user unescrow and burn txs - if err := k.HandleCompleteSend(ctx, msg.Msg, packetData.Memo, connectionID); err != nil { - return err - } - case "/cosmos.distribution.v1beta1.MsgSetWithdrawAddress": - if !success { - // safely ignore this, as we'll try again anyway. - return nil - } - response := distrtypes.MsgSetWithdrawAddressResponse{} - err = proto.Unmarshal(msgResponse, &response) - if err != nil { - k.Logger(ctx).Error("unable to unpack MsgSetWithdrawAddress response", "error", err) - return err - } +func handleMsgBeginRedelegate(k *Keeper, ctx sdk.Context, msg sdk.Msg, msgResponse []byte, success bool, memo string) error { + if success { + response := stakingtypes.MsgBeginRedelegateResponse{} + err := proto.Unmarshal(msgResponse, &response) + k.Logger(ctx).Info("unmarshalling msgResponse", "response", response) + if err != nil { + k.Logger(ctx).Error("unable to unpack MsgBeginRedelegate response", "error", err) + return err + } + k.Logger(ctx).Info("Redelegation initiated", "response", response) + if err := k.HandleBeginRedelegate(ctx, msg, response.CompletionTime, memo); err != nil { + return err + } + } else { + if err := k.HandleFailedBeginRedelegate(ctx, msg, memo); err != nil { + return err + } + } + return nil +} - k.Logger(ctx).Info("Withdraw Address Updated", "response", response) - if err := k.HandleUpdatedWithdrawAddress(ctx, msg.Msg); err != nil { - return err - } - case "/ibc.applications.transfer.v1.MsgTransfer": - k.Logger(ctx).Debug("Received MsgTransfer acknowledgement; no action") - return nil +func handleMsgUndelegate(k *Keeper, ctx sdk.Context, msg sdk.Msg, msgResponse []byte, success bool, memo string) error { + if success { + response := stakingtypes.MsgUndelegateResponse{} + err := proto.Unmarshal(msgResponse, &response) + if err != nil { + k.Logger(ctx).Error("unable to unpack MsgUndelegate response", "error", err) + return err + } + k.Logger(ctx).Info("Undelegation started", "response", response) + if err := k.HandleUndelegate(ctx, msg, response.CompletionTime, memo); err != nil { + return err + } + } else { + if err := k.HandleFailedUndelegate(ctx, msg, memo); err != nil { + return err + } + } + return nil +} - default: - k.Logger(ctx).Error("unhandled acknowledgement packet", "type", reflect.TypeOf(msg.Msg).Name()) +func handleMsgSend(k *Keeper, ctx sdk.Context, msg sdk.Msg, msgResponse []byte, success bool, memo string, connectionID string) error { + if !success { + if err := k.HandleFailedBankSend(ctx, msg, memo, connectionID); err != nil { + k.Logger(ctx).Error("unable to handle failed MsgSend", "error", err) + return err } + return nil + } + response := banktypes.MsgSendResponse{} + err := proto.Unmarshal(msgResponse, &response) + if err != nil { + k.Logger(ctx).Error("unable to unpack MsgSend response", "error", err) + return err + } + k.Logger(ctx).Info("Funds Transferred", "response", response) + if err := k.HandleCompleteSend(ctx, msg, memo, connectionID); err != nil { + return err } + return nil +} +func handleMsgSetWithdrawAddress(k *Keeper, ctx sdk.Context, msg sdk.Msg, msgResponse []byte, success bool) error { + if !success { + return nil + } + response := distrtypes.MsgSetWithdrawAddressResponse{} + err := proto.Unmarshal(msgResponse, &response) + if err != nil { + k.Logger(ctx).Error("unable to unpack MsgSetWithdrawAddress response", "error", err) + return err + } + k.Logger(ctx).Info("Withdraw Address Updated", "response", response) + if err := k.HandleUpdatedWithdrawAddress(ctx, msg); err != nil { + return err + } return nil } diff --git a/x/interchainstaking/keeper/keeper.go b/x/interchainstaking/keeper/keeper.go index f2a619aed..413917f94 100644 --- a/x/interchainstaking/keeper/keeper.go +++ b/x/interchainstaking/keeper/keeper.go @@ -353,7 +353,7 @@ func (k *Keeper) SetValidatorForZone(ctx sdk.Context, zone *types.Zone, data []b return err } - if err := k.updateValidatorFields(ctx, zone, val, validator); err != nil { + if err := k.updateValidatorFields(ctx, val, validator); err != nil { return err } @@ -403,11 +403,7 @@ func (k *Keeper) handleNewValidator(ctx sdk.Context, zone *types.Zone, validator return err } - if err := k.MakePerformanceDelegation(ctx, zone, validator.OperatorAddress); err != nil { - return err - } - - return nil + return k.MakePerformanceDelegation(ctx, zone, validator.OperatorAddress) } func (k *Keeper) handleJailStatusTransition(ctx sdk.Context, zone *types.Zone, val types.Validator, validator lsmstakingtypes.Validator) error { @@ -460,7 +456,7 @@ func (k *Keeper) handleJailStatusTransition(ctx sdk.Context, zone *types.Zone, v return nil } -func (k *Keeper) updateValidatorFields(ctx sdk.Context, zone *types.Zone, val types.Validator, validator lsmstakingtypes.Validator) error { +func (k *Keeper) updateValidatorFields(ctx sdk.Context, val types.Validator, validator lsmstakingtypes.Validator) error { if !val.CommissionRate.Equal(validator.GetCommission()) { k.Logger(ctx).Debug("Validator commission rate change; updating...", "valoper", validator.OperatorAddress, "oldRate", val.CommissionRate, "newRate", validator.GetCommission()) val.CommissionRate = validator.GetCommission() From 54974cba7761f8c728585f34e813b9dcc08041e7 Mon Sep 17 00:00:00 2001 From: Jacob Gadikian Date: Fri, 22 Mar 2024 02:56:22 +0700 Subject: [PATCH 13/18] update .golangci.yml --- .golangci.yml | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 12c5bf7ad..169216f65 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -28,20 +28,20 @@ linters: - typecheck - stylecheck - revive -# - testifylint + # - testifylint - typecheck - tenv - unconvert - - unparam + - unparam - unused - misspell issues: exclude-rules: - - text: 'differs only by capitalization to method' + - text: "differs only by capitalization to method" linters: - revive - - text: 'Use of weak random number generator' + - text: "Use of weak random number generator" linters: - gosec @@ -108,12 +108,12 @@ linters-settings: disabled: true - name: unused-parameter disabled: true - - name: unhandled-error + - name: unhandled-error disabled: true arguments: - - 'b.WriteString' - - 'respJSON.Write' - - 'fmt.Printf' - - 'fmt.Print' - - 'fmt.Println' - - 'fmt.Fprintf' + - "b.WriteString" + - "respJSON.Write" + - "fmt.Printf" + - "fmt.Print" + - "fmt.Println" + - "fmt.Fprintf" From b4540b5e2d3c19be7ba81c448a253994de82dbd3 Mon Sep 17 00:00:00 2001 From: Tuan Tran Date: Fri, 29 Mar 2024 00:18:52 +0700 Subject: [PATCH 14/18] fix syntax error and lint --- .../keeper/ibc_packet_handlers.go | 31 ++++++++++--------- x/participationrewards/keeper/distribution.go | 11 ++----- .../keeper/submodule_osmosis.go | 2 -- 3 files changed, 20 insertions(+), 24 deletions(-) diff --git a/x/interchainstaking/keeper/ibc_packet_handlers.go b/x/interchainstaking/keeper/ibc_packet_handlers.go index 545d8469e..4c44a768e 100644 --- a/x/interchainstaking/keeper/ibc_packet_handlers.go +++ b/x/interchainstaking/keeper/ibc_packet_handlers.go @@ -11,6 +11,7 @@ import ( "github.com/golang/protobuf/proto" // nolint:staticcheck "cosmossdk.io/math" + sdkmath "cosmossdk.io/math" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/telemetry" @@ -502,20 +503,22 @@ func (k *Keeper) HandleWithdrawForUser(ctx sdk.Context, zone *types.Zone, msg *b // New method to process per-validator withdrawals func (k Keeper) processPerValidatorWithdrawal(ctx sdk.Context, withdrawalRecord *types.WithdrawalRecord, msg *banktypes.MsgSend) error { - dlist := make(map[int]struct{}) - for i, dist := range withdrawalRecord.Distribution { - if msg.Amount[0].Amount.Equal(dist.Amount) { // check valoper here too? - dlist[i] = struct{}{} - // matched amount - if len(withdrawalRecord.Distribution) == len(dlist) { - // we just removed the last element - k.Logger(ctx).Info("found matching withdrawal; marking as completed") - k.UpdateWithdrawalRecordStatus(ctx, &withdrawalRecord, types.WithdrawStatusCompleted) - if err := k.BankKeeper.BurnCoins(ctx, types.EscrowModuleAccount, sdk.NewCoins(withdrawalRecord.BurnAmount)); err != nil { - // if we can't burn the coins, fail. - return err - } - k.Logger(ctx).Info("burned coins post-withdrawal", "coins", withdrawalRecord.BurnAmount) + dlist := make(map[int]struct{}) + for i, dist := range withdrawalRecord.Distribution { + if msg.Amount[0].Amount.Equal(dist.Amount) { // check valoper here too? + dlist[i] = struct{}{} + // matched amount + if len(withdrawalRecord.Distribution) == len(dlist) { + // we just removed the last element + k.Logger(ctx).Info("found matching withdrawal; marking as completed") + k.UpdateWithdrawalRecordStatus(ctx, withdrawalRecord, types.WithdrawStatusCompleted) + if err := k.BankKeeper.BurnCoins(ctx, types.EscrowModuleAccount, sdk.NewCoins(withdrawalRecord.BurnAmount)); err != nil { + // if we can't burn the coins, fail. + return err + } + k.Logger(ctx).Info("burned coins post-withdrawal", "coins", withdrawalRecord.BurnAmount) + } + } } if len(dlist) > 0 { withdrawalRecord.Distribution = k.removeMatchedDistributions(withdrawalRecord.Distribution, dlist) diff --git a/x/participationrewards/keeper/distribution.go b/x/participationrewards/keeper/distribution.go index 79d2a97d0..491a56a1f 100644 --- a/x/participationrewards/keeper/distribution.go +++ b/x/participationrewards/keeper/distribution.go @@ -66,16 +66,11 @@ func (k *Keeper) CalcTokenValues(ctx sdk.Context) (TokenValues, error) { baseIBCDenom = ibcDenom } else { zone, ok := k.icsKeeper.GetZone(ctx, pool.Denoms[ibcDenom].ChainID) - if !ok { - return false - } - - if pool.Denoms[ibcDenom].Denom == zone.BaseDenom { - queryIBCDenom = ibcDenom - valueDenom = zone.BaseDenom - } else { + if !ok || pool.Denoms[ibcDenom].Denom != zone.BaseDenom { return false } + queryIBCDenom = ibcDenom + valueDenom = zone.BaseDenom } } diff --git a/x/participationrewards/keeper/submodule_osmosis.go b/x/participationrewards/keeper/submodule_osmosis.go index 7c2f65d1b..18ae2b4ed 100644 --- a/x/participationrewards/keeper/submodule_osmosis.go +++ b/x/participationrewards/keeper/submodule_osmosis.go @@ -73,12 +73,10 @@ func (m *OsmosisModule) Hooks(ctx sdk.Context, k *Keeper) { }) } - func (*OsmosisModule) ValidateClaim(ctx sdk.Context, k *Keeper, msg *types.MsgSubmitClaim) (math.Int, error) { amount := sdk.ZeroInt() var lock osmolockup.PeriodLock for _, proof := range msg.Proofs { - var lock osmolockup.PeriodLock var err error switch proof.ProofType { From 24f448e0c4bada4e2e0c6359c4c2cedf4e190ec9 Mon Sep 17 00:00:00 2001 From: Tuan Tran Date: Fri, 29 Mar 2024 01:05:38 +0700 Subject: [PATCH 15/18] pass pointer as argument to mutate the validator --- x/interchainstaking/keeper/keeper.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x/interchainstaking/keeper/keeper.go b/x/interchainstaking/keeper/keeper.go index 2bc5b242d..9bf7f44c3 100644 --- a/x/interchainstaking/keeper/keeper.go +++ b/x/interchainstaking/keeper/keeper.go @@ -350,11 +350,11 @@ func (k *Keeper) SetValidatorForZone(ctx sdk.Context, zone *types.Zone, data []b return nil } - if err := k.handleJailStatusTransition(ctx, zone, val, validator); err != nil { + if err := k.handleJailStatusTransition(ctx, zone, &val, validator); err != nil { return err } - if err := k.updateValidatorFields(ctx, val, validator); err != nil { + if err := k.updateValidatorFields(ctx, &val, validator); err != nil { return err } @@ -407,7 +407,7 @@ func (k *Keeper) handleNewValidator(ctx sdk.Context, zone *types.Zone, validator return k.MakePerformanceDelegation(ctx, zone, validator.OperatorAddress) } -func (k *Keeper) handleJailStatusTransition(ctx sdk.Context, zone *types.Zone, val types.Validator, validator lsmstakingtypes.Validator) error { +func (k *Keeper) handleJailStatusTransition(ctx sdk.Context, zone *types.Zone, val *types.Validator, validator lsmstakingtypes.Validator) error { if !val.Jailed && validator.IsJailed() { k.Logger(ctx).Info("Transitioning validator to jailed state", "valoper", validator.OperatorAddress, "old_vp", val.VotingPower, "new_vp", validator.Tokens, "new_shares", validator.DelegatorShares, "old_shares", val.DelegatorShares) @@ -457,7 +457,7 @@ func (k *Keeper) handleJailStatusTransition(ctx sdk.Context, zone *types.Zone, v return nil } -func (k *Keeper) updateValidatorFields(ctx sdk.Context, val types.Validator, validator lsmstakingtypes.Validator) error { +func (k *Keeper) updateValidatorFields(ctx sdk.Context, val *types.Validator, validator lsmstakingtypes.Validator) error { if !val.CommissionRate.Equal(validator.GetCommission()) { k.Logger(ctx).Debug("Validator commission rate change; updating...", "valoper", validator.OperatorAddress, "oldRate", val.CommissionRate, "newRate", validator.GetCommission()) val.CommissionRate = validator.GetCommission() From 0d210447bb5fa333a5c77d1c09bd52513d8abf18 Mon Sep 17 00:00:00 2001 From: Tuan Tran Date: Fri, 29 Mar 2024 13:22:14 +0700 Subject: [PATCH 16/18] refactor reduntant err!=nil check --- x/interchainstaking/keeper/callbacks_test.go | 123 +++--------------- .../keeper/ibc_channel_handlers.go | 14 +- .../keeper/ibc_packet_handlers.go | 43 ++---- x/interchainstaking/keeper/keeper.go | 77 ++++++----- 4 files changed, 83 insertions(+), 174 deletions(-) diff --git a/x/interchainstaking/keeper/callbacks_test.go b/x/interchainstaking/keeper/callbacks_test.go index 11bcb746e..56ea30645 100644 --- a/x/interchainstaking/keeper/callbacks_test.go +++ b/x/interchainstaking/keeper/callbacks_test.go @@ -86,7 +86,19 @@ func (suite *KeeperTestSuite) setupIbc() (*app.Quicksilver, sdk.Context) { func (suite *KeeperTestSuite) TestHandleValsetCallback() { newVal := addressutils.GenerateValAddressForTest() - + // helper function to check if a query is found + checkQueryFound := func(require *require.Assertions, ctx sdk.Context, quicksilver *app.Quicksilver, in stakingtypes.Validators) { + foundQuery := false + _, addr, _ := bech32.DecodeAndConvert(in[0].OperatorAddress) + data := stakingtypes.GetValidatorKey(addr) + for _, i := range quicksilver.InterchainQueryKeeper.AllQueries(ctx) { + if i.QueryType == storeStakingKey && bytes.Equal(i.Request, data) { + foundQuery = true + break + } + } + require.True(foundQuery) + } tests := []struct { name string valset func(in stakingtypes.Validators) stakingtypes.QueryValidatorsResponse @@ -108,16 +120,7 @@ func (suite *KeeperTestSuite) TestHandleValsetCallback() { return stakingtypes.QueryValidatorsResponse{Validators: in} }, checks: func(require *require.Assertions, ctx sdk.Context, quicksilver *app.Quicksilver, in stakingtypes.Validators) { - foundQuery := false - _, addr, _ := bech32.DecodeAndConvert(in[0].OperatorAddress) - data := stakingtypes.GetValidatorKey(addr) - for _, i := range quicksilver.InterchainQueryKeeper.AllQueries(ctx) { - if i.QueryType == storeStakingKey && bytes.Equal(i.Request, data) { - foundQuery = true - break - } - } - require.True(foundQuery) + checkQueryFound(require, ctx, quicksilver, in) }, }, { @@ -128,22 +131,7 @@ func (suite *KeeperTestSuite) TestHandleValsetCallback() { return stakingtypes.QueryValidatorsResponse{Validators: in} }, checks: func(require *require.Assertions, ctx sdk.Context, quicksilver *app.Quicksilver, in stakingtypes.Validators) { - foundQuery := false - foundQuery2 := false - _, addr, _ := bech32.DecodeAndConvert(in[1].OperatorAddress) - data := stakingtypes.GetValidatorKey(addr) - _, addr2, _ := bech32.DecodeAndConvert(in[2].OperatorAddress) - data2 := stakingtypes.GetValidatorKey(addr2) - for _, i := range quicksilver.InterchainQueryKeeper.AllQueries(ctx) { - if i.QueryType == storeStakingKey && bytes.Equal(i.Request, data) { - foundQuery = true - } - if i.QueryType == storeStakingKey && bytes.Equal(i.Request, data2) { - foundQuery2 = true - } - } - require.True(foundQuery) - require.True(foundQuery2) + checkQueryFound(require, ctx, quicksilver, in) }, }, { @@ -153,16 +141,7 @@ func (suite *KeeperTestSuite) TestHandleValsetCallback() { return stakingtypes.QueryValidatorsResponse{Validators: in} }, checks: func(require *require.Assertions, ctx sdk.Context, quicksilver *app.Quicksilver, in stakingtypes.Validators) { - foundQuery := false - _, addr, _ := bech32.DecodeAndConvert(in[0].OperatorAddress) - data := stakingtypes.GetValidatorKey(addr) - for _, i := range quicksilver.InterchainQueryKeeper.AllQueries(ctx) { - if i.QueryType == storeStakingKey && bytes.Equal(i.Request, data) { - foundQuery = true - break - } - } - require.True(foundQuery) + checkQueryFound(require, ctx, quicksilver, in) }, }, { @@ -173,22 +152,7 @@ func (suite *KeeperTestSuite) TestHandleValsetCallback() { return stakingtypes.QueryValidatorsResponse{Validators: in} }, checks: func(require *require.Assertions, ctx sdk.Context, quicksilver *app.Quicksilver, in stakingtypes.Validators) { - foundQuery := false - foundQuery2 := false - _, addr, _ := bech32.DecodeAndConvert(in[1].OperatorAddress) - data := stakingtypes.GetValidatorKey(addr) - _, addr2, _ := bech32.DecodeAndConvert(in[2].OperatorAddress) - data2 := stakingtypes.GetValidatorKey(addr2) - for _, i := range quicksilver.InterchainQueryKeeper.AllQueries(ctx) { - if i.QueryType == storeStakingKey && bytes.Equal(i.Request, data) { - foundQuery = true - } - if i.QueryType == storeStakingKey && bytes.Equal(i.Request, data2) { - foundQuery2 = true - } - } - require.True(foundQuery) - require.True(foundQuery2) + checkQueryFound(require, ctx, quicksilver, in) }, }, { @@ -199,22 +163,7 @@ func (suite *KeeperTestSuite) TestHandleValsetCallback() { return stakingtypes.QueryValidatorsResponse{Validators: in} }, checks: func(require *require.Assertions, ctx sdk.Context, quicksilver *app.Quicksilver, in stakingtypes.Validators) { - foundQuery := false - foundQuery2 := false - _, addr, _ := bech32.DecodeAndConvert(in[1].OperatorAddress) - data := stakingtypes.GetValidatorKey(addr) - _, addr2, _ := bech32.DecodeAndConvert(in[2].OperatorAddress) - data2 := stakingtypes.GetValidatorKey(addr2) - for _, i := range quicksilver.InterchainQueryKeeper.AllQueries(ctx) { - if i.QueryType == storeStakingKey && bytes.Equal(i.Request, data) { - foundQuery = true - } - if i.QueryType == storeStakingKey && bytes.Equal(i.Request, data2) { - foundQuery2 = true - } - } - require.True(foundQuery) - require.True(foundQuery2) + checkQueryFound(require, ctx, quicksilver, in) }, }, { @@ -225,22 +174,7 @@ func (suite *KeeperTestSuite) TestHandleValsetCallback() { return stakingtypes.QueryValidatorsResponse{Validators: in} }, checks: func(require *require.Assertions, ctx sdk.Context, quicksilver *app.Quicksilver, in stakingtypes.Validators) { - foundQuery := false - foundQuery2 := false - _, addr, _ := bech32.DecodeAndConvert(in[0].OperatorAddress) - data := stakingtypes.GetValidatorKey(addr) - _, addr2, _ := bech32.DecodeAndConvert(in[2].OperatorAddress) - data2 := stakingtypes.GetValidatorKey(addr2) - for _, i := range quicksilver.InterchainQueryKeeper.AllQueries(ctx) { - if i.QueryType == storeStakingKey && bytes.Equal(i.Request, data) { - foundQuery = true - } - if i.QueryType == storeStakingKey && bytes.Equal(i.Request, data2) { - foundQuery2 = true - } - } - require.True(foundQuery) - require.True(foundQuery2) + checkQueryFound(require, ctx, quicksilver, in) }, }, { @@ -252,14 +186,7 @@ func (suite *KeeperTestSuite) TestHandleValsetCallback() { return stakingtypes.QueryValidatorsResponse{Validators: in} }, checks: func(require *require.Assertions, ctx sdk.Context, quicksilver *app.Quicksilver, in stakingtypes.Validators) { - foundQuery := false - data := stakingtypes.GetValidatorKey(newVal) - for _, i := range quicksilver.InterchainQueryKeeper.AllQueries(ctx) { - if i.QueryType == storeStakingKey && bytes.Equal(i.Request, data) { - foundQuery = true - } - } - require.True(foundQuery) + checkQueryFound(require, ctx, quicksilver, in) }, }, { @@ -269,15 +196,7 @@ func (suite *KeeperTestSuite) TestHandleValsetCallback() { return stakingtypes.QueryValidatorsResponse{Validators: in} }, checks: func(require *require.Assertions, ctx sdk.Context, quicksilver *app.Quicksilver, in stakingtypes.Validators) { - foundQuery := false - _, addr, _ := bech32.DecodeAndConvert(in[0].OperatorAddress) - data := stakingtypes.GetValidatorKey(addr) - for _, i := range quicksilver.InterchainQueryKeeper.AllQueries(ctx) { - if i.QueryType == storeStakingKey && bytes.Equal(i.Request, data) { - foundQuery = true - } - } - require.True(foundQuery) + checkQueryFound(require, ctx, quicksilver, in) }, }, } diff --git a/x/interchainstaking/keeper/ibc_channel_handlers.go b/x/interchainstaking/keeper/ibc_channel_handlers.go index 1591b23fd..61cdc4835 100644 --- a/x/interchainstaking/keeper/ibc_channel_handlers.go +++ b/x/interchainstaking/keeper/ibc_channel_handlers.go @@ -40,10 +40,14 @@ func (k *Keeper) HandleChannelOpenAck(ctx sdk.Context, portID, connectionID stri ctx.Logger().Info("found matching address", "chain", zone.ChainId, "address", address, "port", portID) portParts := strings.Split(portID, ".") - + if len(portParts) != 2 { + err := fmt.Errorf("unexpected portID format: got %s", portID) + ctx.Logger().Error(err.Error()) + return err + } switch { // deposit address - case len(portParts) == 2 && portParts[1] == types.ICASuffixDeposit: + case portParts[1] == types.ICASuffixDeposit: if zone.DepositAddress == nil { zone.DepositAddress, err = types.NewICAAccount(address, portID) @@ -73,7 +77,7 @@ func (k *Keeper) HandleChannelOpenAck(ctx sdk.Context, portID, connectionID stri } // withdrawal address - case len(portParts) == 2 && portParts[1] == types.ICASuffixWithdrawal: + case portParts[1] == types.ICASuffixWithdrawal: if zone.WithdrawalAddress == nil { zone.WithdrawalAddress, err = types.NewICAAccount(address, portID) if err != nil { @@ -83,7 +87,7 @@ func (k *Keeper) HandleChannelOpenAck(ctx sdk.Context, portID, connectionID stri } // delegation addresses - case len(portParts) == 2 && portParts[1] == types.ICASuffixDelegate: + case portParts[1] == types.ICASuffixDelegate: if zone.DelegationAddress == nil { zone.DelegationAddress, err = types.NewICAAccount(address, portID) if err != nil { @@ -94,7 +98,7 @@ func (k *Keeper) HandleChannelOpenAck(ctx sdk.Context, portID, connectionID stri } // performance address - case len(portParts) == 2 && portParts[1] == types.ICASuffixPerformance: + case portParts[1] == types.ICASuffixPerformance: if zone.PerformanceAddress == nil { ctx.Logger().Info("create performance account") zone.PerformanceAddress, err = types.NewICAAccount(address, portID) diff --git a/x/interchainstaking/keeper/ibc_packet_handlers.go b/x/interchainstaking/keeper/ibc_packet_handlers.go index 4c44a768e..32f4d4ff2 100644 --- a/x/interchainstaking/keeper/ibc_packet_handlers.go +++ b/x/interchainstaking/keeper/ibc_packet_handlers.go @@ -11,7 +11,6 @@ import ( "github.com/golang/protobuf/proto" // nolint:staticcheck "cosmossdk.io/math" - sdkmath "cosmossdk.io/math" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/telemetry" @@ -167,18 +166,12 @@ func handleMsgWithdrawDelegatorReward(k *Keeper, ctx sdk.Context, msg sdk.Msg, s return nil } k.Logger(ctx).Info("Rewards withdrawn") - if err := k.HandleWithdrawRewards(ctx, msg, connectionID); err != nil { - return err - } - return nil + return k.HandleWithdrawRewards(ctx, msg, connectionID) } func handleMsgRedeemTokensForShares(k *Keeper, ctx sdk.Context, msg sdk.Msg, msgResponse []byte, success bool, memo string, connectionID string) error { if !success { - if err := k.HandleFailedRedeemTokens(ctx, msg, memo); err != nil { - return err - } - return nil + return k.HandleFailedRedeemTokens(ctx, msg, memo) } response := lsmstakingtypes.MsgRedeemTokensForSharesResponse{} err := proto.Unmarshal(msgResponse, &response) @@ -187,10 +180,7 @@ func handleMsgRedeemTokensForShares(k *Keeper, ctx sdk.Context, msg sdk.Msg, msg return err } k.Logger(ctx).Info("Tokens redeemed for shares", "response", response) - if err := k.HandleRedeemTokens(ctx, msg, response.Amount, memo, connectionID); err != nil { - return err - } - return nil + return k.HandleRedeemTokens(ctx, msg, response.Amount, memo, connectionID) } func handleMsgTokenizeShares(k *Keeper, ctx sdk.Context, msg sdk.Msg, msgResponse []byte, success bool, memo string) error { @@ -204,18 +194,12 @@ func handleMsgTokenizeShares(k *Keeper, ctx sdk.Context, msg sdk.Msg, msgRespons return err } k.Logger(ctx).Info("Shares tokenized", "response", response) - if err := k.HandleTokenizedShares(ctx, msg, response.Amount, memo); err != nil { - return err - } - return nil + return k.HandleTokenizedShares(ctx, msg, response.Amount, memo) } func handleMsgDelegate(k *Keeper, ctx sdk.Context, msg sdk.Msg, msgResponse []byte, success bool, memo string) error { if !success { - if err := k.HandleFailedDelegate(ctx, msg, memo); err != nil { - return err - } - return nil + return k.HandleFailedDelegate(ctx, msg, memo) } response := stakingtypes.MsgDelegateResponse{} err := proto.Unmarshal(msgResponse, &response) @@ -224,10 +208,7 @@ func handleMsgDelegate(k *Keeper, ctx sdk.Context, msg sdk.Msg, msgResponse []by return err } k.Logger(ctx).Info("Delegated", "response", response) - if err := k.HandleDelegate(ctx, msg, memo); err != nil { - return err - } - return nil + return k.HandleDelegate(ctx, msg, memo) } func handleMsgBeginRedelegate(k *Keeper, ctx sdk.Context, msg sdk.Msg, msgResponse []byte, success bool, memo string) error { @@ -286,10 +267,7 @@ func handleMsgSend(k *Keeper, ctx sdk.Context, msg sdk.Msg, msgResponse []byte, return err } k.Logger(ctx).Info("Funds Transferred", "response", response) - if err := k.HandleCompleteSend(ctx, msg, memo, connectionID); err != nil { - return err - } - return nil + return k.HandleCompleteSend(ctx, msg, memo, connectionID) } func handleMsgSetWithdrawAddress(k *Keeper, ctx sdk.Context, msg sdk.Msg, msgResponse []byte, success bool) error { @@ -303,10 +281,7 @@ func handleMsgSetWithdrawAddress(k *Keeper, ctx sdk.Context, msg sdk.Msg, msgRes return err } k.Logger(ctx).Info("Withdraw Address Updated", "response", response) - if err := k.HandleUpdatedWithdrawAddress(ctx, msg); err != nil { - return err - } - return nil + return k.HandleUpdatedWithdrawAddress(ctx, msg) } func (*Keeper) HandleTimeout(_ sdk.Context, _ channeltypes.Packet) error { @@ -498,7 +473,7 @@ func (k *Keeper) HandleWithdrawForUser(ctx sdk.Context, zone *types.Zone, msg *b period := int64(k.GetParam(ctx, types.KeyValidatorSetInterval)) query := stakingtypes.QueryValidatorsRequest{} - return k.EmitValSetQuery(ctx, zone.ConnectionId, zone.ChainId, query, sdkmath.NewInt(period)) + return k.EmitValSetQuery(ctx, zone.ConnectionId, zone.ChainId, query, math.NewInt(period)) } // New method to process per-validator withdrawals diff --git a/x/interchainstaking/keeper/keeper.go b/x/interchainstaking/keeper/keeper.go index 9bf7f44c3..f6a5dad4a 100644 --- a/x/interchainstaking/keeper/keeper.go +++ b/x/interchainstaking/keeper/keeper.go @@ -408,48 +408,21 @@ func (k *Keeper) handleNewValidator(ctx sdk.Context, zone *types.Zone, validator } func (k *Keeper) handleJailStatusTransition(ctx sdk.Context, zone *types.Zone, val *types.Validator, validator lsmstakingtypes.Validator) error { - if !val.Jailed && validator.IsJailed() { - k.Logger(ctx).Info("Transitioning validator to jailed state", "valoper", validator.OperatorAddress, "old_vp", val.VotingPower, "new_vp", validator.Tokens, "new_shares", validator.DelegatorShares, "old_shares", val.DelegatorShares) + if val.Jailed == validator.IsJailed() { + return nil + } - var pk cryptotypes.PubKey - err := k.cdc.UnpackAny(validator.ConsensusPubkey, &pk) - if err != nil { - return err - } - consAddr := sdk.ConsAddress(pk.Address().Bytes()) - k.SetValidatorAddrByConsAddr(ctx, zone.ChainId, validator.OperatorAddress, consAddr) + if !val.Jailed { + k.Logger(ctx).Info("Transitioning validator to jailed state", "valoper", validator.OperatorAddress, "old_vp", val.VotingPower, "new_vp", validator.Tokens, "new_shares", validator.DelegatorShares, "old_shares", val.DelegatorShares) - err = k.EmitSigningInfoQuery(ctx, zone.ConnectionId, zone.ChainId, validator) + err := k.handleValidatorJailed(ctx, zone, val, validator) if err != nil { return err } - val.Jailed = true val.JailedSince = ctx.BlockTime() - - if !val.VotingPower.IsPositive() { - return fmt.Errorf("existing voting power must be greater than zero, received %s", val.VotingPower) - } - if validator.Tokens.IsNegative() { - return fmt.Errorf("incoming voting power must not be negative, received %s", validator.Tokens) - } - if validator.Tokens.IsZero() { - err = k.UpdateWithdrawalRecordsForSlash(ctx, zone, val.ValoperAddress, sdk.ZeroDec()) - if err != nil { - return err - } - } else { - prevRatio := val.DelegatorShares.Quo(sdk.NewDecFromInt(val.VotingPower)) - newRatio := validator.DelegatorShares.Quo(sdk.NewDecFromInt(validator.Tokens)) - delta := newRatio.Quo(prevRatio) - err = k.UpdateWithdrawalRecordsForSlash(ctx, zone, val.ValoperAddress, delta) - if err != nil { - return err - } - } } else if val.Jailed && !validator.IsJailed() { k.Logger(ctx).Debug("Transitioning validator to unjailed state", "valoper", validator.OperatorAddress) - val.Jailed = false val.JailedSince = time.Time{} } @@ -457,6 +430,34 @@ func (k *Keeper) handleJailStatusTransition(ctx sdk.Context, zone *types.Zone, v return nil } +func (k *Keeper) handleValidatorJailed(ctx sdk.Context, zone *types.Zone, val *types.Validator, validator lsmstakingtypes.Validator) error { + var pk cryptotypes.PubKey + err := k.cdc.UnpackAny(validator.ConsensusPubkey, &pk) + if err != nil { + return err + } + consAddr := sdk.ConsAddress(pk.Address().Bytes()) + k.SetValidatorAddrByConsAddr(ctx, zone.ChainId, validator.OperatorAddress, consAddr) + + err = k.EmitSigningInfoQuery(ctx, zone.ConnectionId, zone.ChainId, validator) + if err != nil { + return err + } + + val.Jailed = true + val.JailedSince = ctx.BlockTime() + + if !val.VotingPower.IsPositive() { + return fmt.Errorf("existing voting power must be greater than zero, received %s", val.VotingPower) + } + if validator.Tokens.IsNegative() { + return fmt.Errorf("incoming voting power must not be negative, received %s", validator.Tokens) + } + + // if the validator is jailed, we need to slash the validator + return k.handleUpdateWithdrawalRecords(ctx, zone, val, validator) +} + func (k *Keeper) updateValidatorFields(ctx sdk.Context, val *types.Validator, validator lsmstakingtypes.Validator) error { if !val.CommissionRate.Equal(validator.GetCommission()) { k.Logger(ctx).Debug("Validator commission rate change; updating...", "valoper", validator.OperatorAddress, "oldRate", val.CommissionRate, "newRate", validator.GetCommission()) @@ -491,6 +492,16 @@ func (k *Keeper) updateValidatorFields(ctx sdk.Context, val *types.Validator, va return nil } +func (k *Keeper) handleUpdateWithdrawalRecords(ctx sdk.Context, zone *types.Zone, val *types.Validator, validator lsmstakingtypes.Validator) error { + if validator.Tokens.IsZero() { + return k.UpdateWithdrawalRecordsForSlash(ctx, zone, val.ValoperAddress, sdk.ZeroDec()) + } + prevRatio := val.DelegatorShares.Quo(sdk.NewDecFromInt(val.VotingPower)) + newRatio := validator.DelegatorShares.Quo(sdk.NewDecFromInt(validator.Tokens)) + delta := newRatio.Quo(prevRatio) + return k.UpdateWithdrawalRecordsForSlash(ctx, zone, val.ValoperAddress, delta) +} + func (k *Keeper) depositInterval(ctx sdk.Context) zoneItrFn { return func(index int64, zone *types.Zone) (stop bool) { if zone.DepositAddress != nil { From e7dd00de4caba5a9e4910d456b3e5d9903530a21 Mon Sep 17 00:00:00 2001 From: Tuan Tran Date: Fri, 29 Mar 2024 13:28:18 +0700 Subject: [PATCH 17/18] extract sanity check to other func --- x/interchainstaking/keeper/redemptions.go | 33 ++++++++++++++++------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/x/interchainstaking/keeper/redemptions.go b/x/interchainstaking/keeper/redemptions.go index 77fc2de0f..1b403671c 100644 --- a/x/interchainstaking/keeper/redemptions.go +++ b/x/interchainstaking/keeper/redemptions.go @@ -374,16 +374,35 @@ WITHDRAWAL: } // sanity checks + sumOut, err := sumOutCheck(coinsOutPerValidator, tokensAllocatedForWithdrawalPerValidator, denom) + if err != nil { + return nil, nil, nil, err + } + sumIn, err := sumInCheck(amountToWithdrawPerWithdrawal, distributionsPerWithdrawal, denom) + if err != nil { + return nil, nil, nil, err + } + if !sumIn.Equal(sumOut) { + return nil, nil, nil, fmt.Errorf("sumIn <-> sumOut mismatch; sumIn = %s, sumOut = %s", sumIn.Amount.String(), sumOut.Amount.String()) + } + + return coinsOutPerValidator, txHashesPerValidator, distributionsPerWithdrawal, nil +} + +func sumOutCheck(coinsOutPerValidator map[string]sdk.Coin, tokensAllocatedForWithdrawalPerValidator map[string]sdkmath.Int, denom string) (sdk.Coin, error) { sumOut := sdk.NewCoin(denom, sdkmath.ZeroInt()) for valoper, coinPerVal := range coinsOutPerValidator { - if !coinPerVal.Amount.Equal(_tokensAllocatedForWithdrawalPerValidator[valoper]) { - return nil, nil, nil, fmt.Errorf("allocation <-> coinOut mismatch for %s; in = %v, out = %v", valoper, _tokensAllocatedForWithdrawalPerValidator[valoper], coinPerVal) + if !coinPerVal.Amount.Equal(tokensAllocatedForWithdrawalPerValidator[valoper]) { + return sdk.Coin{}, fmt.Errorf("allocation <-> coinOut mismatch for %s; in = %v, out = %v", valoper, tokensAllocatedForWithdrawalPerValidator[valoper], coinPerVal) } sumOut = sumOut.Add(coinPerVal) } + return sumOut, nil +} +func sumInCheck(amountToWithdrawPerWithdrawal map[string]sdk.Coin, distributionsPerWithdrawal map[string][]*types.Distribution, denom string) (sdk.Coin, error) { sumIn := sdk.NewCoin(denom, sdkmath.ZeroInt()) - for hash, tx := range _amountToWithdrawPerWithdrawal { + for hash, tx := range amountToWithdrawPerWithdrawal { sumIn = sumIn.Add(tx) dist := func(in []*types.Distribution) sdkmath.Int { sum := sdkmath.ZeroInt() @@ -394,13 +413,9 @@ WITHDRAWAL: }(distributionsPerWithdrawal[hash]) if !tx.Amount.Equal(dist) { - return nil, nil, nil, fmt.Errorf("amountToWithdrawPerWithdrawal <-> distributionsPerWithdrawal mismatch for %s; tx = %v, dist = %v", hash, tx, dist) + return sdk.Coin{}, fmt.Errorf("amountToWithdrawPerWithdrawal <-> distributionsPerWithdrawal mismatch for %s; tx = %v, dist = %v", hash, tx, dist) } } - if !sumIn.Equal(sumOut) { - return nil, nil, nil, fmt.Errorf("sumIn <-> sumOut mismatch; sumIn = %s, sumOut = %s", sumIn.Amount.String(), sumOut.Amount.String()) - } - - return coinsOutPerValidator, txHashesPerValidator, distributionsPerWithdrawal, nil + return sumIn, nil } From d873bc62bc323cfed409cb966c56bebf58c5a356 Mon Sep 17 00:00:00 2001 From: Tuan Tran Date: Fri, 29 Mar 2024 13:37:06 +0700 Subject: [PATCH 18/18] disable gocognit lint for cmd and app function --- app/export.go | 1 + app/upgrades/v1_5.go | 1 + cmd/quicksilverd/bulk_airdrop.go | 1 + cmd/quicksilverd/genaccounts.go | 1 + cmd/quicksilverd/genairdrop.go | 1 + test/simulation/state.go | 1 + .../keeper/proposal_handler.go | 231 ++++++++---------- x/interchainstaking/types/redemptions.go | 2 + 8 files changed, 114 insertions(+), 125 deletions(-) diff --git a/app/export.go b/app/export.go index 7efd8a3dc..83cbb1c70 100644 --- a/app/export.go +++ b/app/export.go @@ -53,6 +53,7 @@ func (app *Quicksilver) ExportAppStateAndValidators( // prepForZeroHeightGenesis prepares for fresh start at zero height // NOTE zero height genesis is a temporary feature which will be deprecated // in favor of export at a block height. +// nolint:gocognit func (app *Quicksilver) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []string) error { applyAllowedAddrs := false diff --git a/app/upgrades/v1_5.go b/app/upgrades/v1_5.go index f27f1e41d..ee814f1fa 100644 --- a/app/upgrades/v1_5.go +++ b/app/upgrades/v1_5.go @@ -591,6 +591,7 @@ func reimburseUsersWithdrawnOnLowRR(ctx sdk.Context, appKeepers *keepers.AppKeep // collateRequeuedWithdrawals will iterate, per zone, over requeued queued and active withdrawal records and // collate them into a single record for a delegator/recipient/epoch tuple. +// nolint:gocognit func collateRequeuedWithdrawals(ctx sdk.Context, appKeepers *keepers.AppKeepers) { appKeepers.InterchainstakingKeeper.IterateZones(ctx, func(_ int64, zone *icstypes.Zone) (stop bool) { newRecords := map[string]icstypes.WithdrawalRecord{} diff --git a/cmd/quicksilverd/bulk_airdrop.go b/cmd/quicksilverd/bulk_airdrop.go index 1eccf87f0..df9ae04ac 100644 --- a/cmd/quicksilverd/bulk_airdrop.go +++ b/cmd/quicksilverd/bulk_airdrop.go @@ -117,6 +117,7 @@ func AddZonedropCmd(defaultNodeHome string) *cobra.Command { } // BulkGenesisAirdropCmd returns add-genesis-airdrop cobra Command. +// nolint:gocognit func BulkGenesisAirdropCmd(defaultNodeHome string) *cobra.Command { cmd := &cobra.Command{ Use: "bulk-genesis-airdrop [file.csv] [chain_id]", diff --git a/cmd/quicksilverd/genaccounts.go b/cmd/quicksilverd/genaccounts.go index 664521dab..e38b0108b 100644 --- a/cmd/quicksilverd/genaccounts.go +++ b/cmd/quicksilverd/genaccounts.go @@ -28,6 +28,7 @@ const ( ) // AddGenesisAccountCmd returns add-genesis-account cobra Command. +// nolint:gocognit func AddGenesisAccountCmd(defaultNodeHome string) *cobra.Command { cmd := &cobra.Command{ Use: "add-genesis-account [address_or_key_name] [coin][,[coin]]", diff --git a/cmd/quicksilverd/genairdrop.go b/cmd/quicksilverd/genairdrop.go index b5314743c..35542e6e3 100644 --- a/cmd/quicksilverd/genairdrop.go +++ b/cmd/quicksilverd/genairdrop.go @@ -20,6 +20,7 @@ import ( ) // AddGenesisAirdropCmd returns add-genesis-airdrop cobra Command. +// nolint:gocognit func AddGenesisAirdropCmd(defaultNodeHome string) *cobra.Command { cmd := &cobra.Command{ Use: "add-genesis-airdrop [address] [chain_id] [allocation] [base_value]", diff --git a/test/simulation/state.go b/test/simulation/state.go index 2c66511d2..afabf7af2 100644 --- a/test/simulation/state.go +++ b/test/simulation/state.go @@ -31,6 +31,7 @@ import ( // AppStateFn returns the initial application state using a genesis or the simulation parameters. // It panics if the user provides files for both of them. // If a file is not given for the genesis or the sim params, it creates a randomized one. +// nolint:gocognit func AppStateFn(cdc codec.JSONCodec, simManager *module.SimulationManager) simtypes.AppStateFn { return func(r *rand.Rand, accs []simtypes.Account, config simtypes.Config, ) (appState json.RawMessage, simAccs []simtypes.Account, chainID string, genesisTimestamp time.Time) { diff --git a/x/interchainstaking/keeper/proposal_handler.go b/x/interchainstaking/keeper/proposal_handler.go index 873f06ac8..3af325088 100644 --- a/x/interchainstaking/keeper/proposal_handler.go +++ b/x/interchainstaking/keeper/proposal_handler.go @@ -139,150 +139,131 @@ func (k *Keeper) registerInterchainAccount(ctx sdk.Context, connectionID, portOw func (k *Keeper) HandleUpdateZoneProposal(ctx sdk.Context, p *types.UpdateZoneProposal) error { zone, found := k.GetZone(ctx, p.ChainId) if !found { - err := fmt.Errorf("unable to get registered zone for chain id: %s", p.ChainId) - return err + return fmt.Errorf("unable to get registered zone for chain id: %s", p.ChainId) } for _, change := range p.Changes { - switch change.Key { - case "base_denom": - if err := sdk.ValidateDenom(change.Value); err != nil { - return err - } - if k.BankKeeper.GetSupply(ctx, zone.LocalDenom).Amount.IsPositive() { - return errors.New("zone has assets minted, cannot update base_denom without potentially losing assets") - } - zone.BaseDenom = change.Value + if err := k.applyChange(ctx, &zone, *change); err != nil { + return err + } + } + + k.SetZone(ctx, &zone) + + k.Logger(ctx).Info("applied changes to zone", "changes", p.Changes, "zone", zone.ChainId) - case "local_denom": - if err := sdk.ValidateDenom(change.Value); err != nil { - return err - } - if k.BankKeeper.GetSupply(ctx, zone.LocalDenom).Amount.IsPositive() { - return errors.New("zone has assets minted, cannot update local_denom without potentially losing assets") - } + return nil +} + +func (k *Keeper) applyChange(ctx sdk.Context, zone *types.Zone, change types.UpdateZoneValue) error { + switch change.Key { + case "base_denom", "local_denom": + if err := sdk.ValidateDenom(change.Value); err != nil { + return err + } + if k.BankKeeper.GetSupply(ctx, zone.LocalDenom).Amount.IsPositive() { + return errors.New("zone has assets minted, cannot update denom without potentially losing assets") + } + if change.Key == "base_denom" { + zone.BaseDenom = change.Value + } else { zone.LocalDenom = change.Value + } + case "liquidity_module", "unbonding_enabled", "deposits_enabled", "return_to_sender", "is_118": + boolValue, err := strconv.ParseBool(change.Value) + if err != nil { + return err + } + switch change.Key { case "liquidity_module": - boolValue, err := strconv.ParseBool(change.Value) - if err != nil { - return err - } zone.LiquidityModule = boolValue - case "unbonding_enabled": - boolValue, err := strconv.ParseBool(change.Value) - if err != nil { - return err - } zone.UnbondingEnabled = boolValue - case "deposits_enabled": - boolValue, err := strconv.ParseBool(change.Value) - if err != nil { - return err - } zone.DepositsEnabled = boolValue - case "return_to_sender": - boolValue, err := strconv.ParseBool(change.Value) - if err != nil { - return err - } zone.ReturnToSender = boolValue - - case "messages_per_tx": - intVal, err := strconv.Atoi(change.Value) - if err != nil { - return err - } - if intVal < 1 { - return fmt.Errorf("invalid value for messages_per_tx: %d", intVal) - } - zone.MessagesPerTx = int64(intVal) - - case "account_prefix": - zone.AccountPrefix = change.Value - case "is_118": - boolValue, err := strconv.ParseBool(change.Value) - if err != nil { - return err - } zone.Is_118 = boolValue + } - case "connection_id": - if !strings.HasPrefix(change.Value, "connection-") { - return errors.New("unexpected connection format") - } - if zone.DepositAddress != nil || zone.DelegationAddress != nil || zone.PerformanceAddress != nil || zone.WithdrawalAddress != nil { - return errors.New("zone already intialised, cannot update connection_id") - } - if k.BankKeeper.GetSupply(ctx, zone.LocalDenom).Amount.IsPositive() { - return errors.New("zone has assets minted, cannot update connection_id without potentially losing assets") - } - - connection, found := k.IBCKeeper.ConnectionKeeper.GetConnection(ctx, change.Value) - if !found { - return errors.New("unable to fetch connection") - } - - clientState, found := k.IBCKeeper.ClientKeeper.GetClientState(ctx, connection.ClientId) - if !found { - return errors.New("unable to fetch client state") - } - - tmClientState, ok := clientState.(*tmclienttypes.ClientState) - if !ok { - return errors.New("error unmarshaling client state") - } - - if tmClientState.Status(ctx, k.IBCKeeper.ClientKeeper.ClientStore(ctx, connection.ClientId), k.IBCKeeper.Codec()) != ibcexported.Active { - return errors.New("new connection client state is not active") - } - - zone.ConnectionId = change.Value - - k.SetZone(ctx, &zone) - - // generate deposit account - portOwner := zone.ChainId + ".deposit" - if err := k.registerInterchainAccount(ctx, zone.ConnectionId, portOwner); err != nil { - return err - } - - // generate withdrawal account - portOwner = zone.ChainId + ".withdrawal" - if err := k.registerInterchainAccount(ctx, zone.ConnectionId, portOwner); err != nil { - return err - } - - // generate perf account - portOwner = zone.ChainId + ".performance" - if err := k.registerInterchainAccount(ctx, zone.ConnectionId, portOwner); err != nil { - return err - } - - // generate delegate accounts - portOwner = zone.ChainId + ".delegate" - if err := k.registerInterchainAccount(ctx, zone.ConnectionId, portOwner); err != nil { - return err - } - - period := int64(k.GetParam(ctx, types.KeyValidatorSetInterval)) - query := stakingtypes.QueryValidatorsRequest{} - err := k.EmitValSetQuery(ctx, zone.ConnectionId, zone.ChainId, query, sdkmath.NewInt(period)) - if err != nil { - return err - } - - default: - return fmt.Errorf("unexpected key '%s'", change.Key) + case "messages_per_tx": + intVal, err := strconv.Atoi(change.Value) + if err != nil { + return err } - } - k.SetZone(ctx, &zone) + if intVal < 1 { + return fmt.Errorf("invalid value for messages_per_tx: %d", intVal) + } + zone.MessagesPerTx = int64(intVal) - k.Logger(ctx).Info("applied changes to zone", "changes", p.Changes, "zone", zone.ChainId) + case "account_prefix": + zone.AccountPrefix = change.Value + + case "connection_id": + if err := k.applyConnectionIDChange(ctx, zone, change.Value); err != nil { + return err + } + + default: + return fmt.Errorf("unexpected key '%s'", change.Key) + } return nil } + +func (k *Keeper) applyConnectionIDChange(ctx sdk.Context, zone *types.Zone, connectionID string) error { + if !strings.HasPrefix(connectionID, "connection-") { + return errors.New("unexpected connection format") + } + if zone.DepositAddress != nil || zone.DelegationAddress != nil || zone.PerformanceAddress != nil || zone.WithdrawalAddress != nil { + return errors.New("zone already intialised, cannot update connection_id") + } + if k.BankKeeper.GetSupply(ctx, zone.LocalDenom).Amount.IsPositive() { + return errors.New("zone has assets minted, cannot update connection_id without potentially losing assets") + } + + connection, found := k.IBCKeeper.ConnectionKeeper.GetConnection(ctx, connectionID) + if !found { + return errors.New("unable to fetch connection") + } + + clientState, found := k.IBCKeeper.ClientKeeper.GetClientState(ctx, connection.ClientId) + if !found { + return errors.New("unable to fetch client state") + } + + tmClientState, ok := clientState.(*tmclienttypes.ClientState) + if !ok { + return errors.New("error unmarshaling client state") + } + + if tmClientState.Status(ctx, k.IBCKeeper.ClientKeeper.ClientStore(ctx, connection.ClientId), k.IBCKeeper.Codec()) != ibcexported.Active { + return errors.New("new connection client state is not active") + } + + zone.ConnectionId = connectionID + + // generate deposit account + portOwner := zone.ChainId + ".deposit" + if err := k.registerInterchainAccount(ctx, zone.ConnectionId, portOwner); err != nil { + return err + } + + // generate withdrawal account + portOwner = zone.ChainId + ".withdrawal" + if err := k.registerInterchainAccount(ctx, zone.ConnectionId, portOwner); err != nil { + return err + } + + // generate perf account + portOwner = zone.ChainId + ".performance" + if err := k.registerInterchainAccount(ctx, zone.ConnectionId, portOwner); err != nil { + return err + } + + // generate delegate account + portOwner = zone.ChainId + ".delegate" + return k.registerInterchainAccount(ctx, zone.ConnectionId, portOwner) +} diff --git a/x/interchainstaking/types/redemptions.go b/x/interchainstaking/types/redemptions.go index ec985b026..44e3a7df2 100644 --- a/x/interchainstaking/types/redemptions.go +++ b/x/interchainstaking/types/redemptions.go @@ -21,6 +21,7 @@ func filter(in map[string]math.Int) map[string]math.Int { return out } +// nolint:gocognit func DetermineAllocationsForUndelegation(currentAllocations map[string]math.Int, lockedAllocations map[string]bool, currentSum math.Int, targetAllocations ValidatorIntents, availablePerValidator map[string]math.Int, amount sdk.Coins) (map[string]math.Int, error) { outWeights := make(map[string]math.Int) if len(amount) != 1 { @@ -179,6 +180,7 @@ func DetermineAllocationsForUndelegation(currentAllocations map[string]math.Int, return filter(outWeights), nil } +// nolint:gocognit func DetermineAllocationsForUndelegationPredef(currentAllocations map[string]math.Int, lockedAllocations map[string]bool, currentSum math.Int, targetAllocations ValidatorIntents, availablePerValidator map[string]math.Int, amount sdk.Coins) (map[string]math.Int, error) { outWeights := make(map[string]math.Int, len(availablePerValidator)) if len(amount) != 1 {