auth: Improve cli tx decoding with fallback to direct txBytes protobuf unmarshaling #23611
Conversation
…f unmarshaling IIRC before it supported just signBytes
📝 WalkthroughWalkthroughThe changes add a new function Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Decoder as decodeTxAndGetJSON
participant TxDecoder as clientCtx.TxConfig
participant JSONEncoder
Caller->>Decoder: Call decodeTxAndGetJSON(txBytes)
Decoder->>TxDecoder: Decode transaction (txBytes)
alt Decoding Successful
TxDecoder-->>Decoder: Decoded transaction
Decoder->>JSONEncoder: Encode transaction to JSON
JSONEncoder-->>Decoder: JSON result
else Decoding Failed
Decoder->>Decoder: Unmarshal txBytes into sdktx.Tx
Decoder->>JSONEncoder: Marshal sdktx.Tx to JSON
end
Decoder-->>Caller: Return JSON or error
Suggested reviewers
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
x/auth/client/cli/decode.go (1)
61-63: Consider wrapping errors with more context.To improve debugging, consider wrapping the error with additional context about the fallback attempt.
- return nil, err + return nil, fmt.Errorf("failed to unmarshal tx bytes in fallback path: %w", err)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
x/auth/client/cli/decode.go(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
x/auth/client/cli/decode.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: tests (03)
- GitHub Check: tests (00)
- GitHub Check: test-simapp-v2
- GitHub Check: test-system-v2
- GitHub Check: test-sim-nondeterminism
- GitHub Check: test-integration
- GitHub Check: Analyze
- GitHub Check: build (amd64)
- GitHub Check: Summary
🔇 Additional comments (3)
x/auth/client/cli/decode.go (3)
11-11: LGTM! Import follows style guidelines.The new import is properly grouped with other cosmos-sdk imports and uses clear aliasing.
36-36: LGTM! Good separation of concerns.Extracting the decoding logic into a separate function improves code organization and maintainability.
52-57: LGTM! Clear implementation of the fallback mechanism.The function follows a clear pattern:
- Attempts standard decoding first
- Only falls back if needed
- Includes helpful comments explaining the flow
Co-authored-by: Alex | Interchain Labs <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
x/auth/client/cli/decode.go(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
x/auth/client/cli/decode.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
🪛 GitHub Check: tests (03)
x/auth/client/cli/decode.go
[failure] 62-62:
undefined: fmt
🪛 GitHub Check: tests (02)
x/auth/client/cli/decode.go
[failure] 62-62:
undefined: fmt
🪛 GitHub Check: dependency-review
x/auth/client/cli/decode.go
[failure] 62-62:
undefined: fmt
🪛 GitHub Check: golangci-lint
x/auth/client/cli/decode.go
[failure] 62-62:
undefined: fmt (typecheck)
[failure] 62-62:
undefined: fmt) (typecheck)
[failure] 62-62:
undefined: fmt) (typecheck)
🪛 GitHub Actions: Dependency Review
x/auth/client/cli/decode.go
[error] 62-62: undefined: fmt
🪛 GitHub Actions: Lint
x/auth/client/cli/decode.go
[error] 62-62: undefined: fmt (typecheck)
🪛 GitHub Actions: Build SimApp
x/auth/client/cli/decode.go
[error] 62-62: undefined: fmt
🪛 GitHub Actions: v2 core Tests
x/auth/client/cli/decode.go
[error] 62-62: undefined: fmt
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: tests (00)
- GitHub Check: Analyze
- GitHub Check: Summary
🔇 Additional comments (4)
x/auth/client/cli/decode.go (4)
11-11: LGTM!The import of the
sdktxpackage with a clear alias is appropriate for the new fallback mechanism.
36-36: LGTM!Good refactoring to extract the decoding logic into a separate function, improving code organization and maintainability.
62-62: Improve error message formatting.The error message formatting can be improved as suggested in a previous review.
Apply this change:
- return nil, fmt.Errorf("failed to directly unmarshal txBytes: %w", err) + return nil, fmt.Errorf("failed to directly unmarshal txBytes: %w", err)🧰 Tools
🪛 GitHub Check: tests (03)
[failure] 62-62:
undefined: fmt🪛 GitHub Check: tests (02)
[failure] 62-62:
undefined: fmt🪛 GitHub Check: dependency-review
[failure] 62-62:
undefined: fmt🪛 GitHub Check: golangci-lint
[failure] 62-62:
undefined: fmt (typecheck)
[failure] 62-62:
undefined: fmt) (typecheck)
[failure] 62-62:
undefined: fmt) (typecheck)🪛 GitHub Actions: Dependency Review
[error] 62-62: undefined: fmt
🪛 GitHub Actions: Lint
[error] 62-62: undefined: fmt (typecheck)
🪛 GitHub Actions: Build SimApp
[error] 62-62: undefined: fmt
🪛 GitHub Actions: v2 core Tests
[error] 62-62: undefined: fmt
52-66: LGTM! Well-structured fallback mechanism.The function implements a robust two-step decoding process:
- First attempts decoding with TxDecoder
- Falls back to direct protobuf unmarshaling if needed
The progression is clear and well-commented.
🧰 Tools
🪛 GitHub Check: tests (03)
[failure] 62-62:
undefined: fmt🪛 GitHub Check: tests (02)
[failure] 62-62:
undefined: fmt🪛 GitHub Check: dependency-review
[failure] 62-62:
undefined: fmt🪛 GitHub Check: golangci-lint
[failure] 62-62:
undefined: fmt (typecheck)
[failure] 62-62:
undefined: fmt) (typecheck)
[failure] 62-62:
undefined: fmt) (typecheck)🪛 GitHub Actions: Dependency Review
[error] 62-62: undefined: fmt
🪛 GitHub Actions: Lint
[error] 62-62: undefined: fmt (typecheck)
🪛 GitHub Actions: Build SimApp
[error] 62-62: undefined: fmt
🪛 GitHub Actions: v2 core Tests
[error] 62-62: undefined: fmt
| func decodeTxAndGetJSON(clientCtx client.Context, txBytes []byte) ([]byte, error) { | ||
| // First try decoding with TxDecoder | ||
| tx, err := clientCtx.TxConfig.TxDecoder()(txBytes) | ||
| if err == nil { | ||
| return clientCtx.TxConfig.TxJSONEncoder()(tx) | ||
| } | ||
|
|
||
| // Fallback to direct unmarshaling | ||
| var sdkTx sdktx.Tx | ||
| if err := clientCtx.Codec.Unmarshal(txBytes, &sdkTx); err != nil { | ||
| return nil, fmt.Errorf("failed to directly unmarshal txBytes: %w", err) | ||
| } | ||
|
|
||
| return clientCtx.Codec.MarshalJSON(&sdkTx) | ||
| } |
There was a problem hiding this comment.
Add missing fmt package import.
The fmt package is required for error formatting but is not imported, causing build failures.
Add this import to fix the build:
import (
"encoding/base64"
"encoding/hex"
+ "fmt"
"github.com/spf13/cobra"📝 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.
| func decodeTxAndGetJSON(clientCtx client.Context, txBytes []byte) ([]byte, error) { | |
| // First try decoding with TxDecoder | |
| tx, err := clientCtx.TxConfig.TxDecoder()(txBytes) | |
| if err == nil { | |
| return clientCtx.TxConfig.TxJSONEncoder()(tx) | |
| } | |
| // Fallback to direct unmarshaling | |
| var sdkTx sdktx.Tx | |
| if err := clientCtx.Codec.Unmarshal(txBytes, &sdkTx); err != nil { | |
| return nil, fmt.Errorf("failed to directly unmarshal txBytes: %w", err) | |
| } | |
| return clientCtx.Codec.MarshalJSON(&sdkTx) | |
| } | |
| import ( | |
| "encoding/base64" | |
| "encoding/hex" | |
| "fmt" | |
| "github.com/spf13/cobra" | |
| // ... other imports if any | |
| ) |
🧰 Tools
🪛 GitHub Check: tests (03)
[failure] 62-62:
undefined: fmt
🪛 GitHub Check: tests (02)
[failure] 62-62:
undefined: fmt
🪛 GitHub Check: dependency-review
[failure] 62-62:
undefined: fmt
🪛 GitHub Check: golangci-lint
[failure] 62-62:
undefined: fmt (typecheck)
[failure] 62-62:
undefined: fmt) (typecheck)
[failure] 62-62:
undefined: fmt) (typecheck)
🪛 GitHub Actions: Dependency Review
[error] 62-62: undefined: fmt
🪛 GitHub Actions: Lint
[error] 62-62: undefined: fmt (typecheck)
🪛 GitHub Actions: Build SimApp
[error] 62-62: undefined: fmt
🪛 GitHub Actions: v2 core Tests
[error] 62-62: undefined: fmt
|
hey @assafmo thanks for the PR! Looks like build is failing. Could we also get some tests to reproduce the error case this is fixing |
IIRC before it supported only signBytes.
This mean now
cli tx decodeaccepts the output generated by the mempool in #20802.Summary by CodeRabbit