chore(core): bring changes from serverv2

[tac0turtle]

Description

Closes: #XXXX

This PR adds appmodule v2 and transaction to core to prep the upstream of server/v2 pieces

---

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
  • Consolidated and reorganized the Environment struct declaration.
  • Restructured functionality related to extension interfaces and their implementations.
  • Introduced a custom genesis handling API for app modules.
  • Added a DatabaseService for storing non-consensus data.
  • Enhanced gas tracking functionality and error signaling.
  • Introduced interfaces for transaction encoding, decoding, and handling.

[coderabbitai]

Walkthrough

Walkthrough

The recent update aims to enhance the core components of the system by refining various functionalities. This includes consolidating struct declarations, restructuring extension interfaces, introducing new interfaces for app modules, events, and transactions, improving gas tracking and error handling, and defining interfaces for non-consensus data operations in stores. These changes are designed to enhance modularity, efficiency, and clarity within the system's key components.

Changes

Files	Change Summary
core/appmodule/environment.go, core/appmodule/v2/environment.go, core/appmodule/module.go	Consolidated struct declarations, restructured extension interfaces, and introduced type aliases from appmodule/v2, enhancing modularity and relationships within the module.
core/appmodule/v2/appmodule.go, core/appmodule/v2/genesis.go, core/appmodule/v2/handlers.go, core/appmodule/v2/message.go, core/appmodule/v2/migrations.go	Introduced new interfaces for app modules, custom genesis handling, message handlers, message type retrieval, and module consensus versioning, improving app module functionality.
core/event/event.go	Introduced structures and functions for managing events in the system, defining key structures like Attribute, Events, and Event for streamlined event management.
core/event/service.go	Removed Attribute struct and NewAttribute function, potentially simplifying event service operations.
core/gas/meter.go	Enhanced gas tracking with new error handling like ErrOutOfGas, defining Gas as a uint64 alias, and adding NoGasLimit constant for improved gas functionality and error signaling.
core/store/database.go	Introduced DatabaseService interface and NonConsensusStore interface for CRUD operations on non-consensus data in a key-value store, enhancing data management capabilities.
core/transaction/transaction.go	Introduced a new package for transaction handling, defining interfaces for transaction encoding, decoding, and other transaction-related operations to streamline transaction management.

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

• X
• Mastodon
• Reddit
• LinkedIn

---

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-tests for this file.
• 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 tests 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 from git and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit tests.
  • @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.
• The JSON schema for the configuration file is available here.
• 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/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: .coderabbit.yml

Commits

Files that changed from the base of the PR and between 3d59013 and db29e1e.

Files selected for processing (13)

• core/appmodule/environment.go (1 hunks)
• core/appmodule/module.go (4 hunks)
• core/appmodule/v2/appmodule.go (1 hunks)
• core/appmodule/v2/environment.go (1 hunks)
• core/appmodule/v2/genesis.go (1 hunks)
• core/appmodule/v2/handlers.go (1 hunks)
• core/appmodule/v2/message.go (1 hunks)
• core/appmodule/v2/migrations.go (1 hunks)
• core/event/event.go (1 hunks)
• core/event/service.go (1 hunks)
• core/gas/meter.go (1 hunks)
• core/store/database.go (1 hunks)
• core/transaction/transaction.go (1 hunks)

Files skipped from review due to trivial changes (1)

• core/appmodule/environment.go

Additional comments: 26

core/appmodule/v2/genesis.go (1)

• 7-15: The HasGenesis interface is well-defined, focusing on custom genesis handling for app modules. It adheres to best practices by providing a clear contract for modules to implement genesis-related functionalities. The methods DefaultGenesis, ValidateGenesis, InitGenesis, and ExportGenesis cover the essential aspects of genesis handling, ensuring modules can define their default genesis state, validate genesis data, initialize their state at genesis, and export their current state as genesis data for future chains. This approach promotes modularity and maintainability by allowing each module to encapsulate its genesis logic.

core/appmodule/v2/message.go (2)

