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

test: setup simapp for testing #3

Closed
wants to merge 6 commits into from

Conversation

spoo-bar
Copy link
Contributor

@spoo-bar spoo-bar commented Oct 14, 2024

  • Setup go work
  • Bumping go version
  • Adding simapp codestuff

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced a command for adding genesis accounts to the genesis.json file.
    • Added a new AnteHandler for enhanced transaction processing.
    • Implemented a command-line interface for managing the application.
  • Improvements

    • Updated dependencies and Go version for better performance and compatibility.
    • Enhanced build and testing workflow with new commands in the Justfile.
  • Bug Fixes

    • Added new entries to the .gitignore to prevent tracking unnecessary files.
  • Documentation

    • Improved clarity and functionality in the Justfile and command structures.

@spoo-bar spoo-bar marked this pull request as ready for review October 15, 2024 15:15
@spoo-bar spoo-bar requested a review from a team as a code owner October 15, 2024 15:15
Copy link

coderabbitai bot commented Oct 15, 2024

Walkthrough

This pull request introduces several significant changes across multiple files in the simapp module. Key updates include the addition of a new .gitignore entry, modifications to the go.mod file to update the Go version and dependencies, and enhancements to the justfile for improved build and testing commands. New files are introduced, such as ante.go, app.go, export.go, and genesis.go, which establish foundational structures and functionalities for the Cosmos SDK application, including transaction handling, application state management, and genesis state configuration.

Changes

File Change Summary
.gitignore Added simapp/build to ignore list.
go.mod Updated Go version to 1.23.2 and modified several dependencies, including cosmossdk.io/math, github.com/circlefin/noble-fiattokenfactory, and github.com/ethereum/go-ethereum. Added indirect dependencies.
justfile Enhanced with new commands: help, tidy, build, and modified test command. Default command updated to include tidy, build, and test.
simapp/ante.go Introduced an AnteHandler with HandlerOptions struct and a NewAnteHandler function for transaction processing.
simapp/app.go Defined the App struct and its methods for application initialization, transaction handling, and module management.
simapp/export.go Added methods for exporting application state and validators for genesis files.
simapp/genesis.go Introduced GenesisState type and NewDefaultGenesisState function for managing genesis state.
simapp/go.mod Created a new go.mod file for the simapp module, specifying dependencies and module path.
simapp/justfile New Justfile defining variables and a build target for the simapp project.
simapp/simd/cmd/encoding.go Added EncodingConfig struct and functions for managing encoding configurations.
simapp/simd/cmd/genaccounts.go Introduced command for adding genesis accounts to genesis.json.
simapp/simd/cmd/prefixes.go Added SetPrefixes function for setting address prefixes.
simapp/simd/cmd/root.go Established CLI structure for the application, including command handling and application management.
simapp/simd/cmd/util.go Introduced functions for managing command-line flags and configuration.
simapp/simd/main.go Created entry point for the application, initializing the command-line interface.

Possibly related PRs

  • feat: implement justfile #1: The changes in the go.mod file are related as both PRs update the Go version and modify dependencies, ensuring compatibility with the latest versions. Additionally, both PRs introduce a justfile that enhances the build and testing workflow for the project.

Suggested reviewers

  • johnletey

🐰 In the land of code, where changes bloom,
A new .gitignore sweeps away the gloom.
With go.mod updated, dependencies align,
Justfile commands, oh how they shine!
AnteHandlers and apps, a blockchain delight,
Hop along, dear coder, let’s code through the night! 🌙✨


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>, please review it.
    • 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @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 using 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.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration 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

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

🧹 Outside diff range and nitpick comments (25)
simapp/simd/main.go (2)

1-11: Consider removing or utilizing the blank import.

The blank import of "autocctp.dev/simapp/simd/cmd" on line 10 is unusual. Blank imports are typically used to trigger the package's init() function side effects, but they can also indicate unused code.

Please ensure this import is necessary. If it's not being used for its side effects, consider removing it or explicitly using the package to improve code clarity.


14-21: Consider handling the ignored return value from NewRootCmd.

The second return value from cmd.NewRootCmd is being ignored (line 14). This value might contain important information or an error that should be handled.

Consider capturing and handling this return value. If it's an error, you might want to log it or exit the program if it's critical. If it's not needed, you can explicitly ignore it using the blank identifier _ for better code clarity.

simapp/simd/cmd/prefixes.go (2)

5-5: Function signature looks good, consider adding documentation.

The SetPrefixes function has a clear name and appropriate parameter. To improve maintainability, consider adding a godoc comment explaining the function's purpose and the expected format of accountAddressPrefix.

You could add a comment like this:

// SetPrefixes configures and seals the Bech32 prefixes for the application.
// It takes the base accountAddressPrefix and derives other prefixes from it.

6-19: Implementation looks correct, consider adding a check for sealed config.

The prefix derivation and configuration logic is implemented correctly. The use of config.Seal() at the end is crucial for preventing further modifications.

For improved robustness, consider adding a check to see if the config is already sealed before attempting to modify it. This could prevent potential issues if the function is called multiple times.

You could modify the function like this:

func SetPrefixes(accountAddressPrefix string) {
	config := sdk.GetConfig()
	if config.Sealed() {
		return // or log a warning
	}

	// Rest of the function remains the same
	...
}

This change ensures that the function doesn't attempt to modify an already sealed configuration.

simapp/genesis.go (3)

9-16: LGTM: GenesisState type is well-defined and documented.

The GenesisState type is correctly defined as a map of string keys to raw JSON messages, which allows for flexible representation of module-specific genesis data. The accompanying comment provides clear and comprehensive information about its purpose and usage in the application.

Consider adding a brief example in the comment to illustrate how a module might use this type, which could enhance understanding for new contributors.


18-21: LGTM: NewDefaultGenesisState function is correctly implemented.

The NewDefaultGenesisState function is concise and correctly uses the ModuleBasics.DefaultGenesis method to generate the default genesis state. The function signature is appropriate, taking a codec.JSONCodec as a parameter and returning a GenesisState.

Consider adding a brief comment explaining what ModuleBasics is and where it's defined, as it's not immediately clear from this file. This would improve code readability and help developers understand the broader context.


