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: Update tx lifecycle in beginner/tx-lifecycle #20284

Merged
merged 13 commits into from
Jun 18, 2024
Merged

Conversation

samricotta
Copy link
Contributor

@samricotta samricotta commented May 6, 2024

Description

This updates the Tx lifecycle so it is more generic with server/v2 efforts. Removes all mention of cometbft other than as an example of a consensus engine that could be used.

Closes: #20173 and part of cosmos/cosmos-sdk-docs#144


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

  • Documentation
    • Updated the advanced guide to provide in-depth insights into transaction processing in the Cosmos SDK, including Context initialization, AnteHandler operations, RunMsgs execution, FinalizeBlock, and Commit processes.
    • Enriched the beginner's guide with sections on transaction creation, broadcasting, processing, and post-transaction handling, covering gas, fees, and mempool operations.

@samricotta samricotta requested a review from a team as a code owner May 6, 2024 07:55
Copy link
Contributor

coderabbitai bot commented May 6, 2024

Note

Reviews paused

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Walkthrough

The documentation update focuses on enhancing clarity and coherence in the transaction lifecycle sections, particularly emphasizing transaction processing and state management. The changes aim to provide a more structured and detailed explanation of transaction verification, processing, and post-transaction handling within the Cosmos SDK.

Changes

File Path Change Summary
docs/learn/advanced/00-baseapp.md Updated transaction processing details including Context initialization, AnteHandler, and Commit.
docs/learn/beginner/01-tx-lifecycle.md Revised sections on transaction creation, broadcasting, processing, and handling, enhancing detail and clarity.

Assessment against linked issues

Objective Addressed Explanation
Update Transaction Lifecycle section for clarity and coherence (#20173)
Define the role of FinalizeBlock clearly (#20173)
Mention the application mempool availability (#20173) The provided changes do not explicitly mention the application mempool's availability.
Rewrite for coherence using updated terminology (#20173)

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.
    • @coderabbitai help me debug CodeRabbit configuration file.

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 an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @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.

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: 5

Out of diff range and nitpick comments (9)
docs/learn/beginner/01-tx-lifecycle.md (9)

Line range hint 34-34: Consider a shorter alternative to avoid wordiness.

The phrase "can be used to scale gas up in order to avoid underestimating" could be shortened to "can scale gas up to avoid underestimation."


41-41: Consider shortening this phrase to just ‘whether’.

The phrase "whether or not" can be simplified to "whether" to make the text more concise.


150-150: Do not mix variants of the same word.

The word "finalized" should be consistent with the British English variant "finalised" if used elsewhere in the document. Please choose one variant and use it consistently throughout the document.


86-86: Trailing spaces.

Please remove the trailing spaces at the end of this line to maintain clean code formatting.


103-103: Trailing spaces.

Please remove the trailing spaces at the end of this line to maintain clean code formatting.


105-105: Trailing spaces.

Please remove the trailing spaces at the end of this line to maintain clean code formatting.


123-123: Trailing spaces.

Please remove the trailing spaces at the end of this line to maintain clean code formatting.


16-16: Multiple consecutive blank lines.

Please reduce the multiple consecutive blank lines to a single blank line to maintain clean formatting.


108-108: Multiple consecutive blank lines.

Please reduce the multiple consecutive blank lines to a single blank line to maintain clean formatting.

Review Details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits Files that changed from the base of the PR and between b795646 and 122c792.
Files selected for processing (1)
  • docs/learn/beginner/01-tx-lifecycle.md (3 hunks)
Additional Context Used
LanguageTool (6)
docs/learn/beginner/01-tx-lifecycle.md (6)

Near line 13: Unpaired symbol: ‘[’ seems to be missing
Context: ... * Anatomy of a Cosmos SDK Application ::: ## Transacti...


Near line 34: Consider a shorter alternative to avoid wordiness.
Context: ...optional) can be used to scale gas up in order to avoid underestimating. For example, use...


Near line 41: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...e of the two. Later, validators decide whether or not to include the transaction in their blo...


Near line 88: Possible missing comma found.
Context: ...ee of trasaction processing. Within the SDK we wrap the BaseApp with a runtime la...


Near line 111: Two consecutive dots
Context: ...e state to reflect the transfer if valid.. ### Module Execution 1. The transacti...


Near line 150: Do not mix variants of the same word (‘finalize’ and ‘finalise’) within a single text.
Context: ...to the state during the transaction are finalized and written to the blockchain. This ste...

Path-based Instructions (1)
docs/learn/beginner/01-tx-lifecycle.md (1)

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

docs/learn/beginner/01-tx-lifecycle.md Outdated Show resolved Hide resolved
docs/learn/beginner/01-tx-lifecycle.md Outdated Show resolved Hide resolved
docs/learn/beginner/01-tx-lifecycle.md Outdated Show resolved Hide resolved
docs/learn/beginner/01-tx-lifecycle.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.

Actionable comments posted: 8

Out of diff range and nitpick comments (1)
docs/learn/beginner/01-tx-lifecycle.md (1)

Line range hint 34-34: Consider rephrasing to avoid wordiness.

- can be used to scale `gas` up in order to avoid underestimating.
+ can be used to scale `gas` up to avoid underestimating.
Review Details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits Files that changed from the base of the PR and between 122c792 and 5ea4174.
Files selected for processing (1)
  • docs/learn/beginner/01-tx-lifecycle.md (3 hunks)
Additional Context Used
LanguageTool (5)
docs/learn/beginner/01-tx-lifecycle.md (5)

Near line 13: Unpaired symbol: ‘[’ seems to be missing
Context: ... * Anatomy of a Cosmos SDK Application ::: ## Transacti...


Near line 34: Consider a shorter alternative to avoid wordiness.
Context: ...optional) can be used to scale gas up in order to avoid underestimating. For example, use...


Near line 41: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...e of the two. Later, validators decide whether or not to include the transaction in their blo...


Near line 88: Possible missing comma found.
Context: ...ee of trasaction processing. Within the SDK we wrap the BaseApp with a runtime la...


Near line 150: Do not mix variants of the same word (‘finalize’ and ‘finalise’) within a single text.
Context: ...to the state during the transaction are finalized and written to the blockchain. This ste...

Path-based Instructions (1)
docs/learn/beginner/01-tx-lifecycle.md (1)

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

Additional comments not posted (2)
docs/learn/beginner/01-tx-lifecycle.md (2)

13-13: Unpaired symbol: '[' seems to be missing.


88-88: Consider adding a comma after "SDK" for better readability: "Within the SDK, we wrap the BaseApp with a runtime layer..."


3. Error Handling: If any errors occur during transaction execution, they are handled appropriately, which may include rolling back certain operations to maintain state consistency.

4. State Commitment: Changes made to the state during the transaction are finalized and written to the blockchain. This step is crucial as it ensures that all state transitions are permanently recorded.
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure consistent use of American English spelling for "finalize".

- are finalized and written to the blockchain.
+ are finalized and written to the blockchain.

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
4. State Commitment: Changes made to the state during the transaction are finalized and written to the blockchain. This step is crucial as it ensures that all state transitions are permanently recorded.
4. State Commitment: Changes made to the state during the transaction are finalized and written to the blockchain. This step is crucial as it ensures that all state transitions are permanently recorded.

@@ -13,9 +13,8 @@
* [Anatomy of a Cosmos SDK Application](./00-app-anatomy.md)
:::

## Creation

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove multiple consecutive blank lines.

-

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


#### Guideline

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove multiple consecutive blank lines.

-

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

Copy link
Member

@tac0turtle tac0turtle left a comment

Choose a reason for hiding this comment

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

this seems to be removing lots of information without replacing it. I think there is a lot more information that can be added.

  • Appside mempool
  • finalisation
  • block inclusion
  • the ante handler starless vs stateful is still a concept
  • validate basic is an optional setting users can do.

@samricotta
Copy link
Contributor Author

@tac0turtle a couple of q's

  • We discussed cometbft mempool previously, although we are removing this... we want to replace it with info on appside?
  • I tried to remain a little higher level as its in beginner and didn't delve into the functions that are already inbaseApp but maybe I can just add that section back in the Transaction Execution as it was before and I can try give a bit more explanation and context about why we are using abci functions.

@tac0turtle
Copy link
Member

  • We discussed cometbft mempool previously, although we are removing this... we want to replace it with info on appside?

We should point to comets mempool if cometbft is being used, a link with a short description is useful. Keeping it high level enough for the user to go to comet to find out more

  • I tried to remain a little higher level as its in beginner and didn't delve into the functions that are already inbaseApp but maybe I can just add that section back in the Transaction Execution as it was before and I can try give a bit more explanation and context about why we are using abci functions.

if this is beginner we should move the information somewhere else. I dont think removing information and not replacing it is a path we should take. There is still valuable information in what is being removed.

@samricotta
Copy link
Contributor Author

samricotta commented May 6, 2024

We should point to comets mempool if cometbft is being used, a link with a short description is useful. Keeping it high level enough for the user to go to comet to find out more

But should we have a whole section on the mempool then in that case? Could have a one liner here saying in the case you are using comet the tx would be added to the mempool if it passes checks.... What I would love to do is remove this for now and add a section later for each consensus option such as rollkit, comet.

if this is beginner we should move the information somewhere else. I dont think removing information and not replacing it is a path we should take. There is still valuable information in what is being removed.

I wasnt going to remove it completely. Was planning on adding it into the BaseApp section depending on what already lives there but it needs to be looked at separately and not just added in without rewriting the whole page. Probably should have mentioned that in the comments (will do that for next time)

@tac0turtle
Copy link
Member

can that be done in this pr? ideally we do these things in the same pr to avoid it being removed couple days pass and then its added again. This happens too often in my time in cosmos.

@samricotta
Copy link
Contributor Author

samricotta commented May 7, 2024

Yep, thats completely fine. Ill target both of the issues (baseapp & txlifecycle). Do you have an answer on the comet/mempool part of my question?

But should we have a whole section on the mempool then in that case? Could have a one liner here saying in the case you are using comet the tx would be added to the mempool if it passes checks.... What I would love to do is remove this for now and add a section later for each consensus option such as rollkit, comet.

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: 2

Out of diff range and nitpick comments (1)
docs/learn/beginner/01-tx-lifecycle.md (1)

Line range hint 34-34: Consider a shorter alternative to avoid wordiness.

- can be used to scale `gas` up in order to avoid underestimating.
+ can be used to scale `gas` up to avoid underestimating.
Review Details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits Files that changed from the base of the PR and between 5ea4174 and c0ace62.
Files selected for processing (1)
  • docs/learn/beginner/01-tx-lifecycle.md (4 hunks)
Additional Context Used
LanguageTool (6)
docs/learn/beginner/01-tx-lifecycle.md (6)

Near line 13: Unpaired symbol: ‘[’ seems to be missing
Context: ... * Anatomy of a Cosmos SDK Application ::: ## Transacti...


Near line 34: Consider a shorter alternative to avoid wordiness.
Context: ...optional) can be used to scale gas up in order to avoid underestimating. For example, use...


Near line 41: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...e of the two. Later, validators decide whether or not to include the transaction in their blo...


Near line 41: The verb “is” doesn’t seem to fit in this context, “are” is probably more formally correct.
Context: .... Txis rejected if itsgas-prices` is not high enough, so users are incentivi...


Near line 192: Do not mix variants of the same word (‘finalise’ and ‘finalize’) within a single text.
Context: ... Cosmos SDK, several steps are taken to finalise the process: 1. Event Emission: Module...


Near line 200: Do not mix variants of the same word (‘finalise’ and ‘finalize’) within a single text.
Context: ...to the state during the transaction are finalised and written to the blockchain. This ste...

Path-based Instructions (1)
docs/learn/beginner/01-tx-lifecycle.md (1)

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

Additional comments not posted (1)
docs/learn/beginner/01-tx-lifecycle.md (1)

13-13: Unpaired symbol: '[' seems to be missing.


:::warning
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now living in 00-baseapp.md

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: 8

Out of diff range and nitpick comments (6)
docs/learn/advanced/00-baseapp.md (6)

Line range hint 23-23: Ensure the link to "Service Routers" is correctly formatted.

- * [Service Routers](#service-routers), to route messages an...
+ * [Service Routers](#service-routers), to route messages and...

Line range hint 69-69: Ensure the link to "Msg Service Router" is correctly formatted.

- * [`Msg` Service Router](#msg-service-router): The `msgServiceRo...
+ * [`Msg` Service Router](#msg-service-router): The `msgServiceRouter`...

Line range hint 73-73: Ensure the link to "gRPC Query Router" is correctly formatted.

- * [gRPC Query Router](#grpc-query-router): The `grpcQueryRout...
+ * [gRPC Query Router](#grpc-query-router): The `grpcQueryRouter`...

Line range hint 219-219: Ensure the link to "sdk.Msgs" is correctly formatted.

- [`sdk.Msg`s](../../build/building-modules/02-message...
+ [`sdk.Msg`s](../../build/building-modules/02-messages-and-queries.md#messages)...

Line range hint 447-447: Ensure the link to "Consensus Parameters" is correctly formatted.

- * [Consensus Parameters](https://docs.cometbft.com/v1.0/spec/abc...
+ * [Consensus Parameters](https://docs.cometbft.com/v1.0/spec/abci/abci++_app_requirements#consensus-parameters)...

Line range hint 448-448: Ensure the link to "checkState and finalizeBlockState" is correctly formatted.

- * [`checkState` and `finalizeBlockState`](#state-updates) via `setState`. * The [...
+ * [`checkState` and `finalizeBlockState`](#state-updates) via `setState`. * The [`Block gas meter`](../beginner/04-gas-fees.md#block-gas-meter)...
Review Details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits Files that changed from the base of the PR and between c0ace62 and ba72d1e.
Files ignored due to path filters (1)
  • docs/learn/advanced/blockprocessing.png is excluded by !**/*.png
Files selected for processing (2)
  • docs/learn/advanced/00-baseapp.md (6 hunks)
  • docs/learn/beginner/01-tx-lifecycle.md (4 hunks)
Additional Context Used
LanguageTool (79)
docs/learn/advanced/00-baseapp.md (72)

Near line 23: Unpaired symbol: ‘[’ seems to be missing
Context: ...gine (e.g. CometBFT). * Service Routers, to route messages an...


Near line 63: Loose punctuation mark.
Context: ...iStore`](./04-store.md#commitmultistore): This is the main store of the applicati...


Near line 69: Unpaired symbol: ‘[’ seems to be missing
Context: ...ata persistence. * Msg Service Router: The `msgServiceRo...


Near line 71: Consider a shorter alternative to avoid wordiness.
Context: ...at needs to be processed by a service in order to update the application state, and not t...


Near line 73: Unpaired symbol: ‘[’ seems to be missing
Context: ...g consensus engine. * gRPC Query Router: The `grpcQueryRout...


Near line 76: Loose punctuation mark.
Context: ...b.com/cosmos/cosmos-sdk/types#TxDecoder): It is used to decode raw transaction ...


Near line 78: Loose punctuation mark.
Context: ... engine. * AnteHandler: This handler is used to handle signatur...


Near line 81: Loose punctuation mark.
Context: .../beginner/00-app-anatomy.md#initchainer), [PreBlocker](../beginner/00-app-anato...


Near line 87: Loose punctuation mark.
Context: ...s) (i.e. cached states): * checkState: This state is updated during [CheckTx...


Near line 88: Loose punctuation mark.
Context: ...ommit](#commit). * finalizeBlockState: This state is updated during [Finalize...


Near line 90: Loose punctuation mark.
Context: ...FinalizeBlock. * processProposalState: This state is updated during [ProcessP...


Near line 91: Loose punctuation mark.
Context: ...cess-proposal). * prepareProposalState: This state is updated during [`PrepareP...


Near line 95: Loose punctuation mark.
Context: ...ore important parameters: * voteInfos: This parameter carries the list of vali...


Near line 99: Loose punctuation mark.
Context: ...hing absent validators. * minGasPrices: This parameter defines the minimum gas ...


Near line 106: Loose punctuation mark.
Context: ...n 1uatom OR 1photon). * appVersion: Version of the application. It is set i...


Near line 151: ‘off of’ might be wordy. Consider a shorter alternative.
Context: ...ckTx, the checkState`, which is based off of the last committed state from the root ...


Near line 175: ‘off of’ might be wordy. Consider a shorter alternative.
Context: ...the processProposalState is set based off of the last committed state from the root...


Near line 177: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...called and the AnteHandler is executed and the context used in this state is built...


Near line 187: ‘off of’ might be wordy. Consider a shorter alternative.
Context: ...lock. The finalizeBlockState is based off of the last committed state from the root ...


Near line 215: Consider a shorter alternative to avoid wordiness.
Context: ...ust be routed to the appropriate module in order to be processed. Routing is done via `Base...


Near line 219: Unpaired symbol: ‘[’ seems to be missing
Context: .... ### Msg Service Router [sdk.Msgs](../../build/building-modules/02-message...


Near line 219: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...holds amsgServiceRouter which maps fully-qualified service methods (string`, defined in e...


Near line 221: Possible missing comma found.
Context: ...nt to make use of more stateful routing mechanisms such as allowing governance to disable ...


Near line 227: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ... grpcQueryRouter, which maps modules' fully-qualified service methods (string, defined in t...


Near line 259: Possible missing comma found.
Context: ....Msg`'s. This is done after stateless checks as stateful checks are more computati...


Near line 265: Possible missing comma found.
Context: ...osalcomplements theProcessProposal` method which is executed after this method. Th...


Near line 269: Loose punctuation mark.
Context: ...e response contains: * Txs ([][]byte): List of transactions which will form a ...


Near line 282: Possible missing comma found.
Context: ... that the coherence property is adhered to i.e. all honest validators must accept ...


Near line 283: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...cept a proposal by an honest proposer. It's important to note that `ProcessPropos...


Near line 288: Possible missing comma found.
Context: ...ectly coding these methods could affect liveness as CometBFT is unable to receive 2/3 va...


Near line 292: Loose punctuation mark.
Context: ...e contains: * Status (ProposalStatus): Status of the proposal processing wher...


Near line 314: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...eds GasWanted, the execution is halted and the changes made to the cached copy of ...


Near line 329: Consider a shorter alternative to avoid wordiness.
Context: ...t based on the raw transaction size, in order to avoid spam with transactions that provi...


Near line 342: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...y pay the fees until the transaction is actually included in a block, because `checkStat...


Near line 342: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...nsaction is actually included in a block, because checkState never gets committed to th...


Near line 343: The plural noun “blocks” cannot be used with the article “a”. Did you mean “a block” or “blocks”?
Context: ...atest state of the main state each time a blocks gets committed. CheckTx r...


Near line 348: Loose punctuation mark.
Context: ...he response contains: * Code (uint32): Response Code. 0 if successful. * `Da...


Near line 349: Loose punctuation mark.
Context: ...de. 0 if successful. * Data ([]byte): Result bytes, if any. * Log (string):...


Near line 350: To form a complete sentence, be sure to include a subject.
Context: ...The output of the application's logger. May be non-deterministic. * `Info (string):...


Near line 351: To form a complete sentence, be sure to include a subject.
Context: ...Info (string):Additional information. May be non-deterministic. *GasWanted (int...


Near line 352: Loose punctuation mark.
Context: ...non-deterministic. * GasWanted (int64): Amount of gas requested for transaction...


Near line 353: Loose punctuation mark.
Context: ...ate the transaction. * GasUsed (int64): Amount of gas consumed by transaction. ...


Near line 359: Loose punctuation mark.
Context: ...c.go#L102 ``` * Events ([]cmn.KVPair): Key-Value tags for filtering and indexi...


Near line 359: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...or filtering and indexing transactions (eg. by account). See [events](./08-events...


Near line 360: Loose punctuation mark.
Context: ...nts.md) for more. * Codespace (string): Namespace for the Code. #### RecheckTx...


Near line 425: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...e. First, it retrieves the sdk.Msg's fully-qualified type name, by checking the type_url o...


Near line 429: Consider a shorter alternative to avoid wordiness.
Context: ... receives the Result of the RunMsgs in order to enable this customizable behavior. Lik...


Near line 445: This phrase is redundant. Consider writing “started”.
Context: ...lying CometBFT engine when the chain is first started. It is mainly used to initialize pa...


Near line 447: Unpaired symbol: ‘[’ seems to be missing
Context: ...and state like: * [Consensus Parameters](https://docs.cometbft.com/v1.0/spec/abc...


Near line 448: Unpaired symbol: ‘[’ seems to be missing
Context: ...etConsensusParams. * [checkStateandfinalizeBlockState](#state-updates) via setState`. * The [...


Near line 451: Consider a shorter alternative to avoid wordiness.
Context: ...tomy.md#initchainer) of the application in order to initialize the main state of the applic...


Near line 479: Did you mean to use ‘who’s’ here (the contracted form of ‘who’ and ‘is/has’)?
Context: ...pplication, i.e. the list of validators whose precommit for the previous block was ...


Near line 499: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ...n within FinalizeBlock performs the exact same steps as CheckTx, with a little cav...


Near line 516: Loose punctuation mark.
Context: ...he response contains: * Code (uint32): Response Code. 0 if successful. * `Da...


Near line 517: Loose punctuation mark.
Context: ...de. 0 if successful. * Data ([]byte): Result bytes, if any. * Log (string):...


Near line 518: To form a complete sentence, be sure to include a subject.
Context: ...The output of the application's logger. May be non-deterministic. * `Info (string):...


Near line 519: To form a complete sentence, be sure to include a subject.
Context: ...Info (string):Additional information. May be non-deterministic. *GasWanted (int...


Near line 520: Loose punctuation mark.
Context: ...non-deterministic. * GasWanted (int64): Amount of gas requested for transaction...


Near line 521: Loose punctuation mark.
Context: ...ate the transaction. * GasUsed (int64): Amount of gas consumed by transaction. ...


Near line 522: Loose punctuation mark.
Context: ... store occurs. * Events ([]cmn.KVPair): Key-Value tags for filtering and indexi...


Near line 522: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...or filtering and indexing transactions (eg. by account). See [events](./08-events...


Near line 523: Loose punctuation mark.
Context: ...nts.md) for more. * Codespace (string): Namespace for the Code. #### EndBlock ...


Near line 535: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...nsus but listen for votes to understand whether or not they should commit the state changes. ...


Near line 539: Consider a shorter alternative to avoid wordiness.
Context: ...nd finalizeBlockState resets to nil in order to be consistent and reflect the changes. ...


Near line 541: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...cks have the same number of transactions and it is possible for consensus to result ...


Near line 541: ‘none at all’ might be wordy. Consider a shorter alternative.
Context: ... to result in a nil block or one with none at all. In a public blockchain network, it is ...


Near line 541: Possible missing comma found.
Context: ... censor a Tx by excluding it from the block or a validator voting against the block...


Near line 543: Do not use a colon (:) before a series that is introduced by a preposition (‘over’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ... the transaction lifecycle of a Tx is over: nodes have verified its validity, deliv...


Near line 553: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ... to query. If the path matches a gRPC fully-qualified service method, then BaseApp will def...


Near line 561: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...rocess does NOT have to be deterministic and the data returned can be unique to the ...


Near line 563: Possible missing comma found.
Context: ...o the validator process. In the Cosmos-SDK this is implemented as a NoOp: ``` go ...


Near line 573: Possible missing comma found.
Context: ...itted Application state. In the Cosmos-SDK this is implemented as a NoOp: ```go r...

docs/learn/beginner/01-tx-lifecycle.md (7)

Near line 13: Unpaired symbol: ‘[’ seems to be missing
Context: ... * Anatomy of a Cosmos SDK Application ::: ## Transactio...


Near line 33: Consider a shorter alternative to avoid wordiness.
Context: ...optional) can be used to scale gas up in order to avoid underestimating. For example, use...


Near line 40: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...e of the two. Later, validators decide whether or not to include the transaction in their blo...


Near line 40: The verb “is” doesn’t seem to fit in this context, “are” is probably more formally correct.
Context: .... Txis rejected if itsgas-prices` is not high enough, so users are incentivi...


Near line 149: Possible missing comma found.
Context: ...further consensus processes. The app-side mempool, serves the purpose of keepin...


Near line 211: Do not mix variants of the same word (‘finalise’ and ‘finalize’) within a single text.
Context: ... Cosmos SDK, several steps are taken to finalise the process: 1. Event Emission: Module...


Near line 219: Do not mix variants of the same word (‘finalise’ and ‘finalize’) within a single text.
Context: ...to the state during the transaction are finalised and written to the blockchain. This ste...

Path-based Instructions (2)
docs/learn/beginner/01-tx-lifecycle.md (1)

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

docs/learn/advanced/00-baseapp.md (1)

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

Additional comments not posted (1)
docs/learn/beginner/01-tx-lifecycle.md (1)

16-16: Ensure the command-line example is accurate and up-to-date.


Consensus, the process through which validator nodes come to agreement on which transactions to
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is all in baseapp with a new diagram

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: 10

Out of diff range and nitpick comments (13)
docs/learn/advanced/00-baseapp.md (13)

Line range hint 23-23: Ensure the markdown link is correctly formatted.

- * [Service Routers](#service-routers), to route messages an...
+ * [Service Routers](#service-routers), to route messages and...

Line range hint 69-69: Ensure the markdown link is correctly formatted.

- * [`Msg` Service Router](#msg-service-router): The `msgServiceRo...
+ * [`Msg` Service Router](#msg-service-router): The `msgServiceRouter`...

Line range hint 73-73: Ensure the markdown link is correctly formatted.

- * [gRPC Query Router](#grpc-query-router): The `grpcQueryRout...
+ * [gRPC Query Router](#grpc-query-router): The `grpcQueryRouter`...

Line range hint 219-219: Ensure the markdown link is correctly formatted.

- ### `Msg` Service Router  [`sdk.Msg`s](../../build/building-modules/02-message...
+ ### `Msg` Service Router  [`sdk.Msg`s](../../build/building-modules/02-messages-and-queries.md#messages)...

Line range hint 447-447: Ensure the markdown link is correctly formatted.

- * [Consensus Parameters](https://docs.cometbft.com/v1.0/spec/abc...
+ * [Consensus Parameters](https://docs.cometbft.com/v1.0/spec/abci/abci++_app_requirements#consensus-parameters)...

Line range hint 448-448: Ensure the markdown link is correctly formatted.

- * [`checkState` and `finalizeBlockState`](#state-updates) via `setState`. * The [...
+ * [`checkState` and `finalizeBlockState`](#state-updates) via `setState`. * The [`block gas meter`](../beginner/04-gas-fees.md#block-gas-meter),...

Line range hint 151-151: Consider rephrasing to avoid wordiness.

- ...ckTx`, the `checkState`, which is based off of the last committed state from the root ...
+ ...ckTx`, the `checkState`, which is based on the last committed state from the root ...

Line range hint 175-175: Consider rephrasing to avoid wordiness.

- ...the `processProposalState` is set based off of the last committed state  from the root...
+ ...the `processProposalState` is set based on the last committed state from the root...

Line range hint 187-187: Consider rephrasing to avoid wordiness.

- ...lock. The `finalizeBlockState` is based off of the last committed state from the root ...
+ ...lock. The `finalizeBlockState` is based on the last committed state from the root ...

Line range hint 215-215: Consider rephrasing to avoid wordiness.

- ...ust be routed to the appropriate module in order to be processed. Routing is done via `Base...
+ ...ust be routed to the appropriate module for processing. Routing is done via `Base...

Line range hint 429-429: Consider rephrasing to avoid wordiness.

- ... receives the `Result` of the `RunMsgs` in order to enable this customizable behavior.  Lik...
+ ... receives the `Result` of the `RunMsgs` to enable this customizable behavior.  Lik...

Line range hint 445-445: Consider rephrasing to avoid redundancy.

- ...lying CometBFT engine when the chain is first started. It is mainly used to **initialize** pa...
+ ...lying CometBFT engine when the chain starts. It is mainly used to **initialize** pa...

Line range hint 451-451: Consider rephrasing to avoid wordiness.

- ...tomy.md#initchainer) of the application in order to initialize the main state of the applic...
+ ...tomy.md#initchainer) of the application to initialize the main state of the applic...
Review Details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits Files that changed from the base of the PR and between ba72d1e and 33c4a21.
Files selected for processing (2)
  • docs/learn/advanced/00-baseapp.md (6 hunks)
  • docs/learn/beginner/01-tx-lifecycle.md (4 hunks)
Additional Context Used
LanguageTool (78)
docs/learn/advanced/00-baseapp.md (71)

Near line 23: Unpaired symbol: ‘[’ seems to be missing
Context: ...gine (e.g. CometBFT). * Service Routers, to route messages an...


Near line 63: Loose punctuation mark.
Context: ...iStore`](./04-store.md#commitmultistore): This is the main store of the applicati...


Near line 69: Unpaired symbol: ‘[’ seems to be missing
Context: ...ata persistence. * Msg Service Router: The `msgServiceRo...


Near line 71: Consider a shorter alternative to avoid wordiness.
Context: ...at needs to be processed by a service in order to update the application state, and not t...


Near line 73: Unpaired symbol: ‘[’ seems to be missing
Context: ...g consensus engine. * gRPC Query Router: The `grpcQueryRout...


Near line 76: Loose punctuation mark.
Context: ...b.com/cosmos/cosmos-sdk/types#TxDecoder): It is used to decode raw transaction ...


Near line 78: Loose punctuation mark.
Context: ... engine. * AnteHandler: This handler is used to handle signatur...


Near line 81: Loose punctuation mark.
Context: .../beginner/00-app-anatomy.md#initchainer), [PreBlocker](../beginner/00-app-anato...


Near line 87: Loose punctuation mark.
Context: ...s) (i.e. cached states): * checkState: This state is updated during [CheckTx...


Near line 88: Loose punctuation mark.
Context: ...ommit](#commit). * finalizeBlockState: This state is updated during [Finalize...


Near line 90: Loose punctuation mark.
Context: ...FinalizeBlock. * processProposalState: This state is updated during [ProcessP...


Near line 91: Loose punctuation mark.
Context: ...cess-proposal). * prepareProposalState: This state is updated during [`PrepareP...


Near line 95: Loose punctuation mark.
Context: ...ore important parameters: * voteInfos: This parameter carries the list of vali...


Near line 99: Loose punctuation mark.
Context: ...hing absent validators. * minGasPrices: This parameter defines the minimum gas ...


Near line 106: Loose punctuation mark.
Context: ...n 1uatom OR 1photon). * appVersion: Version of the application. It is set i...


Near line 151: ‘off of’ might be wordy. Consider a shorter alternative.
Context: ...ckTx, the checkState`, which is based off of the last committed state from the root ...


Near line 152: Possible missing comma found.
Context: ...tore, is used for any reads and writes. Here we only execute the AnteHandler and v...


Near line 175: ‘off of’ might be wordy. Consider a shorter alternative.
Context: ...the processProposalState is set based off of the last committed state from the root...


Near line 177: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...called and the AnteHandler is executed and the context used in this state is built...


Near line 187: ‘off of’ might be wordy. Consider a shorter alternative.
Context: ...lock. The finalizeBlockState is based off of the last committed state from the root ...


Near line 215: Consider a shorter alternative to avoid wordiness.
Context: ...ust be routed to the appropriate module in order to be processed. Routing is done via `Base...


Near line 219: Unpaired symbol: ‘[’ seems to be missing
Context: .... ### Msg Service Router [sdk.Msgs](../../build/building-modules/02-message...


Near line 219: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...holds amsgServiceRouter which maps fully-qualified service methods (string`, defined in e...


Near line 221: Possible missing comma found.
Context: ...nt to make use of more stateful routing mechanisms such as allowing governance to disable ...


Near line 227: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ... grpcQueryRouter, which maps modules' fully-qualified service methods (string, defined in t...


Near line 259: Possible missing comma found.
Context: ....Msg`'s. This is done after stateless checks as stateful checks are more computati...


Near line 265: Possible missing comma found.
Context: ...osalcomplements theProcessProposal` method which is executed after this method. Th...


Near line 269: Loose punctuation mark.
Context: ...e response contains: * Txs ([][]byte): List of transactions which will form a ...


Near line 283: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...cept a proposal by an honest proposer. It's important to note that `ProcessPropos...


Near line 288: Possible missing comma found.
Context: ...ectly coding these methods could affect liveness as CometBFT is unable to receive 2/3 va...


Near line 292: Loose punctuation mark.
Context: ...e contains: * Status (ProposalStatus): Status of the proposal processing wher...


Near line 314: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...eds GasWanted, the execution is halted and the changes made to the cached copy of ...


Near line 329: Consider a shorter alternative to avoid wordiness.
Context: ...t based on the raw transaction size, in order to avoid spam with transactions that provi...


Near line 342: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...y pay the fees until the transaction is actually included in a block, because `checkStat...


Near line 342: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...nsaction is actually included in a block, because checkState never gets committed to th...


Near line 343: The plural noun “blocks” cannot be used with the article “a”. Did you mean “a block” or “blocks”?
Context: ...atest state of the main state each time a blocks gets committed. CheckTx r...


Near line 348: Loose punctuation mark.
Context: ...he response contains: * Code (uint32): Response Code. 0 if successful. * `Da...


Near line 349: Loose punctuation mark.
Context: ...de. 0 if successful. * Data ([]byte): Result bytes, if any. * Log (string):...


Near line 350: To form a complete sentence, be sure to include a subject.
Context: ...The output of the application's logger. May be non-deterministic. * `Info (string):...


Near line 351: To form a complete sentence, be sure to include a subject.
Context: ...Info (string):Additional information. May be non-deterministic. *GasWanted (int...


Near line 352: Loose punctuation mark.
Context: ...non-deterministic. * GasWanted (int64): Amount of gas requested for transaction...


Near line 353: Loose punctuation mark.
Context: ...ate the transaction. * GasUsed (int64): Amount of gas consumed by transaction. ...


Near line 359: Loose punctuation mark.
Context: ...c.go#L102 ``` * Events ([]cmn.KVPair): Key-Value tags for filtering and indexi...


Near line 359: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...or filtering and indexing transactions (eg. by account). See [events](./08-events...


Near line 360: Loose punctuation mark.
Context: ...nts.md) for more. * Codespace (string): Namespace for the Code. #### RecheckTx...


Near line 425: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...e. First, it retrieves the sdk.Msg's fully-qualified type name, by checking the type_url o...


Near line 429: Consider a shorter alternative to avoid wordiness.
Context: ... receives the Result of the RunMsgs in order to enable this customizable behavior. Lik...


Near line 445: This phrase is redundant. Consider writing “started”.
Context: ...lying CometBFT engine when the chain is first started. It is mainly used to initialize pa...


Near line 447: Unpaired symbol: ‘[’ seems to be missing
Context: ...and state like: * [Consensus Parameters](https://docs.cometbft.com/v1.0/spec/abc...


Near line 448: Unpaired symbol: ‘[’ seems to be missing
Context: ...etConsensusParams. * [checkStateandfinalizeBlockState](#state-updates) via setState`. * The [...


Near line 451: Consider a shorter alternative to avoid wordiness.
Context: ...tomy.md#initchainer) of the application in order to initialize the main state of the applic...


Near line 479: Did you mean to use ‘who’s’ here (the contracted form of ‘who’ and ‘is/has’)?
Context: ...pplication, i.e. the list of validators whose precommit for the previous block was ...


Near line 499: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ...n within FinalizeBlock performs the exact same steps as CheckTx, with a little cav...


Near line 516: Loose punctuation mark.
Context: ...he response contains: * Code (uint32): Response Code. 0 if successful. * `Da...


Near line 517: Loose punctuation mark.
Context: ...de. 0 if successful. * Data ([]byte): Result bytes, if any. * Log (string):...


Near line 518: To form a complete sentence, be sure to include a subject.
Context: ...The output of the application's logger. May be non-deterministic. * `Info (string):...


Near line 519: To form a complete sentence, be sure to include a subject.
Context: ...Info (string):Additional information. May be non-deterministic. *GasWanted (int...


Near line 520: Loose punctuation mark.
Context: ...non-deterministic. * GasWanted (int64): Amount of gas requested for transaction...


Near line 521: Loose punctuation mark.
Context: ...ate the transaction. * GasUsed (int64): Amount of gas consumed by transaction. ...


Near line 522: Loose punctuation mark.
Context: ... store occurs. * Events ([]cmn.KVPair): Key-Value tags for filtering and indexi...


Near line 522: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...or filtering and indexing transactions (eg. by account). See [events](./08-events...


Near line 523: Loose punctuation mark.
Context: ...nts.md) for more. * Codespace (string): Namespace for the Code. #### EndBlock ...


Near line 535: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...nsus but listen for votes to understand whether or not they should commit the state changes. ...


Near line 539: Consider a shorter alternative to avoid wordiness.
Context: ...nd finalizeBlockState resets to nil in order to be consistent and reflect the changes. ...


Near line 541: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...cks have the same number of transactions and it is possible for consensus to result ...


Near line 541: ‘none at all’ might be wordy. Consider a shorter alternative.
Context: ... to result in a nil block or one with none at all. In a public blockchain network, it is ...


Near line 541: Possible missing comma found.
Context: ... censor a Tx by excluding it from the block or a validator voting against the block...


Near line 543: Do not use a colon (:) before a series that is introduced by a preposition (‘over’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ... the transaction lifecycle of a Tx is over: nodes have verified its validity, deliv...


Near line 553: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ... to query. If the path matches a gRPC fully-qualified service method, then BaseApp will def...


Near line 561: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...rocess does NOT have to be deterministic and the data returned can be unique to the ...


Near line 573: Possible missing comma found.
Context: ...itted Application state. In the Cosmos-SDK this is implemented as a NoOp: ```go r...

docs/learn/beginner/01-tx-lifecycle.md (7)

Near line 13: Unpaired symbol: ‘[’ seems to be missing
Context: ... * Anatomy of a Cosmos SDK Application ::: ## Transactio...


Near line 33: Consider a shorter alternative to avoid wordiness.
Context: ...optional) can be used to scale gas up in order to avoid underestimating. For example, use...


Near line 40: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...e of the two. Later, validators decide whether or not to include the transaction in their blo...


Near line 40: The verb “is” doesn’t seem to fit in this context, “are” is probably more formally correct.
Context: .... Txis rejected if itsgas-prices` is not high enough, so users are incentivi...


Near line 149: Possible missing comma found.
Context: ...further consensus processes. The app-side mempool, serves the purpose of keepin...


Near line 211: Do not mix variants of the same word (‘finalise’ and ‘finalize’) within a single text.
Context: ... Cosmos SDK, several steps are taken to finalise the process: 1. Event Emission: Module...


Near line 219: Do not mix variants of the same word (‘finalise’ and ‘finalize’) within a single text.
Context: ...to the state during the transaction are finalised and written to the blockchain. This ste...

Path-based Instructions (2)
docs/learn/beginner/01-tx-lifecycle.md (1)

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

docs/learn/advanced/00-baseapp.md (1)

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


`ValidateBasic` should not be used anymore. Message validation should be performed in the `Msg` service when [handling a message](../../build/building-modules/03-msg-services.md#validation) in a module Msg Server.
The **app-side mempool**, serves the purpose of keeping track of transactions seen by all full-nodes. Full-nodes maintain a **mempool cache** of the last `mempool.cache_size` transactions they have seen, serving as a first line of defense to prevent replay attacks. Ideally, `mempool.cache_size` should be large enough to encompass all transactions in the full mempool. If the mempool cache is too small to track all transactions, the initial transaction validation process is responsible for identifying and rejecting replayed transactions.
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a missing comma for clarity.

- The **app-side mempool**, serves the purpose of keeping track of transactions seen by all full-nodes.
+ The **app-side mempool**, serves the purpose of keeping track of transactions seen by all full-nodes.

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 **app-side mempool**, serves the purpose of keeping track of transactions seen by all full-nodes. Full-nodes maintain a **mempool cache** of the last `mempool.cache_size` transactions they have seen, serving as a first line of defense to prevent replay attacks. Ideally, `mempool.cache_size` should be large enough to encompass all transactions in the full mempool. If the mempool cache is too small to track all transactions, the initial transaction validation process is responsible for identifying and rejecting replayed transactions.
The **app-side mempool**, serves the purpose of keeping track of transactions seen by all full-nodes. Full-nodes maintain a **mempool cache** of the last `mempool.cache_size` transactions they have seen, serving as a first line of defense to prevent replay attacks. Ideally, `mempool.cache_size` should be large enough to encompass all transactions in the full mempool. If the mempool cache is too small to track all transactions, the initial transaction validation process is responsible for identifying and rejecting replayed transactions.


On the `BaseApp` end, the `Commit(res abci.CommitResponse)` function is implemented to commit all the valid state transitions that occurred during `FinalizeBlock` and to reset state for the next block. It syncs all the state transitions by writing the `finalizeBlockState.ms` into the application's internal state. `finalizeBlockState.ms` is a branched multistore of the main store `app.cms`. The `Commit` function calls the `Write()` function on `finalizeBlockState.ms`, effectively committing the state transitions. Then, the `Commit` function sets `checkState` to the latest header (obtained from `finalizeBlockState.ctx.BlockHeader`) and `finalizeBlockState` to `nil`.

Finally, `Commit` returns the hash of the commitment of `app.cms` back to the underlying consensus engine. This hash is used as a reference in the header of the next block. As soon as the state changes are committed, `checkState` starts afresh from the most recently committed state and `finalizeBlockState` resets to `nil` in order to be consistent and reflect the changes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider rephrasing to avoid wordiness.

- ...nd `finalizeBlockState` resets to `nil` in order to be consistent and reflect the changes. ...
+ ...nd `finalizeBlockState` resets to `nil` to be consistent and reflect the changes. ...

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
Finally, `Commit` returns the hash of the commitment of `app.cms` back to the underlying consensus engine. This hash is used as a reference in the header of the next block. As soon as the state changes are committed, `checkState` starts afresh from the most recently committed state and `finalizeBlockState` resets to `nil` in order to be consistent and reflect the changes.
Finally, `Commit` returns the hash of the commitment of `app.cms` back to the underlying consensus engine. This hash is used as a reference in the header of the next block. As soon as the state changes are committed, `checkState` starts afresh from the most recently committed state and `finalizeBlockState` resets to `nil` to be consistent and reflect the changes.

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: 8

Review Details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits Files that changed from the base of the PR and between 33c4a21 and c714089.
Files selected for processing (1)
  • docs/learn/beginner/01-tx-lifecycle.md (4 hunks)
Additional Context Used
LanguageTool (5)
docs/learn/beginner/01-tx-lifecycle.md (5)

Near line 13: Unpaired symbol: ‘[’ seems to be missing
Context: ... * Anatomy of a Cosmos SDK Application ::: ## Transactio...


Near line 33: Consider a shorter alternative to avoid wordiness.
Context: ...optional) can be used to scale gas up in order to avoid underestimating. For example, use...


Near line 40: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...e of the two. Later, validators decide whether or not to include the transaction in their blo...


Near line 212: Do not mix variants of the same word (‘finalise’ and ‘finalize’) within a single text.
Context: ... Cosmos SDK, several steps are taken to finalise the process: 1. Event Emission: Module...


Near line 220: Do not mix variants of the same word (‘finalise’ and ‘finalize’) within a single text.
Context: ...to the state during the transaction are finalised and written to the blockchain. This ste...

Path-based Instructions (1)
docs/learn/beginner/01-tx-lifecycle.md (1)

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

During the module execution phase, each message that has been routed to the appropriate module is processed according to the module-specific business logic. For example, the `handleMsgSend` function in the bank module processes `MsgSend` messages by checking balances, transferring tokens, and emitting events:


```go
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove trailing spaces.

- func handleMsgSend(ctx sdk.Context, keeper BankKeeper, msg MsgSend) error {
+ func handleMsgSend(ctx sdk.Context, keeper BankKeeper, msg MsgSend) error {

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
```go

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

Review Details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits Files that changed from the base of the PR and between c714089 and e0bd6e0.
Files selected for processing (1)
  • docs/learn/beginner/01-tx-lifecycle.md (4 hunks)
Additional Context Used
LanguageTool (5)
docs/learn/beginner/01-tx-lifecycle.md (5)

Near line 13: Unpaired symbol: ‘[’ seems to be missing
Context: ... * Anatomy of a Cosmos SDK Application ::: ## Transactio...


Near line 33: Consider a shorter alternative to avoid wordiness.
Context: ...optional) can be used to scale gas up in order to avoid underestimating. For example, use...


Near line 40: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...e of the two. Later, validators decide whether or not to include the transaction in their blo...


Near line 209: Do not mix variants of the same word (‘finalise’ and ‘finalize’) within a single text.
Context: ... Cosmos SDK, several steps are taken to finalise the process: 1. Event Emission: Module...


Near line 217: Do not mix variants of the same word (‘finalise’ and ‘finalize’) within a single text.
Context: ...to the state during the transaction are finalised and written to the blockchain. This ste...

Path-based Instructions (1)
docs/learn/beginner/01-tx-lifecycle.md (1)

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

@@ -13,9 +13,7 @@ This document describes the lifecycle of a transaction from creation to committe
* [Anatomy of a Cosmos SDK Application](./00-app-anatomy.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct the Markdown link syntax.

- [Anatomy of a Cosmos SDK Application](./00-app-anatomy.md)
+ [Anatomy of a Cosmos SDK Application](./00-app-anatomy.md)

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
* [Anatomy of a Cosmos SDK Application](./00-app-anatomy.md)
* [Anatomy of a Cosmos SDK Application](./00-app-anatomy.md)

@@ -27,7 +25,7 @@

There are several required and optional flags for transaction creation. The `--from` flag specifies which [account](./03-accounts.md) the transaction is originating from. For example, if the transaction is sending coins, the funds are drawn from the specified `from` address.

#### Gas and Fees
### Gas and Fees
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider simplifying the explanation of the --gas-adjustment flag.

- * `--gas-adjustment` (optional) can be used to scale `gas` up in order to avoid underestimating. For example, users can specify their gas adjustment as 1.5 to use 1.5 times the estimated gas.
+ * `--gas-adjustment` (optional) scales the estimated gas by a specified factor to prevent underestimation, e.g., 1.5 times.

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
### Gas and Fees
### Gas and Fees
* `--gas-adjustment` (optional) scales the estimated gas by a specified factor to prevent underestimation, e.g., 1.5 times.

@@ -41,53 +39,101 @@

Later, validators decide whether or not to include the transaction in their block by comparing the given or calculated `gas-prices` to their local `min-gas-prices`. `Tx` is rejected if its `gas-prices` is not high enough, so users are incentivized to pay more.
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct grammatical error in the description of gas prices.

- `Tx` is rejected if its `gas-prices` is not high enough, so users are incentivized to pay more.
+ `Tx` is rejected if its `gas-prices` are not high enough, so users are incentivized to pay more.

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
Later, validators decide whether or not to include the transaction in their block by comparing the given or calculated `gas-prices` to their local `min-gas-prices`. `Tx` is rejected if its `gas-prices` is not high enough, so users are incentivized to pay more.
Later, validators decide whether or not to include the transaction in their block by comparing the given or calculated `gas-prices` to their local `min-gas-prices`. `Tx` is rejected if its `gas-prices` are not high enough, so users are incentivized to pay more.

In order to verify a `Tx`, full-nodes call `CheckTx`, which includes both _stateless_ and _stateful_
checks. Further validation happens later in the [`DeliverTx`](../advanced/00-baseapp.md#delivertx) stage. `CheckTx` goes
through several steps, beginning with decoding `Tx`.
## Transaction Processing
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove trailing spaces.

- Within the SDK we wrap the `BaseApp` with a runtime layer defined in `runtime/app.go`. 
+ Within the SDK we wrap the `BaseApp` with a runtime layer defined in `runtime/app.go`.

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
## Transaction Processing
## Transaction Processing

How Routing Works:
The transaction is routed to the appropriate module based on the message type. Each message type is associated with a specific module, which is responsible for processing the message. The core transaction processing layer uses a `MsgServiceRouter` to direct the transaction to the correct module.

1. **Transaction Type Identification:**
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove trailing spaces.

- The transaction is routed to the appropriate module based on the message type. 
+ The transaction is routed to the appropriate module based on the message type.

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. **Transaction Type Identification:**
1. **Transaction Type Identification:**


1. **Transaction Type Identification:**
Each transaction contains one or more messages (`sdk.Msg`), and each message has a `Type()` method that identifies its type. This type is used to determine the appropriate module to handle the message.
2. **Module Routing:**
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove trailing spaces.

- Each transaction contains one or more messages (`sdk.Msg`), and each message has a `Type()` method that identifies its type. 
+ Each transaction contains one or more messages (`sdk.Msg`), and each message has a `Type()` method that identifies its type.

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
2. **Module Routing:**
2. **Module Routing:**

Comment on lines +209 to +217
After a transaction is executed in the Cosmos SDK, several steps are taken to finalise the process:

1. Event Emission: Modules emit events that can be used for logging, monitoring, or triggering other workflows. These events are collected during the transaction execution.

2. Logging: Information about the transaction execution, such as success or failure, and any significant state changes, are logged for audit and diagnostic purposes.

3. Error Handling: If any errors occur during transaction execution, they are handled appropriately, which may include rolling back certain operations to maintain state consistency.

4. State Commitment: Changes made to the state during the transaction are finalised and written to the blockchain. This step is crucial as it ensures that all state transitions are permanently recorded.
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure consistent use of American English spelling for "finalize".

- finalise
+ finalize

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
After a transaction is executed in the Cosmos SDK, several steps are taken to finalise the process:
1. Event Emission: Modules emit events that can be used for logging, monitoring, or triggering other workflows. These events are collected during the transaction execution.
2. Logging: Information about the transaction execution, such as success or failure, and any significant state changes, are logged for audit and diagnostic purposes.
3. Error Handling: If any errors occur during transaction execution, they are handled appropriately, which may include rolling back certain operations to maintain state consistency.
4. State Commitment: Changes made to the state during the transaction are finalised and written to the blockchain. This step is crucial as it ensures that all state transitions are permanently recorded.
After a transaction is executed in the Cosmos SDK, several steps are taken to finalize the process:
1. Event Emission: Modules emit events that can be used for logging, monitoring, or triggering other workflows. These events are collected during the transaction execution.
2. Logging: Information about the transaction execution, such as success or failure, and any significant state changes, are logged for audit and diagnostic purposes.
3. Error Handling: If any errors occur during transaction execution, they are handled appropriately, which may include rolling back certain operations to maintain state consistency.
4. State Commitment: Changes made to the state during the transaction are finalized and written to the blockchain. This step is crucial as it ensures that all state transitions are permanently recorded.

@samricotta
Copy link
Contributor Author

samricotta commented May 8, 2024

@coderabbitai pause

Copy link
Contributor

coderabbitai bot commented May 8, 2024

Actions Performed

Reviews paused.

Copy link
Member

@tac0turtle tac0turtle left a comment

Choose a reason for hiding this comment

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

this is super good!! nice job. We should add some mermaid diagrams on sections to help show. Trying to create more diagrams as its very informational. lmk if you want to pair on a few, I also need to learn how to use mermaidjs

(should add diagrams before merge)

docs/learn/advanced/00-baseapp.md Outdated Show resolved Hide resolved
Comment on lines 487 to 495
flowchart TD
A[Receive Block Proposal] --> B[FinalizeBlock]
B -->|BeginBlock| C{Execute Transactions}
C --> D[ExecuteTx(tx0)]
C --> E[ExecuteTx(tx1)]
C --> F[ExecuteTx(tx2)]
C --> G[ExecuteTx(tx3)]
C -->|EndBlock| H[Consensus]
H --> I[Commit]
Copy link
Contributor

Choose a reason for hiding this comment

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

the "(tx)" are giving trouble to the renderer, this should work:

Suggested change
flowchart TD
A[Receive Block Proposal] --> B[FinalizeBlock]
B -->|BeginBlock| C{Execute Transactions}
C --> D[ExecuteTx(tx0)]
C --> E[ExecuteTx(tx1)]
C --> F[ExecuteTx(tx2)]
C --> G[ExecuteTx(tx3)]
C -->|EndBlock| H[Consensus]
H --> I[Commit]
flowchart TD
A[Receive Block Proposal] --> B[FinalizeBlock]
B -->|BeginBlock| C{Execute Transactions}
C --> D["ExecuteTx(tx0)"]
C --> E["ExecuteTx(tx1)"]
C --> F["ExecuteTx(tx2)"]
C --> G["ExecuteTx(tx3)"]
C -->|EndBlock| H[Consensus]
H --> I[Commit]

Before the first transaction of a given block is processed, a [volatile state](#state-updates) called `finalizeBlockState` is initialized during FinalizeBlock. This state is updated each time a transaction is processed via `FinalizeBlock`, and committed to the [main state](#state-updates) when the block is [committed](#commit), after what it is set to `nil`.

The `FinalizeBlock` ABCI function defined in `BaseApp` and does the bulk of the state transitions: it is run for each transaction in the block in sequential order as committed to during consensus. Under the hood, transaction execution is almost identical to `CheckTx` but calls the [`runTx`](#runtx) function in deliver mode instead of check mode.
Copy link
Contributor

Choose a reason for hiding this comment

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

is the "and" in BaseApp and does unnecessary ?

@@ -408,6 +418,10 @@ Click [here](../beginner/04-gas-fees.md#antehandler) for more on the `anteHandle

`RunMsgs` is called from `RunTx` with `runTxModeCheck` as parameter to check the existence of a route for each message the transaction, and with `execModeFinalize` to actually process the `sdk.Msg`s.

After `CheckTx` exits, `FinalizeBlock` continues to run [`runMsgs`](../advanced/00-baseapp.md#runtx-antehandler-runmsgs-posthandler) to fully execute each `Msg` within the transaction. Since the transaction may have messages from different modules, `BaseApp` needs to know which module to find the appropriate handler. This is achieved using `BaseApp`'s `MsgServiceRouter` so that it can be processed by the module's Protobuf [`Msg` service](../../build/building-modules/03-msg-services.md).
Copy link
Member

Choose a reason for hiding this comment

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

This is not accurate, CheckTx is ran when including the tx to the mempool, not before FinalizeBlock. These 2 are in different flows, maybe just mention that FinalizeBlock also calls runmsgs and remove the After CheckTx exits,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ugh this is actually something prexisting from the intro page and was pasted here :/ and missed this. I think a lot is outdated

Copy link
Member

@facundomedica facundomedica left a comment

Choose a reason for hiding this comment

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

LGTM, left a comment

@samricotta samricotta added this pull request to the merge queue Jun 18, 2024
Merged via the queue into main with commit 3267b0a Jun 18, 2024
62 of 63 checks passed
@samricotta samricotta deleted the sam/lifecycle branch June 18, 2024 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🥳 Done
Development

Successfully merging this pull request may close these issues.

[Documentation]: Transaction Lifecycle section to be updated
4 participants