Skip to content
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

docs(lockup): add documentation #19898

Merged
merged 15 commits into from
Apr 26, 2024
Merged

docs(lockup): add documentation #19898

merged 15 commits into from
Apr 26, 2024

Conversation

tac0turtle
Copy link
Member

@tac0turtle tac0turtle commented Mar 28, 2024

Description

add docs to the lockup accounts


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...

  • included the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • reviewed "Files changed" and left comments if necessary
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • updated the relevant documentation or specification, including comments for documenting Go code
  • confirmed all CI checks have passed

Reviewers 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.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

Summary by CodeRabbit

  • New Features
    • Renamed and expanded the x/accounts/lockup module to Lockup Accounts.
    • Added various lockup account types like BaseLockup, ContinuousLockup, DelayedLockup, PeriodicLockup, and PermanentLocked.
    • Detailed unlocking rules and functionalities for each account type in the README.

Copy link
Contributor

coderabbitai bot commented Mar 28, 2024

Walkthrough

Walkthrough

The update enriches the Lockup Accounts module by rebranding it and introducing diverse vesting account types with distinct unlocking mechanisms. This expansion enhances the module's versatility and usability, catering to a wider array of scenarios for users.

Changes

File Path Change Summary
x/accounts/defaults/lockup/README.md Module renamed to Lockup Accounts and now includes various lockup account types (BaseLockup, ContinuousLockup, DelayedLockup, PeriodicLockup, PermanentLocked) with detailed unlocking behaviors and functionalities outlined in the README.

Recent Review Details

Configuration used: .coderabbit.yml

Commits Files that changed from the base of the PR and between 6bfafa6 and 3f3e9cf.
Files selected for processing (1)
  • x/accounts/defaults/lockup/README.md (1 hunks)
Additional Context Used
LanguageTool (23)
x/accounts/defaults/lockup/README.md (23)