1-21: Overall, excellent implementation of genesis state management.

This file provides a solid foundation for managing the genesis state in a Cosmos SDK application. The code is well-structured, follows Go best practices, and aligns with Cosmos SDK patterns. The GenesisState type and NewDefaultGenesisState function are correctly implemented and documented.

To enhance the file's completeness, consider adding:

  1. A brief package-level comment explaining the purpose of the app package.
  2. A simple example of how to use NewDefaultGenesisState in a typical application setup.

These additions would provide valuable context for developers working with this code.

simapp/justfile (4)

8-9: LGTM with suggestion: Tag version retrieval is well-implemented.

The tagversion variable correctly fetches the tag of the current commit, with appropriate handling for different operating systems. This is a good practice for cross-platform compatibility.

Consider capturing the error output instead of suppressing it. This could be useful for debugging if tag retrieval fails. You could modify the command to:

tagversion := if os_family() == "windows" { 
  `git describe --exact-match 2>&1 || echo "No tag found"` 
} else { 
  `git describe --exact-match 2>&1 || echo "No tag found"` 
}

This way, you'll have more informative output if there's no tag or if the command fails for any reason.


10-17: Suggestion: Rename dirtybranch for clarity and LGTM for branch.

The branch variable correctly retrieves the current git branch name.

However, the dirtybranch variable name might be misleading. It doesn't actually check if the branch is dirty, but rather it checks if there's a tag or returns the status of the working directory. Consider renaming it to something like tagOrStatus for clarity:

tagOrStatus := if tagversion == "" {
  `git status --porcelain || echo ""`
} else {
  tagversion
}

This name better reflects what the variable actually contains.


18-23: LGTM with suggestion: Version setting logic is correct.

The version variable correctly sets the version based on the branch status, appending "-dirty" if there are uncommitted changes.

If you rename the dirtybranch variable to tagOrStatus as suggested earlier, update this section accordingly:

version := if tagOrStatus == "" {
  branch
} else {
  branch + "-dirty"
}

This maintains the correct logic while using the more accurately named variable.


25-27: LGTM with suggestion: Build target is well-defined.

The build target is comprehensive and correctly uses the defined variables to set version information. The use of -mod=readonly is a good practice for ensuring reproducible builds.

Consider adding a clean target to remove the build/ directory before building. This ensures a fresh build each time. You could add:

clean:
    rm -rf build/

build: clean
    @go build -mod=readonly -ldflags "-X github.com/cosmos/cosmos-sdk/version.Name={{name}} -X github.com/cosmos/cosmos-sdk/version.AppName={{appname}} -X github.com/cosmos/cosmos-sdk/version.Version={{version}} -X github.com/cosmos/cosmos-sdk/version.Commit={{commit}}" -o build/ ./...

This addition would ensure that old artifacts are removed before each build, preventing potential issues with stale binaries.

justfile (4)

4-6: Helpful addition of a help command

The new help command is a great addition that improves the usability of the justfile. It allows users to easily see all available commands.

Consider adding a brief description for each command in the justfile. This can be done by adding comments above each command, which will be displayed when running just --list. For example:

# Format, build and test the simapp
default: format tidy build test

# Display all available commands
help:
    just --list

12-16: Effective dependency management with new tidy command

The new tidy command is a valuable addition that ensures proper dependency management across the project. It cleans up the go.mod files and syncs the Go workspace, which is crucial for maintaining a healthy project structure.

Consider adding error checking to ensure the command fails if any of the steps encounter an error. You can achieve this by removing the @ symbol at the beginning of each command, which will make the output visible, and by using set -e at the beginning of the command to exit on any error. Here's an example:

tidy:
    set -e
    go mod tidy
    go work sync
    cd simapp && go mod tidy

This way, if any of the commands fail, the entire tidy command will fail, alerting the user to potential issues.


36-44: Effective build command added

The new build command is a valuable addition that compiles the simd binary. The use of echo statements provides clear feedback to the user about the build process.

For consistency with the tidy command, consider changing the cd simapp && just build line to use multiple lines:

build:
    @echo "🤖 Building simd..."
    @cd simapp
    @just build
    @echo "✅ Completed build!"

This change makes it easier to add error handling in the future if needed, similar to the suggestion for the tidy command.


Line range hint 50-56: Enhanced test command with improved coverage and race detection

The modifications to the test command, including the addition of the test-unit target, significantly improve the testing process. The inclusion of coverage reporting and race detection enhances the quality of the tests.

Consider adding flags to control the verbosity and race detection. This would allow users to run quicker tests when needed. Here's an example implementation:

test: test-unit

# Run unit tests with optional flags for verbosity and race detection
# Usage: just test-unit [verbose] [race]
test-unit verbose='false' race='true':
    @echo "🤖 Running unit tests..."
    @go test -cover -coverprofile=coverage.out {{if race == "true"}}-race{{end}} {{if verbose == "true"}}-v{{end}}
    @echo "✅ Completed unit tests!"

This allows users to run tests with different options, e.g., just test-unit verbose=true or just test-unit race=false.

simapp/go.mod (1)

5-18: Consider using tagged versions for dependencies

Some dependencies are using specific commit hashes (e.g., github.com/circlefin/noble-cctp v0.0.0-20240510135541-253cf7eb9436). While this ensures reproducibility, it can make version tracking and updates more challenging. Consider using tagged versions when available.

For example, if a tagged version is available for noble-cctp, you could update it like this:

-github.com/circlefin/noble-cctp v0.0.0-20240510135541-253cf7eb9436
+github.com/circlefin/noble-cctp v1.0.0

Please review each dependency and update to tagged versions where possible.

go.mod (1)

Line range hint 290-309: Consider reviewing replace directives periodically.

The replace directives at the end of the file remain unchanged. While these are important for ensuring compatibility with specific versions of dependencies, it's good practice to periodically review their necessity.

Consider setting up a regular schedule (e.g., quarterly) to review these replace directives. This will help ensure that the project stays up-to-date with the latest compatible versions of these dependencies, potentially reducing technical debt over time.

