-
Notifications
You must be signed in to change notification settings - Fork 40
feat: add cosmos liquid staking module #267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds a new Cosmos SDK native liquid staking module Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant CLI
participant App
participant MsgServer
participant LiquidKeeper
participant StakingKeeper
participant BankKeeper
participant DistKeeper
User->>CLI: submit MsgTokenizeShares tx
CLI->>App: broadcast tx
App->>MsgServer: deliver MsgTokenizeShares
MsgServer->>LiquidKeeper: TokenizeShares(goCtx, msg)
LiquidKeeper->>StakingKeeper: ValidateUnbondAmount / Unbond / Delegate
LiquidKeeper->>BankKeeper: MintCoins / SendCoinsFromModuleToAccount
LiquidKeeper->>LiquidKeeper: AddTokenizeShareRecord / Update totals & shares
LiquidKeeper->>DistKeeper: (withdraw/calc rewards on withdraw flows)
LiquidKeeper->>MsgServer: return result & events
MsgServer->>App: finalize tx result
App->>CLI: tx response
CLI->>User: display result
Note: BeginBlock flow removes expired locks via Keeper.BeginBlocker -> RemoveExpiredTokenizeShareLocks. Upgrade flow invokes LiquidKeeper.Hooks().AfterValidatorCreated for existing validators during migration. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 22
🧹 Nitpick comments (16)
x/liquid/keeper/keeper_test.go (1)
90-106: Consider expanding parameter testing coverage.The current test only covers basic parameter get/set operations. Consider adding tests for:
- Parameter validation edge cases
- Invalid parameter values
- Parameter update authorization
func (s *KeeperTestSuite) TestParams() { ctx, keeper := s.ctx, s.lsmKeeper require := s.Require() expParams := lsmtypes.DefaultParams() // check that the empty keeper loads the default resParams, err := keeper.GetParams(ctx) require.NoError(err) require.Equal(expParams, resParams) expParams.GlobalLiquidStakingCap = sdkmath.LegacyNewDec(1) expParams.ValidatorLiquidStakingCap = sdkmath.LegacyNewDec(1) require.NoError(keeper.SetParams(ctx, expParams)) resParams, err = keeper.GetParams(ctx) require.NoError(err) require.True(expParams.Equal(resParams)) + + // Test invalid parameters + invalidParams := lsmtypes.DefaultParams() + invalidParams.GlobalLiquidStakingCap = sdkmath.LegacyNewDec(-1) // negative value + require.Error(keeper.SetParams(ctx, invalidParams)) }x/liquid/keeper/keeper.go (1)
41-44: Consider returning an error instead of panicking in the constructor.Based on the KYVE codebase preferences for error handling, consider returning an error from
NewKeeperinstead of panicking when the authority address is invalid. This allows proper error handling at the application initialization level.-func NewKeeper( +func NewKeeper( cdc codec.BinaryCodec, storeService storetypes.KVStoreService, ak types.AccountKeeper, bk types.BankKeeper, sk types.StakingKeeper, dk types.DistributionKeeper, authority string, -) *Keeper { +) (*Keeper, error) { // ensure that authority is a valid AccAddress if _, err := ak.AddressCodec().StringToBytes(authority); err != nil { - panic("authority is not a valid acc address") + return nil, fmt.Errorf("authority is not a valid acc address: %w", err) } - return &Keeper{ + return &Keeper{ storeService: storeService, cdc: cdc, authKeeper: ak, bankKeeper: bk, stakingKeeper: sk, distKeeper: dk, authority: authority, - } + }, nil }x/liquid/types/codec.go (1)
15-24: Verify amino route compatibility requirements.The amino route names use "gaia/" prefixes instead of "kyve/". While this might be intentional for compatibility with existing Cosmos liquid staking implementations, please verify this is the intended behavior for KYVE.
Also, address the TODO comment about
MsgUnbondValidator- either implement it or document why it's excluded.Would you like me to help determine the appropriate amino route names for KYVE or assist with the missing
MsgUnbondValidatorimplementation?x/liquid/keeper/genesis.go (1)
18-46: Consider returning errors instead of panicking.The genesis initialization logic is correct, including the important consistency check for tokenize share record IDs. However, consider returning errors instead of panicking to allow callers to handle failures gracefully.
The ID consistency validation (lines 37-39) is particularly well implemented.
x/liquid/types/params.go (1)
39-56: Consider migration to protobuf codec.These functions use
*codec.LegacyAminowhich is deprecated in favor of protobuf codecs in newer Cosmos SDK versions. Verify if this is required for backward compatibility or if it should be updated to use protobuf serialization.x/liquid/types/errors.go (1)
13-13: Fix typo in error message."Allowd" should be "Allowed" in the error message.
- ErrOnlyBondDenomAllowdForTokenize = errors.Register(ModuleName, 106, "only bond denom is allowed for tokenize") + ErrOnlyBondDenomAllowedForTokenize = errors.Register(ModuleName, 106, "only bond denom is allowed for tokenize")x/liquid/keeper/distribution.go (2)
157-157: Inconsistent error message.The error message uses "external funds" while other methods use just "funds". This inconsistency could be confusing.
-return nil, errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to receive external funds", ownerAddr) +return nil, errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to receive funds", ownerAddr)
170-178: Consider returning partial errors to caller.When a validator is not found, the method silently continues without informing the caller. This could hide issues where records point to non-existent validators.
Consider collecting and returning information about skipped records so callers can handle these cases if needed:
+type SkippedRecord struct { + RecordID uint64 + Reason string +} + -func (k Keeper) WithdrawAllTokenizeShareRecordReward(ctx sdk.Context, ownerAddr sdk.AccAddress) (sdk.Coins, error) { +func (k Keeper) WithdrawAllTokenizeShareRecordReward(ctx sdk.Context, ownerAddr sdk.AccAddress) (sdk.Coins, []SkippedRecord, error) {x/liquid/README.md (2)
10-31: Fix markdown list indentation.The unordered lists should use 2-space indentation instead of 4 spaces for consistency with markdown standards.
* [State](#state) - * [TotalLiquidStakedTokens](#totalliquidstakedtokens) - * [PendingTokenizeShareAuthorizations](#pendingtokenizeshareauthorizations) + * [TotalLiquidStakedTokens](#totalliquidstakedtokens) + * [PendingTokenizeShareAuthorizations](#pendingtokenizeshareauthorizations) * [Messages](#messages) - * [MsgUpdateParams](#msgupdateparams) - * [MsgTokenizeShares](#msgtokenizeshares) + * [MsgUpdateParams](#msgupdateparams) + * [MsgTokenizeShares](#msgtokenizeshares)Apply similar changes to all nested list items throughout the document.
25-25: Fix invalid link fragment.The link fragment
#endblockerdoesn't match the actual section header.- * [EndBlocker](#endblocker) + * [Events](#events)x/liquid/keeper/tokenize_share_record.go (4)
23-42: Review panic usage in store operations.Both functions use
panic(err)when store operations fail. While this may be acceptable for critical store failures that indicate system-level issues, consider whether these should return errors instead to allow callers to handle failures gracefully.If store operations can fail due to recoverable conditions, consider this pattern:
func (k Keeper) GetLastTokenizeShareRecordID(ctx context.Context) (uint64, error) { store := k.storeService.OpenKVStore(ctx) bytes, err := store.Get(types.LastTokenizeShareRecordIDKey) if err != nil { - panic(err) + return 0, err } if bytes == nil { return 0, nil } - return sdk.BigEndianToUint64(bytes) + return sdk.BigEndianToUint64(bytes), nil }
44-58: Consider using Unmarshal instead of MustUnmarshal.Using
MustUnmarshalat line 56 can cause a panic if the stored data is corrupted or malformed. For better error handling, consider usingUnmarshaland returning the error.- k.cdc.MustUnmarshal(bz, &tokenizeShareRecord) - return tokenizeShareRecord, nil + if err := k.cdc.Unmarshal(bz, &tokenizeShareRecord); err != nil { + return tokenizeShareRecord, errorsmod.Wrap(err, "failed to unmarshal tokenize share record") + } + return tokenizeShareRecord, nil
60-77: Consider logging errors instead of silently continuing.The function silently continues when
GetTokenizeShareRecordfails (line 72), which may hide data corruption or consistency issues. Consider logging these errors for debugging purposes.tokenizeShareRecord, err := k.GetTokenizeShareRecord(ctx, id.Value) if err != nil { + // Log the error to help with debugging data consistency issues + k.Logger(ctx).Error("failed to get tokenize share record", "id", id.Value, "error", err) continue }
134-158: Consider transaction atomicity for multi-index deletion.The function performs three separate delete operations (lines 145, 149, 153). If one fails, the record could be left in an inconsistent state across indices. Consider wrapping these operations to ensure all-or-nothing behavior.
While the current implementation handles individual errors properly, you might want to collect all operations and only commit if all succeed, or implement a rollback mechanism for failed operations.
x/liquid/keeper/msg_server.go (1)
56-236: Well-implemented tokenization logic with comprehensive validationsThe TokenizeShares handler properly validates all inputs, checks lock status, enforces vesting restrictions, and manages the complex unbonding/re-delegation flow. The liquid staking cap enforcement is correctly implemented.
One minor suggestion: Consider adding more specific error context when retrieving the validator after unbonding (line 209).
x/liquid/keeper/grpc_query.go (1)
145-174: Consider using regularPaginateinstead ofFilteredPaginateSince the filter function always returns
true, usingquery.Paginatewould be more efficient thanquery.FilteredPaginate.- pageRes, err := query.FilteredPaginate(valStore, req.Pagination, func(key, value []byte, accumulate bool) (bool, error) { + pageRes, err := query.Paginate(valStore, req.Pagination, func(key, value []byte) error { var tokenizeShareRecord types.TokenizeShareRecord if err := k.cdc.Unmarshal(value, &tokenizeShareRecord); err != nil { - return false, err + return err } - - if accumulate { - records = append(records, tokenizeShareRecord) - } - return true, nil + records = append(records, tokenizeShareRecord) + return nil })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
x/liquid/types/genesis.pb.gois excluded by!**/*.pb.gox/liquid/types/liquid.pb.gois excluded by!**/*.pb.gox/liquid/types/query.pb.gois excluded by!**/*.pb.gox/liquid/types/query.pb.gw.gois excluded by!**/*.pb.gw.gox/liquid/types/tx.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (43)
CHANGELOG.md(1 hunks)api/kyve/liquid/module/v1/module.pulsar.go(1 hunks)app/app.go(4 hunks)app/app_config.go(5 hunks)proto/kyve/liquid/LICENSE(1 hunks)proto/kyve/liquid/module/v1/module.proto(1 hunks)proto/kyve/liquid/v1beta1/genesis.proto(1 hunks)proto/kyve/liquid/v1beta1/liquid.proto(1 hunks)proto/kyve/liquid/v1beta1/query.proto(1 hunks)proto/kyve/liquid/v1beta1/tx.proto(1 hunks)x/liquid/LICENSE(1 hunks)x/liquid/README.md(1 hunks)x/liquid/autocli.go(1 hunks)x/liquid/client/cli/tx.go(1 hunks)x/liquid/keeper/abci.go(1 hunks)x/liquid/keeper/distribution.go(1 hunks)x/liquid/keeper/genesis.go(1 hunks)x/liquid/keeper/grpc_query.go(1 hunks)x/liquid/keeper/grpc_query_test.go(1 hunks)x/liquid/keeper/hooks.go(1 hunks)x/liquid/keeper/keeper.go(1 hunks)x/liquid/keeper/keeper_test.go(1 hunks)x/liquid/keeper/liquid_stake.go(1 hunks)x/liquid/keeper/liquid_stake_test.go(1 hunks)x/liquid/keeper/msg_server.go(1 hunks)x/liquid/keeper/params.go(1 hunks)x/liquid/keeper/tokenize_share_record.go(1 hunks)x/liquid/keeper/tokenize_share_record_test.go(1 hunks)x/liquid/module.go(1 hunks)x/liquid/types/codec.go(1 hunks)x/liquid/types/errors.go(1 hunks)x/liquid/types/events.go(1 hunks)x/liquid/types/expected_keepers.go(1 hunks)x/liquid/types/genesis.go(1 hunks)x/liquid/types/keys.go(1 hunks)x/liquid/types/liquid_validator.go(1 hunks)x/liquid/types/mocks/AccountKeeper.go(1 hunks)x/liquid/types/mocks/BankKeeper.go(1 hunks)x/liquid/types/mocks/DistributionKeeper.go(1 hunks)x/liquid/types/mocks/StakingKeeper.go(1 hunks)x/liquid/types/msg.go(1 hunks)x/liquid/types/params.go(1 hunks)x/liquid/types/tokenize_share_record.go(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
x/liquid/keeper/distribution.go (1)
Learnt from: troykessler
PR: #244
File: x/multi_coin_rewards/keeper/exported.go:19-23
Timestamp: 2025-02-11T09:41:10.529Z
Learning: When handling errors in reward distribution, prefer returning all rewards as a fallback to prevent potential loss of funds, rather than restricting to native rewards only.
x/liquid/types/mocks/StakingKeeper.go (1)
Learnt from: troykessler
PR: #211
File: x/stakers/keeper/exported_functions.go:334-342
Timestamp: 2024-12-20T08:52:36.443Z
Learning: The GetValidatorPoolStakes() method ensures that the effective stake cannot exceed the validator's total bonded tokens, so no additional check is necessary.
x/liquid/keeper/liquid_stake.go (1)
Learnt from: troykessler
PR: #211
File: x/stakers/keeper/exported_functions.go:334-342
Timestamp: 2024-12-20T08:52:36.443Z
Learning: The GetValidatorPoolStakes() method ensures that the effective stake cannot exceed the validator's total bonded tokens, so no additional check is necessary.
x/liquid/keeper/liquid_stake_test.go (1)
Learnt from: troykessler
PR: #211
File: x/stakers/keeper/exported_functions.go:334-342
Timestamp: 2024-12-20T08:52:36.443Z
Learning: The GetValidatorPoolStakes() method ensures that the effective stake cannot exceed the validator's total bonded tokens, so no additional check is necessary.
app/app_config.go (1)
Learnt from: troykessler
PR: #244
File: x/multi_coin_rewards/keeper/logic_distribution.go:45-47
Timestamp: 2025-02-11T09:41:07.954Z
Learning: In the KYVE codebase, prefer returning errors over panicking to allow proper error handling at the BeginBlock level, especially for operations that could fail like module-to-module transfers.
🧬 Code Graph Analysis (19)
x/liquid/types/msg.go (1)
x/liquid/types/tx.pb.go (6)
MsgWithdrawTokenizeShareRecordReward(550-553)MsgWithdrawTokenizeShareRecordReward(557-557)MsgWithdrawTokenizeShareRecordReward(558-560)MsgWithdrawAllTokenizeShareRecordReward(632-634)MsgWithdrawAllTokenizeShareRecordReward(640-640)MsgWithdrawAllTokenizeShareRecordReward(641-643)
x/liquid/keeper/distribution.go (3)
x/liquid/keeper/keeper.go (1)
Keeper(21-29)x/liquid/types/errors.go (2)
ErrNoValidatorFound(23-23)ErrNotTokenizeShareRecordOwner(11-11)x/liquid/types/events.go (3)
EventTypeWithdrawTokenizeShareReward(8-8)AttributeKeyWithdrawAddress(16-16)AttributeKeyAmount(14-14)
x/liquid/keeper/hooks.go (2)
x/liquid/keeper/keeper.go (1)
Keeper(21-29)x/liquid/types/liquid_validator.go (1)
NewLiquidValidator(10-15)
x/liquid/keeper/keeper_test.go (11)
x/liquid/keeper/keeper.go (2)
Keeper(21-29)NewKeeper(32-55)x/liquid/types/expected_keepers.go (3)
StakingKeeper(37-57)BankKeeper(22-34)AccountKeeper(14-19)x/liquid/types/mocks/StakingKeeper.go (2)
StakingKeeper(20-22)NewStakingKeeper(942-953)x/liquid/types/mocks/BankKeeper.go (2)
BankKeeper(14-16)NewBankKeeper(561-572)x/liquid/types/mocks/AccountKeeper.go (2)
AccountKeeper(16-18)NewAccountKeeper(223-234)x/liquid/types/query.pb.go (1)
QueryClient(1278-1307)x/liquid/types/keys.go (2)
StoreKey(15-15)ModuleName(12-12)x/liquid/types/mocks/DistributionKeeper.go (1)
NewDistributionKeeper(208-219)x/liquid/types/params.go (1)
DefaultParams(31-36)x/liquid/types/codec.go (1)
RegisterInterfaces(30-44)x/liquid/keeper/msg_server.go (1)
NewMsgServerImpl(32-34)
x/liquid/types/params.go (1)
x/liquid/types/liquid.pb.go (3)
Params(70-77)Params(81-81)Params(82-84)
x/liquid/types/mocks/AccountKeeper.go (1)
x/liquid/types/expected_keepers.go (1)
AccountKeeper(14-19)
x/liquid/keeper/tokenize_share_record_test.go (2)
x/liquid/keeper/keeper_test.go (1)
KeeperTestSuite(37-47)x/liquid/types/liquid.pb.go (3)
TokenizeShareRecord(113-118)TokenizeShareRecord(122-122)TokenizeShareRecord(123-125)
x/liquid/types/mocks/BankKeeper.go (1)
x/liquid/types/expected_keepers.go (1)
BankKeeper(22-34)
x/liquid/types/tokenize_share_record.go (1)
x/liquid/types/liquid.pb.go (3)
TokenizeShareRecord(113-118)TokenizeShareRecord(122-122)TokenizeShareRecord(123-125)
x/liquid/types/mocks/DistributionKeeper.go (1)
x/liquid/types/expected_keepers.go (1)
DistributionKeeper(60-64)
x/liquid/keeper/keeper.go (2)
x/liquid/types/expected_keepers.go (4)
AccountKeeper(14-19)BankKeeper(22-34)StakingKeeper(37-57)DistributionKeeper(60-64)x/liquid/types/keys.go (1)
ModuleName(12-12)
x/liquid/autocli.go (3)
x/liquid/module.go (2)
AppModule(104-111)AppModule(164-164)x/liquid/types/query.pb.go (1)
Query_serviceDesc(1718-1718)x/liquid/types/tx.pb.go (1)
Msg_serviceDesc(1125-1125)
x/liquid/types/mocks/StakingKeeper.go (2)
x/liquid/types/expected_keepers.go (1)
StakingKeeper(37-57)x/liquid/types/liquid.pb.go (3)
Params(70-77)Params(81-81)Params(82-84)
x/liquid/keeper/abci.go (1)
x/liquid/keeper/keeper.go (1)
Keeper(21-29)
x/liquid/keeper/liquid_stake.go (6)
x/liquid/keeper/keeper.go (1)
Keeper(21-29)x/liquid/types/keys.go (6)
TotalLiquidStakedTokensKey(33-33)GetTokenizeSharesLockKey(64-66)TokenizeSharesLockPrefix(34-34)GetTokenizeShareAuthorizationTimeKey(70-73)TokenizeSharesUnlockQueuePrefix(35-35)GetLiquidValidatorKey(40-42)x/liquid/types/liquid.pb.go (11)
LiquidValidator(268-274)LiquidValidator(278-278)LiquidValidator(279-281)TokenizeShareLockStatus(33-33)TokenizeShareLockStatus(65-67)TOKENIZE_SHARE_LOCK_STATUS_UNLOCKED(41-41)TOKENIZE_SHARE_LOCK_STATUS_LOCKED(39-39)TOKENIZE_SHARE_LOCK_STATUS_LOCK_EXPIRING(44-44)PendingTokenizeShareAuthorizations(183-185)PendingTokenizeShareAuthorizations(189-189)PendingTokenizeShareAuthorizations(190-192)x/liquid/types/errors.go (5)
ErrGlobalLiquidStakingCapExceeded(14-14)ErrTotalLiquidStakedUnderflow(20-20)ErrValidatorLiquidStakingCapExceeded(15-15)ErrValidatorLiquidSharesUnderflow(19-19)ErrNoValidatorFound(23-23)x/liquid/types/genesis.pb.go (3)
TokenizeShareLock(108-115)TokenizeShareLock(119-119)TokenizeShareLock(120-122)x/liquid/types/liquid_validator.go (2)
MustMarshalValidator(17-19)UnmarshalValidator(22-25)
x/liquid/types/errors.go (1)
x/liquid/types/keys.go (1)
ModuleName(12-12)
x/liquid/types/codec.go (2)
x/liquid/types/tx.pb.go (24)
MsgUpdateParams(40-48)MsgUpdateParams(52-52)MsgUpdateParams(53-55)MsgTokenizeShares(136-141)MsgTokenizeShares(145-145)MsgTokenizeShares(146-148)MsgRedeemTokensForShares(223-226)MsgRedeemTokensForShares(230-230)MsgRedeemTokensForShares(231-233)MsgTransferTokenizeShareRecord(308-312)MsgTransferTokenizeShareRecord(316-316)MsgTransferTokenizeShareRecord(317-319)MsgDisableTokenizeShares(389-391)MsgDisableTokenizeShares(395-395)MsgDisableTokenizeShares(396-398)MsgEnableTokenizeShares(465-467)MsgEnableTokenizeShares(471-471)MsgEnableTokenizeShares(472-474)MsgWithdrawTokenizeShareRecordReward(550-553)MsgWithdrawTokenizeShareRecordReward(557-557)MsgWithdrawTokenizeShareRecordReward(558-560)MsgWithdrawAllTokenizeShareRecordReward(632-634)MsgWithdrawAllTokenizeShareRecordReward(640-640)MsgWithdrawAllTokenizeShareRecordReward(641-643)x/liquid/types/liquid.pb.go (3)
Params(70-77)Params(81-81)Params(82-84)
x/liquid/keeper/msg_server.go (7)
x/liquid/keeper/keeper.go (1)
Keeper(21-29)x/liquid/types/tx.pb.go (49)
MsgServer(922-946)MsgUpdateParams(40-48)MsgUpdateParams(52-52)MsgUpdateParams(53-55)MsgUpdateParamsResponse(99-100)MsgUpdateParamsResponse(104-104)MsgUpdateParamsResponse(105-107)MsgTokenizeShares(136-141)MsgTokenizeShares(145-145)MsgTokenizeShares(146-148)MsgTokenizeSharesResponse(177-179)MsgTokenizeSharesResponse(183-183)MsgTokenizeSharesResponse(184-186)MsgRedeemTokensForShares(223-226)MsgRedeemTokensForShares(230-230)MsgRedeemTokensForShares(231-233)MsgRedeemTokensForSharesResponse(263-265)MsgRedeemTokensForSharesResponse(269-269)MsgRedeemTokensForSharesResponse(270-272)MsgTransferTokenizeShareRecord(308-312)MsgTransferTokenizeShareRecord(316-316)MsgTransferTokenizeShareRecord(317-319)MsgTransferTokenizeShareRecordResponse(349-350)MsgTransferTokenizeShareRecordResponse(356-356)MsgTransferTokenizeShareRecordResponse(357-359)MsgDisableTokenizeShares(389-391)MsgDisableTokenizeShares(395-395)MsgDisableTokenizeShares(396-398)MsgDisableTokenizeSharesResponse(428-429)MsgDisableTokenizeSharesResponse(433-433)MsgDisableTokenizeSharesResponse(434-436)MsgEnableTokenizeShares(465-467)MsgEnableTokenizeShares(471-471)MsgEnableTokenizeShares(472-474)MsgEnableTokenizeSharesResponse(504-506)MsgEnableTokenizeSharesResponse(510-510)MsgEnableTokenizeSharesResponse(511-513)MsgWithdrawTokenizeShareRecordReward(550-553)MsgWithdrawTokenizeShareRecordReward(557-557)MsgWithdrawTokenizeShareRecordReward(558-560)MsgWithdrawTokenizeShareRecordRewardResponse(590-591)MsgWithdrawTokenizeShareRecordRewardResponse(599-599)MsgWithdrawTokenizeShareRecordRewardResponse(600-602)MsgWithdrawAllTokenizeShareRecordReward(632-634)MsgWithdrawAllTokenizeShareRecordReward(640-640)MsgWithdrawAllTokenizeShareRecordReward(641-643)MsgWithdrawAllTokenizeShareRecordRewardResponse(673-674)MsgWithdrawAllTokenizeShareRecordRewardResponse(682-682)MsgWithdrawAllTokenizeShareRecordRewardResponse(683-685)x/liquid/types/liquid.pb.go (9)
Params(70-77)Params(81-81)Params(82-84)TOKENIZE_SHARE_LOCK_STATUS_LOCKED(39-39)TOKENIZE_SHARE_LOCK_STATUS_LOCK_EXPIRING(44-44)TokenizeShareRecord(113-118)TokenizeShareRecord(122-122)TokenizeShareRecord(123-125)TOKENIZE_SHARE_LOCK_STATUS_UNLOCKED(41-41)x/liquid/types/errors.go (11)
ErrTokenizeSharesDisabledForAccount(16-16)ErrOnlyBondDenomAllowdForTokenize(13-13)ErrExceedingFreeVestingDelegations(12-12)ErrInsufficientShares(8-8)ErrRedelegationInProgress(7-7)ErrNotEnoughBalance(21-21)ErrTinyRedemptionAmount(22-22)ErrTokenizeShareRecordNotExists(9-9)ErrNotTokenizeShareRecordOwner(11-11)ErrTokenizeSharesAlreadyDisabledForAccount(18-18)ErrTokenizeSharesAlreadyEnabledForAccount(17-17)x/liquid/keeper/liquid_stake.go (1)
CheckVestedDelegationInVestingAccount(440-459)x/liquid/types/keys.go (2)
TokenizeShareModuleAccountPrefix(21-21)ModuleName(12-12)x/liquid/types/events.go (9)
EventTypeTokenizeShares(5-5)AttributeKeyDelegator(11-11)AttributeKeyValidator(10-10)AttributeKeyShareOwner(12-12)AttributeKeyShareRecordID(13-13)AttributeKeyAmount(14-14)AttributeKeyTokenizedShares(15-15)EventTypeRedeemShares(6-6)EventTypeTransferTokenizeShareRecord(7-7)
x/liquid/types/liquid_validator.go (1)
x/liquid/types/liquid.pb.go (3)
LiquidValidator(268-274)LiquidValidator(278-278)LiquidValidator(279-281)
🪛 markdownlint-cli2 (0.17.2)
x/liquid/README.md
11-11: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
12-12: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
14-14: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
15-15: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
16-16: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
17-17: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
18-18: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
19-19: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
20-20: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
21-21: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
23-23: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
25-25: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
25-25: Link fragments should be valid
(MD051, link-fragments)
26-26: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
29-29: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
30-30: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
31-31: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (88)
proto/kyve/liquid/LICENSE (1)
1-201: LGTM! Proper license coverage for the liquid staking module.The Apache License 2.0 provides appropriate legal coverage for the newly integrated liquid staking module from Cosmos Gaia.
x/liquid/LICENSE (1)
1-201: LGTM! Consistent licensing for the liquid staking implementation.The Apache License 2.0 provides proper legal coverage for the liquid staking module implementation files, consistent with the protobuf license.
CHANGELOG.md (2)
19-21: LGTM! Proper documentation of the liquid staking module addition.The changelog entry correctly documents the new feature with appropriate state machine breaking change indicator and PR reference.
25-25: Good clarification of the affected module.Adding the module specification (
x/bundles) improves clarity for the existing storage cost payout improvement.x/liquid/keeper/abci.go (1)
9-14: LGTM! Standard ABCI lifecycle implementation.The
BeginBlockermethod correctly follows Cosmos SDK patterns for cleanup operations, appropriately unwrapping the context and handling errors from the expired lock removal.app/app.go (4)
86-86: LGTM! Proper module import.The side-effects import for the liquid module follows the established pattern used by other modules.
99-99: LGTM! Necessary keeper import.The explicit import of the liquid keeper package is needed for direct keeper usage in the App struct.
165-166: LGTM! Proper keeper field addition.The LiquidKeeper field is correctly placed with other Cosmos module keepers and follows the established naming convention.
316-316: LGTM! Correct dependency injection integration.The LiquidKeeper is properly included in the dependency injection configuration, ensuring it will be initialized with the necessary dependencies.
x/liquid/types/tokenize_share_record.go (2)
12-18: LGTM: Well-documented hardcoded module name for address stability.The intentional hardcoding of the "lsm" module name with explanatory comment is a good practice to ensure address stability across potential future module refactorings.
20-22: LGTM: Clean denomination generation logic.The share token denomination format using lowercase validator and record ID is straightforward and follows a logical naming convention.
proto/kyve/liquid/module/v1/module.proto (1)
11-24: LGTM: Standard and well-structured module protobuf definition.The module configuration follows Cosmos SDK conventions with appropriate fields for authority delegation and bech32 prefix configuration. The go_import path correctly points to the KYVE chain module.
x/liquid/types/genesis.go (2)
5-19: LGTM: Clean genesis state constructor pattern.The constructor follows standard Cosmos SDK patterns for initializing genesis state with all required fields.
21-25: LGTM: Appropriate default genesis state.The default genesis state correctly initializes with default parameters, leaving other fields as zero values which is appropriate for a new module.
x/liquid/keeper/keeper_test.go (2)
37-47: LGTM: Well-structured test suite with comprehensive mocking.The test suite properly sets up all necessary components with mocked dependencies, following Cosmos SDK testing best practices.
49-88: LGTM: Comprehensive test setup with proper service registration.The setup method correctly initializes the keeper with mocked dependencies, registers interfaces, and sets up query/message servers for comprehensive testing.
x/liquid/types/liquid_validator.go (3)
9-15: LGTM: Standard constructor with appropriate zero initialization.The constructor properly initializes a new
LiquidValidatorwith zero liquid shares using the appropriatemath.LegacyZeroDec()function.
17-19: LGTM: Proper "Must" pattern implementation.The
MustMarshalValidatorfunction correctly follows the Cosmos SDK "Must" pattern that panics on marshaling errors, which is appropriate for cases where marshaling should never fail.
21-25: LGTM: Clean unmarshaling with proper error handling.The unmarshaling function properly handles errors and returns both the unmarshaled value and any error encountered.
x/liquid/types/msg.go (1)
1-24: LGTM! Clean implementation of message constructors.The message type definitions follow standard Cosmos SDK patterns with proper interface assertions and straightforward constructor functions. The implementation is minimal, focused, and correctly structured.
x/liquid/keeper/keeper.go (1)
57-61: LGTM! Proper logger implementation.The Logger method correctly follows the Cosmos SDK pattern for module-specific logging with proper context unwrapping and module name prefixing.
proto/kyve/liquid/v1beta1/genesis.proto (2)
15-38: LGTM! Well-structured genesis state definition.The GenesisState message properly encapsulates all necessary state for module initialization, including params, records, ID tracking, and token amounts. The use of custom types for math.Int and proper gogoproto annotations is correct.
40-52: LGTM! Appropriate lock state definition.The TokenizeShareLock message correctly defines the lock state with address, status, and completion time. The timestamp handling with gogoproto annotations is properly configured.
x/liquid/keeper/tokenize_share_record_test.go (2)
14-21: LGTM! Good test coverage for ID management.The test properly verifies both getting the initial default value and setting/getting a custom ID value. The assertions are correct and the test structure is clean.
23-69: LGTM! Comprehensive test coverage for record operations.The test effectively covers multiple scenarios:
- Adding records with different owners and module accounts
- Retrieving records by ID and denomination
- Querying all records and filtering by owner
- Proper assertion of expected results
The test data setup is well-structured and the assertions verify the correct behavior.
app/app_config.go (5)
67-68: LGTM! Proper import additions for liquid module.The imports correctly add the necessary liquid module API and types for integration.
114-114: LGTM! Appropriate genesis order placement.The liquid module is correctly placed in the genesis initialization order after other core KYVE modules, which is appropriate for a staking-related module.
146-146: LGTM! Correct begin blocker inclusion.Adding the liquid module to the begin blockers is appropriate as it likely needs to process state changes and handle expired locks during block processing.
196-197: LGTM! Necessary module account permissions.The liquid module correctly requires both minter and burner permissions for its module account, which are essential for liquid staking operations involving token minting and burning.
379-382: LGTM! Proper module configuration registration.The liquid module is correctly registered in the appConfig with proper configuration wrapping, following the established pattern used by other modules.
x/liquid/keeper/params.go (4)
27-41: LGTM!Proper parameter retrieval implementation with appropriate default handling and error propagation.
43-47: LGTM!Clean convenience method with proper error propagation.
49-53: LGTM!Clean convenience method with proper error propagation.
16-25: Parameter validation confirmed in MsgUpdateParams handlerValidation is performed in x/liquid/keeper/msg_server.go – the
MsgUpdateParamsmethod callsmsg.Params.Validate()beforeSetParams, so the “no validation” contract onSetParamsis intentional and safe.• x/liquid/keeper/msg_server.go:
if err := msg.Params.Validate(); err != nil { return nil, err }x/liquid/types/codec.go (1)
30-44: LGTM!Proper interface registration following Cosmos SDK conventions. All message types are correctly registered as
sdk.Msgimplementations and the message service descriptor is registered.x/liquid/keeper/genesis.go (2)
48-75: LGTM!Proper handling of tokenize share lock statuses with appropriate state setup for both LOCKED and LOCK_EXPIRING cases. The panic on unsupported status is appropriate for genesis validation.
77-90: LGTM!Comprehensive genesis export that includes all relevant module state. The exported fields match what's expected for complete state reconstruction.
x/liquid/types/params.go (2)
58-65: LGTM!Clean validation pattern that checks both parameters and returns appropriate errors.
67-97: LGTM!Comprehensive validation with proper type checking, boundary validation, and descriptive error messages. The 100% upper bound check is appropriate for percentage-based caps.
x/liquid/types/errors.go (1)
6-24: LGTM! Comprehensive error definitions.The error definitions provide good coverage of liquid staking scenarios with descriptive messages. The non-sequential error codes appear intentional, possibly for compatibility with Cosmos Gaia or existing SDK patterns.
x/liquid/autocli.go (1)
19-122: LGTM! Well-structured AutoCLI configuration.The AutoCLI implementation is comprehensive and follows best practices:
- Query commands cover all liquid staking operations with proper examples
- Dynamic Bech32 prefixes ensure accurate example addresses
- Transaction commands are appropriately disabled in favor of custom implementations
- Consistent naming and descriptions across all commands
x/liquid/keeper/grpc_query_test.go (2)
20-79: LGTM! Comprehensive test coverage for single validator query.The test cases properly cover:
- Edge cases (nil/empty requests)
- Invalid but well-formed addresses
- Valid existing validators
- Proper error handling and response validation
81-192: LGTM! Thorough pagination testing for multiple validators.The test implementation excellently covers:
- Default behavior (empty request returns all)
- Limit-based pagination
- Offset pagination
- Count total functionality
- Proper response validation for each scenario
x/liquid/types/mocks/DistributionKeeper.go (1)
1-219: LGTM! Auto-generated mock follows standard patterns.The mock implementation correctly:
- Implements all three DistributionKeeper interface methods
- Uses proper testify/mock patterns with expecter helpers
- Includes constructor with cleanup registration
- Method signatures match the expected interface exactly
Since this is auto-generated code, the implementation follows established patterns and correctly mocks the required interface.
proto/kyve/liquid/v1beta1/liquid.proto (4)
15-38: LGTM! Well-defined parameters with proper annotations.The Params message correctly defines:
- Global and validator-specific liquid staking caps as decimal types
- Appropriate protobuf annotations for Go generation
- Proper amino naming and field tags
40-48: LGTM! TokenizeShareRecord structure is well-designed.The message properly captures:
- Unique ID for each tokenized share record
- Owner and module account addresses
- Associated validator information
- Proper equality annotation for comparisons
58-70: LGTM! Comprehensive enum for tokenization lock status.The enum properly defines all necessary states:
- Unspecified default value
- Locked/unlocked states
- Lock expiring transition state
- Proper protobuf enum configuration
85-101: LGTM! LiquidValidator message is properly structured.The message correctly defines:
- Validator operator address with proper cosmos address annotation
- Liquid shares as decimal type with appropriate tags
- Generator options for Go code generation
x/liquid/types/events.go (1)
3-17: LGTM! Well-organized event constants following Cosmos SDK conventions.The event definitions are comprehensive and properly structured:
- Event types cover all major liquid staking operations
- Attribute keys are descriptive and consistent
- Naming follows established Cosmos SDK patterns
- Good organization with clear categorization
x/liquid/types/mocks/AccountKeeper.go (1)
1-235: LGTM! Well-structured auto-generated mock.The mock implementation correctly follows the testify/mock patterns and provides all necessary methods for testing the AccountKeeper interface. The panic behavior for missing return values ensures test correctness.
x/liquid/keeper/tokenize_share_record.go (3)
96-109: LGTM with same MustUnmarshal consideration.The iterator pattern is implemented correctly. Same recommendation as previous functions regarding
MustUnmarshalusage applies here.
111-132: LGTM! Well-structured function with proper validation.The function correctly:
- Validates record doesn't already exist
- Updates multiple indices atomically
- Uses structured error handling
- Maintains data consistency across indices
160-201: LGTM! Helper functions follow established patterns.The remaining helper functions (
hasTokenizeShareRecord,setTokenizeShareRecord, etc.) are well-implemented and consistent with the established patterns in the file. They properly handle the multi-index storage requirements.x/liquid/client/cli/tx.go (3)
26-47: LGTM! Standard root command implementation.The function follows Cosmos SDK conventions and properly registers all transaction subcommands.
49-107: LGTM! Comprehensive input validation and good UX.The function includes:
- Proper address validation for validator and delegator addresses
- Coin amount parsing with validation
- Clear help text with practical examples
- Consistent error handling
109-343: LGTM! Well-implemented CLI commands with consistent patterns.The remaining commands demonstrate:
- Consistent input validation
- Proper error handling
- Clear help documentation
- Correct message construction
- Good use of Cosmos SDK utilities
x/liquid/types/mocks/BankKeeper.go (1)
1-572: LGTM! Auto-generated mock aligns with interface definition.The mock correctly implements all methods from the
BankKeeperinterface defined inx/liquid/types/expected_keepers.go(lines 21-33). The implementation includes:
- All required methods with correct signatures
- Proper expectation handling with testify/mock
- Type-safe call structures for fluent expectations
- Constructor with automatic test cleanup
x/liquid/types/keys.go (2)
10-37: LGTM! Well-defined module constants and key prefixes.The constants and key prefixes are properly defined with:
- Unique byte prefixes to avoid key collisions
- Descriptive names following conventions
- Proper module account prefix naming
39-73: LGTM! Proper key construction utilities.The key construction functions demonstrate good practices:
- Use of
address.MustLengthPrefixfor length-prefixed address keyssdk.Uint64ToBigEndianfor consistent uint64 encodingsdk.FormatTimeBytesfor timestamp keys- Proper composite key construction for multi-part indices
proto/kyve/liquid/v1beta1/tx.proto (2)
18-52: LGTM! Comprehensive service definition for liquid staking.The
Msgservice properly defines all liquid staking operations:
- Parameter updates
- Share tokenization and redemption
- Record ownership transfers
- Enable/disable tokenization controls
- Reward withdrawal operations
All methods follow consistent naming and documentation patterns.
54-197: LGTM! Well-structured message definitions with proper annotations.The message definitions demonstrate good practices:
- Proper
cosmos.msg.v1.signerannotations for transaction authorization- Consistent
amino.nameannotations (inheriting "gaia/" prefix from source)- Appropriate
gogoprotooptions for Go code generation- Proper field types with validation tags
- Clear documentation for each message type
api/kyve/liquid/module/v1/module.pulsar.go (1)
1-14: LGTM!The generated protobuf code follows the standard patterns and includes proper attribution for the original source from Cosmos Gaia.
x/liquid/keeper/liquid_stake_test.go (7)
1-26: LGTM!The file header properly attributes the original source and imports are well-organized.
28-38: LGTM!Simple and effective test for basic Set/Get functionality of total liquid staked tokens.
79-268: Excellent test coverage!Well-structured table-driven test with comprehensive coverage of various liquid staking cap scenarios. Good use of mocks with proper cleanup.
270-346: LGTM!Well-designed test that properly validates liquid staking cap enforcement and the behavior when caps are adjusted.
380-606: LGTM!Comprehensive test coverage for tokenize shares lock functionality, including proper testing of time-based expiration and queue mechanisms.
608-622: LGTM!Correctly tests the identification of ICA module accounts as liquid staking providers.
624-721: LGTM!Comprehensive test coverage for vesting account delegation checks with well-structured test cases covering all important scenarios.
x/liquid/types/mocks/StakingKeeper.go (1)
1-953: LGTM!Auto-generated mock implementation follows standard mockery patterns and correctly implements all methods of the StakingKeeper interface.
x/liquid/keeper/msg_server.go (5)
38-54: LGTM!UpdateParams handler correctly validates authority and parameters before updating.
238-382: Excellent implementation of share redemptionThe RedeemTokensForShares handler properly handles all edge cases including precision rounding for full redemptions, prevents tiny redemptions, and correctly manages the tokenize share record lifecycle including deletion when fully redeemed.
384-432: LGTM! Good security checksThe TransferTokenizeShareRecord handler properly validates ownership and includes important security checks to prevent transfers to blocked addresses.
486-501: LGTM!Simple and correct implementation for withdrawing rewards from a single tokenize share record.
503-518: LGTM!Correct implementation for withdrawing rewards from all tokenize share records owned by an address.
x/liquid/keeper/grpc_query.go (10)
38-44: LGTM!The
Paramsquery implementation is correct and follows best practices for error handling.
47-61: LGTM!The
LiquidValidatorquery properly validates input, converts addresses using the appropriate codec, and handles errors correctly.
64-90: LGTM!The
LiquidValidatorsquery correctly implements pagination and proper error handling for retrieving all liquid validators.
93-107: LGTM!The implementation correctly queries tokenize share records by ID with proper validation and error handling. The nolint comment appropriately acknowledges the proto-derived naming constraint.
110-124: LGTM!The
TokenizeShareRecordByDenomquery correctly validates input and retrieves records by denomination with proper error handling.
127-142: LGTM!The
TokenizeShareRecordsOwnedquery properly validates input, converts addresses using the appropriate codec, and retrieves records by owner.
177-186: LGTM!The implementation correctly returns the last tokenize share record ID with proper validation. The nolint comment appropriately acknowledges the proto-derived naming constraint.
189-226: LGTM!The
TotalTokenizeSharedAssetsquery correctly calculates the total tokenized assets by aggregating delegation tokens across all records with proper error handling.
231-240: LGTM!The
TotalLiquidStakedquery correctly returns the total liquid staked tokens as a string with proper validation.
268-339: LGTM!The
TokenizeShareRecordRewardquery implementation is comprehensive, correctly handling edge cases for missing validators/delegations and properly aggregating rewards including module account balances.x/liquid/keeper/liquid_stake.go (2)
73-142: LGTM!The liquid staking cap check functions correctly implement the cap validation logic with proper handling of the
sharesAlreadyBondedflag to avoid double counting.
440-459: LGTM!The
CheckVestedDelegationInVestingAccountutility function correctly implements the vesting delegation check logic without any error handling issues.x/liquid/types/expected_keepers.go (1)
1-64: LGTM!The expected keeper interfaces are well-defined, follow Cosmos SDK conventions, and properly return errors where operations can fail. This provides good abstraction for dependency injection and testing.
# Conflicts: # CHANGELOG.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
♻️ Duplicate comments (1)
x/liquid/module.go (1)
93-98: Don’t panic during gRPC gateway route registration.Panicking can crash the node at startup if registration fails. Log the error instead and continue.
func (AppModuleBasic) RegisterGRPCGatewayRoutes(clientCtx client.Context, mux *gwruntime.ServeMux) { if err := types.RegisterQueryHandlerClient(context.Background(), mux, types.NewQueryClient(clientCtx)); err != nil { - panic(err) + clientCtx.Logger.Error("failed to register gRPC gateway routes", "module", types.ModuleName, "err", err) } }
🧹 Nitpick comments (14)
docs/config.json (1)
82-94: Optional: keep KYVE module entries grouped/sorted for readabilityNot a blocker, but consider placing the Liquid entry near other kyve/* module entries in a predictable order (e.g., alphabetical) to keep the list easy to scan as it grows.
docs/static/openapi.yml (5)
10050-10052: Fix grammar in TotalLiquidStaked summaryUse “a liquid staking provider.”
- Query for total liquid staked (including tokenized shares or owned by an - liquid staking provider) + Query for total liquid staked (including tokenized shares or owned by a + liquid staking provider)
9810-9811: Tighten summary phrasingRemove duplicate “by” and clarify.
- summary: Query for individual tokenize share record information by share by id + summary: Query tokenize share record information by id
9774-9776: Fix grammar in module_account titleSubject-verb agreement and add “the” for clarity.
- title: module account take the role of delegator + title: module account takes the role of the delegatorAlso applies to: 9827-9829, 9883-9885, 9940-9942
9718-9730: Consider documenting lock status as an enum (if applicable)If the proto defines discrete lock states, expose them as an enum to improve client UX. If it’s free-form, ignore.
Would you like me to propose the enum values based on the proto?
10192-10195: Remove stray line break in TokenizeShareRecordReward descriptionThere’s an isolated “the” and extra blank line, which renders oddly in Swagger UIs.
- QueryTokenizeShareRecordRewardResponse is the response type for - the - - Query/TokenizeShareRecordReward RPC method. + QueryTokenizeShareRecordRewardResponse is the response type for the + Query/TokenizeShareRecordReward RPC method.proto/kyve/liquid/v1beta1/tx.proto (5)
41-44: Fix typo in comments (“addresseses”).Minor but user-facing in generated docs.
- // addresseses stake after it has been disabled + // address's stake after it has been disabled
45-47: Polish message docstrings (grammar and correctness).
- Use “rewards” plural.
- Clarify ownership phrasing.
- “tokenize” -> “tokenized”.
- Last response comment references the wrong message.
- // WithdrawTokenizeShareRecordReward defines a method to withdraw reward for + // WithdrawTokenizeShareRecordReward defines a method to withdraw rewards for // an owning TokenizeShareRecord ... - // WithdrawAllTokenizeShareRecordReward defines a method to withdraw reward - // for all owning TokenizeShareRecord + // WithdrawAllTokenizeShareRecordReward defines a method to withdraw rewards + // for all owned TokenizeShareRecords ... -// MsgTransferTokenizeShareRecord transfer a tokenize share record +// MsgTransferTokenizeShareRecord transfers a tokenized share record ... -// MsgWithdrawAllTokenizeShareRecordRewardResponse defines the -// Msg/WithdrawTokenizeShareRecordReward response type. +// MsgWithdrawAllTokenizeShareRecordRewardResponse defines the +// Msg/WithdrawAllTokenizeShareRecordReward response type.Also applies to: 49-51, 113-115, 195-197
83-87: Annotate addresses with cosmos.AddressString for consistency and validation.Only
authorityis annotated. Best practice is to annotate all address strings to aid client generation and validation.- string delegator_address = 1 [(gogoproto.moretags) = "yaml:\"delegator_address\""]; - string validator_address = 2 [(gogoproto.moretags) = "yaml:\"validator_address\""]; + string delegator_address = 1 [(gogoproto.moretags) = "yaml:\"delegator_address\"", (cosmos_proto.scalar) = "cosmos.AddressString"]; + string validator_address = 2 [(gogoproto.moretags) = "yaml:\"validator_address\"", (cosmos_proto.scalar) = "cosmos.AddressString"]; ... - string delegator_address = 1 [(gogoproto.moretags) = "yaml:\"delegator_address\""]; + string delegator_address = 1 [(gogoproto.moretags) = "yaml:\"delegator_address\"", (cosmos_proto.scalar) = "cosmos.AddressString"]; ... - string sender = 2; - string new_owner = 3; + string sender = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"]; + string new_owner = 3 [(cosmos_proto.scalar) = "cosmos.AddressString"]; ... - string delegator_address = 1 [(gogoproto.moretags) = "yaml:\"delegator_address\""]; + string delegator_address = 1 [(gogoproto.moretags) = "yaml:\"delegator_address\"", (cosmos_proto.scalar) = "cosmos.AddressString"]; ... - string delegator_address = 1 [(gogoproto.moretags) = "yaml:\"delegator_address\""]; + string delegator_address = 1 [(gogoproto.moretags) = "yaml:\"delegator_address\"", (cosmos_proto.scalar) = "cosmos.AddressString"]; ... - string owner_address = 1 [(gogoproto.moretags) = "yaml:\"owner_address\""]; + string owner_address = 1 [(gogoproto.moretags) = "yaml:\"owner_address\"", (cosmos_proto.scalar) = "cosmos.AddressString"]; ... - string owner_address = 1 [(gogoproto.moretags) = "yaml:\"owner_address\""]; + string owner_address = 1 [(gogoproto.moretags) = "yaml:\"owner_address\"", (cosmos_proto.scalar) = "cosmos.AddressString"];Optional: add yaml tags for
sender,new_owner, andtokenized_share_ownerto mirror others.Also applies to: 103-105, 121-124, 139-140, 154-155, 175-176, 192-193
175-177: Use consistent field name for TokenizeShareRecord IDs.Other messages use
tokenize_share_record_id; here it’srecord_id. Rename for API consistency (name-only change; field number unchanged).- uint64 record_id = 2; + uint64 tokenize_share_record_id = 2;
86-87: Consider tagging tokenized_share_owner like other addresses.It’s an address but lacks yaml and cosmos address annotations. Add for parity.
- string tokenized_share_owner = 4; + string tokenized_share_owner = 4 [(gogoproto.moretags) = "yaml:\"tokenized_share_owner\"", (cosmos_proto.scalar) = "cosmos.AddressString"];app/upgrades/v2_2/store.go (1)
11-11: Use the typed store key constant instead of a string literal.Prevents drift if the store key ever changes and improves readability.
Apply within this hunk:
- Added: []string{"liquid"}, + Added: []string{liquidtypes.StoreKey},Additional change outside this hunk (add import):
import ( storetypes "cosmossdk.io/store/types" upgradetypes "cosmossdk.io/x/upgrade/types" "github.com/cosmos/cosmos-sdk/baseapp" + liquidtypes "github.com/KYVENetwork/chain/x/liquid/types" )app/upgrades/v2_2/upgrade.go (1)
7-9: Minor: avoid reusing thestakingKeepername for both the import alias and the parameter.This can be confusing to readers since the parameter shadows the import alias within the function scope. Consider renaming the parameter to
skfor clarity.x/liquid/module.go (1)
120-135: Minor: AppModuleBasic.ak is unused in the current code path.If you don’t plan to use the account keeper in AppModuleBasic, consider removing it from the struct to reduce noise. If it will be used soon (e.g., for address codecs), feel free to ignore.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
x/liquid/types/query.pb.gois excluded by!**/*.pb.gox/liquid/types/query.pb.gw.gois excluded by!**/*.pb.gw.gox/liquid/types/tx.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (9)
CHANGELOG.md(1 hunks)app/app.go(5 hunks)app/upgrades/v2_2/store.go(1 hunks)app/upgrades/v2_2/upgrade.go(3 hunks)docs/config.json(1 hunks)docs/static/openapi.yml(1 hunks)proto/kyve/liquid/v1beta1/tx.proto(1 hunks)x/liquid/module.go(1 hunks)x/liquid/types/expected_keepers.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/app.go
- x/liquid/types/expected_keepers.go
- CHANGELOG.md
🧰 Additional context used
🧬 Code graph analysis (2)
app/upgrades/v2_2/upgrade.go (1)
x/liquid/keeper/hooks.go (1)
Hooks(20-22)
x/liquid/module.go (3)
x/liquid/types/expected_keepers.go (4)
AccountKeeper(18-23)BankKeeper(26-38)StakingKeeper(41-61)DistributionKeeper(64-68)x/liquid/types/query.pb.gw.go (1)
RegisterQueryHandlerClient(825-1068)x/liquid/keeper/keeper.go (2)
Keeper(21-29)NewKeeper(32-55)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test / test
- GitHub Check: Summary
🔇 Additional comments (8)
docs/config.json (2)
82-94: LGTM: Liquid module query mapping added correctlyThe new kyve/liquid/v1beta1 mapping mirrors the pattern used by other KYVE modules (Params → LiquidParams, Query → QueryLiquid). Looks consistent and should prevent operationId/tag collisions for Params and Query.
82-94: docs/config.json is valid JSON – duplicate-ID check inconclusive
- ✅ docs/config.json parses without errors.
⚠️ All./tmp-swagger-gen/.../query.swagger.jsonfiles were missing locally, so no operationIds were inspected.- To avoid CI or doc-generation failures, please regenerate your swagger artifacts (e.g. via your proto-to-swagger build) and rerun the duplicate-ID scan before merging. If any duplicate
operationIds are reported, add them under theoperationIds.renamesection indocs/config.json.docs/static/openapi.yml (2)
9526-9659: Pagination parameters look consistent and completeThe inclusion of key/offset/limit/count_total/reverse mirrors Cosmos SDK conventions and should work well with gRPC-Gateway and clients.
Also applies to: 9920-10045
10133-10139: Confirmed: HTTP path matches proto annotationThe OpenAPI spec entry
/kyve/liquid/v1beta1/{owner_address}/tokenize_share_record_rewards(docs/static/openapi.yml:10132) exactly mirrors thegoogle.api.httpannotation onTokenizeShareRecordRewardinproto/kyve/liquid/v1beta1/query.proto(line 83). No changes are necessary.proto/kyve/liquid/v1beta1/tx.proto (3)
16-16: Confirm go_package matches actual module path after vendoring.Ensure the generated Go paths (github.com/KYVENetwork/chain/x/liquid/types) are correct relative to where this module lands; mismatches break imports.
If this repo module path differs in go.mod, adjust go_package accordingly. I can generate a follow-up PR if needed.
18-52: Service definition LGTM.Service wiring, cosmos.msg annotation, and RPC set mirror Gaia’s surface with KYVE types. No issues found.
90-92: MsgServer Response Payloads VerifiedAll gRPC MsgServer methods return response messages that exactly match their
.protodefinitions:
MsgTokenizeSharesResponsecarries a singleCoin amountfield, and the keeper implementation returns&MsgTokenizeSharesResponse{Amount: shareToken}, matching the proto .MsgEnableTokenizeSharesResponsecarries acompletion_timetimestamp, and the keeper returns&MsgEnableTokenizeSharesResponse{CompletionTime: completionTime}, matching the proto .MsgDisableTokenizeSharesResponse,MsgWithdrawTokenizeShareRecordRewardResponse, andMsgWithdrawAllTokenizeShareRecordRewardResponseare defined as empty messages—and the corresponding keeper handlers return empty structs—so they too align with the .proto definitions .No mismatches were found between the response payloads declared in
proto/kyve/liquid/v1beta1/tx.protoand the keeper–msg_server implementations.Optional consideration:
- Withdraw handlers do not emit module-specific events (e.g.,
EventTypeWithdrawTokenizeShareReward). They rely on the bank module’s transfer events for coin movements. If you’d like to surface withdrawals under your own module events for richer telemetry, consider emitting ansdk.NewEvent(types.EventTypeWithdrawTokenizeShareReward, …)in each withdraw handler.x/liquid/module.go (1)
201-207: Review ProvideModule: Authority Default MismatchThe code in x/liquid/module.go (lines 201–207) currently reads:
func ProvideModule(in ModuleInputs) ModuleOutputs { // default to governance authority if not provided authority := authtypes.NewModuleAddress(types.ModuleName) if in.Config.Authority != "" { authority = authtypes.NewModuleAddressOrBech32Address(in.Config.Authority) } // … }• types.ModuleName is defined as
"liquid", soauthtypes.NewModuleAddress(types.ModuleName)creates the liquid module’s account, not the governance module’s account.
• This contradicts the comment (“default to governance authority”), which impliesx/govshould control parameter changes.Please confirm the intended behavior for the liquid module’s authority:
Option A (default to gov module account):
func ProvideModule(in ModuleInputs) ModuleOutputs { - // default to governance authority if not provided - authority := authtypes.NewModuleAddress(types.ModuleName) + // default to governance authority if not provided + authority := authtypes.NewModuleAddress(govtypes.ModuleName) if in.Config.Authority != "" { authority = authtypes.NewModuleAddressOrBech32Address(in.Config.Authority) }Import:
import ( // … + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" )Option B (keep current module account but correct comment):
- // default to governance authority if not provided + // default to the module’s own authority if not providedLet me know which approach matches KYVE’s governance model.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
proto/kyve/liquid/v1beta1/tx.proto (1)
1-198: Update Amino Name Options to Include theliquidNamespaceThe grep run uncovered several
option (amino.name)entries that still use the top‐levelkyve/…prefix instead of the intendedkyve/liquid/…scope. All of these must be corrected to avoid collisions and ensure proper routing.• proto/kyve/liquid/v1beta1/liquid.proto
– Line 17:
Change
option (amino.name) = "kyve/x/liquid/Params";
to
option (amino.name) = "kyve/liquid/Params";• proto/kyve/liquid/v1beta1/tx.proto
– Line 78:
Change
option (amino.name) = "kyve/MsgTokenizeShares";
to
option (amino.name) = "kyve/liquid/MsgTokenizeShares";
– Line 98:
Change
option (amino.name) = "kyve/MsgRedeemTokensForShares";
to
option (amino.name) = "kyve/liquid/MsgRedeemTokensForShares";
– Line 116:
Change
option (amino.name) = "kyve/MsgTransferTokenizeShareRecord";
to
option (amino.name) = "kyve/liquid/MsgTransferTokenizeShareRecord";
– Line 134:
Change
option (amino.name) = "kyve/MsgDisableTokenizeShares";
to
option (amino.name) = "kyve/liquid/MsgDisableTokenizeShares";
– Line 149:
Change
option (amino.name) = "kyve/MsgEnableTokenizeShares";
to
option (amino.name) = "kyve/liquid/MsgEnableTokenizeShares";
– Line 170:
Change
option (amino.name) = "kyve/MsgWithdrawTokenizeShareRecordReward";
to
option (amino.name) = "kyve/liquid/MsgWithdrawTokenizeShareRecordReward";
– Line 187:
Change
option (amino.name) = "kyve/MsgWithdrawAllTokenizeShareRecordReward";
to
option (amino.name) = "kyve/liquid/MsgWithdrawAllTokenizeShareRecordReward";These updates are necessary to fully namespace all Amino message names under
kyve/liquidand prevent any stray top‐levelkyve/…entries.
♻️ Duplicate comments (6)
docs/static/openapi.yml (3)
9445-9446: Fix duplicated “Query/Query…” in response descriptionsRemove the extra “Query/” prefix so user-facing docs aren’t confusing.
Apply:
- QueryLastTokenizeShareRecordIdResponse is response type for the - Query/QueryLastTokenizeShareRecordId RPC method. + QueryLastTokenizeShareRecordIdResponse is response type for the + Query/LastTokenizeShareRecordId RPC method.- QueryLiquidValidatorsResponse is response type for the - Query/QueryLiquidValidators RPC method. + QueryLiquidValidatorsResponse is response type for the + Query/LiquidValidators RPC method.- QueryTokenizeShareRecordByDenomResponse is response type for the - Query/QueryTokenizeShareRecordByDenom RPC method. + QueryTokenizeShareRecordByDenomResponse is response type for the + Query/TokenizeShareRecordByDenom RPC method.- QueryAllTokenizeShareRecordsResponse is response type for the - Query/QueryAllTokenizeShareRecords RPC method. + QueryAllTokenizeShareRecordsResponse is response type for the + Query/AllTokenizeShareRecords RPC method.- QueryTotalLiquidStakedResponse is response type for the - Query/QueryTotalLiquidStaked RPC method. + QueryTotalLiquidStakedResponse is response type for the + Query/TotalLiquidStaked RPC method.- QueryTotalTokenizeSharedAssetsResponse is response type for the - Query/QueryTotalTokenizeSharedAssets RPC method. + QueryTotalTokenizeSharedAssetsResponse is response type for the + Query/TotalTokenizeSharedAssets RPC method.Also applies to: 9582-9584, 9781-9783, 9968-9969, 10062-10064, 10110-10111
9482-9487: Use “bech32-encoded” and fix broken wrapAlign with Cosmos wording and keep it on one logical line.
- operator_address defines the address of the validator's - operator; bech - - encoded in JSON. + operator_address defines the address of the validator's + operator; bech32-encoded in JSON.(Apply in both LiquidValidator and LiquidValidators item descriptions.)
Also applies to: 9547-9552
9725-9727: Mark expiration_time as RFC3339Add date-time format for better client generation/validation.
expiration_time: - type: string + type: string + format: date-timeapp/upgrades/v2_2/upgrade.go (1)
43-50: Don’t ignore errors; use staking keeper’s address codec for robustnessHandle the error from GetAllValidators and decode operator addresses via the keeper’s ValidatorAddressCodec to avoid bech32 prefix pitfalls. Also wrap errors with context.
Apply:
- validators, _ := stakingKeeper.GetAllValidators(ctx) - for _, v := range validators { - valAddress, _ := sdk.ValAddressFromBech32(v.OperatorAddress) - err := liquidStakingKeeper.Hooks().AfterValidatorCreated(ctx, valAddress) - if err != nil { - return nil, err - } - } + validators, err := stakingKeeper.GetAllValidators(ctx) + if err != nil { + return nil, fmt.Errorf("upgrade %s: failed to fetch validators: %w", UpgradeName, err) + } + for _, v := range validators { + addrBz, err := stakingKeeper.ValidatorAddressCodec().StringToBytes(v.OperatorAddress) + if err != nil { + return nil, fmt.Errorf("upgrade %s: invalid validator operator address %q: %w", UpgradeName, v.OperatorAddress, err) + } + valAddress := sdk.ValAddress(addrBz) + if err := liquidStakingKeeper.Hooks().AfterValidatorCreated(ctx, valAddress); err != nil { + return nil, fmt.Errorf("upgrade %s: liquid.AfterValidatorCreated failed for %q: %w", UpgradeName, v.OperatorAddress, err) + } + }proto/kyve/liquid/v1beta1/tx.proto (2)
1-4: Add SPDX and original license header for Apache-2.0 complianceSame requirement as noted earlier; ensure license header is present.
Apply:
+// SPDX-License-Identifier: Apache-2.0 +// Copyright (c) Cosmos Contributors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// // This file was modified from its original version. // https://github.com/cosmos/gaia/blob/b3583a755ad2947582f8e9c969806b187668ea23/proto/gaia/liquid/v1beta1/tx.proto // Modifications made by KYVE, 2025.
77-79: Unify Amino names under kyve/liquid/ namespace*Mixed kyve/* vs kyve/liquid/* can break legacy JSON signing.
Apply:
- option (amino.name) = "kyve/MsgTokenizeShares"; + option (amino.name) = "kyve/liquid/MsgTokenizeShares"; @@ - option (amino.name) = "kyve/MsgRedeemTokensForShares"; + option (amino.name) = "kyve/liquid/MsgRedeemTokensForShares"; @@ - option (amino.name) = "kyve/MsgTransferTokenizeShareRecord"; + option (amino.name) = "kyve/liquid/MsgTransferTokenizeShareRecord"; @@ - option (amino.name) = "kyve/MsgDisableTokenizeShares"; + option (amino.name) = "kyve/liquid/MsgDisableTokenizeShares"; @@ - option (amino.name) = "kyve/MsgEnableTokenizeShares"; + option (amino.name) = "kyve/liquid/MsgEnableTokenizeShares"; @@ - option (amino.name) = "kyve/MsgWithdrawTokenizeShareRecordReward"; + option (amino.name) = "kyve/liquid/MsgWithdrawTokenizeShareRecordReward"; @@ - option (amino.name) = "kyve/MsgWithdrawAllTokenizeShareRecordReward"; + option (amino.name) = "kyve/liquid/MsgWithdrawAllTokenizeShareRecordReward";Also applies to: 97-99, 115-117, 133-135, 148-150, 169-171, 186-188
🧹 Nitpick comments (11)
docs/static/openapi.yml (6)
9519-9523: Add bech32 hints to address path paramsClarify expected address encoding for clients.
- name: validator_addr in: path required: true type: string + description: bech32-encoded validator operator address.- name: address in: path required: true type: string + description: bech32-encoded account address.- name: owner in: path required: true type: string + description: bech32-encoded account address of the records owner.- name: owner_address in: path required: true type: string + description: bech32-encoded account address.Also applies to: 9749-9753, 9913-9917, 10215-10219
9810-9810: Tighten summary wordingRemove duplicate “by”.
- summary: Query for individual tokenize share record information by share by id + summary: Query for individual tokenize share record information by id
9636-9636: Fix double spaceMinor copyedit.
- count_total is set to true to indicate that the result set should + count_total is set to true to indicate that the result set should
9774-9776: Subject-verb agreement (“takes”)Polish titles for consistency.
- title: module account take the role of delegator + title: module account takes the role of delegator(Apply to all listed occurrences.)
Also applies to: 9827-9829, 9883-9885, 9940-9942
10050-10052: Grammar: “a liquid staking provider”Indefinite article fix.
- Query for total liquid staked (including tokenized shares or owned by an - liquid staking provider) + Query for total liquid staked (including tokenized shares or owned by a + liquid staking provider)
10164-10167: Grammar: “a Dec” (not “an Dec”)Minor doc fix in DecCoin notes.
- NOTE: The amount field is an Dec which implements the + NOTE: The amount field is a Dec which implements the(Apply to both occurrences.)
Also applies to: 10186-10189
app/upgrades/v2_2/upgrade.go (2)
23-23: Remove package-level logger; keep it local to the handlerThe global logger isn’t needed and can confuse concurrency/readability. Use a local variable.
Apply:
-var logger log.Logger +// no package-level logger @@ return func(ctx context.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { sdkCtx := sdk.UnwrapSDKContext(ctx) - logger = sdkCtx.Logger().With("upgrade", UpgradeName) - logger.Info(fmt.Sprintf("performing upgrade %v", UpgradeName)) + logger := sdkCtx.Logger().With("upgrade", UpgradeName) + logger.Info(fmt.Sprintf("performing upgrade %v", UpgradeName)) @@ - logger.Info(fmt.Sprintf("finished upgrade %v", UpgradeName)) + logger.Info(fmt.Sprintf("finished upgrade %v", UpgradeName))Also applies to: 31-34, 60-60
62-62: Return nil error explicitlyMinor clarity: err is guaranteed nil here.
Apply:
- return migratedVersionMap, err + return migratedVersionMap, nilproto/kyve/liquid/v1beta1/liquid.proto (1)
44-48: Annotate address fields with cosmos.AddressString for stronger typingAdd cosmos_proto.scalar to ensure consistent address validation and codegen.
Apply:
uint64 id = 1; - string owner = 2; - string module_account = 3; // module account take the role of delegator - string validator = 4; // validator delegated to for tokenize share record creation + string owner = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"]; + string module_account = 3 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // module account take the role of delegator + string validator = 4 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // validator delegated to for tokenize share record creation @@ message PendingTokenizeShareAuthorizations { - repeated string addresses = 1; + repeated string addresses = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"]; } @@ - string operator_address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"]; + string operator_address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];Also applies to: 53-54, 91-94
proto/kyve/liquid/v1beta1/tx.proto (2)
41-43: Fix typo in comment“addresseses” → “address’s”.
Apply:
- // addresseses stake after it has been disabled + // address's stake after it has been disabled
83-86: Annotate address fields with cosmos.AddressString for consistencyAdd scalars to delegator/validator/owner addresses to improve validation and client codegen.
Apply:
- string delegator_address = 1 [(gogoproto.moretags) = "yaml:\"delegator_address\""]; - string validator_address = 2 [(gogoproto.moretags) = "yaml:\"validator_address\""]; + string delegator_address = 1 [(gogoproto.moretags) = "yaml:\"delegator_address\"", (cosmos_proto.scalar) = "cosmos.AddressString"]; + string validator_address = 2 [(gogoproto.moretags) = "yaml:\"validator_address\"", (cosmos_proto.scalar) = "cosmos.AddressString"]; @@ - string delegator_address = 1 [(gogoproto.moretags) = "yaml:\"delegator_address\""]; + string delegator_address = 1 [(gogoproto.moretags) = "yaml:\"delegator_address\"", (cosmos_proto.scalar) = "cosmos.AddressString"]; @@ - uint64 tokenize_share_record_id = 1; - string sender = 2; - string new_owner = 3; + uint64 tokenize_share_record_id = 1; + string sender = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"]; + string new_owner = 3 [(cosmos_proto.scalar) = "cosmos.AddressString"]; @@ - string delegator_address = 1 [(gogoproto.moretags) = "yaml:\"delegator_address\""]; + string delegator_address = 1 [(gogoproto.moretags) = "yaml:\"delegator_address\"", (cosmos_proto.scalar) = "cosmos.AddressString"]; @@ - string delegator_address = 1 [(gogoproto.moretags) = "yaml:\"delegator_address\""]; + string delegator_address = 1 [(gogoproto.moretags) = "yaml:\"delegator_address\"", (cosmos_proto.scalar) = "cosmos.AddressString"]; @@ - string owner_address = 1 [(gogoproto.moretags) = "yaml:\"owner_address\""]; + string owner_address = 1 [(gogoproto.moretags) = "yaml:\"owner_address\"", (cosmos_proto.scalar) = "cosmos.AddressString"]; @@ - string owner_address = 1 [(gogoproto.moretags) = "yaml:\"owner_address\""]; + string owner_address = 1 [(gogoproto.moretags) = "yaml:\"owner_address\"", (cosmos_proto.scalar) = "cosmos.AddressString"];Also applies to: 103-105, 121-124, 139-140, 154-155, 175-177, 192-193
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
x/liquid/types/liquid.pb.gois excluded by!**/*.pb.gox/liquid/types/query.pb.gois excluded by!**/*.pb.gox/liquid/types/tx.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (6)
app/upgrades/v2_2/upgrade.go(3 hunks)docs/static/openapi.yml(1 hunks)proto/kyve/liquid/v1beta1/liquid.proto(1 hunks)proto/kyve/liquid/v1beta1/query.proto(1 hunks)proto/kyve/liquid/v1beta1/tx.proto(1 hunks)x/liquid/client/cli/tx.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- x/liquid/client/cli/tx.go
- proto/kyve/liquid/v1beta1/query.proto
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-02-11T09:41:07.954Z
Learnt from: troykessler
PR: KYVENetwork/chain#244
File: x/multi_coin_rewards/keeper/logic_distribution.go:45-47
Timestamp: 2025-02-11T09:41:07.954Z
Learning: In the KYVE codebase, prefer returning errors over panicking to allow proper error handling at the BeginBlock level, especially for operations that could fail like module-to-module transfers.
Applied to files:
app/upgrades/v2_2/upgrade.go
🧬 Code graph analysis (1)
app/upgrades/v2_2/upgrade.go (2)
x/liquid/keeper/hooks.go (1)
Hooks(20-22)x/liquid/types/liquid.pb.go (3)
Params(70-77)Params(81-81)Params(82-84)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Summary
- GitHub Check: test / test
🔇 Additional comments (2)
docs/static/openapi.yml (1)
9431-9444: Overall looks solid — endpoints, tags, and operationIds are coherentNice alignment with the gRPC Query service and Cosmos REST conventions.
app/upgrades/v2_2/upgrade.go (1)
37-40: Good: aborting on Cosmos SDK migration errorsEarly return on migration failure prevents partial KYVE steps. Thanks for addressing this.
This adds the liquid staking module from https://github.com/cosmos/gaia/tree/main/x/liquid to KYVE
Summary by CodeRabbit
New Features
User-facing APIs
Documentation
Tests
Bug Fixes