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

feat: module hash by height query #20779

Merged
merged 25 commits into from
Aug 10, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
0836685
module hash by height query
czarcas7ic Jun 25, 2024
a289320
add changelog
czarcas7ic Jun 25, 2024
0f9d159
correct comment
czarcas7ic Jun 25, 2024
91e9438
lint
czarcas7ic Jun 25, 2024
cebf849
lint order
czarcas7ic Jun 25, 2024
268af2e
add more detail to changelog entry
czarcas7ic Jun 26, 2024
39ef821
move to server and make default
czarcas7ic Jun 26, 2024
c5bc58e
Merge branch 'main' into adam/module-hash-by-height
czarcas7ic Jun 26, 2024
8e2f8bb
output flag to format json
czarcas7ic Jun 26, 2024
d9f8467
remove comments
czarcas7ic Jun 26, 2024
49e89be
use appd for comment name
czarcas7ic Jun 29, 2024
0d25c7a
Merge branch 'main' into adam/module-hash-by-height
czarcas7ic Jun 29, 2024
06dd7cd
use cmd.Println
czarcas7ic Jun 29, 2024
8354731
Merge branch 'main' into adam/module-hash-by-height
czarcas7ic Jul 5, 2024
86c0af5
Merge branch 'main' into adam/module-hash-by-height
facundomedica Jul 15, 2024
f983727
Merge branch 'main' into adam/module-hash-by-height
czarcas7ic Jul 15, 2024
2b586e1
rename verbose file
czarcas7ic Jul 15, 2024
425db57
use example attribute
czarcas7ic Jul 15, 2024
bd42bd8
add more context to error
czarcas7ic Jul 15, 2024
9c01f7c
use existing OpenDB method
czarcas7ic Jul 15, 2024
ccb2582
remove osmosisd from error message
czarcas7ic Jul 15, 2024
660aa8d
add timestamp
czarcas7ic Jul 15, 2024
3dc6775
use print proto instead of manually defining prints
czarcas7ic Jul 15, 2024
f883653
Merge branch 'main' into adam/module-hash-by-height
tac0turtle Aug 9, 2024
61dd9e3
Merge branch 'main' into adam/module-hash-by-height
czarcas7ic Aug 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
* (client) [#19870](https://github.com/cosmos/cosmos-sdk/pull/19870) Add new query command `wait-tx`. Alias `event-query-tx-for` to `wait-tx` for backward compatibility.
* (crypto/keyring) [#20212](https://github.com/cosmos/cosmos-sdk/pull/20212) Expose the db keyring used in the keystore.
* (genutil) [#19971](https://github.com/cosmos/cosmos-sdk/pull/19971) Allow manually setting the consensus key type in genesis
* (cli) [#20779](https://github.com/cosmos/cosmos-sdk/pull/20779) Added `module-hash-by-height` command to query and retrieve module hashes at a specified blockchain height, enhancing debugging capabilities.

### Improvements

Expand Down
125 changes: 125 additions & 0 deletions server/module_hash_by_height.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
package server
czarcas7ic marked this conversation as resolved.
Show resolved Hide resolved

import (
"encoding/hex"
"encoding/json"
"fmt"
"path/filepath"
"sort"
"strconv"
"strings"

dbm "github.com/cosmos/cosmos-db"
"github.com/spf13/cobra"

"cosmossdk.io/store/rootmulti"
storetypes "cosmossdk.io/store/types"

"github.com/cosmos/cosmos-sdk/client/flags"
servertypes "github.com/cosmos/cosmos-sdk/server/types"
)

// ModuleHashByHeightQuery retrieves the module hashes at a given height.
func ModuleHashByHeightQuery[T servertypes.Application](appCreator servertypes.AppCreator[T]) *cobra.Command {
cmd := &cobra.Command{
Use: "module-hash-by-height [height]",
Short: "Get module hashes at a given height",
Long: `Get module hashes at a given height. This command is useful for debugging and verifying the state of the application at a given height. Daemon should not be running when calling this command.
Example:
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use the Example attribute instead and generic syntax so that it works for all apps.

Example: fmt.Sprintf(
	"$ %s your command",
	version.AppName,
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

osmosisd module-hash-by-height 16841115,
czarcas7ic marked this conversation as resolved.
Show resolved Hide resolved
`,
Args: cobra.ExactArgs(1), // Ensure exactly one argument is provided
RunE: func(cmd *cobra.Command, args []string) error {
heightToRetrieveString := args[0]

serverCtx := GetServerContextFromCmd(cmd)

height, err := strconv.ParseInt(heightToRetrieveString, 10, 64)
if err != nil {
return err
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: it would be helpful to add some context: `fmt.Errorf("invalid height: %w", err)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

}

commitInfoForHeight, err := getModuleHashesAtHeight(serverCtx, appCreator, height)
if err != nil {
return err
}

outputFormat, err := cmd.Flags().GetString(flags.FlagOutput)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use the client context instead to print the output?

			clientCtx := client.GetClientContextFromCmd(cmd)
			return clientCtx.PrintProto(commitInfoForHeight)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TIL, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if err != nil {
return err
}

switch outputFormat {
case flags.OutputFormatJSON:
jsonOutput, err := json.MarshalIndent(commitInfoForHeight, "", " ")
if err != nil {
return err
}
fmt.Println(string(jsonOutput))
Copy link
Member

Choose a reason for hiding this comment

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

Nit: cmd.Println everywhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

case flags.OutputFormatText:
fallthrough
Copy link
Contributor

Choose a reason for hiding this comment

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

yaml is used for text type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct me if I am wrong, but I think after taking your recommendation here 3dc6775 we should no longer need this logic branch.

default:
fmt.Println(commitInfoForHeight.String())
}

return nil
},
}

cmd.Flags().StringP(flags.FlagOutput, "o", flags.OutputFormatText, "Output format (text|json)")

return cmd
}

func getModuleHashesAtHeight[T servertypes.Application](svrCtx *Context, appCreator servertypes.AppCreator[T], height int64) (*storetypes.CommitInfo, error) {
home := svrCtx.Config.RootDir
db, err := openDB(home, GetAppDBBackend(svrCtx.Viper))
Copy link
Contributor

Choose a reason for hiding this comment

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

Use OpenDB() instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if err != nil {
return nil, fmt.Errorf("error opening DB, make sure osmosisd is not running when calling this query: %w", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

osmosisd ? Please revisit the error message to make it more generic

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, habits 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

}
app := appCreator(svrCtx.Logger, db, nil, svrCtx.Viper)
rms, ok := app.CommitMultiStore().(*rootmulti.Store)
if !ok {
return nil, fmt.Errorf("expected rootmulti.Store, got %T", app.CommitMultiStore())
}

commitInfoForHeight, err := rms.GetCommitInfo(height)
if err != nil {
return nil, err
}

// Create a new slice of StoreInfos for storing the modified hashes.
storeInfos := make([]storetypes.StoreInfo, len(commitInfoForHeight.StoreInfos))

for i, storeInfo := range commitInfoForHeight.StoreInfos {
// Convert the hash to a hexadecimal string.
hash := strings.ToUpper(hex.EncodeToString(storeInfo.CommitId.Hash))

// Create a new StoreInfo with the modified hash.
storeInfos[i] = storetypes.StoreInfo{
Name: storeInfo.Name,
CommitId: storetypes.CommitID{
Version: storeInfo.CommitId.Version,
Hash: []byte(hash),
},
}
}

// Sort the storeInfos slice based on the module name.
sort.Slice(storeInfos, func(i, j int) bool {
return storeInfos[i].Name < storeInfos[j].Name
})

// Create a new CommitInfo with the modified StoreInfos.
commitInfoForHeight = &storetypes.CommitInfo{
Version: commitInfoForHeight.Version,
StoreInfos: storeInfos,
Copy link
Contributor

Choose a reason for hiding this comment

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

please add Timestamp: commitInfoForHeight.Timestamp,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

}

return commitInfoForHeight, nil
}
czarcas7ic marked this conversation as resolved.
Show resolved Hide resolved

func openDB(rootDir string, backendType dbm.BackendType) (dbm.DB, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be removed in favour of OpenDB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dataDir := filepath.Join(rootDir, "data")
return dbm.NewDB("application", backendType, dataDir)
}
1 change: 1 addition & 0 deletions server/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ func AddCommands[T types.Application](rootCmd *cobra.Command, appCreator types.A
cometCmd,
version.NewVersionCommand(),
NewRollbackCmd(appCreator),
ModuleHashByHeightQuery(appCreator),
Copy link
Contributor

Choose a reason for hiding this comment

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

This adds the command to the root context which makes it very prominent.
I don't have a strong opinion on this. Debug could be an option, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did this because including it in debug would require a change in the API of Cmd() (we would need to include appCreator as an input param). If this is desired I would be happy to do so.

)
}

Expand Down
Loading