simapp/export.go (1)

108-124: Unused variable counter

In lines 108-124, the variable counter is incremented within the loop but not used elsewhere in the code. If counter is not required, consider removing it to clean up the code.

Apply this diff to remove the unused variable:

-    counter := int16(0)
...
-        counter++
simapp/simd/cmd/util.go (3)

96-100: Simplify the logging writer assignment

The assignment of logWriter can be simplified for better readability by initializing it with the default value and only changing it if needed.

Apply this diff for clarity:

-var logWriter io.Writer
-if strings.ToLower(serverCtx.Viper.GetString(flags.FlagLogFormat)) == tmcfg.LogFormatPlain {
-	logWriter = zerolog.ConsoleWriter{Out: os.Stderr}
-} else {
-	logWriter = os.Stderr
+logWriter := os.Stderr
+if strings.EqualFold(serverCtx.Viper.GetString(flags.FlagLogFormat), tmcfg.LogFormatPlain) {
+	logWriter = zerolog.ConsoleWriter{Out: os.Stderr}
 }

118-190: Simplify error handling in interceptConfigs function

The switch-case block for error handling when checking the existence of config.toml can be refactored for better readability.

Consider the following refactor:

- switch _, err := os.Stat(tmCfgFile); {
- case os.IsNotExist(err):
+ if _, err := os.Stat(tmCfgFile); os.IsNotExist(err) {
     tmcfg.EnsureRoot(rootDir)
     // ... (rest of the configuration code)
- case err != nil:
+ } else if err != nil {
     return nil, err
- default:
+ } else {
     rootViper.SetConfigType("toml")
     // ... (rest of the code for existing configuration)
 }

This change makes the flow of error handling more straightforward.


119-190: Add documentation comments for exported functions

The exported function interceptConfigs lacks a documentation comment. Adding a descriptive comment enhances code readability and provides better understanding for other developers.

Include a documentation comment like this:

+// interceptConfigs parses and updates the Tendermint and application configuration files.
+// It ensures that default configurations are set if the configuration files do not exist.
+// The function returns the Tendermint configuration.
 func interceptConfigs(rootViper *viper.Viper, customAppTemplate string, customConfig interface{}) (*tmcfg.Config, error) {
simapp/simd/cmd/genaccounts.go (1)

145-146: Ensure Genesis Accounts Are Deduplicated Properly

After appending the new account and sanitizing the accounts list, it's important to ensure no duplicate accounts exist. While SanitizeGenesisAccounts helps with this, consider explicitly checking for duplicates to prevent potential state inconsistencies.

You might add a check before appending the new account:

// Ensure the account doesn't already exist
for _, acc := range accs {
    if acc.GetAddress().Equals(addr) {
        return fmt.Errorf("account %s already exists in genesis state", addr)
    }
}
simapp/simd/cmd/root.go (2)

171-171: Remove Unused Commented-Out Code

The line // startProxyForTunneledPeers(initClientCtx, cmd) is commented out. If this code is no longer needed, consider removing it to keep the codebase clean and maintainable.


421-436: Clarify App Configuration Defaults

The initAppConfig function mentions returning "" and nil if no custom configuration is required, but it always returns a default configuration.

// initAppConfig helps to override default appConfig template and configs.
// return "", nil if no custom configuration is required for the application.
func initAppConfig() (string, interface{}) {
    // ...
}

If a default configuration is always returned, update the comment to reflect this behavior accurately.

simapp/app.go (1)

139-146: Consider handling errors gracefully in the init function

In the init function, any error retrieving the user's home directory results in a panic. While panicking in an init function is acceptable for unrecoverable errors, you might consider handling the error more gracefully or providing a more descriptive error message to improve the user experience.

Alternatively, you could set a default path if the home directory cannot be determined.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 1a1aa2d and 267985b.

⛔ Files ignored due to path filters (4)
  • go.sum is excluded by !**/*.sum
  • go.work is excluded by !**/*.work
  • go.work.sum is excluded by !**/*.sum
  • simapp/go.sum is excluded by !**/*.sum
📒 Files selected for processing (15)
  • .gitignore (1 hunks)
  • go.mod (6 hunks)
  • justfile (2 hunks)
  • simapp/ante.go (1 hunks)
  • simapp/app.go (1 hunks)
  • simapp/export.go (1 hunks)
  • simapp/genesis.go (1 hunks)
  • simapp/go.mod (1 hunks)
  • simapp/justfile (1 hunks)
  • simapp/simd/cmd/encoding.go (1 hunks)
  • simapp/simd/cmd/genaccounts.go (1 hunks)
  • simapp/simd/cmd/prefixes.go (1 hunks)
  • simapp/simd/cmd/root.go (1 hunks)
  • simapp/simd/cmd/util.go (1 hunks)
  • simapp/simd/main.go (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • .gitignore
🧰 Additional context used
🔇 Additional comments (23)
simapp/simd/main.go (1)

13-26: LGTM: Main function structure and error handling.

The main function is well-structured:

  • It correctly sets up the root command with appropriate parameters.
  • The error handling for svrcmd.Execute is proper, exiting the program if an error occurs.

This implementation follows good practices for a Cosmos SDK application entry point.

simapp/simd/cmd/prefixes.go (1)

1-3: LGTM: Package declaration and import look good.

The package name cmd is appropriate for command-related functionality, and the SDK import is correctly aliased.

simapp/genesis.go (1)

3-7: LGTM: Import statements are correct and necessary.

The import statements are properly formatted and include the required packages for JSON handling and Cosmos SDK codec functionality.

simapp/justfile (3)

1-4: LGTM: Project name variables are well-defined.

The name and appname variables are correctly set for the simapp project. These will be useful for referencing throughout the build process.


5-6: LGTM: Commit hash retrieval is correct.

The commit variable correctly fetches the latest commit hash using Git. This is crucial for versioning and will be used in the build process.


1-27: Overall: Well-structured Justfile with minor suggestions for improvement.

This Justfile provides a solid foundation for building the simapp project. It correctly defines necessary variables for versioning and implements a comprehensive build target. The use of Git commands to fetch version information is particularly useful for maintaining accurate build metadata.

To further enhance this file:

  1. Consider renaming the dirtybranch variable for clarity.
  2. Add error handling or more informative output for the tagversion variable.
  3. Implement a clean target for ensuring fresh builds.

These changes will improve clarity, error handling, and build process robustness.

justfile (2)

1-2: Improved default workflow

The default command now includes tidy and build steps, which enhances the overall workflow. This change ensures that dependencies are managed and the application is compiled before running tests, leading to a more robust development process.


Line range hint 1-56: Overall improvements to the build and test workflow

The changes to this justfile significantly enhance the project's development workflow. The additions of tidy and build commands, along with improvements to the test command, provide a more robust and comprehensive set of tools for developers. The new help command also improves usability.

While some minor suggestions have been made for potential improvements, the overall changes are very positive and align well with the PR objectives of setting up the simapp for testing and improving the project structure.

simapp/go.mod (3)

172-172: Clarify the purpose of the autocctp.dev replacement

The replacement autocctp.dev => ../ suggests that the module is referencing a local directory. Could you please clarify the purpose of this replacement and confirm if this is intended for local development only?

If this is for local development, consider adding a comment explaining its purpose:

-replace autocctp.dev => ../
+// For local development only. Remove or update for production builds.
+replace autocctp.dev => ../

1-172: Overall go.mod review summary

The go.mod file sets up the necessary dependencies for the autocctp.dev/simapp module. While it generally looks good, there are a few points to address:

  1. Update the Go version to a valid, stable release.
  2. Consider using tagged versions for dependencies where possible.
  3. Clarify the purpose of the autocctp.dev replacement directive.

Once these issues are addressed, the go.mod file should be in good shape for the project.


1-3: ⚠️ Potential issue

Update Go version to a valid release

The specified Go version (1.23.2) is not a valid released version. As of October 2024, the latest stable version is in the 1.21.x series. Please update to a valid and stable Go version.

Apply this change:

-go 1.23.2
+go 1.21.3

Run the following script to check the latest stable Go version:

go.mod (3)

3-3: Go version update looks good.

Updating to Go 1.23.2 is a positive change as it likely includes bug fixes and performance improvements. Keeping the Go version up-to-date is important for security and stability reasons.


43-43: New indirect dependencies added - please clarify their necessity.

Two new indirect dependencies have been added:

  1. github.com/benbjohnson/clock v1.3.0
  2. github.com/onsi/ginkgo v1.16.2

Could you please clarify why these new dependencies were introduced? It's important to understand their purpose and ensure they don't unnecessarily bloat the project. Run the following commands to check where these dependencies are being used:

If these dependencies are not directly used in our project, consider investigating which direct dependency introduced them and if there's a way to avoid them.

Also applies to: 187-187


6-6: Dependency updates look good, but verify compatibility.

The updates to cosmossdk.io/math, github.com/circlefin/noble-fiattokenfactory, and github.com/ethereum/go-ethereum are positive changes, moving to more recent versions. Notably, cosmossdk.io/math has been updated from a beta to a stable version.

Please verify that these updates don't introduce any breaking changes and check if there are any new features that could be beneficial to the project. Run the following commands to check for any potential issues:

Also applies to: 60-60, 85-85

✅ Verification successful

Dependency updates verified successfully. No issues found.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any breaking changes or new features in the updated dependencies

# For cosmossdk.io/math
echo "Checking cosmossdk.io/math changes:"
rg -A 5 'cosmossdk\.io/math' --type go

# For noble-fiattokenfactory
echo "Checking noble-fiattokenfactory usage:"
rg -A 5 'github\.com/circlefin/noble-fiattokenfactory' --type go

# For go-ethereum
echo "Checking go-ethereum usage:"
rg -A 5 'github\.com/ethereum/go-ethereum' --type go

Length of output: 1991


Script:

#!/bin/bash
# Refined search for any usage of github.com/ethereum/go-ethereum

echo "Refined search: Listing all imports of github.com/ethereum/go-ethereum"
fd -e go | xargs grep -H 'github\.com/ethereum/go-ethereum'

echo "Searching for function implementations related to go-ethereum using ast-grep"
ast-grep --lang go --pattern $'import (
    $$$
    "github.com/ethereum/go-ethereum" $$$
)'

Length of output: 13666

simapp/simd/cmd/encoding.go (2)

1-10: Imports are appropriate and necessary

The imported packages are relevant and necessary for the functionality provided.


12-19: Definition of EncodingConfig is clear and comprehensive

The EncodingConfig struct effectively encapsulates all necessary encoding configurations for the application.

simapp/export.go (1)

78-82: ⚠️ Potential issue

Redundant context height reset without intermediate operations

Between lines 78-82, the context block height is set to zero and then immediately reset to the original height without any operations in between. This seems redundant and may indicate missing code that should execute with the block height set to zero. Please verify if any actions are intended to occur while the block height is zero.

simapp/simd/cmd/genaccounts.go (1)

183-187: Review the Necessity of Query Flags in Command

The command adds query flags using flags.AddQueryFlagsToCmd(cmd), but it doesn't perform any queries.

Please verify if these flags are required for this command. If not, consider removing them to avoid confusion.

simapp/simd/cmd/root.go (3)

338-341: Verify Skip Upgrade Heights Parsing

The code parses skip upgrade heights but does not check for potential parsing errors or invalid heights.

skipUpgradeHeights := make(map[int64]bool)
for _, h := range cast.ToIntSlice(appOpts.Get(server.FlagUnsafeSkipUpgrades)) {
    skipUpgradeHeights[int64(h)] = true
}

Ensure that the values obtained from appOpts are valid integers and represent correct block heights.


231-236: Ensure Start Command Customization

The startCmdCustomizer is optional but should be properly integrated if provided.

if options.startCmdCustomizer != nil {
    options.startCmdCustomizer(cmd)
}

Verify that any customizations to the start command are correctly applied.


325-380: Confirm Application Initialization Parameters

Ensure that all parameters passed to the buildApp function are correctly set, especially those derived from appOpts. Incorrect configurations may lead to unexpected application behavior.

simapp/app.go (2)

127-127: Review the permissions for the fiattokenfactory module account

The fiattokenfactory module account includes the authtypes.Staking permission. Ensure that this permission is necessary for the module's functionality. Including unnecessary permissions can pose security risks.

To verify if the authtypes.Staking permission is required, review the module's usage of staking functionalities.


182-183: ⚠️ Potential issue

Unused scoped keepers declared in the App struct

The ScopedICAHostKeeper and ScopedCCVConsumerKeeper are declared in the App struct but are not initialized or used elsewhere in the code. This might lead to confusion and unnecessary memory usage. Consider removing these declarations if they are not needed, or ensure they are properly initialized and utilized within the application.

To verify if these scoped keepers are used elsewhere in the codebase, run the following script:

This script uses rg (ripgrep) to find any references to the scoped keepers in the codebase, excluding simapp/app.go where they are declared.

✅ Verification successful

Further verification needed for scoped keepers usage

To ensure accurate verification, please run the following scripts:


Scoped keepers are unused and can be removed

The ScopedICAHostKeeper and ScopedCCVConsumerKeeper are declared in the App struct but are not utilized elsewhere in the codebase. Removing these declarations will help eliminate unnecessary memory usage and reduce potential confusion.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for usages of ScopedICAHostKeeper and ScopedCCVConsumerKeeper outside of their declarations.

# Search for usages of ScopedICAHostKeeper
echo "Searching for usages of ScopedICAHostKeeper:"
rg --type go "app\.ScopedICAHostKeeper" --glob "!simapp/app.go"

# Search for usages of ScopedCCVConsumerKeeper
echo "Searching for usages of ScopedCCVConsumerKeeper:"
rg --type go "app\.ScopedCCVConsumerKeeper" --glob "!simapp/app.go"

Length of output: 341


Script:

#!/bin/bash
# Description: Refined search for usages of ScopedICAHostKeeper and ScopedCCVConsumerKeeper outside of their declarations.

# Search for usages of ScopedICAHostKeeper without the 'app.' prefix
echo "Searching for usages of ScopedICAHostKeeper:"
rg --type go "ScopedICAHostKeeper" --glob "!simapp/app.go" -A 2

# Search for usages of ScopedCCVConsumerKeeper without the 'app.' prefix
echo "Searching for usages of ScopedCCVConsumerKeeper:"
rg --type go "ScopedCCVConsumerKeeper" --glob "!simapp/app.go" -A 2

Length of output: 337

Comment on lines +36 to +44
// MakeEncodingConfig creates an EncodingConfig for testing
func MakeEncodingConfig(moduleBasics module.BasicManager) EncodingConfig {
encodingConfig := makeEncodingConfig()
std.RegisterLegacyAminoCodec(encodingConfig.Amino)
std.RegisterInterfaces(encodingConfig.InterfaceRegistry)
moduleBasics.RegisterLegacyAminoCodec(encodingConfig.Amino)
moduleBasics.RegisterInterfaces(encodingConfig.InterfaceRegistry)
return encodingConfig
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Ensure consistent function naming and improve clarity

The functions makeEncodingConfig and MakeEncodingConfig have similar names but different export levels and purposes, which might cause confusion. Consider renaming makeEncodingConfig to enhance clarity and align with Go naming conventions.

For example, rename makeEncodingConfig to newEncodingConfig to indicate it's a constructor function.

Apply this diff:

-func makeEncodingConfig() EncodingConfig {
+func newEncodingConfig() EncodingConfig {

...

-func MakeEncodingConfig(moduleBasics module.BasicManager) EncodingConfig {
-    encodingConfig := makeEncodingConfig()
+func MakeEncodingConfig(moduleBasics module.BasicManager) EncodingConfig {
+    encodingConfig := newEncodingConfig()
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// MakeEncodingConfig creates an EncodingConfig for testing
func MakeEncodingConfig(moduleBasics module.BasicManager) EncodingConfig {
encodingConfig := makeEncodingConfig()
std.RegisterLegacyAminoCodec(encodingConfig.Amino)
std.RegisterInterfaces(encodingConfig.InterfaceRegistry)
moduleBasics.RegisterLegacyAminoCodec(encodingConfig.Amino)
moduleBasics.RegisterInterfaces(encodingConfig.InterfaceRegistry)
return encodingConfig
}
// MakeEncodingConfig creates an EncodingConfig for testing
func MakeEncodingConfig(moduleBasics module.BasicManager) EncodingConfig {
encodingConfig := newEncodingConfig()
std.RegisterLegacyAminoCodec(encodingConfig.Amino)
std.RegisterInterfaces(encodingConfig.InterfaceRegistry)
moduleBasics.RegisterLegacyAminoCodec(encodingConfig.Amino)
moduleBasics.RegisterInterfaces(encodingConfig.InterfaceRegistry)
return encodingConfig
}

Comment on lines +21 to +34
// makeEncodingConfig creates an EncodingConfig for an amino based test configuration.
func makeEncodingConfig() EncodingConfig {
amino := codec.NewLegacyAmino()
interfaceRegistry := types.NewInterfaceRegistry()
marshaler := codec.NewProtoCodec(interfaceRegistry)
txCfg := tx.NewTxConfig(marshaler, tx.DefaultSignModes)

return EncodingConfig{
InterfaceRegistry: interfaceRegistry,
Marshaler: marshaler,
TxConfig: txCfg,
Amino: amino,
}
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Clarify the purpose in the makeEncodingConfig function comment

The comment for makeEncodingConfig mentions "an amino based test configuration," but the function initializes both Amino and Protobuf codecs. To prevent confusion, consider updating the comment to accurately reflect that it sets up both codecs.

Apply this diff to update the comment:

-// makeEncodingConfig creates an EncodingConfig for an amino based test configuration.
+// makeEncodingConfig creates an EncodingConfig with both Amino and Protobuf codecs for testing purposes.
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// makeEncodingConfig creates an EncodingConfig for an amino based test configuration.
func makeEncodingConfig() EncodingConfig {
amino := codec.NewLegacyAmino()
interfaceRegistry := types.NewInterfaceRegistry()
marshaler := codec.NewProtoCodec(interfaceRegistry)
txCfg := tx.NewTxConfig(marshaler, tx.DefaultSignModes)
return EncodingConfig{
InterfaceRegistry: interfaceRegistry,
Marshaler: marshaler,
TxConfig: txCfg,
Amino: amino,
}
}
// makeEncodingConfig creates an EncodingConfig with both Amino and Protobuf codecs for testing purposes.
func makeEncodingConfig() EncodingConfig {
amino := codec.NewLegacyAmino()
interfaceRegistry := types.NewInterfaceRegistry()
marshaler := codec.NewProtoCodec(interfaceRegistry)
txCfg := tx.NewTxConfig(marshaler, tx.DefaultSignModes)
return EncodingConfig{
InterfaceRegistry: interfaceRegistry,
Marshaler: marshaler,
TxConfig: txCfg,
Amino: amino,
}
}

Comment on lines +17 to +25
type HandlerOptions struct {
ante.HandlerOptions
cdc codec.Codec
fiatTokenFactoryKeeper *fiattokenfactorykeeper.Keeper
IBCKeeper *ibckeeper.Keeper
GlobalFeeSubspace paramtypes.Subspace
StakingSubspace paramtypes.Subspace
ForwardingKeeper *forwardingkeeper.Keeper
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Inconsistent field naming in HandlerOptions struct

The fields cdc and fiatTokenFactoryKeeper are unexported (start with lowercase), while the other fields are exported (start with uppercase). To maintain consistency and adhere to Go naming conventions, consider capitalizing these fields if they need to be accessed externally.

Apply this diff to adjust the field names:

 type HandlerOptions struct {
     ante.HandlerOptions
-    cdc                    codec.Codec
-    fiatTokenFactoryKeeper *fiattokenfactorykeeper.Keeper
+    Cdc                    codec.Codec
+    FiatTokenFactoryKeeper *fiattokenfactorykeeper.Keeper
     IBCKeeper              *ibckeeper.Keeper
     GlobalFeeSubspace      paramtypes.Subspace
     StakingSubspace        paramtypes.Subspace
     ForwardingKeeper       *forwardingkeeper.Keeper
 }
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
type HandlerOptions struct {
ante.HandlerOptions
cdc codec.Codec
fiatTokenFactoryKeeper *fiattokenfactorykeeper.Keeper
IBCKeeper *ibckeeper.Keeper
GlobalFeeSubspace paramtypes.Subspace
StakingSubspace paramtypes.Subspace
ForwardingKeeper *forwardingkeeper.Keeper
}
type HandlerOptions struct {
ante.HandlerOptions
Cdc codec.Codec
FiatTokenFactoryKeeper *fiattokenfactorykeeper.Keeper
IBCKeeper *ibckeeper.Keeper
GlobalFeeSubspace paramtypes.Subspace
StakingSubspace paramtypes.Subspace
ForwardingKeeper *forwardingkeeper.Keeper
}

Comment on lines +31 to +39
if options.AccountKeeper == nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "account keeper is required for AnteHandler")
}
if options.BankKeeper == nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "bank keeper is required for AnteHandler")
}
if options.SignModeHandler == nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "sign mode handler is required for ante builder")
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add nil checks for additional required HandlerOptions fields

The options Cdc, FiatTokenFactoryKeeper, ForwardingKeeper, and IBCKeeper are used in the AnteHandler but are not checked for nil. To prevent potential nil pointer dereference errors, please add nil checks for these fields.

Apply this diff to add the nil checks:

 func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
     if options.AccountKeeper == nil {
         return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "account keeper is required for AnteHandler")
     }
     if options.BankKeeper == nil {
         return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "bank keeper is required for AnteHandler")
     }
     if options.SignModeHandler == nil {
         return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "sign mode handler is required for ante builder")
     }
+    if options.Cdc == nil {
+        return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "codec is required for AnteHandler")
+    }
+    if options.FiatTokenFactoryKeeper == nil {
+        return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "fiatTokenFactoryKeeper is required for AnteHandler")
+    }
+    if options.ForwardingKeeper == nil {
+        return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "forwarding keeper is required for AnteHandler")
+    }
+    if options.IBCKeeper == nil {
+        return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "IBCKeeper is required for AnteHandler")
+    }
     sigGasConsumer := options.SigGasConsumer
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if options.AccountKeeper == nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "account keeper is required for AnteHandler")
}
if options.BankKeeper == nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "bank keeper is required for AnteHandler")
}
if options.SignModeHandler == nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "sign mode handler is required for ante builder")
}
if options.AccountKeeper == nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "account keeper is required for AnteHandler")
}
if options.BankKeeper == nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "bank keeper is required for AnteHandler")
}
if options.SignModeHandler == nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "sign mode handler is required for ante builder")
}
if options.Cdc == nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "codec is required for AnteHandler")
}
if options.FiatTokenFactoryKeeper == nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "fiatTokenFactoryKeeper is required for AnteHandler")
}
if options.ForwardingKeeper == nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "forwarding keeper is required for AnteHandler")
}
if options.IBCKeeper == nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "IBCKeeper is required for AnteHandler")
}

