Open
Conversation
Contributor
📝 WalkthroughWalkthroughExtends the get-tss-address CLI command to query TSS addresses across multiple Bitcoin networks. Adds btcNetworks enumeration and modifies command behavior: single argument queries one network (backward compatible), no argument queries all networks and prints results with per-network error handling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@x/observer/client/cli/query_get_tss_address.go`:
- Around line 31-40: The help text (the Long string in query_get_tss_address.go)
claims that giving a bitcoinChainId prints only the Bitcoin address, but the
command's handler still prints the full query response in the bitcoin-argument
branch (the printing block that handles bitcoinChainId in the command's
RunE/handler). Fix by either (A) updating the Long string to accurately describe
that the full response is printed even when a bitcoinChainId is supplied, or (B)
change the handler printing logic so when a bitcoinChainId argument is provided
it filters the query response and prints only the Bitcoin address for that chain
(modify the bitcoin-argument branch in the RunE/handler to extract and print the
single address instead of printing the entire response). Ensure you update the
same Long text and the bitcoin-argument printing branch so both behavior and
help text remain consistent.
- Around line 89-101: The loop over btcNetworks in the function using
queryClient.GetTssAddress currently prints per-network errors and continues, but
the function still returns nil; instead, track failures (e.g., collect failed
net identifiers into a slice) while continuing to print successes, and after the
loop return a non-nil error if any failures occurred (e.g., fmt.Errorf with the
list of failed networks). Update the function that performs the loop
(references: btcNetworks, queryClient.GetTssAddress, btcRes) to accumulate
failed networks and return a combined error after printing all results rather
than returning nil on partial failures.
- Around line 75-98: The no-arg branch in printAllTSSAddresses currently uses
fmt.Printf for output; change it to use the same client output path as the
single-chain branch by getting clientCtx := client.GetClientContextFromCmd(cmd)
and calling clientCtx.PrintProto(...) instead of fmt.Printf. Specifically,
replace the fmt.Printf("eth: ..."), fmt.Printf("sui: ..."), and fmt.Printf("btc
(...) : ...") calls to print the QueryGetTssAddressResponse (res) via
clientCtx.PrintProto(res) and likewise print each btcRes with
clientCtx.PrintProto(btcRes) (or aggregate into a single proto message and
PrintProto) so output respects configured format and Cobra's output writer.
Ensure errors still print to the command writer (use
fmt.Fprintf(cmd.OutOrStdout(), ...) or return the error) consistent with other
query commands.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 13ad8914-68fc-4663-a369-602c03f5fc79
📒 Files selected for processing (1)
x/observer/client/cli/query_get_tss_address.go
Comment on lines
+31
to
+40
| Long: `Query current TSS addresses for EVM, Sui, and Bitcoin. | ||
|
|
||
| Without arguments, prints the EVM and Sui addresses once, and the Bitcoin address for all | ||
| supported networks (mainnet, testnet3, signet, testnet4, regtest). | ||
|
|
||
| With a bitcoinChainId argument, prints only the Bitcoin address for that specific chain. | ||
|
|
||
| Examples: | ||
| zetacored query observer get-tss-address | ||
| zetacored query observer get-tss-address 18333`, |
Contributor
There was a problem hiding this comment.
Help text does not match the single-chain behavior.
Line 36 says this mode prints only the Bitcoin address, but Lines 55-61 still print the full query response. Please either narrow the output or update the help text to describe the actual behavior.
Suggested wording
-With a bitcoinChainId argument, prints only the Bitcoin address for that specific chain.
+With a bitcoinChainId argument, prints the TSS addresses, using that Bitcoin chain ID to derive the BTC address.Also applies to: 55-61
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@x/observer/client/cli/query_get_tss_address.go` around lines 31 - 40, The
help text (the Long string in query_get_tss_address.go) claims that giving a
bitcoinChainId prints only the Bitcoin address, but the command's handler still
prints the full query response in the bitcoin-argument branch (the printing
block that handles bitcoinChainId in the command's RunE/handler). Fix by either
(A) updating the Long string to accurately describe that the full response is
printed even when a bitcoinChainId is supplied, or (B) change the handler
printing logic so when a bitcoinChainId argument is provided it filters the
query response and prints only the Bitcoin address for that chain (modify the
bitcoin-argument branch in the RunE/handler to extract and print the single
address instead of printing the entire response). Ensure you update the same
Long text and the bitcoin-argument printing branch so both behavior and help
text remain consistent.
Comment on lines
+75
to
+98
| func printAllTSSAddresses(cmd *cobra.Command, queryClient types.QueryClient) error { | ||
| // Query with first BTC network to get EVM and Sui addresses | ||
| first := btcNetworks[0] | ||
| res, err := queryClient.GetTssAddress(cmd.Context(), &types.QueryGetTssAddressRequest{ | ||
| BitcoinChainId: first.ChainID, | ||
| }) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| fmt.Printf("eth: %s\n", res.Eth) | ||
| fmt.Printf("sui: %s\n", res.Sui) | ||
| fmt.Printf("btc (%s, %d): %s\n", first.Name, first.ChainID, res.Btc) | ||
|
|
||
| // Query remaining BTC networks | ||
| for _, net := range btcNetworks[1:] { | ||
| btcRes, err := queryClient.GetTssAddress(cmd.Context(), &types.QueryGetTssAddressRequest{ | ||
| BitcoinChainId: net.ChainID, | ||
| }) | ||
| if err != nil { | ||
| fmt.Printf("btc (%s, %d): error: %v\n", net.Name, net.ChainID, err) | ||
| continue | ||
| } | ||
| fmt.Printf("btc (%s, %d): %s\n", net.Name, net.ChainID, btcRes.Btc) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the file
find . -name "query_get_tss_address.go" -type fRepository: zeta-chain/node
Length of output: 107
🏁 Script executed:
# Read the file to see the actual code and context
cat -n x/observer/client/cli/query_get_tss_address.go | head -120Repository: zeta-chain/node
Length of output: 4519
🏁 Script executed:
# Search for other CLI query functions to see the pattern used for output
rg "OutOrStdout\|Printf" x/observer/client/cli/ -n -A 2 -B 2 | head -100Repository: zeta-chain/node
Length of output: 41
🏁 Script executed:
# Check for Cobra usage patterns in this CLI directory
fd "\.go$" x/observer/client/cli/ -x grep -l "cmd.Out" {} \;Repository: zeta-chain/node
Length of output: 41
🏁 Script executed:
# Search for output patterns in CLI queries
rg "Print|Printf|OutOrStdout|cmd\.Out" x/observer/client/cli/ --type go -n | head -50Repository: zeta-chain/node
Length of output: 2848
🏁 Script executed:
# Check other query commands in the same directory
find x/observer/client/cli -name "query_*.go" -type fRepository: zeta-chain/node
Length of output: 716
🏁 Script executed:
# Look at how other commands handle output - check a few similar query files
cat -n x/observer/client/cli/query.go | head -100Repository: zeta-chain/node
Length of output: 1697
🏁 Script executed:
# Check if clientCtx.PrintProto is the standard pattern used elsewhere
rg "clientCtx\.Print|PrintProto" x/observer/client/cli/ --type go -n -B 2 -A 2 | head -80Repository: zeta-chain/node
Length of output: 4093
Make the no-arg path use the same output layer as the single-chain branch.
Lines 85–98 use direct fmt.Printf output, creating an inconsistency within the same command. The single-chain path (line 61) routes output through clientCtx.PrintProto(res), which respects the configured output format and Cobra's output writer. The no-arg path should do the same by using clientCtx.PrintProto() to ensure consistent behavior, proper scripting support, and adherence to the established codebase pattern used throughout other query commands in this module.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@x/observer/client/cli/query_get_tss_address.go` around lines 75 - 98, The
no-arg branch in printAllTSSAddresses currently uses fmt.Printf for output;
change it to use the same client output path as the single-chain branch by
getting clientCtx := client.GetClientContextFromCmd(cmd) and calling
clientCtx.PrintProto(...) instead of fmt.Printf. Specifically, replace the
fmt.Printf("eth: ..."), fmt.Printf("sui: ..."), and fmt.Printf("btc (...) :
...") calls to print the QueryGetTssAddressResponse (res) via
clientCtx.PrintProto(res) and likewise print each btcRes with
clientCtx.PrintProto(btcRes) (or aggregate into a single proto message and
PrintProto) so output respects configured format and Cobra's output writer.
Ensure errors still print to the command writer (use
fmt.Fprintf(cmd.OutOrStdout(), ...) or return the error) consistent with other
query commands.
Comment on lines
+89
to
+101
| // Query remaining BTC networks | ||
| for _, net := range btcNetworks[1:] { | ||
| btcRes, err := queryClient.GetTssAddress(cmd.Context(), &types.QueryGetTssAddressRequest{ | ||
| BitcoinChainId: net.ChainID, | ||
| }) | ||
| if err != nil { | ||
| fmt.Printf("btc (%s, %d): error: %v\n", net.Name, net.ChainID, err) | ||
| continue | ||
| } | ||
| fmt.Printf("btc (%s, %d): %s\n", net.Name, net.ChainID, btcRes.Btc) | ||
| } | ||
|
|
||
| return nil |
Contributor
There was a problem hiding this comment.
Do not return success after partial network failures.
Lines 94-96 downgrade per-network query failures to text and continue, but Line 101 still returns nil. Any automation invoking this command will get exit code 0 even when some BTC addresses were not retrieved. Consider collecting the failed networks and returning a combined error after printing the successful rows.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@x/observer/client/cli/query_get_tss_address.go` around lines 89 - 101, The
loop over btcNetworks in the function using queryClient.GetTssAddress currently
prints per-network errors and continues, but the function still returns nil;
instead, track failures (e.g., collect failed net identifiers into a slice)
while continuing to print successes, and after the loop return a non-nil error
if any failures occurred (e.g., fmt.Errorf with the list of failed networks).
Update the function that performs the loop (references: btcNetworks,
queryClient.GetTssAddress, btcRes) to accumulate failed networks and return a
combined error after printing all results rather than returning nil on partial
failures.
ws4charlie
approved these changes
Mar 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Updates the query to print all variants of BTC address
How Has This Been Tested?
Note
Medium Risk
Changes the CLI’s default output format and performs multiple sequential queries, which may break downstream scripts and alters failure behavior (partial results with per-network errors).
Overview
Updates
observer get-tss-addressto support a multi-network default output: when run with no args it now printsethandsuionce and then queries/prints the BTC TSS address for each supported Bitcoin network (mainnet,testnet3,signet,testnet4,regtest).Preserves the prior single-query behavior when a
bitcoinChainIdis provided, and expands command help/usage text; the no-arg path now uses customfmt.Printfoutput (and logs per-network errors instead of failing the whole command).Written by Cursor Bugbot for commit 3887f2c. Configure here.
Summary by CodeRabbit
New Features
Improvements