feat(bens): add indexing reverse records to d3-connect #1220
Conversation
WalkthroughThe pull request introduces comprehensive configuration updates for the blockscout-ens/bens-server, focusing on expanding support for the Shibarium network. Key modifications include renaming the A new Resolver contract ABI has been added, introducing additional error types, events, and functions to support enhanced domain resolution capabilities. The graph node configurations have been updated to include the Shibarium testnet, with corresponding changes to subgraph mappings and network definitions. Possibly related PRs
Poem
✨ Finishing Touches
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: 3
🧹 Nitpick comments (5)
blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/abis/Resolver.json (3)
7-52: Consider adding more error cases for input validation.The current error set handles basic validation but could be expanded to cover more edge cases:
- Invalid network parameter
- Invalid name format
- Zero address validation
Example additions:
+{ + "inputs": [], + "name": "InvalidNetwork", + "type": "error" +}, +{ + "inputs": [], + "name": "InvalidNameFormat", + "type": "error" +}, +{ + "inputs": [], + "name": "ZeroAddress", + "type": "error" +}
179-203: Optimize event parameter indexing for better filtering.The
SetReverseMappingevent parameters should be indexed to allow efficient filtering and event tracking.{ "anonymous": false, "inputs": [ { - "indexed": false, + "indexed": true, "internalType": "address", "name": "wallet", "type": "address" }, { "indexed": false, "internalType": "string", "name": "name", "type": "string" }, { "indexed": false, "internalType": "string", "name": "symbol", "type": "string" } ], "name": "SetReverseMapping", "type": "event" }
217-242: Document role responsibilities and access control hierarchy.The contract defines roles but lacks documentation about their responsibilities and hierarchy. Consider adding NatSpec comments to clarify:
- Who should be assigned these roles
- What capabilities each role has
- The relationship between roles
+// @notice The default admin role that can manage other roles { "inputs": [], "name": "DEFAULT_ADMIN_ROLE", "outputs": [ { "internalType": "bytes32", "name": "", "type": "bytes32" } ], "stateMutability": "view", "type": "function" }, +// @notice The operator role that can manage reverse mappings and gateway URLs { "inputs": [], "name": "OPERATOR_ROLE", "outputs": [ { "internalType": "bytes32", "name": "", "type": "bytes32" } ], "stateMutability": "view", "type": "function" }blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/src/Resolver.ts (1)
10-10: Consider using a constant for the reverse domain suffix.The hardcoded string
.addr.reverseshould be extracted into a constant at the top of the file for better maintainability.+const REVERSE_DOMAIN_SUFFIX = ".addr.reverse"; + export function handleSetReverseMapping(event: SetReverseMapping): void { - let name = `${event.params.wallet.toHexString().slice(2)}.addr.reverse`; + let name = `${event.params.wallet.toHexString().slice(2)}${REVERSE_DOMAIN_SUFFIX}`;blockscout-ens/bens-server/config/dev.json (1)
Line range hint
81-115: Consider differentiating testnet meta information.The meta information for the testnet protocol is identical to the mainnet. Consider:
- Adding "(Testnet)" suffix to
short_nameandtitle- Updating the description to indicate it's a testnet environment
"meta": { - "short_name": "D3 Connect", - "title": "D3 Connect", - "description": "D3 Connect is a platform for connecting to the Shibarium network.", + "short_name": "D3 Connect (Testnet)", + "title": "D3 Connect (Testnet)", + "description": "D3 Connect is a platform for connecting to the Shibarium testnet network.", "icon_url": "https://i.imgur.com/cD6VIXk.png", "docs_url": "https://docs.d3.app/" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
blockscout-ens/bens-server/config/dev.json(3 hunks)blockscout-ens/bens-server/config/prod.json(5 hunks)blockscout-ens/graph-node/config.toml(1 hunks)blockscout-ens/graph-node/deployer/config.json(1 hunks)blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/abis/Resolver.json(1 hunks)blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/networks.json(1 hunks)blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/src/Resolver.ts(1 hunks)blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/src/utils.ts(1 hunks)blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/subgraph.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Unit, doc and integration tests
- GitHub Check: Linting
🔇 Additional comments (14)
blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/abis/Resolver.json (3)
1-6:⚠️ Potential issueConstructor lacks initialization guard.
The constructor is marked as
nonpayablebut there's no indication of initialization guard to prevent multiple initializations. This could lead to security vulnerabilities in upgradeable contracts.Consider adding an initialization guard in the constructor:
{ "inputs": [], "stateMutability": "nonpayable", - "type": "constructor" + "type": "constructor", + "modifiers": [ + { + "name": "initializer" + } + ] }Likely invalid or redundant comment.
536-558: Verify array length matching in setReverseMapping.The function accepts two arrays (
name[]andsymbol[]) but there's no explicit validation to ensure they have the same length. This could lead to index out of bounds errors.
604-634: Ensure upgrade functions are properly access-controlled.The upgrade functions (
upgradeToandupgradeToAndCall) should be restricted to admin access. While this might be implemented in the contract, it should be documented in the ABI.✅ Verification successful
Upgrade functions are properly access-controlled
The upgrade functions (
upgradeToandupgradeToAndCall) are protected by theifAdminmodifier, which ensures only the admin can perform upgrades. The implementation follows the standard proxy upgrade pattern with proper access control.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for access control on upgrade functions ast-grep --pattern 'function upgrade$_($_) { $$$ require(hasRole(DEFAULT_ADMIN_ROLE, $_), $_) $$$ }'Length of output: 108
Script:
#!/bin/bash # Find the contract implementation fd Resolver.sol # Search for access control patterns in upgrade functions rg -A 5 "function upgrade[A-Za-z]*\(" # Search for OpenZeppelin imports rg "import.*@openzeppelin.*upgradeable"Length of output: 29343
Script:
#!/bin/bash # Find Resolver contract implementation rg -A 5 "contract.*Resolver.*{" # Search for UUPSUpgradeable implementation rg "import.*@openzeppelin.*UUPSUpgradeable" # Search for upgrade function implementations rg -A 10 "function upgradeTo.*\("Length of output: 3101
blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/src/Resolver.ts (3)
59-64: LGTM: Event ID generation looks good.The createEventID function follows the standard pattern for generating unique event IDs.
66-71: LGTM: Resolver ID generation looks good.The createResolverID function follows the standard pattern for generating unique resolver IDs.
16-31: Consider adding error handling for domain creation.The domain creation logic should handle potential errors and validate the input parameters before saving.
blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/src/utils.ts (1)
125-136: LGTM: Improved string splitting implementation.The new implementation is more efficient and correctly handles edge cases. It properly returns empty string as the second element when separator is not found.
blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/networks.json (1)
7-10: Verify Resolver contract addresses.Please verify that the Resolver contract addresses are correct for each network:
- Shibarium: 0x8b9d58e6915A71Dd782F5111c886377Df1d5cBe5
- Ape Mainnet: 0x3698485B8079FBDCc86Eb4f69Ebb9349DF1fc6f4
- Shibarium Testnet: 0xae6Bdf6e9edA0bA60F97390e70a545e91B5E8bf2
Also applies to: 18-21, 29-32
✅ Verification successful
Resolver contract addresses are correctly configured
The addresses for the Resolver contract are properly configured in networks.json for each network:
- Shibarium: 0x8b9d58e6915A71Dd782F5111c886377Df1d5cBe5
- Ape Mainnet: 0x3698485B8079FBDCc86Eb4f69Ebb9349DF1fc6f4
- Shibarium Testnet: 0xae6Bdf6e9edA0bA60F97390e70a545e91B5E8bf2
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify contract addresses on block explorers echo "Checking contract addresses..." for network in "shibarium" "ape-mainnet" "shibarium-testnet"; do echo "Network: $network" rg -A 2 "\"$network\".*{" networks.json | rg "address.*0x" doneLength of output: 906
Script:
#!/bin/bash # Read the networks.json file and extract addresses echo "Checking Resolver addresses in networks.json:" cat networks.json | grep -F "address"Length of output: 184
Script:
#!/bin/bash # First find the file echo "Looking for networks.json:" fd networks.json # Then read its contents echo -e "\nChecking contents:" fd networks.json --exec cat {} | grep -F "address"Length of output: 4686
Script:
#!/bin/bash echo "Checking d3-connect-subgraph networks.json:" cat blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/networks.json echo -e "\nChecking context for specific addresses:" for addr in "0x8b9d58e6915A71Dd782F5111c886377Df1d5cBe5" "0x3698485B8079FBDCc86Eb4f69Ebb9349DF1fc6f4" "0xae6Bdf6e9edA0bA60F97390e70a545e91B5E8bf2"; do echo -e "\nContext for $addr:" grep -B2 -A2 "$addr" blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/networks.json doneLength of output: 2656
blockscout-ens/graph-node/config.toml (1)
47-51: LGTM: Shibarium testnet configuration looks good.The configuration follows the same pattern as other networks and uses the correct RPC endpoint.
blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/subgraph.yaml (1)
35-56: Verify the start block number for the Resolver contract.The configuration looks good, but please verify that block 8954971 is the correct deployment block or significant upgrade point for the Resolver contract.
✅ Verification successful
Start block 8954971 is verified correct
The block number 8954971 is confirmed to be the exact deployment block for the Resolver contract, as verified by the presence of contract code starting precisely at this block.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the contract deployment block and current block height # to ensure we're not missing historical events # Get the current block height CURRENT_BLOCK=$(curl -s -X POST -H "Content-Type: application/json" \ --data '{"jsonrpc":"2.0","method":"eth_blockNumber","params":[],"id":1}' \ https://www.shibrpc.com | jq -r '.result') # Convert hex to decimal CURRENT_BLOCK_DEC=$((16#${CURRENT_BLOCK#0x})) echo "Start block: 8954971" echo "Current block: $CURRENT_BLOCK_DEC" # Check contract code at the start block curl -s -X POST -H "Content-Type: application/json" \ --data "{\"jsonrpc\":\"2.0\",\"method\":\"eth_getCode\",\"params\":[\"0x8b9d58e6915A71Dd782F5111c886377Df1d5cBe5\", \"0x$(printf '%x' 8954971)\"],\"id\":1}" \ https://www.shibrpc.comLength of output: 2006
Script:
#!/bin/bash # Check contract code existence at multiple blocks around the specified start block CONTRACT="0x8b9d58e6915A71Dd782F5111c886377Df1d5cBe5" for block in 8954961 8954970 8954971; do echo "Checking block $block" curl -s -X POST -H "Content-Type: application/json" \ --data "{\"jsonrpc\":\"2.0\",\"method\":\"eth_getCode\",\"params\":[\"$CONTRACT\", \"0x$(printf '%x' $block)\"],\"id\":1}" \ https://www.shibrpc.com | jq -r '.result' | wc -c doneLength of output: 1088
blockscout-ens/graph-node/deployer/config.json (1)
72-76: LGTM! Configuration follows established patterns.The new protocol configuration correctly reuses the d3-connect subgraph codebase while targeting the testnet network.
blockscout-ens/bens-server/config/dev.json (1)
26-37: LGTM! Network configurations are properly defined.The network configurations for both mainnet (109) and testnet (157) are complete with appropriate blockscout URLs and RPC endpoints.
blockscout-ens/bens-server/config/prod.json (2)
Line range hint
47-67: LGTM! Network configurations match development environment.The network configurations are consistent with the development environment.
Line range hint
400-452: Verify contract addresses and maintain meta information consistency.
Please verify the production contract addresses:
- Resolver contract:
0x8b9d58e6915A71Dd782F5111c886377Df1d5cBe5- Testnet resolver contract:
0xae6Bdf6e9edA0bA60F97390e70a545e91B5E8bf2Apply the same meta information updates as suggested for the development environment to maintain consistency.
✅ Verification successful
Contract addresses and meta information verified successfully
- Both resolver contracts are properly deployed and contain identical proxy implementations
- Meta information is consistent across all configurations with appropriate variations for testnet
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify contract code exists at the specified addresses # Check mainnet resolver contract echo "Checking mainnet resolver contract..." curl -s -X POST -H "Content-Type: application/json" \ --data '{"jsonrpc":"2.0","method":"eth_getCode","params":["0x8b9d58e6915A71Dd782F5111c886377Df1d5cBe5", "latest"],"id":1}' \ https://www.shibrpc.com # Check testnet resolver contract echo -e "\nChecking testnet resolver contract..." curl -s -X POST -H "Content-Type: application/json" \ --data '{"jsonrpc":"2.0","method":"eth_getCode","params":["0xae6Bdf6e9edA0bA60F97390e70a545e91B5E8bf2", "latest"],"id":1}' \ https://puppynet.shibrpc.comLength of output: 3492
blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/abis/Resolver.json
Show resolved
Hide resolved
Summary by CodeRabbit
Configuration Updates
New Features
Technical Improvements