Comment on lines +75 to +76
/* Handle fee distribution state. */

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Incomplete implementation: Missing handling of fee distribution state

At lines 75-76, there's a comment indicating that fee distribution state should be handled, but no implementation is provided. Please ensure that the required logic is implemented to correctly handle the fee distribution state during zero height genesis preparation.

Would you like assistance in implementing the fee distribution state handling?

Comment on lines +429 to +430
srvCfg.MinGasPrices = "0stake"

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Review Minimum Gas Prices Configuration

Setting MinGasPrices to "0stake" allows transactions with zero gas fees, which might lead to network congestion due to spam transactions.

 srvCfg.MinGasPrices = "0stake"

Consider setting a non-zero MinGasPrices to deter spam and ensure network stability.

Committable suggestion was skipped due to low confidence.

Comment on lines +199 to +567
distrtypes.ModuleName,
slashingtypes.ModuleName,
genutiltypes.ModuleName,
evidencetypes.ModuleName,
paramstypes.ModuleName,
upgradetypes.ModuleName,
vestingtypes.ModuleName,
fiattokenfactorymoduletypes.ModuleName,
cctptypes.ModuleName,
forwardingtypes.ModuleName,
)

// NOTE: The genutils module must occur after staking so that pools are
// properly initialized with tokens from genesis accounts.
// NOTE: Capability module must occur first so that it can initialize any capabilities
// so that other modules that want to create or claim capabilities afterwards in InitChain
// can do so safely.
app.mm.SetOrderInitGenesis(
capabilitytypes.ModuleName,
authtypes.ModuleName,
banktypes.ModuleName,
stakingtypes.ModuleName,
distrtypes.ModuleName,
slashingtypes.ModuleName,
crisistypes.ModuleName,
fiattokenfactorymoduletypes.ModuleName,
genutiltypes.ModuleName,
ibchost.ModuleName,
ibctransfertypes.ModuleName,
evidencetypes.ModuleName,
paramstypes.ModuleName,
upgradetypes.ModuleName,
vestingtypes.ModuleName,
cctptypes.ModuleName,
forwardingtypes.ModuleName,

// this line is used by starport scaffolding # stargate/app/initGenesis
)

