perf: send multiple txs on the same height concurrently #16
Conversation
WalkthroughThe pull request introduces modifications to the transaction handling process in the Changes
Sequence DiagramsequenceDiagram
participant DebugPriceFeeder
participant PendingRequestManager
participant ExoClient
DebugPriceFeeder->>ExoClient: GetSignedTxBytesDebug()
ExoClient-->>DebugPriceFeeder: Return tx bytes
DebugPriceFeeder->>PendingRequestManager: Add sendBytesReq
PendingRequestManager->>ExoClient: SendSignedTxBytesDebug()
ExoClient-->>PendingRequestManager: Broadcast result
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: 0
🧹 Nitpick comments (6)
cmd/feeder_debug.go (4)
27-30: Clarify responsibility of the newsendBytesReqstruct
ThesendBytesReqstruct now encapsulates transaction bytes and a results channel. Consider including metadata (e.g., block height or a correlation ID) to facilitate logging, diagnostics, or correlation.
45-56: Concurrent request processing aligns with PR objective
Theprocessmethod correctly uses goroutines and aWaitGroupto handle multiple transactions concurrently at or below a given height. Consider capturing partial failures or logging them to track concurrency issues.
164-171: No guaranteed ordering in concurrent execution
CallingSendSignedTxBytesDebugconcurrently fulfills the PR’s goal of sending multiple transactions at once, but there is no inherent ordering guarantee. Ensure this behavior is acceptable and well-documented.
184-195: Error handling strategy with switch break
If an error occurs while signing the transaction (GetSignedTxBytesDebug), the code breaks from theselectcase but continues the outer loop. Verify that this is intentional and won’t omit further processing inadvertently. Also consider whether the user should be notified of partial failures.exoclient/tx.go (2)
132-143: Typographical fix and improved broadcasting logs
The reintroducedSendTxDebugmethod works as intended. However, there is a minor typo in the error message “braodcast.” Recommend correcting it to “broadcast.”- return nil, fmt.Errorf("failed to braodcast transaction, msg:%v, ... + return nil, fmt.Errorf("failed to broadcast transaction, msg:%v, ...
149-156: Duplicate logic and minor typo
SendSignedTxBytesDebugmirrorsSendTxDebug’s broadcast logic with signed bytes. Consider refactoring to avoid duplication. Also fix the “braodcast” typo.- return nil, fmt.Errorf("failed to braodcast transaction, txBytes:%v, ... + return nil, fmt.Errorf("failed to broadcast transaction, txBytes:%v, ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cmd/feeder_debug.go(4 hunks)exoclient/tx.go(1 hunks)
🔇 Additional comments (5)
cmd/feeder_debug.go (4)
8-8: Confirm concurrency usage with the sync package
The import of the sync package indicates concurrency usage such as WaitGroup. Ensure that any shared data structures are handled in a concurrency-safe manner or accessed from a single goroutine.
31-31: Map-based manager might need synchronization
pendingRequestManager is defined as a shared map of block heights to slice pointers. If accessed concurrently from multiple goroutines, consider appropriate locking or guaranteeing single-threaded access.
34-34: Straightforward map initialization
The constructor function returning an empty map of int64 to *sendBytesReq slices is correct and clear.
37-41: Ensure atomic addition of pending requests
The add method appends to a slice stored in a map. If multiple goroutines call add at the same time, a race might occur. Confirm that only one goroutine writes to this map or add a locking mechanism.
exoclient/tx.go (1)
145-147: Simple wrapper method
GetSignedTxBytesDebug delegates directly to getSignedTxBytes, keeping code DRY and readable. No issues found.
Notable changes
Note:when sending multiple transactions on the same height, the execution order of those transactions is not guaranteed.
Summary by CodeRabbit
New Features
Refactor