BE-661 | Fix: claimbot panics #597
Conversation
Fixed claimbot panics due nil routerrepo.RouterRepository in txGasCalulator
|
WalkthroughThe changes include updates to the Changes
Possibly related PRs
Suggested labels
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
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
🧹 Outside diff range and nitpick comments (3)
ingest/usecase/plugins/orderbook/claimbot/config.go (1)
34-34: LGTM! Consider adding documentation.The change to inject
msgSimulatorthrough the constructor is a good practice as it makes dependencies explicit and helps prevent nil references. Consider adding documentation to describe the expected behavior of themsgSimulatorparameter.Add a comment above the
msgSimulatorparameter:func NewConfig( keyring keyring.Keyring, orderbookusecase mvc.OrderBookUsecase, poolsUseCase mvc.PoolsUsecase, + // msgSimulator is used to simulate messages before sending them to ensure they will succeed msgSimulator sqstx.MsgSimulator, logger log.Logger, chainGRPCGatewayEndpoint string, chainID string, )Also applies to: 49-49
CHANGELOG.md (1)
40-40: LGTM! Consider following the changelog format consistently.The entry accurately describes the fix. Consider following the established format with a bullet point and issue reference.
-Fixed claimbot panics due nil routerrepo.RouterRepository in txGasCalulator +- Fixed claimbot panics due to nil routerrepo.RouterRepository in txGasCalculatorapp/sidecar_query_server.go (1)
Line range hint
319-324: Good architectural design: Consistent gas calculation across componentsThe changes demonstrate good architectural design by reusing the same
gasCalculatorinstance across both thequoteSimulatorandclaimbot. This ensures consistent gas calculations throughout the system and reduces the likelihood of discrepancies in gas estimations between different components.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)app/sidecar_query_server.go(1 hunks)ingest/usecase/plugins/orderbook/claimbot/config.go(2 hunks)ingest/usecase/plugins/orderbook/claimbot/plugin.go(2 hunks)
🔇 Additional comments (3)
ingest/usecase/plugins/orderbook/claimbot/plugin.go (1)
51-51: LGTM! The changes properly address the nil reference issue.
The addition of the msgSimulator parameter and its proper initialization through the constructor chain helps prevent the nil reference panic in txGasCalculator. The error handling and logging are well implemented.
Also applies to: 56-56
app/sidecar_query_server.go (2)
Line range hint 324-327: LGTM: Proper error handling for plugin initialization
The code properly checks and handles errors from the claimbot initialization. If the initialization fails, the error is immediately returned, preventing the system from continuing with an improperly initialized plugin.
Line range hint 320-324: LGTM: Proper initialization of claimbot with gasCalculator
The changes correctly address the nil reference issue by properly initializing the claimbot with a non-nil gasCalculator. The gasCalculator is initialized earlier in the code using tx.NewMsgSimulator with all required dependencies, ensuring it's properly configured before being passed to the claimbot.
Let's verify the initialization of gasCalculator and its dependencies:
✅ Verification successful
Let me gather more information about the orderbookclaimbot.New function to ensure it properly handles the gasCalculator parameter.
Let me try a different approach to verify the claimbot implementation.
Let me try one final verification approach focusing on the plugin package structure.
Let me check the implementation of the claimbot plugin specifically.
LGTM: Proper initialization and usage of gasCalculator in claimbot
The code shows that:
- The
gasCalculator(MsgSimulator) is properly passed through the constructor and stored in the config - It's correctly used in
sendBatchClaimTxfor transaction simulation and building - The implementation includes proper error handling and sequence number management
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the proper initialization and usage of gasCalculator
# Check if gasCalculator is used consistently across the codebase
rg -A 3 "gasCalculator.*:=.*NewMsgSimulator"
# Verify the claimbot's usage of gasCalculator
ast-grep --pattern 'orderbookclaimbot.New($$$, gasCalculator, $$$)'
Length of output: 1230
Script:
#!/bin/bash
# Check the implementation of orderbookclaimbot.New to verify gasCalculator usage
ast-grep --pattern 'func New($$$, gasCalculator tx.MsgSimulator, $$$) (*OrderbookClaimBot, error) {
$$$
}'
# Check for any potential nil checks on gasCalculator
rg -A 3 "if.*gasCalculator.*nil"
Length of output: 145
Script:
#!/bin/bash
# Find the orderbookclaimbot package and check its implementation
fd "orderbookclaimbot" --type f
# Check for any usage of gasCalculator in the codebase
rg -A 3 "gasCalculator.*tx\.MsgSimulator"
# Look for the OrderbookClaimBot struct definition
ast-grep --pattern 'type OrderbookClaimBot struct {
$$$
}'
Length of output: 138
Script:
#!/bin/bash
# Search for plugin-related files
fd "plugin" --type f
# Look for any references to gasCalculator in plugin-related files
rg -A 3 "gasCalculator" --type go
# Check for the orderbookclaimbot package in a potential plugins directory
fd -e go . | rg -i "claim.*bot"
Length of output: 1865
Script:
#!/bin/bash
# Check the claimbot plugin implementation
cat ingest/usecase/plugins/orderbook/claimbot/plugin.go
# Look for gasCalculator usage in tx.go
cat ingest/usecase/plugins/orderbook/claimbot/tx.go
Length of output: 10116
Fixes claimbot panics due nil routerrepo.RouterRepository in txGasCalulator (cherry picked from commit 8cb0091)
Fixes claimbot panics due nil routerrepo.RouterRepository in txGasCalulator
Summary by CodeRabbit
Release Notes
Bug Fixes
New Features
orderbookclaimbotfunctionality by integrating a gas calculator during initialization.claimbotby incorporating transaction simulation capabilities.Documentation