// Uncomment if you want to set a custom migration order here.
// app.mm.SetOrderMigrations(custom order)

app.mm.RegisterInvariants(&app.CrisisKeeper)
app.mm.RegisterRoutes(app.Router(), app.QueryRouter(), encodingConfig.Amino)

app.configurator = module.NewConfigurator(app.appCodec, app.MsgServiceRouter(), app.GRPCQueryRouter())
app.mm.RegisterServices(app.configurator)

// create the simulation manager and define the order of the modules for deterministic simulations
overrideModules := map[string]module.AppModuleSimulation{
authtypes.ModuleName: auth.NewAppModule(app.appCodec, app.AccountKeeper, authsims.RandomGenesisAccounts),
}
app.sm = module.NewSimulationManagerFromAppModules(app.mm.Modules, overrideModules)
app.sm.RegisterStoreDecoders()

// initialize stores
app.MountKVStores(keys)
app.MountTransientStores(tkeys)
app.MountMemoryStores(memKeys)

// initialize BaseApp
anteHandler, err := NewAnteHandler(
HandlerOptions{
HandlerOptions: ante.HandlerOptions{
AccountKeeper: app.AccountKeeper,
BankKeeper: app.BankKeeper,
FeegrantKeeper: nil,
SignModeHandler: encodingConfig.TxConfig.SignModeHandler(),
SigGasConsumer: ante.DefaultSigVerificationGasConsumer,
},
cdc: appCodec,
fiatTokenFactoryKeeper: app.FiatTokenFactoryKeeper,

IBCKeeper: app.IBCKeeper,
StakingSubspace: app.GetSubspace(stakingtypes.ModuleName),

ForwardingKeeper: app.ForwardingKeeper,
},
)
if err != nil {
panic(fmt.Errorf("failed to create AnteHandler: %s", err))
}