Near line 5: Possible spelling mistake found.
Context: ...nt Types](#lockup-account-types) * BaseLockup * [ContinuousLockup](...


Near line 6: Possible spelling mistake found.
Context: ... * BaseLockup * ContinuousLockup * [DelayedLocku...


Near line 7: Possible spelling mistake found.
Context: ...inuousLockup](#continuouslockup) * DelayedLockup * [PeriodicLockup]...


Near line 8: Possible spelling mistake found.
Context: ... DelayedLockup * PeriodicLockup * [PermanentLocke...


Near line 9: Possible spelling mistake found.
Context: ...PeriodicLockup](#periodiclockup) * PermanentLocked * [Genesis Initializ...


Near line 20: Possible spelling mistake found.
Context: ...s module. ## Lockup Account Types ### BaseLockup The base lockup account is used by all...


Near line 39: Possible spelling mistake found.
Context: ... collections.Item[time.Time] } ``` ### ContinuousLockup The continuous lockup account has a fu...


Near line 43: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ...e specified end date. To determine the amount of coins that are vested for a given bl...


Near line 46: This phrase is duplicated. You should probably use “Compute Compute” only once.
Context: ...me T, the following is performed: 1. Compute X := T - StartTime 2. Compute Y := EndTime - StartTime 3. Compute V' := OV * (X / Y) 4. Compute V := OV - V' Thus, the total amount ...


Near line 51: Unpaired symbol: ‘'’ seems to be missing
Context: ..., the total amount of vested coins is V' and the remaining amount, V, is _loc...


Near line 61: Possible spelling mistake found.
Context: ... collections.Item[time.Time] } ``` ### DelayedLockup The delayed lockup account unlocks all...


Near line 71: Possible spelling mistake found.
Context: ...ccount struct { *BaseLockup } ``` ### PeriodicLockup The periodic lockup account locks toke...


Near line 75: Possible spelling mistake found.
Context: ... periods could have passed when calling GetVestedCoins, so we must iterate over each period u...


Near line 77: Possible typo: you repeated a word
Context: ...he end of that period is after T. 1. Set CT := StartTime 2. Set V' := 0 For each Period P: 1. Com...


Near line 84: Possible typo: you repeated a word
Context: ... - CT 2. IFX >= P.Length 1. ComputeV' += P.Amount 2. ComputeCT += P.Length` 3. ELSE break ...


Near line 97: Possible spelling mistake found.
Context: ...ions.Vec[lockuptypes.Period] } ``` ### PermanentLocked The permanent lockup account permanent...


Near line 152: Possible agreement error. The noun ‘vest’ seems to be countable.
Context: ... 5. More time passes, 2 more coins vest text V = 6 V' = 4 `...


Near line 160: Use a comma before ‘or’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ot send anymore until further coins vest or it receives additional coins. It can st...


Near line 170: Possible agreement error. The noun ‘vest’ seems to be countable.
Context: ...imple example. 1. Time passes, 5 coins vest ```text V = 5 V' = 5 `...


Near line 192: Possible spelling mistake found.
Context: ... delegation to A now worth 2.5 coins 5. Undelegate from validator A (2.5 coins) ```te...


Near line 199: Possible spelling mistake found.
Context: ... 2.5 BC = 0 + 2.5 = 2.5 ``` 6. Undelegate from validator B (5 coins). The account...


Near line 239: Possible agreement error. The noun ‘vest’ seems to be countable.
Context: ...`` 2. Lockup period 1 passes, 25 coins vest ```text V = 75 V' = 25 ...


Near line 253: Possible agreement error. The noun ‘vest’ seems to be countable.
Context: ...`` 4. Lockup period 2 passes, 25 coins vest ```text V = 50 V' = 50 ...

Path-based Instructions (1)
x/accounts/defaults/lockup/README.md (1)

Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@tac0turtle tac0turtle marked this pull request as ready for review March 28, 2024 16:48
@tac0turtle tac0turtle requested a review from a team as a code owner March 28, 2024 16:48
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 7

Configuration used: .coderabbit.yml

Commits Files that changed from the base of the PR and between d54e940 and 5bc3ca8.
Files selected for processing (1)
  • x/accounts/defaults/lockup/README.md (1 hunks)
Path instructions used (1)
x/accounts/defaults/lockup/README.md (1)

**/*.md
Assess the documentation for misspellings, grammatical errors, missing documentation and correctness

Additional comments (8)
x/accounts/defaults/lockup/README.md (8)
  • 1-1: The title "LockUp Accounts" might be a spelling mistake or could be improved for clarity. Consider using "Lockup Accounts" for consistency with common terminology.
  • 44-44: The phrase "amount of coins" is used, which might be better expressed as "number of coins" since coins are countable.

Consider revising to "number of coins" for grammatical accuracy.

  • 47-47: The phrase "Compute Compute" seems to be a duplication error in the context provided by the static analysis tool. However, this appears to be a false positive as the context does not show duplicated "Compute" in the documentation.
  • 52-52: The symbol ‘'’ in V' seems to be flagged as unpaired, which is likely a false positive since this notation is commonly used in mathematical expressions to denote a variant of a variable.
  • 76-76: The phrase "calculating the coins released during each period for a given block time T" might be missing a preposition before "each period." However, this seems to be a false positive as the sentence is grammatically correct and clear in its current form.
  • 78-78: The phrase "Set CT := StartTime 2. Set V' := 0" is flagged for repeating a word, which seems to be a misunderstanding of the context. This is a step-by-step instruction in a list and does not constitute a typo.
  • 85-85: The phrase "Compute V' += P.Amount 2. Compute CT += P.Length" is flagged for repeating a word, which seems to be a misunderstanding of the context. This is a step-by-step instruction in a list and does not constitute a typo.
  • 215-215: The phrase "1/4 of tokens vesting each quarter" might be missing an article before "tokens." However, this seems to be a stylistic choice rather than a grammatical error, as the sentence is clear and understandable in its current form.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 8

Configuration used: .coderabbit.yml

Commits Files that changed from the base of the PR and between 5bc3ca8 and e78b35b.
Files selected for processing (1)
  • x/accounts/defaults/lockup/README.md (1 hunks)
Path instructions used (1)
x/accounts/defaults/lockup/README.md (1)

**/*.md
Assess the documentation for misspellings, grammatical errors, missing documentation and correctness

x/accounts/defaults/lockup/README.md Outdated Show resolved Hide resolved
tac0turtle and others added 4 commits March 29, 2024 03:16
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 7

Configuration used: .coderabbit.yml

Commits Files that changed from the base of the PR and between e78b35b and 1440a9e.
Files selected for processing (1)
  • x/accounts/defaults/lockup/README.md (1 hunks)
Path instructions used (1)
x/accounts/defaults/lockup/README.md (1)

**/*.md
Assess the documentation for misspellings, grammatical errors, missing documentation and correctness

x/accounts/defaults/lockup/README.md Outdated Show resolved Hide resolved
x/accounts/defaults/lockup/README.md Outdated Show resolved Hide resolved
x/accounts/defaults/lockup/README.md Outdated Show resolved Hide resolved
x/accounts/defaults/lockup/README.md Outdated Show resolved Hide resolved
x/accounts/defaults/lockup/README.md Outdated Show resolved Hide resolved
x/accounts/defaults/lockup/README.md Outdated Show resolved Hide resolved
x/accounts/defaults/lockup/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: .coderabbit.yml

Commits Files that changed from the base of the PR and between 1440a9e and 162300e.
Files selected for processing (1)
  • x/accounts/defaults/lockup/README.md (1 hunks)
Path instructions used (1)
x/accounts/defaults/lockup/README.md (1)

**/*.md
Assess the documentation for misspellings, grammatical errors, missing documentation and correctness

Additional comments (6)
x/accounts/defaults/lockup/README.md (6)
  • 1-1: The title "Lockup Accounts" might be more accurately represented as "Lockup Accounts" to maintain consistency with the module naming convention and improve readability.
  • 41-41: The term "enddate" should be two words. Consider revising to "end date" for correctness.
  • 73-73: Consider adding a comma after "periods" for clarity and grammatical correctness.
  • 99-99: The term "permentally" seems to be a typo. Consider correcting it to "permanently" for accuracy.
  • 159-159: Consider adding a comma after "anymore" for clarity and grammatical correctness.
  • 160-160: Consider adding a comma before "however" for clarity and grammatical correctness.

Copy link
Member

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to dive into lockup to be able to verify this. I'll give it a read later on this week.

x/accounts/defaults/lockup/README.md Outdated Show resolved Hide resolved
x/accounts/defaults/lockup/README.md Outdated Show resolved Hide resolved
@julienrbrt julienrbrt changed the title chore(lockup): add documentation dpcs(lockup): add documentation Mar 28, 2024
@julienrbrt julienrbrt changed the title dpcs(lockup): add documentation docs(lockup): add documentation Mar 28, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review Status

Configuration used: .coderabbit.yml

Commits Files that changed from the base of the PR and between 162300e and 4e1b21b.
Files selected for processing (1)
  • x/accounts/defaults/lockup/README.md (1 hunks)
Additional Context Used
Path-based Instructions (1)
x/accounts/defaults/lockup/README.md (1)

**/*.md:
Assess the documentation for misspellings, grammatical errors, missing documentation and correctness

Additional comments not posted (6)
x/accounts/defaults/lockup/README.md (6)

1-1: The title "Lockup Accounts" might be more accurately represented as "Lockup Accounts" to maintain consistency with the module naming convention and improve readability.


22-22: The term "enddate" should be two words. Consider revising to "end date" for correctness.


73-73: Consider adding a comma after "periods" for clarity and grammatical correctness.


99-99: The term "permentally" seems to be a typo. Consider correcting it to "permanently" for accuracy.


160-160: Consider adding a comma after "anymore" for clarity and grammatical correctness.


160-160: Consider adding a comma before "however" for clarity and grammatical correctness.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Review Status

Configuration used: .coderabbit.yml

Commits Files that changed from the base of the PR and between 4e1b21b and 0bee846.
Files selected for processing (1)
  • x/accounts/defaults/lockup/README.md (1 hunks)
Additional Context Used
Path-based Instructions (1)
x/accounts/defaults/lockup/README.md (1)

**/*.md:
Assess the documentation for misspellings, grammatical errors, missing documentation and correctness

Additional comments not posted (8)
x/accounts/defaults/lockup/README.md (8)

1-1: The title "Lockup Accounts" might be more accurately represented as "Lockup Accounts" to maintain consistency with the module naming convention and improve readability.


22-22: There are extra spaces before "account." Consider removing the extra spaces for consistency and readability.


41-41: The term "enddate" should be two words. Consider revising to "end date" for correctness.


73-73: Consider adding a comma after "periods" for clarity and grammatical correctness.


99-99: The term "permentally" seems to be a typo. Consider correcting it to "permanently" for accuracy.


160-160: Consider adding a comma after "anymore" for clarity and grammatical correctness.


160-160: Consider adding a comma before "however" for clarity and grammatical correctness.


51-51: The unpaired symbol ' in V' might confuse readers. Ensure consistency in the use of symbols for variables.

Consider using a consistent notation for variables throughout the documentation to avoid confusion.

* [Slashing](#slashing)
* [Periodic Lockup](#periodic-lockup)

The x/accounts/defaults/lockup module provides the implementation for lockup accounts within the x/accounts module.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The introduction sentence could be enhanced for clarity and conciseness. Consider rephrasing to directly state the purpose of the module.

- The x/accounts/defaults/lockup module provides the implementation for lockup accounts within the x/accounts module.
+ The `x/accounts/defaults/lockup` module implements lockup account functionalities within the Cosmos SDK's `x/accounts` module.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
The x/accounts/defaults/lockup module provides the implementation for lockup accounts within the x/accounts module.
The `x/accounts/defaults/lockup` module implements lockup account functionalities within the Cosmos SDK's `x/accounts` module.

To determine the amount of coins that are vested for a given block time `T`, the
following is performed:

1. Compute `X := T - StartTime`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The phrase "Compute Compute" appears to be duplicated in the documentation. This seems to be a typo or copy-paste error.

- 1. Compute `X := T - StartTime`
+ 1. Compute `X := T - StartTime`

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
1. Compute `X := T - StartTime`
1. Compute `X := T - StartTime`


The periodic vesting account locks tokens for a series of periods. The account can receive coins and send coins. After all the periods, all the coins are unlocked and the account can send coins.

Periodic vesting accounts require calculating the coins released during each period for a given block time `T`. Note that multiple periods could have passed when calling `GetVestedCoins`, so we must iterate over each period until the end of that period is after `T`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The phrase "could have passed" might be missing a preposition. Consider rephrasing for clarity.

- Note that multiple periods could have passed when calling `GetVestedCoins`, so we must iterate over each period until the end of that period is after `T`.
+ Note that multiple periods could have elapsed by the time `GetVestedCoins` is called, necessitating iteration over each period until finding one that ends after `T`.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
Periodic vesting accounts require calculating the coins released during each period for a given block time `T`. Note that multiple periods could have passed when calling `GetVestedCoins`, so we must iterate over each period until the end of that period is after `T`.
Periodic vesting accounts require calculating the coins released during each period for a given block time `T`. Note that multiple periods could have elapsed by the time `GetVestedCoins` is called, necessitating iteration over each period until finding one that ends after `T`.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6


### BaseLockup

The base vesting account is used by all default lockup accounts. It contains the basic information for a vesting account. The Base vesting account keeps knowledge of the staking delegations from the account.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we capitalise the B to keep it consistent?

Copy link
Contributor

@samricotta samricotta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than that one nit, looks good

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11


<!--- TODO: need to expand more on this --->

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider removing the extra blank line for better formatting consistency.

- 

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change

Comment on lines +4 to +14
* [Lockup Account Types](#lockup-account-types)
* [BaseLockup](#baselockup)
* [ContinuousLockup](#continuouslockup)
* [DelayedLockup](#delayedlockup)
* [PeriodicLockup](#periodiclockup)
* [PermanentLocked](#permanentlocked)
* [Genesis Initialization](#genesis-initialization)
* [Examples](#examples)
* [Simple](#simple)
* [Slashing](#slashing)
* [Periodic Lockup](#periodic-lockup)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adjust the indentation of the unordered list items to align with Markdown best practices.

-    * [BaseLockup](#baselockup)
-    * [ContinuousLockup](#continuouslockup)
-    * [DelayedLockup](#delayedlockup)
-    * [PeriodicLockup](#periodiclockup)
-    * [PermanentLocked](#permanentlocked)
+  * [BaseLockup](#baselockup)
+  * [ContinuousLockup](#continuouslockup)
+  * [DelayedLockup](#delayedlockup)
+  * [PeriodicLockup](#periodiclockup)
+  * [PermanentLocked](#permanentlocked)

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
* [Lockup Account Types](#lockup-account-types)
* [BaseLockup](#baselockup)
* [ContinuousLockup](#continuouslockup)
* [DelayedLockup](#delayedlockup)
* [PeriodicLockup](#periodiclockup)
* [PermanentLocked](#permanentlocked)
* [Genesis Initialization](#genesis-initialization)
* [Examples](#examples)
* [Simple](#simple)
* [Slashing](#slashing)
* [Periodic Lockup](#periodic-lockup)
* [Lockup Account Types](#lockup-account-types)
* [BaseLockup](#baselockup)
* [ContinuousLockup](#continuouslockup)
* [DelayedLockup](#delayedlockup)
* [PeriodicLockup](#periodiclockup)
* [PermanentLocked](#permanentlocked)
* [Genesis Initialization](#genesis-initialization)
* [Examples](#examples)
* [Simple](#simple)
* [Slashing](#slashing)
* [Periodic Lockup](#periodic-lockup)

Comment on lines +16 to +22
The x/accounts/defaults/lockup module provides the implementation for lockup accounts within the x/accounts module.

## Lockup Account Types

### BaseLockup

The base lockup account is used by all default lockup accounts. It contains the basic information for a lockup account. The Base lockup account keeps knowledge of the staking delegations from the account.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enhance the introduction sentence for clarity and conciseness.

- The x/accounts/defaults/lockup module provides the implementation for lockup accounts within the x/accounts module.
+ The `x/accounts/defaults/lockup` module implements lockup account functionalities within the Cosmos SDK's `x/accounts` module.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
The x/accounts/defaults/lockup module provides the implementation for lockup accounts within the x/accounts module.
## Lockup Account Types
### BaseLockup
The base lockup account is used by all default lockup accounts. It contains the basic information for a lockup account. The Base lockup account keeps knowledge of the staking delegations from the account.
The `x/accounts/defaults/lockup` module implements lockup account functionalities within the Cosmos SDK's `x/accounts` module.
## Lockup Account Types
### BaseLockup
The base lockup account is used by all default lockup accounts. It contains the basic information for a lockup account. The Base lockup account keeps knowledge of the staking delegations from the account.


### BaseLockup

The base lockup account is used by all default lockup accounts. It contains the basic information for a lockup account. The Base lockup account keeps knowledge of the staking delegations from the account.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove extra spaces before "account" for consistency and readability.

- The Base lockup account keeps knowledge of the staking delegations from the  account.
+ The Base lockup account keeps knowledge of the staking delegations from the account.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
The base lockup account is used by all default lockup accounts. It contains the basic information for a lockup account. The Base lockup account keeps knowledge of the staking delegations from the account.
The base lockup account is used by all default lockup accounts. It contains the basic information for a lockup account. The Base lockup account keeps knowledge of the staking delegations from the account.

Comment on lines +24 to +36
```go
type BaseLockup struct {
// Owner is the address of the account owner.
Owner collections.Item[[]byte]
OriginalLocking collections.Map[string, math.Int]
DelegatedFree collections.Map[string, math.Int]
DelegatedLocking collections.Map[string, math.Int]
WithdrawedCoins collections.Map[string, math.Int]
addressCodec address.Codec
headerService header.Service
// lockup end time.
EndTime collections.Item[time.Time]
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace tabs with spaces in the Go code snippet to maintain consistency and readability in the documentation.

-	// Owner is the address of the account owner.
-	Owner            collections.Item[[]byte]
-	OriginalLocking  collections.Map[string, math.Int]
-	DelegatedFree    collections.Map[string, math.Int]
-	DelegatedLocking collections.Map[string, math.Int]
-	WithdrawedCoins  collections.Map[string, math.Int]
-	addressCodec     address.Codec
-	headerService    header.Service
-	// lockup end time.
-	EndTime collections.Item[time.Time]
+  // Owner is the address of the account owner.
+  Owner            collections.Item[[]byte]
+  OriginalLocking  collections.Map[string, math.Int]
+  DelegatedFree    collections.Map[string, math.Int]
+  DelegatedLocking collections.Map[string, math.Int]
+  WithdrawedCoins  collections.Map[string, math.Int]
+  addressCodec     address.Codec
+  headerService    header.Service
+  // lockup end time.
+  EndTime collections.Item[time.Time]

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
```go
type BaseLockup struct {
// Owner is the address of the account owner.
Owner collections.Item[[]byte]
OriginalLocking collections.Map[string, math.Int]
DelegatedFree collections.Map[string, math.Int]
DelegatedLocking collections.Map[string, math.Int]
WithdrawedCoins collections.Map[string, math.Int]
addressCodec address.Codec
headerService header.Service
// lockup end time.
EndTime collections.Item[time.Time]
}
type BaseLockup struct {
// Owner is the address of the account owner.
Owner collections.Item[[]byte]
OriginalLocking collections.Map[string, math.Int]
DelegatedFree collections.Map[string, math.Int]
DelegatedLocking collections.Map[string, math.Int]
WithdrawedCoins collections.Map[string, math.Int]
addressCodec address.Codec
headerService header.Service
// lockup end time.
EndTime collections.Item[time.Time]
}

Comment on lines +55 to +58
type ContinuousLockingAccount struct {
*BaseLockup
StartTime collections.Item[time.Time]
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace tabs with spaces in the Go code snippet for the ContinuousLockingAccount struct to maintain consistency and readability.

-	*BaseLockup
-	StartTime collections.Item[time.Time]
+  *BaseLockup
+  StartTime collections.Item[time.Time]

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
type ContinuousLockingAccount struct {
*BaseLockup
StartTime collections.Item[time.Time]
}
type ContinuousLockingAccount struct {
*BaseLockup
StartTime collections.Item[time.Time]
}

Comment on lines +66 to +68
type DelayedLockingAccount struct {
*BaseLockup
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace tabs with spaces in the Go code snippet for the DelayedLockingAccount struct to maintain consistency and readability.

-	*BaseLockup
+  *BaseLockup

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
type DelayedLockingAccount struct {
*BaseLockup
}
type DelayedLockingAccount struct {
*BaseLockup
}

Comment on lines +90 to +93
type PeriodicLockingAccount struct {
*BaseLockup
StartTime collections.Item[time.Time]
LockingPeriods collections.Vec[lockuptypes.Period]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace tabs with spaces in the Go code snippet for the PeriodicLockingAccount struct to maintain consistency and readability.

-	*BaseLockup
-	StartTime      collections.Item[time.Time]
-	LockingPeriods collections.Vec[lockuptypes.Period]
+  *BaseLockup
+  StartTime      collections.Item[time.Time]
+  LockingPeriods collections.Vec[lockuptypes.Period]

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
type PeriodicLockingAccount struct {
*BaseLockup
StartTime collections.Item[time.Time]
LockingPeriods collections.Vec[lockuptypes.Period]
type PeriodicLockingAccount struct {
*BaseLockup
StartTime collections.Item[time.Time]
LockingPeriods collections.Vec[lockuptypes.Period]

Comment on lines +102 to +104
type PermanentLockingAccount struct {
*BaseLockup
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace tabs with spaces in the Go code snippet for the PermanentLocked struct to maintain consistency and readability.

-	*BaseLockup
+  *BaseLockup

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
type PermanentLockingAccount struct {
*BaseLockup
}
type PermanentLockingAccount struct {
*BaseLockup
}

```

6. Sends 2 coins. At this point, the account cannot send anymore until further
coins vest or it receives additional coins. It can still, however, delegate.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a comma before "or" if it connects two independent clauses (unless they are closely connected and short).

- At this point, the account cannot send anymore until further coins vest or it receives additional coins.
+ At this point, the account cannot send anymore until further coins vest, or it receives additional coins.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
coins vest or it receives additional coins. It can still, however, delegate.
coins vest, or it receives additional coins. It can still, however, delegate.

@tac0turtle tac0turtle added this pull request to the merge queue Apr 26, 2024
Merged via the queue into main with commit 84bae07 Apr 26, 2024
53 checks passed
@tac0turtle tac0turtle deleted the marko/lockup_docs branch April 26, 2024 15:42
meetrick pushed a commit to meetrick/cosmos-sdk that referenced this pull request Apr 29, 2024
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants