fix: remove etherscan activity for bsc #579
Conversation
WalkthroughThe changes involve modifications to the activity handling mechanisms across several Ethereum network configurations in the extension. The Binance Smart Chain (BSC) configuration has been simplified to return a static response, while other networks like Arb Nova, Holesky, opBNB, and Palm have been updated to utilize a new Changes
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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
|
|
💼 Build Files |
![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)
packages/extension/src/providers/ethereum/networks/bsc.ts (1)
24-24: Consider adding a code comment explaining the removal of activity trackingTo improve maintainability, consider adding a comment explaining why BSC activity tracking was removed. This will help future maintainers understand the intentional design decision.
- activityHandler: () => Promise.resolve([]), + // BSC activity tracking removed as per PR #579 + activityHandler: () => Promise.resolve([]),
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/extension/src/providers/ethereum/networks/bsc.ts(1 hunks)
🔇 Additional comments (1)
packages/extension/src/providers/ethereum/networks/bsc.ts (1)
24-24: Verify the implications of removing activity tracking for BSC
The change to return an empty array means BSC transaction history won't be available to users. While this aligns with the PR objective, please ensure this is the intended behavior.
Let's verify there are no remaining references to BSC activity tracking:
✅ Verification successful
Removal of BSC activity tracking is consistent with other networks
The change to return an empty array for BSC's activity handler is consistent with the codebase patterns. Several other Ethereum-based networks also use the same empty array pattern (e.g., palm, holesky, arb-nova, op-bnb). This appears to be an intentional design choice for networks where activity tracking is not supported or has been deliberately disabled.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to BSC activity tracking
rg -i "bsc.*activity|activity.*bsc" ./packages/extension/src/
# Check if other network configurations have similar changes
fd -e ts$ . ./packages/extension/src/providers/ethereum/networks/ --exec grep -l "activityHandler"
Length of output: 4540
Script:
#!/bin/bash
# Let's check ethereum network configurations for activity handlers
rg -A 2 "activityHandler" ./packages/extension/src/providers/ethereum/networks/
Length of output: 16076
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/extension/src/providers/ethereum/networks/arb-nova.ts (1)
Line range hint
1-1: Consider updating PR title and description to reflect broader changesThe PR title "fix: remove etherscan activity for bsc" doesn't fully capture the scope of changes:
- Activity handling is being standardized across multiple networks
- Empty activity lists are being implemented for both test and production networks
- This appears to be an architectural change rather than a BSC-specific fix
Consider:
- Updating the PR title to reflect the broader scope
- Adding PR description explaining the architectural decision
- Documenting the impact on user experience (empty activity lists)
packages/extension/src/providers/ethereum/networks/palm.ts (1)
24-24: Consider documenting the rationale for empty activity handlingThe implementation correctly uses
wrapActivityHandlerto return an empty array, which aligns with the PR's objective of removing specific activity handling. However, given the existing comments about Palm API issues, it would be helpful to document why we're choosing to return empty activities.- activityHandler: wrapActivityHandler(() => Promise.resolve([])), + // Return empty activities as Palm's API is currently not supported + activityHandler: wrapActivityHandler(() => Promise.resolve([])),
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/extension/src/providers/ethereum/networks/arb-nova.ts(2 hunks)packages/extension/src/providers/ethereum/networks/bsc.ts(1 hunks)packages/extension/src/providers/ethereum/networks/holesky.ts(2 hunks)packages/extension/src/providers/ethereum/networks/op-bnb.ts(2 hunks)packages/extension/src/providers/ethereum/networks/palm.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/extension/src/providers/ethereum/networks/bsc.ts
🔇 Additional comments (5)
packages/extension/src/providers/ethereum/networks/holesky.ts (1)
4-4: LGTM! Implementation is consistent with other networks
The changes correctly implement the new activity handling pattern using wrapActivityHandler. The empty array response is appropriate for the Holesky test network.
Also applies to: 18-18
packages/extension/src/providers/ethereum/networks/op-bnb.ts (1)
6-6: Verify if empty activity handler is intended for production network
While the implementation is consistent with other networks, please confirm if returning an empty array for activities is the intended behavior for opBNB, which is a production network (isTestNetwork: false).
Let's check if other production networks follow the same pattern:
Also applies to: 24-24
✅ Verification successful
Empty activity handler is consistent with several production networks
The empty activity handler implementation for opBNB is consistent with several other production networks. Looking at the codebase, we can see multiple production networks that also return empty arrays:
- BSC (Binance Smart Chain)
- Klaytn
- Palm
- ZCD
- Bitrock
- Arb Nova
This appears to be an intentional pattern for networks where activity tracking might not be implemented yet or where a different activity tracking mechanism is used.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for activity handler implementations in production network configurations
rg "isTestNetwork:\s*false" -A 10 -B 10 "packages/extension/src/providers/ethereum/networks/*.ts" | rg "activityHandler|isTestNetwork"
Length of output: 232
Script:
#!/bin/bash
# First, let's find all network configuration files
fd ".*\.ts$" packages/extension/src/providers/ethereum/networks/
# Then, let's examine the activity handlers and test network flags in these files
rg "isTestNetwork|activityHandler" packages/extension/src/providers/ethereum/networks/
Length of output: 18600
packages/extension/src/providers/ethereum/networks/arb-nova.ts (1)
6-6: Document the architectural decision to remove activity tracking
The implementation is consistent with other networks, but there's a pattern of removing activity tracking across production networks (Arbitrum Nova, opBNB). Consider:
- Adding a comment explaining why activity tracking is being removed
- Updating relevant documentation
- Ensuring the UI appropriately handles the empty activity lists
Let's check for any related documentation or comments:
Also applies to: 24-24
packages/extension/src/providers/ethereum/networks/palm.ts (2)
5-5: LGTM: Clean import addition
The import statement for wrapActivityHandler is properly placed with other imports.
Line range hint 7-9: Verify the impact of returning empty activities
Given the comments about Palm's API issues and the change to return empty activities, we should verify if this is the intended behavior for Palm network users.
Also applies to: 24-24
✅ Verification successful
Returning empty activities for Palm network is acceptable
The implementation is correct and intentional for the following reasons:
- The Palm network's API endpoints are explicitly documented as non-functional due to DNS resolution issues
- The network is properly configured with a fallback behavior using
wrapActivityHandler(() => Promise.resolve([]))) - The activity state management system (
ActivityStateclass) can handle empty activity arrays gracefully - No UI components directly depend on Palm-specific activity handling
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if Palm network activity was previously used in the UI or other components
# Look for references to Palm network activity in components
rg -l "Palm.*activity" --type ts --type vue
# Check for any existing error handling related to Palm activities
rg -l "Palm.*error" --type ts --type vue
# Look for any tests that might need updating
fd -e test.ts -e spec.ts | xargs rg -l "Palm"
Length of output: 190
Script:
#!/bin/bash
# Let's try a different approach to find Palm network references
# Search for Palm network configuration and usage
rg -l "Palm" --type ts
# Look for activity-related code in the network providers
rg -l "getActivities|fetchActivities" --type ts
# Check for any UI components that might be affected
rg -l "\.(vue|tsx?)$" | xargs rg -l "Palm"
# Look for test files specifically
find . -type f -name "*.test.ts" -o -name "*.spec.ts" | xargs rg -l "Palm"
Length of output: 434
Script:
#!/bin/bash
# Let's examine the content of relevant files to understand the impact
# Check Palm network definition and configuration
cat packages/types/src/networks.ts | grep -A 10 -B 10 "Palm"
# Check activity handling implementation
cat packages/extension/src/libs/activity-state/index.ts
# Examine Palm network provider implementation
cat packages/extension/src/providers/ethereum/networks/palm.ts
# Look for any error handling or UI feedback mechanisms
rg "NetworkError|ActivityError" --type ts
Length of output: 5617
Summary by CodeRabbit
New Features
Bug Fixes