app.SetAnteHandler(anteHandler)
app.SetInitChainer(app.InitChainer)
app.SetBeginBlocker(app.BeginBlocker)
app.SetEndBlocker(app.EndBlocker)

if loadLatest {
if err := app.LoadLatestVersion(); err != nil {
tmos.Exit(err.Error())
}
}

app.ScopedIBCKeeper = scopedIBCKeeper
app.ScopedTransferKeeper = scopedTransferKeeper
// this line is used by starport scaffolding # stargate/app/beforeInitReturn

return app
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider refactoring the New function to improve maintainability

The New function spans over 368 lines, making it lengthy and potentially difficult to read and maintain. Consider breaking it down into smaller helper functions that handle specific initialization tasks, such as setting up keys, initializing keepers, configuring modules, and setting up middleware. This will enhance readability and simplify future modifications.

Comment on lines +612 to +615
func (app *App) BlockedModuleAccountAddrs() map[string]bool {
modAccAddrs := app.ModuleAccountAddrs()
return modAccAddrs
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

All module accounts are blocked from receiving tokens

The BlockedModuleAccountAddrs function currently blocks all module accounts from receiving external tokens by returning all module account addresses. If certain module accounts, such as the fee collector (authtypes.FeeCollectorName), need to receive tokens from external accounts, consider unblocking them to ensure proper functionality.

You can modify the function to unblock specific module accounts:

 func (app *App) BlockedModuleAccountAddrs() map[string]bool {
     modAccAddrs := app.ModuleAccountAddrs()
+    // Unblock the FeeCollector module account to allow it to receive tokens
+    delete(modAccAddrs, authtypes.NewModuleAddress(authtypes.FeeCollectorName).String())
     return modAccAddrs
 }
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (app *App) BlockedModuleAccountAddrs() map[string]bool {
modAccAddrs := app.ModuleAccountAddrs()
return modAccAddrs
}
func (app *App) BlockedModuleAccountAddrs() map[string]bool {
modAccAddrs := app.ModuleAccountAddrs()
// Unblock the FeeCollector module account to allow it to receive tokens
delete(modAccAddrs, authtypes.NewModuleAddress(authtypes.FeeCollectorName).String())
return modAccAddrs
}

Comment on lines +705 to +721
paramsKeeper := paramskeeper.NewKeeper(appCodec, legacyAmino, key, tkey)

paramsKeeper.Subspace(authtypes.ModuleName)
paramsKeeper.Subspace(banktypes.ModuleName)
paramsKeeper.Subspace(stakingtypes.ModuleName)
paramsKeeper.Subspace(distrtypes.ModuleName)
paramsKeeper.Subspace(slashingtypes.ModuleName)
paramsKeeper.Subspace(crisistypes.ModuleName)
paramsKeeper.Subspace(ibctransfertypes.ModuleName)
paramsKeeper.Subspace(ibchost.ModuleName)
paramsKeeper.Subspace(fiattokenfactorymoduletypes.ModuleName)
paramsKeeper.Subspace(upgradetypes.ModuleName)
paramsKeeper.Subspace(cctptypes.ModuleName)
// this line is used by starport scaffolding # stargate/app/paramSubspace

return paramsKeeper
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Missing parameter subspace initialization for the forwarding module

In the initParamsKeeper function, the parameter subspace for the forwarding module is not initialized. If the forwarding module relies on parameters, this could lead to issues during runtime. Consider adding the subspace initialization for the forwarding module.

Apply this diff to include the missing subspace:

 func initParamsKeeper(appCodec codec.BinaryCodec, legacyAmino *codec.LegacyAmino, key, tkey storetypes.StoreKey) paramskeeper.Keeper {
     paramsKeeper := paramskeeper.NewKeeper(appCodec, legacyAmino, key, tkey)

     paramsKeeper.Subspace(authtypes.ModuleName)
     paramsKeeper.Subspace(banktypes.ModuleName)
     paramsKeeper.Subspace(stakingtypes.ModuleName)
     paramsKeeper.Subspace(distrtypes.ModuleName)
     paramsKeeper.Subspace(slashingtypes.ModuleName)
     paramsKeeper.Subspace(crisistypes.ModuleName)
     paramsKeeper.Subspace(ibctransfertypes.ModuleName)
     paramsKeeper.Subspace(ibchost.ModuleName)
     paramsKeeper.Subspace(fiattokenfactorymoduletypes.ModuleName)
     paramsKeeper.Subspace(upgradetypes.ModuleName)
     paramsKeeper.Subspace(cctptypes.ModuleName)
+    paramsKeeper.Subspace(forwardingtypes.ModuleName)
     // this line is used by starport scaffolding # stargate/app/paramSubspace

     return paramsKeeper
 }
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
paramsKeeper := paramskeeper.NewKeeper(appCodec, legacyAmino, key, tkey)
paramsKeeper.Subspace(authtypes.ModuleName)
paramsKeeper.Subspace(banktypes.ModuleName)
paramsKeeper.Subspace(stakingtypes.ModuleName)
paramsKeeper.Subspace(distrtypes.ModuleName)
paramsKeeper.Subspace(slashingtypes.ModuleName)
paramsKeeper.Subspace(crisistypes.ModuleName)
paramsKeeper.Subspace(ibctransfertypes.ModuleName)
paramsKeeper.Subspace(ibchost.ModuleName)
paramsKeeper.Subspace(fiattokenfactorymoduletypes.ModuleName)
paramsKeeper.Subspace(upgradetypes.ModuleName)
paramsKeeper.Subspace(cctptypes.ModuleName)
// this line is used by starport scaffolding # stargate/app/paramSubspace
return paramsKeeper
}
paramsKeeper := paramskeeper.NewKeeper(appCodec, legacyAmino, key, tkey)
paramsKeeper.Subspace(authtypes.ModuleName)
paramsKeeper.Subspace(banktypes.ModuleName)
paramsKeeper.Subspace(stakingtypes.ModuleName)
paramsKeeper.Subspace(distrtypes.ModuleName)
paramsKeeper.Subspace(slashingtypes.ModuleName)
paramsKeeper.Subspace(crisistypes.ModuleName)
paramsKeeper.Subspace(ibctransfertypes.ModuleName)
paramsKeeper.Subspace(ibchost.ModuleName)
paramsKeeper.Subspace(fiattokenfactorymoduletypes.ModuleName)
paramsKeeper.Subspace(upgradetypes.ModuleName)
paramsKeeper.Subspace(cctptypes.ModuleName)
paramsKeeper.Subspace(forwardingtypes.ModuleName)
// this line is used by starport scaffolding # stargate/app/paramSubspace
return paramsKeeper
}