• 9-10: The type alias Message for protoiface.MessageV1 is correctly defined and simplifies the usage of protobuf messages within the app module context. This aliasing promotes code readability and maintainability by abstracting the underlying protobuf interface, making it easier for developers to work with messages without directly dealing with protobuf specifics.
• 12-20: The messageName function is a clever utility that provides a way to dynamically retrieve the full name of a protobuf message based on its type. This function supports both protov2.Message and gogoproto.Message, covering the common use cases in the Cosmos SDK ecosystem. The use of type switches and the panic in the default case is appropriate here, as it enforces the function's contract that only supported message types are passed as arguments. However, it's important to ensure that this function is used in contexts where the message type is known and controlled to avoid runtime panics.

core/appmodule/v2/environment.go (1)

• 12-22: The Environment struct is well-designed, serving as a central point for app modules to access various services like BranchService, EventService, GasService, and others. This design promotes modularity and maintainability by decoupling modules from direct dependencies on specific service implementations, allowing for easier testing and future changes to service implementations. The inclusion of a Logger is also a good practice, enabling modules to perform logging without directly depending on a specific logging library.

core/event/event.go (3)

• 3-10: The Attribute struct and its constructor function NewAttribute are correctly implemented, providing a simple yet effective way to represent key-value pair event attributes. This approach enhances code readability and maintainability by encapsulating the attribute creation logic within a dedicated function, promoting consistency across the codebase where event attributes are needed.
• 12-19: The Events struct and its constructor function NewEvents are well-implemented, offering a straightforward way to aggregate multiple events. This design supports modularity and ease of event management within the system, allowing for the collection and handling of events in a structured manner. The use of variadic arguments in NewEvents is particularly beneficial, providing flexibility in how events are aggregated.
• 21-29: The Event struct and its constructor function NewEvent are effectively designed, facilitating the creation and handling of events with a specific type and a list of attributes. This structure is crucial for the event system's flexibility and extensibility, enabling modules to define and emit custom events that can be handled appropriately by other parts of the system. The clear separation of event type and attributes promotes better organization and readability of event-related code.

core/store/database.go (2)

• 3-7: The DatabaseService interface is correctly defined, providing a clear contract for accessing the underlying database for CRUD operations of non-consensus data. The warning comment about the potential unprovability of modules using this API is crucial, ensuring developers are aware of the implications of using non-consensus data in their modules. This approach promotes careful consideration and responsible use of the database service in the context of blockchain applications.
• 9-23: The NonConsensusStore interface is well-designed, offering a simple yet comprehensive set of methods for CRUD operations on non-consensus data. The emphasis on error handling, especially for nil keys, enhances the robustness of the interface. However, it's important to ensure that implementations of this interface properly document the behavior and expectations for each method, especially regarding error conditions and the handling of non-existent keys, to ensure consistency and predictability across different implementations.

core/transaction/transaction.go (2)

• 12-17: The Codec interface is well-defined, providing a clear and concise contract for decoding transaction bytes into their concrete representation. This design promotes modularity and flexibility in transaction handling, allowing for different implementations of the codec that can handle various transaction formats. Ensuring that implementations of this interface handle decoding errors appropriately is crucial for the robustness of the transaction processing system.
• 19-32: The Tx interface encapsulates the essential functionalities required for handling transactions, including hashing, retrieving messages and senders, getting the gas limit, and obtaining the byte representation of the transaction. This comprehensive approach supports a wide range of transaction types and processing requirements. However, the TODO comments regarding the evaluation of the hash size and the reduction of senders to a single identity highlight areas for future consideration and potential optimization, which should be addressed to ensure the interface's effectiveness and efficiency in various use cases.

core/event/service.go (1)

• 36-38: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]

The Service and Manager interfaces in the event package are well-designed, providing a comprehensive API for event handling within app modules. These interfaces allow for emitting both consensus and non-consensus events, supporting a wide range of use cases. The distinction between consensus and non-consensus events is crucial for blockchain applications, ensuring that developers can correctly categorize events based on their impact on the blockchain state. Implementations of these interfaces should ensure deterministic and consistent event emission to maintain the integrity of the blockchain's state machine.

core/appmodule/v2/migrations.go (3)

