feat(bens): index reverse mapping changes from d3 connect protocol #1189
Conversation
|
Warning Rate limit exceeded@renatoalencar has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 45 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 (6)
WalkthroughThis pull request introduces significant configuration updates for the Shibarium network's blockchain infrastructure within the blockscout-ens repository. Key changes include the modification of the subgraphs reader configuration to transition the protocol from "d3_connect" to "d3_connect_shib" and the addition of a new protocol "d3_connect_shib_testnet". A new chain configuration for the Shibarium testnet has been established, detailing provider settings and network specifications. Furthermore, a new Resolver contract has been introduced to enhance domain resolution through reverse mapping functionality. The overall structure of the configuration files has been expanded to accommodate these new protocols and networks. 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 (
|
renatoalencar
force-pushed
the
renatoalencar/d3-connect-reverse-mapping-index
branch
from
1f3d105 to
3c12699
Compare
| resolvedDomain.resolver = resolverId; | ||
|
|
||
| resolvedDomain.subdomainCount = 0; | ||
| resolvedDomain.isMigrated = false; |
There was a problem hiding this comment.
| resolvedDomain.isMigrated = false; | |
| resolvedDomain.isMigrated = true; |
| domain.resolver = resolverId | ||
|
|
||
| domain.subdomainCount = 0; | ||
| domain.isMigrated = false; |
There was a problem hiding this comment.
| domain.isMigrated = false; | |
| domain.isMigrated = true; |
There was a problem hiding this comment.
this is field for ENS legacy, only old domains have isMigrated=false
| "address": "0x1443bC2bBAB07437BCF9C577b647523A736bB33E", | ||
| "startBlock": 1927346 | ||
| }, | ||
| "Resolver": { |
There was a problem hiding this comment.
To deploy this subgraph on shibarium-mainnet, we need to specify Resolver for mainnet also
There was a problem hiding this comment.
Once the new mainnet resolvers are deployed I'll update them.
renatoalencar
force-pushed
the
renatoalencar/d3-connect-reverse-mapping-index
branch
2 times, most recently
from
42bb7a6 to
f738e6d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/src/Resolver.ts (2)
9-31: Consider validating wallet address format.The function assumes the wallet address is valid and can be converted to a hex string. Consider adding validation to handle potential invalid addresses.
export function handleSetReverseMapping(event: SetReverseMapping): void { + if (!event.params.wallet) { + log.warning('Invalid wallet address in SetReverseMapping event', []); + return; + } let name = `${event.params.wallet.toHexString().slice(2)}.addr.reverse`;
51-57: Consider adding timestamp to NameChanged event.The event entity should include a timestamp for better tracking and querying capabilities.
let resolverEvent = new NameChanged(createEventID(event)); resolverEvent.resolver = resolverId resolverEvent.blockNumber = event.block.number.toI32(); resolverEvent.transactionID = event.transaction.hash; resolverEvent.name = event.params.name; + resolverEvent.timestamp = event.block.timestamp; resolverEvent.save();blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/abis/Resolver.json (1)
1-648: Consider documenting the contract interface.The ABI is comprehensive but lacks documentation. Consider adding NatSpec documentation to explain:
- The purpose of each function
- The role requirements for restricted functions
- The expected format of input arrays for setReverseMapping
- The relationship between resolve and reverseResolve functions
Would you like me to generate NatSpec documentation for the contract interface?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
blockscout-ens/bens-server/config/dev.json(3 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)
🧰 Additional context used
🪛 Biome (1.9.4)
blockscout-ens/graph-node/deployer/config.json
[error] 72-72: Expected a property but instead found '}'.
Expected a property here.
(parse)
🪛 Gitleaks (8.21.2)
blockscout-ens/bens-server/config/dev.json
106-106: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (9)
blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/src/Resolver.ts (1)
7-7: Verify the hardcodedADDR_REVERSE_NODEvalue.Please ensure this hash value matches the deployed contract's configuration for reverse node address.
✅ Verification successful
The
ADDR_REVERSE_NODEvalue is correctThe hash value
0x91d1777781884d03a6757a803996e38de2a42967fb37eeaca72729271025a9e2is verified to be the correct namehash of "addr.reverse" domain, as confirmed by multiple independent implementations and documentation in the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any other occurrences or definitions of this hash rg -i "91d1777781884d03a6757a803996e38de2a42967fb37eeaca72729271025a9e2"Length of output: 68038
blockscout-ens/graph-node/config.toml (1)
47-51: Verify RPC endpoint availability and rate limits.Please ensure that the RPC endpoint
https://puppynet.shibrpc.comis:
- Publicly accessible
- Has sufficient rate limits for graph-node indexing
- Provides archive node capabilities if historical data indexing is required
✅ Verification successful
RPC endpoint is accessible and provides archive node capabilities ✅
The endpoint
https://puppynet.shibrpc.com:
- Responds to JSON-RPC requests correctly
- Provides historical block data (archive node functionality confirmed)
- Manual verification of rate limits during indexing is recommended
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Test RPC endpoint availability and basic functionality curl -X POST -H "Content-Type: application/json" \ --data '{"jsonrpc":"2.0","method":"eth_blockNumber","params":[],"id":1}' \ https://puppynet.shibrpc.comLength of output: 514
Script:
#!/bin/bash # Test archive node capabilities by querying an early block curl -X POST -H "Content-Type: application/json" \ --data '{"jsonrpc":"2.0","method":"eth_getBlockByNumber","params":["0x1", false],"id":1}' \ https://puppynet.shibrpc.comLength of output: 2111
blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/subgraph.yaml (1)
35-56: LGTM! The Resolver data source configuration is well-structured.The configuration correctly sets up indexing for the
SetReverseMappingevent with appropriate entities and mappings.Let's verify the contract address deployment:
✅ Verification successful
Resolver contract implementation verified successfully
The contract address, ABI definition, and event handling implementation are all properly configured and consistent. The
SetReverseMappingevent is correctly defined in the ABI and the handler implementation properly processes the event data.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the Resolver contract address exists on the network # and check if there are any other contracts at this address # Search for any references to the Resolver contract address rg -i "0x8b9d58e6915A71Dd782F5111c886377Df1d5cBe5"Length of output: 313
Script:
#!/bin/bash # Check the event signature in Resolver ABI and handler implementation cat blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/abis/Resolver.json echo "---" cat blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/src/Resolver.tsLength of output: 14735
blockscout-ens/graph-node/deployer/config.json (1)
73-76: LGTM! The testnet protocol configuration is properly structured.The new protocol "d3-connect-shib-testnet" is correctly configured with appropriate subgraph path and network settings.
blockscout-ens/bens-server/config/dev.json (3)
26-26: LGTM! Protocol name updated correctly.The protocol name change from "d3_connect" to "d3_connect_shib" aligns with the new naming convention.
30-37: LGTM! Testnet network configuration is properly structured.The new network 157 configuration is correctly set up with appropriate blockscout URL and RPC endpoint.
Line range hint
81-98: Verify resolver contract addresses across environments.The resolver contract addresses differ between mainnet and testnet:
- Mainnet: 0xD60D40674E678F0089736D6381071973a75B4B6f
- Testnet: 0xae6Bdf6e9edA0bA60F97390e70a545e91B5E8bf2
Please ensure these are the correct contract addresses for each environment.
Also applies to: 99-115
✅ Verification successful
Resolver contract addresses are correctly configured ✅
The resolver contract addresses are properly configured for each network:
- Shibarium mainnet (network 109):
0xD60D40674E678F0089736D6381071973a75B4B6f- Shibarium testnet (network 157):
0xae6Bdf6e9edA0bA60F97390e70a545e91B5E8bf2- APE network (network 33139):
0xeA54b0E40E956383C95e28cb407FF35235Aa31fE🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify resolver contract addresses across configurations # Search for any references to the resolver contract addresses to ensure consistency echo "Checking mainnet resolver contract references:" rg -i "0xD60D40674E678F0089736D6381071973a75B4B6f" echo "Checking testnet resolver contract references:" rg -i "0xae6Bdf6e9edA0bA60F97390e70a545e91B5E8bf2"Length of output: 799
Script:
#!/bin/bash # Search for all resolver contract configurations and environment config files echo "Checking all resolver contract configurations:" rg -i "resolver_contract" echo -e "\nChecking for environment config files:" fd -e json -p "config/"Length of output: 1668
Script:
#!/bin/bash # Get full context of resolver configurations in environment files echo "Checking resolver configurations in dev.json:" rg -A 10 -B 10 "resolver_contract" blockscout-ens/bens-server/config/dev.json echo -e "\nChecking resolver configurations in prod.json:" rg -A 10 -B 10 "resolver_contract" blockscout-ens/bens-server/config/prod.json echo -e "\nChecking resolver configurations in staging.json:" rg -A 10 -B 10 "resolver_contract" blockscout-ens/bens-server/config/staging.jsonLength of output: 3463
🧰 Tools
🪛 Gitleaks (8.21.2)
88-88: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/abis/Resolver.json (2)
179-203: LGTM! The SetReverseMapping event is properly defined.The event includes all necessary parameters for reverse mapping:
- wallet (address)
- name (string)
- symbol (string)
536-558: Verify array parameters in setReverseMapping function.The
setReverseMappingfunction accepts arrays (string[]) for both name and symbol parameters, but the event emits single strings. This might indicate a potential mismatch in the implementation.
| let resolvedDomainNode = hashByName(event.params.name); | ||
| let resolvedDomain = Domain.load(resolvedDomainNode.toHexString()); | ||
| if (resolvedDomain == null) { | ||
| resolvedDomain = new Domain(resolvedDomainNode.toHexString()); | ||
| resolvedDomain.createdAt = event.block.timestamp; | ||
| resolvedDomain.name = event.params.name; | ||
|
|
||
| resolvedDomain.subdomainCount = 0; | ||
| resolvedDomain.isMigrated = true; | ||
| resolvedDomain.owner = event.params.wallet.toHexString(); | ||
| resolvedDomain.storedOffchain = false; | ||
| resolvedDomain.resolvedWithWildcard = false; | ||
| } | ||
|
|
||
| resolvedDomain.resolver = resolverId; | ||
| resolvedDomain.resolvedAddress = event.params.wallet.toHexString(); | ||
| resolvedDomain.save(); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for resolved domain name.
The function should validate event.params.name to ensure it's a valid domain name before processing.
+ if (!event.params.name || event.params.name.trim().length === 0) {
+ log.warning('Empty or invalid domain name in SetReverseMapping event', []);
+ return;
+ }
let resolvedDomainNode = hashByName(event.params.name);Committable suggestion skipped: line range outside the PR's diff.
| function splitStringOnce(input: string, separator: string): string[] { | ||
| const separatorIndex = input.indexOf(separator) | ||
|
|
||
| if (separatorIndex === -1) { | ||
| return [input, '']; | ||
| } | ||
|
|
||
| return [ | ||
| input.slice(0, separatorIndex), | ||
| input.slice(separatorIndex + separator.length) | ||
| ] | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding input validation for empty separator.
The function should handle the case when the separator is an empty string to prevent potential issues.
function splitStringOnce(input: string, separator: string): string[] {
+ if (!separator) {
+ return [input, ''];
+ }
const separatorIndex = input.indexOf(separator)
if (separatorIndex === -1) {
return [input, ''];
}
return [
input.slice(0, separatorIndex),
input.slice(separatorIndex + separator.length)
]
}📝 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.
| function splitStringOnce(input: string, separator: string): string[] { | |
| const separatorIndex = input.indexOf(separator) | |
| if (separatorIndex === -1) { | |
| return [input, '']; | |
| } | |
| return [ | |
| input.slice(0, separatorIndex), | |
| input.slice(separatorIndex + separator.length) | |
| ] | |
| } | |
| function splitStringOnce(input: string, separator: string): string[] { | |
| if (!separator) { | |
| return [input, '']; | |
| } | |
| const separatorIndex = input.indexOf(separator) | |
| if (separatorIndex === -1) { | |
| return [input, '']; | |
| } | |
| return [ | |
| input.slice(0, separatorIndex), | |
| input.slice(separatorIndex + separator.length) | |
| ] | |
| } |
| "startBlock": 1927346 | ||
| }, | ||
| "Resolver": { | ||
| "address": "0xae6Bdf6e9edA0bA60F97390e70a545e91B5E8bf2.", |
There was a problem hiding this comment.
Fix invalid address format.
The Resolver address for shibarium-testnet contains an extra period at the end which makes it invalid.
- "address": "0xae6Bdf6e9edA0bA60F97390e70a545e91B5E8bf2.",
+ "address": "0xae6Bdf6e9edA0bA60F97390e70a545e91B5E8bf2"📝 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.
| "address": "0xae6Bdf6e9edA0bA60F97390e70a545e91B5E8bf2.", | |
| "address": "0xae6Bdf6e9edA0bA60F97390e70a545e91B5E8bf2" |
| "network": "ape-mainnet", | ||
| }, |
There was a problem hiding this comment.
Fix JSON syntax error.
Remove the trailing comma after "network": "ape-mainnet" as it's causing a parsing error.
Apply this diff:
- "network": "ape-mainnet",
+ "network": "ape-mainnet"📝 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.
| "network": "ape-mainnet", | |
| }, | |
| "network": "ape-mainnet" | |
| }, |
🧰 Tools
🪛 Biome (1.9.4)
[error] 72-72: Expected a property but instead found '}'.
Expected a property here.
(parse)
renatoalencar
force-pushed
the
renatoalencar/d3-connect-reverse-mapping-index
branch
from
a6d68e6 to
7935973
Compare
renatoalencar
force-pushed
the
renatoalencar/d3-connect-reverse-mapping-index
branch
from
7935973 to
a1f878e
Compare
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/networks.json (1)
Contract verification results require attention
- ✅ Shibarium mainnet: Both Registry and Resolver contracts are deployed and operational
- ❌ Shibarium testnet: Invalid Resolver address format (contains trailing period: "0xae6Bdf6e9edA0bA60F97390e70a545e91B5E8bf2.")
- ❌ No contract bytecode found for either contract on Shibarium testnet and Ape mainnet networks
🔗 Analysis chain
Line range hint
1-34: Verify contract addresses on-chain.Please verify that all contract addresses are correctly deployed and operational on their respective networks.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify contract addresses exist on their respective networks # Function to check contract code existence check_contract() { local network=$1 local name=$2 local address=$3 echo "Checking $name contract at $address on $network..." # Use eth_getCode RPC call to verify contract existence curl -s -X POST -H "Content-Type: application/json" --data "{ \"jsonrpc\":\"2.0\", \"method\":\"eth_getCode\", \"params\":[\"$address\", \"latest\"], \"id\":1 }" "$network" | jq -r '.result' } # Check Shibarium contracts check_contract "https://rpc.shibrpc.com" "Registry" "0xDe74799371Ceac11A0F52BA2694392A391D0dA18" check_contract "https://rpc.shibrpc.com" "Resolver" "0x8b9d58e6915A71Dd782F5111c886377Df1d5cBe5" # Check Shibarium Testnet contracts check_contract "https://rpc-shibuya.dog" "Registry" "0x1443bC2bBAB07437BCF9C577b647523A736bB33E" check_contract "https://rpc-shibuya.dog" "Resolver" "0xae6Bdf6e9edA0bA60F97390e70a545e91B5E8bf2" # Check Ape-mainnet contracts check_contract "https://mainnet.apexlayer.xyz" "Registry" "0x0D435A6c16045Abeaf6A442Bf162fd52597B4Ed3" check_contract "https://mainnet.apexlayer.xyz" "Resolver" "0x3698485B8079FBDCc86Eb4f69Ebb9349DF1fc6f4"Length of output: 6911
🧹 Nitpick comments (1)
blockscout-ens/bens-server/config/dev.json (1)
99-115: Consider adding environment indicators to prevent confusion.The meta information (title, description) is identical between mainnet and testnet configurations. Consider adding "(Testnet)" suffix to distinguish them, similar to how the ENS configuration does it.
Apply this diff to improve clarity:
"meta": { - "short_name": "D3 Connect", - "title": "D3 Connect", + "short_name": "D3 Connect (Testnet)", + "title": "D3 Connect (Testnet)", "description": "D3 Connect is a platform for connecting to the Shibarium network.",🧰 Tools
🪛 Gitleaks (8.21.2)
106-106: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🛑 Comments failed to post (2)
blockscout-ens/graph-node/subgraphs/d3-connect-subgraph/src/Resolver.ts (2)
26-26:
⚠️ Potential issueVerify ownership assignment.
Using
transaction.fromas the owner might not be secure as it could be different from the actual wallet address. Consider using the wallet parameter from the event instead.- domain.owner = event.transaction.from.toHexString(); + domain.owner = event.params.wallet.toHexString();📝 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.domain.owner = event.params.wallet.toHexString();
47-49:
⚠️ Potential issueVerify domain ownership before updates.
The function updates the resolver and resolved address without verifying if the caller has permission to modify the domain.
+ // Only allow updates if the domain is owned by the wallet + if (resolvedDomain.owner != event.params.wallet.toHexString()) { + log.warning('Unauthorized attempt to update domain resolution', []); + return; + } resolvedDomain.resolver = resolverId; resolvedDomain.resolvedAddress = event.params.wallet.toHexString();Committable suggestion skipped: line range outside the PR's diff.
|
will be merged in #1220 |
This adds support for indexing the new
SetReverseMappingevent from the D3 Connect Resolver contract. With also a few things included:splitStringOnce(which only worked for names with at maximum two labels)A few things are still unresolved:
Summary by CodeRabbit
New Features
Configuration Updates
Technical Improvements