feat(accounts): add genesis account initialization #20642
Conversation
|
@testinginprod your pull request is missing a changelog! |
|
Warning Rate limit exceeded@testinginprod has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 51 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe recent updates introduce a new list type and methods for managing account initialization messages in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MsgServer
participant Keeper
participant GenesisState
User ->> MsgServer: Init(request)
MsgServer ->> Keeper: initFromMsg(ctx, request)
Keeper ->> GenesisState: decode(init_account_msgs)
GenesisState -->> Keeper: account data
Keeper -->> MsgServer: resp, accAddr, error
MsgServer -->> User: MsgInitResponse, error
Possibly related issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (3)
api/cosmos/circuit/v1/tx_grpc.pb.gois excluded by!**/*.pb.gox/accounts/v1/genesis.pb.gois excluded by!**/*.pb.gox/circuit/types/tx.pb.gois excluded by!**/*.pb.go
Files selected for processing (6)
- api/cosmos/accounts/v1/genesis.pulsar.go (15 hunks)
- x/accounts/genesis.go (1 hunks)
- x/accounts/genesis_test.go (2 hunks)
- x/accounts/keeper.go (2 hunks)
- x/accounts/msg_server.go (2 hunks)
- x/accounts/proto/cosmos/accounts/v1/genesis.proto (1 hunks)
Files skipped from review due to trivial changes (1)
- x/accounts/genesis.go
Additional context used
Path-based instructions (4)
x/accounts/msg_server.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/accounts/genesis_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/accounts/keeper.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.api/cosmos/accounts/v1/genesis.pulsar.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (18)
x/accounts/proto/cosmos/accounts/v1/genesis.proto (2)
7-7: The import ofcosmos/accounts/v1/tx.protois appropriate given the context of initializing accounts via genesis messages, ensuring that all necessary types fromtx.protoare available.
15-16: Addinginit_account_msgstoGenesisStateis crucial for enabling the initialization of accounts through genesis messages. This change aligns with the PR's objective to enhance the account initialization process during the genesis phase.x/accounts/msg_server.go (1)
23-25: The error handling ininitFromMsgis robust, usingfmt.Errorfto wrap the original error provides clarity on the failure point which is good practice. However, ensure that the error message is clear and actionable.x/accounts/genesis_test.go (2)
42-50: The test setup for initializing a genesis account usinginitMsgis well-implemented. It correctly appends the initialization message tostate.InitAccountMsgsand checks the initialization status, which is crucial for validating the new functionality.
64-69: The test for querying the newly initialized genesis account and checking its state is a good practice. It ensures that the account initialization via genesis messages works as expected.x/accounts/keeper.go (1)
160-175: TheinitFromMsgfunction is a key addition, facilitating the initialization of accounts fromMsgInitmessages. This function effectively decodes the message and delegates to the existingInitmethod, maintaining a clean separation of concerns and reusing existing logic.api/cosmos/accounts/v1/genesis.pulsar.go (12)
66-115: The implementation of_GenesisState_3_listis consistent with protobuf list handling in Go. It properly manages list operations forMsgInittypes, ensuring type safety and correct protobuf interactions.
121-129: The field descriptor forinit_account_msgsis correctly defined and matches the protobuf field number and type. This ensures that the protobuf encoding and decoding will handle this field as expected.
209-214: TheRangemethod correctly handles the newinit_account_msgslist. It ensures that the list is only processed if it is not empty, which is a good practice for performance.
234-235: TheHasmethod correctly checks for the presence ofinit_account_msgsbased on its length. This is a standard approach in protobuf generated code to handle repeated fields.
256-257: TheClearmethod forinit_account_msgssets the slice to nil, which is the correct way to clear a repeated field in protobufs in Go.
283-288: TheGetmethod correctly returns a list representation forinit_account_msgs. It handles the case where the list is empty by returning an empty list, which is crucial for avoiding nil dereferences.
315-318: TheSetmethod correctly assigns a new list toinit_account_msgs. This method ensures that the entire list is replaced, which is appropriate for the setter behavior in protobufs.
345-350: TheMutablemethod implementation forinit_account_msgscorrectly initializes the list if it is nil and returns a mutable reference. This is essential for enabling modifications to the list after its creation.
371-373: TheNewFieldmethod forinit_account_msgscorrectly creates a new, empty list. This is used in protobufs to initialize fields with their default values when needed.
452-457: TheSizemethod inProtoMethodscorrectly calculates the size ofinit_account_msgsfor marshaling purposes. This includes handling each element in the list, which is necessary for correct protobuf serialization.
487-501: TheMarshalmethod inProtoMethodscorrectly handles the marshaling ofinit_account_msgs. It ensures that each message is encoded and added to the buffer in the correct order, which is crucial for maintaining the integrity of the data.
626-659: TheUnmarshalmethod correctly handles the unmarshaling ofinit_account_msgsfrom a protobuf stream. It properly allocates new instances ofMsgInitand fills them with data from the input buffer, ensuring data integrity and type safety.
# Conflicts: # x/accounts/genesis.go # x/accounts/genesis_test.go
| "app_state": { | ||
| "accounts": { | ||
| "init_account_msgs": [ |
There was a problem hiding this comment.
ahh can we add a excerpt that offline generating messages is the way to do this if that is the intended
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (1)
x/accounts/README.md (1)
8-8: Consider revising the phrase "In order to" to a shorter alternative like "To" for conciseness.Tools
LanguageTool
[style] ~8-~8: Consider a shorter alternative to avoid wordiness. (IN_ORDER_TO_PREMIUM)
Context: ...nesis ## Creating accounts on genesis In order to create accounts at genesis, the `x/acco...
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- x/accounts/README.md (1 hunks)
Additional context used
Path-based instructions (1)
x/accounts/README.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
LanguageTool
x/accounts/README.md
[style] ~8-~8: Consider a shorter alternative to avoid wordiness. (IN_ORDER_TO_PREMIUM)
Context: ...nesis ## Creating accounts on genesis In order to create accounts at genesis, the `x/acco...
Markdownlint
x/accounts/README.md
12-12: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
5-5: null (MD025, single-title, single-h1)
Multiple top-level headings in the same document
44-44: null (MD047, single-trailing-newline)
Files should end with a single newline character
Additional comments not posted (1)
x/accounts/README.md (1)
9-44: The documentation clearly explains the process of initializing accounts at genesis usingMsgInitmessages. The example provided is detailed and helps illustrate the setup in thegenesis.jsonfile.Tools
Markdownlint
12-12: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
44-44: null (MD047, single-trailing-newline)
Files should end with a single newline character
| a list of genesis `MsgInit` messages that will be executed in the `x/accounts` genesis flow. | ||
|
|
||
| This follows the same initialization flow and rules that would happen if the chain is running. |
There was a problem hiding this comment.
Remove the trailing space at the end of line 12.
Tools
Markdownlint
12-12: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
| The x/accounts module provides module and facilities for writing smart cosmos-sdk accounts. | ||
| The x/accounts module provides module and facilities for writing smart cosmos-sdk accounts. | ||
|
|
||
| # Genesis |
There was a problem hiding this comment.
Ensure there is only one top-level heading in the document to comply with best practices in Markdown formatting.
Tools
Markdownlint
5-5: null (MD025, single-title, single-h1)
Multiple top-level headings in the same document
| } | ||
| ``` | ||
|
|
||
| The accounts module will run the lockup account initialization message. |
There was a problem hiding this comment.
Ensure the file ends with a single newline character.
Tools
Markdownlint
44-44: null (MD047, single-trailing-newline)
Files should end with a single newline character
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (1)
x/accounts/v1/genesis.pb.gois excluded by!**/*.pb.go
Files selected for processing (4)
- api/cosmos/accounts/v1/genesis.pulsar.go (21 hunks)
- x/accounts/genesis.go (3 hunks)
- x/accounts/genesis_test.go (3 hunks)
- x/accounts/proto/cosmos/accounts/v1/genesis.proto (1 hunks)
Files not summarized due to errors (1)
- api/cosmos/accounts/v1/genesis.pulsar.go: Error: Server error. Please try again later.
Files skipped from review as they are similar to previous changes (3)
- x/accounts/genesis.go
- x/accounts/genesis_test.go
- x/accounts/proto/cosmos/accounts/v1/genesis.proto
Additional context used
Path-based instructions (1)
api/cosmos/accounts/v1/genesis.pulsar.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (12)
api/cosmos/accounts/v1/genesis.pulsar.go (12)
17-64: Review the implementation of_GenesisState_1_listfor handlingGenesisAccount.This custom list implementation for
GenesisAccountappears to be correctly handling the protobuf message reflection, which is crucial for the dynamic handling of these objects in Go. The methods such asLen,Get,Set,Append,AppendMutable,Truncate, andNewElementare well-implemented for managing slices ofGenesisAccount.
Line range hint
69-109: Review the implementation of_GenesisState_2_listfor handlingMsgInit.The implementation for
_GenesisState_2_list, which manages slices ofMsgInit, is consistent with the protobuf handling seen in_GenesisState_1_list. The methods provided ensure that the list can be dynamically manipulated, which is essential for the initialization messages in the genesis state.
118-127: Check the initialization of field descriptors ininit().The initialization of field descriptors for
GenesisStatewithin theinit()function is correctly set up. This setup is crucial for the correct serialization and deserialization of theGenesisStateprotobuf messages.
195-203: Review theRangemethod implementation infastReflection_GenesisState.The
Rangemethod implementation correctly iterates over the fields ofGenesisState, handling bothAccountsandInitAccountMsgs. This method is essential for protobuf's reflection-based operations, allowing for dynamic field handling based on their presence.
224-225: Check theHasandClearmethods forinit_account_msgs.The
HasandClearmethods for theinit_account_msgsfield are implemented following protobuf standards, ensuring that these operations are safe and reflect the expected behavior for mutable protobuf fields.Also applies to: 244-245
264-272: Review theGetmethod forinit_account_msgs.The
Getmethod forinit_account_msgsis correctly implemented to handle cases where the list might be empty, returning an appropriate empty list in such cases. This is a crucial aspect of robust message handling in protobuf.
296-301: Check theSetmethod forinit_account_msgs.The
Setmethod properly handles the assignment of new values to theinit_account_msgsfield, ensuring that the list is directly manipulated based on the input. This method is crucial for maintaining the integrity of the data withinGenesisState.
326-332: Review theMutablemethod forinit_account_msgs.The
Mutablemethod forinit_account_msgscorrectly ensures that a mutable reference is returned, allowing for modifications to the list of initialization messages. This method is essential for scenarios where the genesis state needs to be dynamically adjusted.
349-351: Check theNewFieldmethod forinit_account_msgs.The
NewFieldmethod is correctly implemented to provide a new, empty list forinit_account_msgswhen needed. This is important for ensuring that fields can be dynamically initialized in protobuf without prior values.
428-433: Review theProtoMethodssize calculation forInitAccountMsgs.The size calculation within the
ProtoMethodsforInitAccountMsgsis accurately implemented, ensuring that the size of each message is correctly computed and added to the total size. This is crucial for performance optimizations in serialization.
463-478: Check the marshaling logic forInitAccountMsgs.The marshaling logic for
InitAccountMsgsin theProtoMethodsis robust, handling the serialization of each message in the list correctly. This ensures that the data integrity is maintained during the serialization process.
Line range hint
545-608: Review the unmarshaling logic forInitAccountMsgs.The unmarshaling logic for
InitAccountMsgsis thoroughly implemented, ensuring that each message is correctly parsed and added to the list. This is essential for correctly reconstructing theGenesisStatefrom its serialized form.
* main: fix(x/staking): stop validators from rotating to the same key on the same block (#20649) perf: add cache to address codec (#20122) build(deps): Bump google.golang.org/protobuf from 1.34.1 to 1.34.2 (#20632) fix: remove recipient amount from map (#20625) fix(proto): remove conditional preventing proper generated file placement (#20650) (serverv2/cometbft) Read config from commands & handle `FlagNode` (#20621) fix(x/consensus): fix .proto file placement (#20646) fix(store): avoid nil error on not exhausted payload stream (#20644) fix (x/accounts): Fix genesis condition check (#20645) feat(accounts): add genesis account initialization (#20642) fix(x/gov): limit execution in gov (#20348)
Description
This PR allows init messages of account to be put in genesis.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
MsgInitmessages.Bug Fixes
Documentation
x/accountsREADME to include instructions for creating accounts at genesis.Refactor