• 5-11: The HasConsensusVersion interface provides a clear and essential method, ConsensusVersion, for declaring a module's consensus version. This method is crucial for versioning and migration management within the blockchain application, ensuring that consensus-breaking changes are tracked and handled appropriately. The emphasis on incrementing the version number on each consensus-breaking change promotes careful change management and compatibility across versions.
• 14-21: The HasMigrations interface, along with the RegisterMigrations method, is effectively designed to support module upgrades and migrations. This interface encourages modular design by allowing each module to define its migration logic, facilitating smooth transitions between consensus versions. It's important for implementations to carefully document the migration process and ensure that migrations are tested thoroughly to prevent issues during upgrades.
• 24-34: The MigrationRegistrar interface and its Register method provide a structured way for modules to register their in-place store migrations. This approach is beneficial for managing complex migrations across module versions, ensuring that each step of the migration process is clearly defined and executable. The requirement to register a no-op function for consensus version bumps without store changes is a good practice, maintaining the integrity of the migration process even when no changes are necessary.

core/gas/meter.go (4)

• 10-13: The introduction of the ErrOutOfGas error is a significant improvement in error handling for gas tracking. This explicit error type allows for more precise handling of out-of-gas scenarios, improving the robustness and user experience of the system by clearly signaling when a transaction has consumed all allowed gas. Implementations should ensure that this error is handled appropriately in all contexts where gas consumption is tracked.
• 15-17: The Gas type alias for uint64 is a good practice, enhancing code readability and maintainability by providing a clear and specific type for gas values. This alias helps to avoid confusion with other uint64 values that may represent different concepts, promoting clearer and more maintainable code.
• 20-21: The NoGasLimit constant, set to math.MaxUint64, is a clear and effective way to signal that no gas limit should be applied. This constant is useful in scenarios where transactions or operations should not be restricted by gas consumption, providing flexibility in gas management. It's important for developers to use this constant judiciously to avoid unintended consequences, such as allowing transactions that could potentially consume excessive resources.
• 22-22: The Service and Meter interfaces are well-designed, providing a comprehensive API for gas tracking and management within app modules. These interfaces support both transaction-level and block-level gas metering, offering flexibility in how gas consumption is tracked and managed across different contexts. Implementations of these interfaces should ensure accurate and efficient gas tracking to maintain the performance and reliability of the blockchain application.

core/appmodule/v2/appmodule.go (5)

• 9-19: The AppModule interface serves as a foundational tag interface for app module implementations, providing a basis for extension interfaces. This design promotes modularity and interoperability among modules by establishing a common type that all app modules should implement. The inclusion of dummy methods to tag and help resolve modules is a clever approach to leveraging Go's type system for module identification and dependency injection.
• 21-29: The HasBeginBlocker interface, with its BeginBlock method, is correctly designed to allow modules to execute custom logic before transaction processing in a block. This lifecycle hook is essential for modules that need to perform setup or state checks at the beginning of each block, enhancing the flexibility and control modules have over the blockchain's execution flow.
• 31-39: The HasEndBlocker interface, featuring the EndBlock method, provides a mechanism for modules to execute custom logic after transaction processing in a block. This capability is crucial for cleanup, state finalization, or triggering actions based on the block's transactions, further enhancing the modularity and extensibility of the blockchain application.
• 41-75: The HasTxValidation interface introduces a creative and flexible approach to transaction validation, allowing modules to implement custom logic for validating transactions. The detailed ASCII art comment effectively emphasizes the importance of the TxValidator method in preventing invalid transactions from passing. This interface is a significant improvement over previous patterns, such as the AnteHandler, providing a more modular and expressive way to define transaction validation logic. It's important for implementations to carefully document the expected behavior and conditions under which transactions are considered valid or invalid.
• 78-92: The HasUpdateValidators interface and the ValidatorUpdate struct provide a structured way for modules to participate in the validator set update process. This interface allows modules to propose changes to the validator set at the end of a block, supporting dynamic and decentralized validator management. The clear definition of the ValidatorUpdate struct, including the validator's public key, type, and updated power, facilitates precise and secure updates to the validator set.