Comment on lines +529 to +546
anteHandler, err := NewAnteHandler(
HandlerOptions{
HandlerOptions: ante.HandlerOptions{
AccountKeeper: app.AccountKeeper,
BankKeeper: app.BankKeeper,
FeegrantKeeper: nil,
SignModeHandler: encodingConfig.TxConfig.SignModeHandler(),
SigGasConsumer: ante.DefaultSigVerificationGasConsumer,
},
cdc: appCodec,
fiatTokenFactoryKeeper: app.FiatTokenFactoryKeeper,

IBCKeeper: app.IBCKeeper,
StakingSubspace: app.GetSubspace(stakingtypes.ModuleName),

ForwardingKeeper: app.ForwardingKeeper,
},
)
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Potential misconfiguration of HandlerOptions in NewAnteHandler

In the call to NewAnteHandler, the HandlerOptions struct is nested within another HandlerOptions struct. This nesting might not be intentional and could lead to incorrect configuration of the AnteHandler. Ensure that the correct types are used and that the inner HandlerOptions is correctly set without unnecessary nesting.

Consider adjusting the code as follows:

 anteHandler, err := NewAnteHandler(
-    HandlerOptions{
+    ante.HandlerOptions{
         AccountKeeper:   app.AccountKeeper,
         BankKeeper:      app.BankKeeper,
         FeegrantKeeper:  nil,
         SignModeHandler: encodingConfig.TxConfig.SignModeHandler(),
         SigGasConsumer:  ante.DefaultSigVerificationGasConsumer,
-    },
-    cdc:                    appCodec,
-    fiatTokenFactoryKeeper: app.FiatTokenFactoryKeeper,
-
-    IBCKeeper:       app.IBCKeeper,
-    StakingSubspace: app.GetSubspace(stakingtypes.ModuleName),
-
-    ForwardingKeeper: app.ForwardingKeeper,
+        cdc:                    appCodec,
+        fiatTokenFactoryKeeper: app.FiatTokenFactoryKeeper,
+        IBCKeeper:              app.IBCKeeper,
+        StakingSubspace:        app.GetSubspace(stakingtypes.ModuleName),
+        ForwardingKeeper:       app.ForwardingKeeper,
     },
 )

Committable suggestion was skipped due to low confidence.

@spoo-bar spoo-bar marked this pull request as draft October 17, 2024 14:25
@spoo-bar spoo-bar closed this Oct 17, 2024
@spoo-bar spoo-bar deleted the spoorthi/eng-186-setup-simapp-for-testing branch October 17, 2024 16:00
@spoo-bar
Copy link
Contributor Author

Closing in favour of #6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant