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(iaviewer): Add "data-full" and "hash" commands #959

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
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
141 changes: 94 additions & 47 deletions cmd/iaviewer/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package main
import (
"bytes"
"crypto/sha256"
"encoding/hex"
"fmt"
"os"
"path/filepath"
Expand All @@ -22,41 +21,68 @@ const (
DefaultCacheSize int = 10000
)

var cmds = map[string]bool{
"data": true,
"data-full": true,
"hash": true,
"shape": true,
"versions": true,
}

func main() {
args := os.Args[1:]
if len(args) < 3 || (args[0] != "data" && args[0] != "shape" && args[0] != "versions") {
fmt.Fprintln(os.Stderr, "Usage: iaviewer <data|shape|versions> <leveldb dir> <prefix> [version number]")
fmt.Fprintln(os.Stderr, "<prefix> is the prefix of db, and the iavl tree of different modules in cosmos-sdk uses ")
fmt.Fprintln(os.Stderr, "different <prefix> to identify, just like \"s/k:gov/\" represents the prefix of gov module")
if len(args) < 3 || len(args) > 4 || !cmds[args[0]] {
fmt.Fprintln(os.Stderr, strings.TrimSpace(`
Usage: iaviewer <data|data-full|hash|shape|versions> <leveldb dir> <prefix> [version number]
<prefix> is the prefix of db, and the iavl tree of different modules in cosmos-sdk uses
different <prefix> to identify, just like "s/k:gov/" represents the prefix of gov module
`))
os.Exit(1)
}

version := 0
if len(args) == 4 {
version := int64(0)
Copy link

Choose a reason for hiding this comment

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

personal preference: a more verbose name would be more readable.

Suggested change
version := int64(0)
wantVersion := int64(0)

if len(args) >= 4 {
var err error
version, err = strconv.Atoi(args[3])
if err != nil {
fmt.Fprintf(os.Stderr, "Invalid version number: %s\n", err)
os.Exit(1)
}
version, err = strconv.ParseInt(args[3], 10, 0)
assertNoError(err, "Invalid version number")
}

tree, err := ReadTree(args[1], version, []byte(args[2]))
if err != nil {
fmt.Fprintf(os.Stderr, "Error reading data: %s\n", err)
os.Exit(1)
mutableTree, latestVersion, err := ReadTree(args[1], []byte(args[2]))
assertNoError(err, "Error reading database")

if args[0] == "versions" {
PrintVersions(mutableTree)
return
}

if version == 0 {
version = latestVersion
}
tree, err := mutableTree.GetImmutable(version)
assertNoError(err, "Error reading target version")
fmt.Printf("Got version: %d\n", tree.Version())

fullValues := false
switch args[0] {
case "data-full":
fullValues = true
fallthrough
case "data":
PrintKeys(tree)
PrintKeys(tree, fullValues)
fallthrough
case "hash":
hash := tree.Hash()
fmt.Printf("Hash: %X\n", hash)
fmt.Printf("Size: %X\n", tree.Size())
case "shape":
PrintShape(tree)
case "versions":
PrintVersions(tree)
}
Comment on lines +65 to +79
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 unintended command execution due to 'fallthrough'

The use of fallthrough statements causes commands "data", "data-full", and "hash" to execute sequentially. This means that when the "data" or "data-full" command is specified, the "hash" command's logic is also executed. If this behavior is unintended, consider removing the fallthrough statements to prevent unexpected command execution.

Apply this diff to remove unintended fallthroughs:

switch args[0] {
case "data-full":
	fullValues = true
-	fallthrough
case "data":
	PrintKeys(tree, fullValues)
-	fallthrough
case "hash":
	hash := tree.Hash()
	fmt.Printf("Hash: %X\n", hash)
	fmt.Printf("Size: %X\n", tree.Size())
}
📝 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
fullValues := false
switch args[0] {
case "data-full":
fullValues = true
fallthrough
case "data":
PrintKeys(tree)
PrintKeys(tree, fullValues)
fallthrough
case "hash":
hash := tree.Hash()
fmt.Printf("Hash: %X\n", hash)
fmt.Printf("Size: %X\n", tree.Size())
case "shape":
PrintShape(tree)
case "versions":
PrintVersions(tree)
}
fullValues := false
switch args[0] {
case "data-full":
fullValues = true
PrintKeys(tree, fullValues)
case "data":
PrintKeys(tree, fullValues)
case "hash":
hash := tree.Hash()
fmt.Printf("Hash: %X\n", hash)
fmt.Printf("Size: %X\n", tree.Size())
case "shape":
PrintShape(tree)
}

}

func assertNoError(err error, msg string) {
if err != nil {
fmt.Fprintf(os.Stderr, "%s: %s\n", msg, err)
os.Exit(1)
}
}

Expand Down Expand Up @@ -111,57 +137,81 @@ func PrintDBStats(db corestore.KVStoreWithBatch) {
}
}

// ReadTree loads an iavl tree from the directory
// If version is 0, load latest, otherwise, load named version
// The prefix represents which iavl tree you want to read. The iaviwer will always set a prefix.
func ReadTree(dir string, version int, prefix []byte) (*iavl.MutableTree, error) {
// ReadTree loads an iavl tree from disk, applying the specified prefix if non-empty.
func ReadTree(dir string, prefix []byte) (tree *iavl.MutableTree, latestVersion int64, err error) {
db, err := OpenDB(dir)
if err != nil {
return nil, err
return nil, 0, err
}
if len(prefix) != 0 {
db = dbm.NewPrefixDB(db, prefix)
}

tree := iavl.NewMutableTree(db, DefaultCacheSize, false, log.NewLogger(os.Stdout))
ver, err := tree.LoadVersion(int64(version))
fmt.Printf("Got version: %d\n", ver)
return tree, err
tree = iavl.NewMutableTree(db, DefaultCacheSize, true, log.NewLogger(os.Stdout))
latestVersion, err = tree.Load()
if err != nil {
return nil, 0, err
} else if tree.IsEmpty() {
return nil, 0, fmt.Errorf("tree is empty")
}
fmt.Printf("Got latest version: %d\n", latestVersion)
return tree, latestVersion, err
}

func PrintKeys(tree *iavl.MutableTree) {
fmt.Println("Printing all keys with hashed values (to detect diff)")
tree.Iterate(func(key []byte, value []byte) bool { //nolint:errcheck
printKey := parseWeaveKey(key)
digest := sha256.Sum256(value)
fmt.Printf(" %s\n %X\n", printKey, digest)
func PrintKeys(tree *iavl.ImmutableTree, fullValues bool) {
valuesLabel := "hashed values"
valueToString := func(value []byte) string {
return fmt.Sprintf("%X", sha256.Sum256(value))
}
if fullValues {
valuesLabel = "values"
valueToString = encodeData
}
fmt.Printf("Printing all keys with %s (to detect diff)\n", valuesLabel)
tree.Iterate(func(key []byte, value []byte) bool {
keyStr := parseWeaveKey(key)
valueStr := valueToString(value)
fmt.Printf(" %s\n %s\n", keyStr, valueStr)
Comment on lines +161 to +174
Copy link

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Unit tests for PrintKeys function are missing.

The search did not find any unit tests covering the PrintKeys function. Please add unit tests to ensure both modes of operation (hashed values and full values) are verified.

  • Location: cmd/iaviewer/main.go lines 160-173
Analysis chain

Flexible implementation of key printing with mode switching.

The use of a function pointer to switch between hashed and full values is a clever design choice. Ensure that this function is covered by unit tests to verify both modes of operation.

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify unit tests for `PrintKeys` function.

# Test: Search for unit tests covering both `hashed values` and `full values`. Expect: Tests validating both modes.
rg --type go $'PrintKeys'

Length of output: 156



Script:

#!/bin/bash
# Description: Find test files and check for tests covering `PrintKeys` function.

# Step 1: Find all Go test files
test_files=$(fd --type f --extension go '_test.go')

# Step 2: Search for `PrintKeys` function usage within the test files
if [ -n "$test_files" ]; then
  echo "$test_files" | xargs rg 'PrintKeys'
else
  echo "No test files found."
fi

Length of output: 2183

return false
})
}

// parseWeaveKey assumes a separating : where all in front should be ascii,
// and all afterwards may be ascii or binary
// parseWeaveKey returns a string representation of a key, splitting on the first ":"
// into a prefix (presumably an all-ASCII type label) followed by a possibly-binary suffix.
func parseWeaveKey(key []byte) string {
cut := bytes.IndexRune(key, ':')
if cut == -1 {
return encodeID(key)
return encodeData(key)
}
prefix := key[:cut]
id := key[cut+1:]
return fmt.Sprintf("%s:%s", encodeID(prefix), encodeID(id))
return fmt.Sprintf("%s:%s", encodeData(prefix), encodeData(id))
Comment on lines +179 to +188
Copy link

Choose a reason for hiding this comment

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

Optimize and clarify parseWeaveKey function.

The parseWeaveKey function now handles potentially binary data, which enhances its robustness. However, the function could be optimized by avoiding multiple calls to encodeData. Consider caching the result or restructuring the logic to minimize redundancy.

-	return fmt.Sprintf("%s:%s", encodeData(prefix), encodeData(id))
+	encodedPrefix := encodeData(prefix)
+	encodedId := encodeData(id)
+	return fmt.Sprintf("%s:%s", encodedPrefix, encodedId)
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
// parseWeaveKey returns a string representation of a key, splitting on the first ":"
// into a prefix (presumably an all-ASCII type label) followed by a possibly-binary suffix.
func parseWeaveKey(key []byte) string {
cut := bytes.IndexRune(key, ':')
if cut == -1 {
return encodeID(key)
return encodeData(key)
}
prefix := key[:cut]
id := key[cut+1:]
return fmt.Sprintf("%s:%s", encodeID(prefix), encodeID(id))
return fmt.Sprintf("%s:%s", encodeData(prefix), encodeData(id))
// parseWeaveKey returns a string representation of a key, splitting on the first ":"
// into a prefix (presumably an all-ASCII type label) followed by a possibly-binary suffix.
func parseWeaveKey(key []byte) string {
cut := bytes.IndexRune(key, ':')
if cut == -1 {
return encodeData(key)
}
prefix := key[:cut]
id := key[cut+1:]
encodedPrefix := encodeData(prefix)
encodedId := encodeData(id)
return fmt.Sprintf("%s:%s", encodedPrefix, encodedId)

}

// casts to a string if it is printable ascii, hex-encodes otherwise
func encodeID(id []byte) string {
for _, b := range id {
// encodeData returns a printable ASCII representation of its input;
// hexadecimal if it is not already printable ASCII, otherwise plain
// or quoted as necessary to avoid misinterpretation.
func encodeData(src []byte) string {
hexConfusable := true
forceQuotes := false
for _, b := range src {
if b < 0x20 || b >= 0x80 {
return strings.ToUpper(hex.EncodeToString(id))
return fmt.Sprintf("%X", src)
}
if b < '0' || (b > '9' && b < 'A') || (b > 'F') {
hexConfusable = false
if b == ' ' || b == '"' || b == '\\' {
forceQuotes = true
}
}
}
return string(id)
if hexConfusable || forceQuotes {
return fmt.Sprintf("%q", src)
}
return string(src)
}

func PrintShape(tree *iavl.MutableTree) {
func PrintShape(tree *iavl.ImmutableTree) {
Copy link

Choose a reason for hiding this comment

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

Handle errors explicitly in PrintShape.

The function currently ignores errors silently. It's a good practice to handle errors explicitly to avoid hidden bugs.

- shape, _ := tree.RenderShape("  ", nodeEncoder)
+ shape, err := tree.RenderShape("  ", nodeEncoder)
+ if err != nil {
+     fmt.Fprintf(os.Stderr, "Error rendering tree shape: %s\n", err)
+     return
+ }
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 PrintShape(tree *iavl.ImmutableTree) {
func PrintShape(tree *iavl.ImmutableTree) {
shape, err := tree.RenderShape(" ", nodeEncoder)
if err != nil {
fmt.Fprintf(os.Stderr, "Error rendering tree shape: %s\n", err)
return
}
fmt.Println(shape)
}

// shape := tree.RenderShape(" ", nil)
// TODO: handle this error
shape, _ := tree.RenderShape(" ", nodeEncoder)
Expand All @@ -173,9 +223,6 @@ func nodeEncoder(id []byte, depth int, isLeaf bool) string {
if isLeaf {
prefix = fmt.Sprintf("*%d ", depth)
}
if len(id) == 0 {
return fmt.Sprintf("%s<nil>", prefix)
}
return fmt.Sprintf("%s%s", prefix, parseWeaveKey(id))
}

Expand Down
Loading