core/appmodule/module.go (2)

• 15-21: The use of type aliases to reference entities from the appmodule/v2 package is a good practice, promoting code readability and maintainability by clearly indicating the source and version of the referenced entities. This approach helps to streamline the interface definitions and their relationships within the module, making it easier for developers to understand and work with the module's API.
• 61-65: The redeclaration of HasBeginBlocker and HasEndBlocker as type aliases from the appmodule/v2 package is consistent with the goal of streamlining interface definitions. This change ensures that the module's API remains clear and maintainable, with a direct reference to the latest version of these interfaces. It's important for developers to be aware of these aliases to correctly implement the required functionalities in their modules.

[julienrbrt]

I think this is the moment to add changelogs as well to be sure to miss nothing

[tac0turtle]

I think this is the moment to add changelogs as well to be sure to miss nothing

added

[coderabbitai]

Review Status

Actionable comments generated: 1

Configuration used: .coderabbit.yml

Commits

Files that changed from the base of the PR and between db29e1e and f55abd1.

Files selected for processing (2)

• core/CHANGELOG.md (1 hunks)
• core/appmodule/module.go (3 hunks)

Files skipped from review as they are similar to previous changes (1)

• core/appmodule/module.go

Additional comments: 2

core/CHANGELOG.md (2)

• 45-53: The changelog entry for PR chore(core): bring changes from serverv2 #19617 is comprehensive and follows the guiding principles outlined at the beginning of the file. However, it's important to ensure that each sub-point under this entry is directly related to the changes made in PR chore(core): bring changes from serverv2 #19617. If any of these sub-points are not part of the PR, they should be removed or adjusted accordingly. Additionally, consider breaking down complex entries into simpler, more digestible points for better readability.
• 55-55: The "API Breaking Changes" section correctly documents changes that could affect users of the SDK. Ensure that each entry in this section is accurate and provides enough detail for developers to understand the impact of the changes. It's also important to verify that all breaking changes introduced in PR chore(core): bring changes from serverv2 #19617 are documented here. If any are missing, they should be added.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: .coderabbit.yml

Commits

Files that changed from the base of the PR and between f55abd1 and bcb69a6.

Files selected for processing (1)

• core/appmodule/v2/genesis.go (1 hunks)

Files skipped from review as they are similar to previous changes (1)

• core/appmodule/v2/genesis.go

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: .coderabbit.yml

Commits

Files that changed from the base of the PR and between bcb69a6 and 29c0bb2.

Files selected for processing (2)

• core/go.mod (2 hunks)
• core/go.sum (3 hunks)

Additional comments: 6

core/go.mod (3)

• 7-7: Adding github.com/cosmos/gogoproto v1.4.11 introduces new or updated protobuf definitions. Ensure compatibility with existing protobuf structures and check for necessary migrations or code generation steps.
• 16-16: The inclusion of github.com/google/go-cmp v0.6.0 as an indirect dependency suggests updates to testing utilities. Verify that this version is compatible with existing tests and consider direct usage if comparison utilities are needed in tests.
• 24-24: golang.org/x/exp v0.0.0-20230811145659-89c5cff77bcb being added as an indirect dependency indicates experimental features might be in use. Ensure that the stability of these features is acceptable for the project's requirements and review their usage for potential risks.

core/go.sum (3)

• 4-5: Checksums for github.com/cosmos/gogoproto v1.4.11 have been added. Verify that these checksums match the expected values for the version specified to ensure the integrity of the dependency.
• 15-15: Checksums for github.com/google/go-cmp v0.6.0 are included. It's important to confirm these checksums align with the expected values for the specified version, safeguarding against corrupted or tampered dependencies.
• 42-43: Added checksums for golang.org/x/exp v0.0.0-20230811145659-89c5cff77bcb. As this is an indirect dependency, ensure that its inclusion is necessary and that the checksums are accurate, reflecting the correct version and integrity of the module.

[sontrinh16]

minor nit

Suggested change

	// Get returns nil iff key doesn't exist. Errors on nil key.
	// Get returns nil if key doesn't exist. Errors on nil key.

[tac0turtle]

if and only if, it should work