Conversation
|
Warning Rate limit exceeded@faddat has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 24 minutes and 0 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request introduces a comprehensive P2P broadcasting system and refines transaction and peer handling across multiple modules. New files and types establish P2P broadcasters and clients for handling transactions over a network. Enhancements include additional validation in transaction building, updated client context retrieval, and improved error handling. Utility functions for HTTP contexts, keyring management, and node connectivity are added. Peer discovery and registry modules now feature adjusted timeouts, retry logic, and expanded configuration options to support hybrid and multisend operations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Broadcaster
participant Client
participant Peer
User->>Broadcaster: Call BroadcastTx(txBytes)
Broadcaster->>Broadcaster: Validate & generate tx hash
Broadcaster->>Client: Delegate tx broadcast
Client->>Client: Start peer connection if needed
Client->>Peer: Send transaction message
Peer-->>Client: Acknowledge receipt
Client-->>Broadcaster: Return broadcast result
Possibly related PRs
Poem
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (20)
lib/chainregistry/cli.go (1)
105-105: Increased peer discovery timeout is a reasonable enhancement.Extending the timeout from 45 seconds to 2 minutes gives the system more time to discover additional peers before timing out, which can be particularly helpful in networks with many nodes or slower connectivity.
Consider making this timeout configurable rather than hardcoded, allowing users to adjust it based on their network conditions. You could add a configuration option or environment variable.
lib/peerdiscovery/discovery.go (1)
179-209: Well-implemented retry logic for node connections.The retry mechanism is properly implemented with exponential backoff (increasing delay between retries), clear logging, and proper connection cleanup.
Consider extracting the retry logic into a reusable utility function that could be used in other parts of the codebase that need similar retry functionality:
// retryWithBackoff attempts an operation with exponential backoff retries func retryWithBackoff(operation func() error, maxRetries int, operationName string) error { var err error for retry := 0; retry < maxRetries; retry++ { // If this is a retry, wait with exponential backoff if retry > 0 { delay := time.Duration(retry) * time.Second fmt.Printf("Retry %d/%d for %s in %v\n", retry, maxRetries, operationName, delay) time.Sleep(delay) } // Attempt the operation if err = operation(); err == nil { return nil // Success } fmt.Printf("Attempt %d: %s failed: %v\n", retry+1, operationName, err) } // If we get here, all retries failed return fmt.Errorf("failed after %d attempts: %w", maxRetries, err) }broadcast/transaction.go (1)
1007-1014: Added logging for transaction node selection improves observability.The new log statements make it clear which node is being used for a given transaction, which is helpful for debugging and monitoring.
Consider adding more detailed logging of parameters that affect node selection to make it easier to debug unexpected behavior:
- fmt.Printf("📝 Transaction for account %s using node %s\n", - fromAddress, txParams.NodeURL) + fmt.Printf("📝 Transaction for account %s using node %s (selected from %d available nodes)\n", + fromAddress, txParams.NodeURL, len(txParams.Config.Nodes.RPC))broadcast/utils.go (3)
148-168: Consider making broadcast mode configurable.
Currently,GetHTTPClientContextsets the broadcast mode to"sync", noting that it will be overridden. Make the broadcast mode an explicit method parameter or read it from config to avoid confusion.
170-200: Keyring creation logic looks sound.
Switching on different backends (test, file, os, memory) is straightforward. If you want multi-chain support, consider a chain-specific keyring name instead of the hard-coded"cosmos".
202-237: Robust node health checks.
IsNodeReachablecleverly checks/health, but no further validation is performed on the response body. Consider more in-depth checks or reusing the approach fromGetNodeInfoif deeper details are needed.p2p/client.go (5)
39-80: Initialization flow for NewClient is coherent.
Generating a unique node ID, logging setup, and channel creation is straightforward. You might consider injecting the logger and default P2P port to allow for different logging frameworks or addresses.
81-97: Graceful Start method.
The client setsisRunningand launches goroutines for both transaction processing and peer connections. Ensure you handle potential goroutine errors, e.g., goroutine panics, for production readiness.
118-145: BroadcastTx logic is currently minimal.
It sets up a response channel and enqueues the transaction totxChan. To avoid timeouts, consider increasing the queue buffer or ensuring the consumer drains quickly.
191-214: processMessages concurrency is handled with a select on context.
This is a good pattern to ensure you exit cleanly when the client is stopped. Just be mindful that larger transaction volumes may need more advanced queue management.
216-237: Helper functions for generating IDs.
Using crypto/rand is good for randomness. The fallback for peer ID generation on error is acceptable for a prototype, but logs or an error might be better in production code.broadcast/p2p_broadcast.go (7)
42-67: Global concurrency pattern for broadcaster.
Thep2pBroadcasterOnceusage with a global instance is convenient for a singleton. Just be aware that singletons can limit testability if multiple concurrent networks are needed.
121-185: BroadcastTxP2P fallback to RPC is a nice feature.
If no peers are connected, it gracefully callsBroadcastTxSync. This ensures reliability. Evaluate how you plan to handle partial peer connectivity in the future.
234-265: Local getSeedNodesForChain with hardcoded seeds.
This is fine for initial usage. As the comment states, a dynamic registry or external config might be more flexible.
267-285: NewP2PBroadcaster returns an initialized instance in the background.
It mirrors the global approach but is decoupled from the singleton logic. This is consistent, but you might unify them or clarify usage for consistency.
287-311: Initialize method sets up the broadcaster.
Lock usage is correct, but check if the background goroutine in the constructor orFindPeersmight duplicate work.
360-389: FindPeers simulates discovery.
Logs the seed nodes you would connect to. The imitation is a good stepping stone, but be sure to unify it with your other peer management calls.
417-453: P2PGetClientContext tries to connect to a node, logging warnings on failure.
This is a sensible approach. If you want offline or partial connectivity logic, consider distinguishing fatal vs non-fatal connection errors more clearly.p2p/broadcaster.go (1)
90-101: Consider waiting for in-flight operations during shutdown.Currently, the
Shutdownmethod stops the client immediately without waiting for any ongoing peer connections or broadcasts to complete. If a graceful shutdown is desired, consider joining or signaling any in-flight operations before clearing theclientreference.modes/registry/registry.go (1)
2011-2055: Check for timeouts in continuous transaction loops.While flooding multiple sends, the loop processes up to
numTransactions. If external factors cause slow broadcasts or unresponsive nodes, consider adding periodic context-cancellation checks or more robust retry logic to avoid unexpected long hangs.🧰 Tools
🪛 GitHub Check: test (1.23.x, macos-latest)
[failure] 2036-2036:
fmt.Printf format %q reads arg #4, but call has 3 args🪛 GitHub Check: test (1.23.x, ubuntu-latest)
[failure] 2036-2036:
fmt.Printf format %q reads arg #4, but call has 3 args🪛 GitHub Actions: Test
[error] 2036-2036: fmt.Printf format %q reads arg #4, but call has 3 args
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
broadcast/p2p_broadcast.go(1 hunks)broadcast/transaction.go(3 hunks)broadcast/utils.go(3 hunks)lib/chainregistry/cli.go(1 hunks)lib/peerdiscovery/discovery.go(4 hunks)modes/registry/registry.go(16 hunks)p2p/broadcaster.go(1 hunks)p2p/client.go(1 hunks)types/types.go(2 hunks)
🧰 Additional context used
🪛 GitHub Check: test (1.23.x, macos-latest)
modes/registry/registry.go
[failure] 2036-2036:
fmt.Printf format %q reads arg #4, but call has 3 args
🪛 GitHub Check: test (1.23.x, ubuntu-latest)
modes/registry/registry.go
[failure] 2036-2036:
fmt.Printf format %q reads arg #4, but call has 3 args
🪛 GitHub Actions: Test
modes/registry/registry.go
[error] 2036-2036: fmt.Printf format %q reads arg #4, but call has 3 args
🔇 Additional comments (26)
types/types.go (1)
137-141: Support for hybrid mode and enhanced multisend controls are good additions.The new configuration options for multisend functionality provide more flexibility in transaction handling. The
Hybridflag allows a mix of sends and multisends, whileNumOutReceiversgives control over the number of recipients in multisend transactions.lib/peerdiscovery/discovery.go (3)
17-21: Added retry mechanism improves peer discovery robustness.Increasing the
DefaultTimeoutand adding theMaxRetriesconstant enhances the robustness of peer discovery by allowing for temporary connection issues.
63-70: Well-implemented tracking of initial registry endpoints.The code now properly normalizes and tracks the initial registry endpoints, which will help with prioritizing endpoints in the results.
105-129: Improved endpoint prioritization logic.The new code separates discovered endpoints into registry and non-registry sources, prioritizing non-registry endpoints in the results. This is beneficial as it gives preference to newly discovered peers while still maintaining the original registry endpoints as fallbacks.
broadcast/transaction.go (5)
176-186: Added input validation for from_address improves transaction safety.The additional validation ensures that the sender address is both present and valid before attempting to build a transaction, preventing potential errors later in the transaction lifecycle.
194-197: Explicit setting of from address in client context is good practice.This change ensures that the client context has the correct sender address, which is important for proper transaction building and signing.
646-646: Consistent use of P2PGetClientContext throughout the codebase.The function has been changed to use
P2PGetClientContexthere as well, maintaining consistency with the earlier changes.
1023-1023: Consistent use of P2PGetClientContext ensures compatibility with P2P broadcasting.Using the same context retrieval method here ensures that the transaction is broadcasted using the P2P system consistently.
189-189:Details
❓ Verification inconclusive
Changed client context retrieval method may require broader changes.
The function has been changed from
GetClientContexttoP2PGetClientContext, which suggests a switch to P2P broadcasting functionality. Ensure that all callers are updated and that the new function properly handles all the same use cases.Check for any occurrences of
GetClientContextthat might need to be updated:
🏁 Script executed:
#!/bin/bash # Search for all instances of GetClientContext that might need to be updated rg "GetClientContext" --type goLength of output: 1148
Attention: P2P Client Context Update Verification
The search results confirm that the update to use
P2PGetClientContextinbroadcast/transaction.gois consistent with similar changes inbroadcast/p2p_broadcast.goandmodes/registry/registry.go. The remaining use ofGetClientContextinbroadcast/utils.goappears to be intentional for HTTP-specific cases. Please verify that this separation between P2P and HTTP client context retrieval is deliberate and that no functionality was inadvertently affected.
- Confirm that HTTP-related callers still correctly use
GetClientContextinbroadcast/utils.go.- Ensure that the P2P-specific paths now using
P2PGetClientContextfully cover the intended use cases.broadcast/utils.go (3)
5-12: Imports for new HTTP-based features look appropriate.
They enable network requests (net/http), streaming I/O (io), OS interactions (os), string operations, and timeouts, which are needed for the newly introduced HTTP context and node reachability checks.
21-21: Keyring import is correctly added.
This import is used for managing keys and appears consistent with the new keyring-related functionality in this file.
239-306: Comprehensive node status retrieval.
GetNodeInfothoroughly requests/statusand parses JSON to extract node info. This is a helpful complement toIsNodeReachable. Just keep an eye on possible schema changes on the node side that might break the map-based parsing.p2p/client.go (4)
14-37: Concurrency structures and fields in Client look well-defined.
The use of mutexes (peersMtx, txRespMtx) and channels for broadcasting indicates good concurrency safety. Beware of potential race conditions if external goroutines modify these fields without locks.
99-117: Stop ensures graceful shutdown.
Cancelling the context, closing channels, and resetting state is appropriate. This method is a good complement to Start. If more resources are open, ensure they’re also cleaned.
147-168: connectToPeers effectively simulates peer connections.
The current placeholder approach for TCP connections is understandable for a first version. Just ensure to expand it when real networking is implemented.
170-189: connectToPeer is aligned with connectToPeers approach.
Again, placeholders for real networking are fine to keep the logic straightforward.broadcast/p2p_broadcast.go (8)
23-40: P2PBroadcaster, Peer, and TxInfo structures provide clear separation of concerns.
P2PBroadcaster tracks peers and transactions, while Peer focuses on connection state, and TxInfo handles transaction broadcast details. Good design.
69-91: GetP2PBroadcaster enforces a lazy initialization.
Kicking offinitializein a goroutine is an interesting approach. Mind that early calls to the broadcaster might expect an already connected state.
93-120: Broadcaster initialization.
The simulation of seed peer connections is a good starting point. Just ensure robust error handling or fallback logic once real P2P is implemented.
187-200: RunCommandWithOutput fosters external interactions.
CapturingCombinedOutput()is good for debugging. Keep an eye on security implications if passing untrusted args in production.
313-327: Shutdown logic is minimal but clean.
Unsetsinitializedafter printing a shutdown message. For real networking, consider graceful peer disconnection.
328-358: BroadcastTx method rechecks initialization.
This is a good safeguard. Consider the effect of repeated calls under heavy load—some concurrency tests would be valuable.
455-473: Config struct and GetDefaultConfig are straightforward.
They provide typical fields for chain interactions, including gas settings. This approach is good for simple local usage.
475-509: MapAccountToNode fosters a sticky assignment.
The use of account address bytes to pick a node index is a creative approach. Watch for concurrency issues inaccountToNodeMapif frequently updated from multiple goroutines.p2p/broadcaster.go (1)
153-180: Revisit concurrency in peer discovery.After spawning separate goroutines to connect to each seed address, the channel is closed immediately, which signals the original goroutine to return. If you want to wait until peer connections actually succeed or fail, consider synchronizing on the goroutines that attempt these connections (for instance, using a
sync.WaitGroup) before closing thedonechannel.modes/registry/registry.go (1)
657-660: Confirm skip-balance-check logic.When
config.SkipBalanceCheckis true, the code bypasses all balance verification and adjustment. This might lead to unbalanced accounts if prior attempts to fix balances failed. Verify that skipping is indeed desired under those conditions.
| Medium int64 `toml:"medium"` // Renamed from Mid -> Medium | ||
| High int64 `toml:"high"` // Renamed from Max -> High | ||
| Max int64 `toml:"max"` // New maximum gas setting | ||
| Mid int64 `toml:"mid"` // Alias for Medium | ||
| Precision int64 `toml:"precision"` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Confusing gas configuration parameter naming.
The renaming from Mid to Medium and Max to High while also introducing a new Max field and keeping Mid as an alias could lead to confusion. It's unclear which field should be used in different situations.
Consider one of these approaches:
- Remove the
Midalias entirely and update all code to useMedium - Mark
Midas deprecated with a comment indicating to useMediuminstead - Document the relationship between
HighandMaxclearly
- Medium int64 `toml:"medium"` // Renamed from Mid -> Medium
- High int64 `toml:"high"` // Renamed from Max -> High
- Max int64 `toml:"max"` // New maximum gas setting
- Mid int64 `toml:"mid"` // Alias for Medium
+ Medium int64 `toml:"medium"` // Medium gas setting
+ High int64 `toml:"high"` // High gas setting
+ Max int64 `toml:"max"` // Maximum gas setting (higher than High)
+ Mid int64 `toml:"mid"` // Deprecated: Use Medium instead📝 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.
| Medium int64 `toml:"medium"` // Renamed from Mid -> Medium | |
| High int64 `toml:"high"` // Renamed from Max -> High | |
| Max int64 `toml:"max"` // New maximum gas setting | |
| Mid int64 `toml:"mid"` // Alias for Medium | |
| Precision int64 `toml:"precision"` | |
| Medium int64 `toml:"medium"` // Medium gas setting | |
| High int64 `toml:"high"` // High gas setting | |
| Max int64 `toml:"max"` // Maximum gas setting (higher than High) | |
| Mid int64 `toml:"mid"` // Deprecated: Use Medium instead | |
| Precision int64 `toml:"precision"` |
| NumMultisend int `toml:"num_multisend"` // Number of transactions to include in a multisend | ||
| NumOutReceivers int `toml:"num_out_receivers"` // Number of receivers for multisend transactions | ||
| SkipBalanceCheck bool `toml:"skip_balance_check"` // Whether to skip balance checking and adjustment | ||
| MsgParams MsgParams `toml:"msg_params"` |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Explicit balance check skipping may lead to transaction failures.
The SkipBalanceCheck option could result in transactions being built that will fail due to insufficient funds. Ensure there's clear documentation about when this flag should be used.
Before making a decision about whether to use this flag, consider running this script to see how balance checks are currently implemented:
🏁 Script executed:
#!/bin/bash
# Search for balance checking code to understand implementation
echo "Looking for balance check implementation..."
rg "balance" --type go -A 3 -B 3
echo "Looking for places where SkipBalanceCheck would be used..."
rg "SkipBalanceCheck" --type go -A 3 -B 3Length of output: 54117
Action: Clarify and Document SkipBalanceCheck Usage
The SkipBalanceCheck flag disables balance verification and adjustment, which can allow transactions to be constructed even when accounts lack sufficient funds—potentially leading to failures at runtime. Our investigation confirms that the flag is used in both the configuration (in types/types.go) and during runtime in modes/registry/registry.go (triggered either by explicit config or via the .skip_balance_check marker file).
Suggestions:
- Enhance Inline Comments: Update the in-code comment for the
SkipBalanceCheckfield (currently “Whether to skip balance checking and adjustment”) to explain the risks and specific conditions under which skipping balance checks is acceptable. - External Documentation: Add or expand documentation (e.g., in the README) detailing:
- When and why to use this flag (for instance, as a fallback only when balance adjustments repeatedly fail).
- The potential impact on transaction reliability if enabled inadvertently.
- Recommended best practices for troubleshooting balance check issues.
- Runtime Warnings (Optional): Consider implementing additional runtime logging or warnings when the flag is active to ensure users are aware of the disabled verification.
Please address these improvements to help users avoid inadvertently triggering transaction failures.
| // FindRandomRecipient finds a random recipient from a keyring | ||
| func FindRandomRecipient(keyringBackend, excludeAddress string) (string, error) { | ||
| // List accounts in keyring | ||
| output, err := RunCommandWithOutput("", "meteorite", "keys", "list", "--keyring-backend", keyringBackend) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| // Parse output to find addresses | ||
| addresses := []string{} | ||
| lines := strings.Split(output, "\n") | ||
|
|
||
| for _, line := range lines { | ||
| if strings.Contains(line, "address:") { | ||
| parts := strings.Split(line, "address: ") | ||
| if len(parts) > 1 { | ||
| addr := strings.TrimSpace(parts[1]) | ||
| if addr != excludeAddress { | ||
| addresses = append(addresses, addr) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if len(addresses) == 0 { | ||
| return "", errors.New("no other addresses found in keyring") | ||
| } | ||
|
|
||
| // Pick a random address | ||
| return addresses[rand.Intn(len(addresses))], nil | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
FindRandomRecipient is strictly reliant on CLI output.
Parsing key addresses from meteorite keys list works, but might be brittle if output formats change. Consider a stable library interface for the key store if available.
| // GetSeedNodesForChain returns seed nodes for a specific chain | ||
| func GetSeedNodesForChain(chainID string) []string { | ||
| switch chainID { | ||
| case "cosmoshub-4": | ||
| return []string{ | ||
| "e1d7ff02b78044795371bff4c36b240262d8479c@65.108.2.41:26656", | ||
| "ade4d8bc8cbe014af6ebdf3cb7b1e9ad36f412c0@seeds.polkachu.com:14956", | ||
| "20e1000e88125698264454a884812746c2eb4807@seeds.lavenderfive.com:14956", | ||
| } | ||
| case "osmosis-1": | ||
| return []string{ | ||
| "f94c92c75ec370b23d7408e32c28e6a3b138dd57@65.108.2.41:26656", | ||
| "5a37f1f701b3634add5a2034c4d0cc0c95f48a3f@seeds.polkachu.com:12556", | ||
| "3255e3620984c891204251d9eeb3e981745913b2@seeds.lavenderfive.com:12556", | ||
| } | ||
| case "stargaze-1": | ||
| return []string{ | ||
| "d95a7770a5f43570303d3d538538ca03f2a3c2c7@65.108.2.41:26656", | ||
| "6c2377646af8c2d99a26ff2256bd1f93382b46ad@seeds.polkachu.com:13756", | ||
| "def2c8a5c85d2f528e4311dcadc8080b91bf5a69@seeds.lavenderfive.com:13756", | ||
| } | ||
| default: | ||
| // Default to returning empty list, the application should have a way to look up seed nodes | ||
| return []string{} | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
GetSeedNodesForChain is largely duplicated from getSeedNodesForChain.
Having two similar functions suggests some refactoring is needed to avoid maintenance overhead. Possibly unify them in one function.
-// getSeedNodesForChain (private) and GetSeedNodesForChain (public) appear to duplicate logic
-// Consider removing one of them or internally reusing one from the other.Committable suggestion skipped: line range outside the PR's diff.
| // BroadcastTx broadcasts a transaction through the P2P network | ||
| func (b *Broadcaster) BroadcastTx(txBytes []byte) (*BroadcastResult, error) { | ||
| b.mtx.Lock() | ||
| initialized := b.isInitialized | ||
| b.mtx.Unlock() | ||
|
|
||
| if !initialized { | ||
| if err := b.Initialize(); err != nil { | ||
| return nil, fmt.Errorf("failed to initialize P2P broadcaster: %w", err) | ||
| } | ||
| } | ||
|
|
||
| if b.client == nil { | ||
| return nil, errors.New("P2P client is not initialized") | ||
| } | ||
|
|
||
| // Create a hash of the transaction for tracking | ||
| hash := sha256.Sum256(txBytes) | ||
| txHash := hex.EncodeToString(hash[:]) | ||
|
|
||
| // Log the transaction broadcast | ||
| b.logger.Printf("Broadcasting transaction with hash %s", txHash) | ||
|
|
||
| // Broadcast the transaction | ||
| err := b.client.BroadcastTx(txBytes) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to broadcast transaction: %w", err) | ||
| } | ||
|
|
||
| // Return a synthetic success result | ||
| return &BroadcastResult{ | ||
| Code: 0, | ||
| Log: "Transaction broadcast through P2P network", | ||
| TxHash: txHash, | ||
| Height: -1, // unknown height | ||
| }, nil | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Return meaningful broadcasting feedback.
The BroadcastTx method always returns a synthetic success result (Code: 0, Height: -1). If you need to track or confirm success, consider fetching a real response from the network to differentiate between successful broadcasts and failures.
| // Process the response | ||
| if broadcastErr != nil { | ||
| failedTxs++ | ||
| fmt.Printf("[POS-%d] Transaction FAILED: seq=%d broadcast=%s error=%q\n", |
There was a problem hiding this comment.
Fix the printf argument mismatch.
The format string uses four placeholders (%d, %d, %s, %q) but only provides three arguments, causing a runtime error. Include the missing argument (e.g., elapsed) or remove the extra placeholder. For example:
- fmt.Printf("[POS-%d] Transaction FAILED: seq=%d broadcast=%s error=%q\n", position, sequence, broadcastErr)
+ fmt.Printf("[POS-%d] Transaction FAILED: seq=%d broadcast=%s error=%q\n",
+ position, sequence, elapsed, broadcastErr)📝 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.
| fmt.Printf("[POS-%d] Transaction FAILED: seq=%d broadcast=%s error=%q\n", | |
| fmt.Printf("[POS-%d] Transaction FAILED: seq=%d broadcast=%s error=%q\n", | |
| position, sequence, elapsed, broadcastErr) |
🧰 Tools
🪛 GitHub Check: test (1.23.x, macos-latest)
[failure] 2036-2036:
fmt.Printf format %q reads arg #4, but call has 3 args
🪛 GitHub Check: test (1.23.x, ubuntu-latest)
[failure] 2036-2036:
fmt.Printf format %q reads arg #4, but call has 3 args
🪛 GitHub Actions: Test
[error] 2036-2036: fmt.Printf format %q reads arg #4, but call has 3 args
1 participant
Summary by CodeRabbit
New Features
Enhancements