-
Notifications
You must be signed in to change notification settings - Fork 83
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Single Request Proxy
deployment to mainnets
#1486
Conversation
WalkthroughThis pull request introduces a new TypeScript deployment script for the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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 (
|
5489935
to
8ced522
Compare
Single Request Proxy
deployment to mainnets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Outside diff range comments (1)
packages/smart-contracts/hardhat.config.ts (1)
Line range hint
28-36
: Consider improving address management for maintainability.While the function correctly handles the Base network case, consider moving the hardcoded addresses to a configuration file or environment variables for better maintainability. This would make it easier to update addresses for other networks in the future if similar issues arise.
Consider this approach:
- function liveDeployerAddress(): string { - switch (process.env.NETWORK?.toLowerCase()) { - case `base`: - return `0xe7E02e5e94d668C5630959e4791B1977f3b74fcC`; - default: - return LIVE_DEPLOYER_ADDRESS; - } - } + const NETWORK_DEPLOYER_ADDRESSES: Record<string, string> = { + base: '0xe7E02e5e94d668C5630959e4791B1977f3b74fcC', + }; + + function liveDeployerAddress(): string { + const network = process.env.NETWORK?.toLowerCase(); + return NETWORK_DEPLOYER_ADDRESSES[network] || LIVE_DEPLOYER_ADDRESS; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (3)
packages/smart-contracts/deploy/deploy-zk-single-request-proxy.ts
(1 hunks)packages/smart-contracts/hardhat.config.ts
(2 hunks)packages/smart-contracts/src/lib/artifacts/SingleRequestProxyFactory/index.ts
(1 hunks)
🔇 Additional comments (6)
packages/smart-contracts/deploy/deploy-zk-single-request-proxy.ts (1)
1-24
: 🧹 Nitpick (assertive)
Verify network configurations for all target mainnets.
The PR objectives mention deployment to multiple mainnets (Sepolia, Ethereum, Optimism, etc.). Let's verify the network configurations and fee proxy addresses for all target networks.
Consider creating a network configuration file that maps network identifiers consistently across the codebase to avoid hardcoding values.
✅ Verification successful
Network configurations are properly set up for zkSync deployment
Based on the verification results:
- The hardhat.config.ts includes zkSync network configurations for both testnet and mainnet
- The fee proxy addresses are correctly configured for zkSyncEra:
- EthereumFeeProxy:
0xE9A708db0D30409e39810C44cA240fd15cdA9b1a
- ERC20FeeProxy:
0x6e28Cc56C2E64c9250f39Cb134686C87dB196532
- EthereumFeeProxy:
- The SingleRequestProxyFactory is already deployed on zkSyncEra at
0xC52F0f36b4f0Cdf55D4349640C32929c5E85Ab1a
The deployment script correctly uses the zkSync-specific fee proxy addresses and follows the network's deployment patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify network configurations and fee proxy addresses for target networks
# Test: Check if network configurations exist for all target networks
echo "Checking network configurations..."
networks=("sepolia" "mainnet" "optimism" "arbitrum" "base" "zksyncera" "gnosis" "polygon" "bnb" "avalanche" "celo" "mantle")
for network in "${networks[@]}"; do
echo "Checking configuration for $network..."
# Check if network exists in hardhat config
rg -A 5 "networks.*${network}.*{" packages/smart-contracts/hardhat.config.ts
# Check if fee proxy addresses are configured
rg "getAddress.*${network}" packages/smart-contracts/src/lib/artifacts
done
Length of output: 4597
Script:
#!/bin/bash
# Let's try a different approach to check network configurations
# Check hardhat config for network configurations
echo "Checking hardhat.config.ts for network configurations..."
rg "networks:\s*{" -A 50 packages/smart-contracts/hardhat.config.ts
# Check for network-specific addresses in artifacts
echo -e "\nChecking fee proxy addresses in artifacts..."
rg "getAddress" packages/smart-contracts/src/lib/artifacts/
# Check deploy scripts for network handling
echo -e "\nChecking deploy scripts for network handling..."
fd "deploy.*\.ts$" packages/smart-contracts/deploy/ --exec cat {}
Length of output: 2916
Script:
#!/bin/bash
# Let's check the network configuration and types specifically
# Check currency types definition
echo "Checking currency types definition..."
rg "EvmChainName" packages/smart-contracts/
# Check hardhat config with simpler pattern
echo -e "\nChecking hardhat network config..."
cat packages/smart-contracts/hardhat.config.ts
# Check artifact addresses
echo -e "\nChecking artifact addresses..."
fd -e ts -e js . packages/smart-contracts/src/lib/artifacts/ --exec cat {}
Length of output: 75585
packages/smart-contracts/src/lib/artifacts/SingleRequestProxyFactory/index.ts (3)
20-63
: Network coverage aligns with deployment objectives.
All networks specified in the PR objectives have been properly configured with their respective deployment details.
20-63
: Block numbers are within expected ranges.
The creation block numbers appear reasonable for each network, taking into account their different block times and chain histories.
28-51
: Verify the repeated contract addresses across networks.
Several networks share the same contract address 0x4D417AA04DBb207201a794E5B7381B3cde815281
(Polygon, Avalanche, Arbitrum One, Gnosis, and BNB). While this is possible with deterministic deployment methods like CREATE2, please verify that this is intentional and not a copy-paste error.
✅ Verification successful
Let me gather more information about the CREATE2 deployment setup to verify if this pattern is intentional.
Based on the gathered information, I can now provide a final response:
Identical contract addresses across networks are intentional and verified
The identical contract addresses across networks are a result of deterministic deployment using CREATE2 through the RequestDeployer contract. This is confirmed by:
- The project uses xdeployer (based on CREATE2) for cross-chain deployments
- A dedicated RequestDeployer contract is used at a fixed address (
0xE99Ab70a5FAE59551544FA326fA048f7B95A24B2
) - The deployment process is well-documented and includes specific scripts for computing addresses beforehand
- SingleRequestProxyFactory is explicitly listed in
create2ContractDeploymentList
for deterministic deployment
The shared address 0x4D417AA04DBb207201a794E5B7381B3cde815281
across multiple networks is the expected behavior of the system's design, not a copy-paste error.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for deployment scripts or tests that might confirm the intentional use of identical addresses
# Test 1: Look for CREATE2 usage in deployment scripts
echo "Searching for CREATE2 deployment patterns..."
rg -l "CREATE2|create2" --type ts
# Test 2: Search for these specific addresses in test files
echo "Searching for address usage in tests..."
rg "0x4D417AA04DBb207201a794E5B7381B3cde815281" --type ts
Length of output: 1439
Script:
#!/bin/bash
# Check the CREATE2 deployment configuration and salt values
echo "Checking CREATE2 deployment scripts..."
cat packages/smart-contracts/scripts-create2/deploy.ts
# Look for any configuration or constants related to deployment
echo -e "\nChecking deployment utilities..."
cat packages/smart-contracts/scripts-create2/utils.ts
# Check if there's any documentation about the deployment strategy
echo -e "\nChecking for deployment documentation..."
cat packages/smart-contracts/README.md 2>/dev/null || echo "README not found"
Length of output: 17050
packages/smart-contracts/hardhat.config.ts (2)
280-287
: LGTM! Verify Celo network configuration.
The Celo custom chain configuration is correctly implemented with the appropriate chainId and URLs.
#!/bin/bash
# Description: Verify Celo network configuration consistency across the codebase
# Test 1: Check if the chainId is consistently used
echo "Checking chainId consistency..."
rg -A 2 "chainId.*42220" || echo "Warning: Celo chainId might be inconsistent"
# Test 2: Check if the Celo URLs are referenced elsewhere
echo "Checking URL references..."
rg -l "celoscan.io" || echo "Warning: Celo URLs might need to be updated in other files"
Line range hint 1-424
: Verify configuration coverage for all target networks.
The PR aims to deploy to multiple mainnets. While the changes for Celo and Base look good, let's verify the configuration for all target networks mentioned in the PR objectives.
#!/bin/bash
# Description: Verify configuration coverage for all target networks
# List of networks from PR objectives
NETWORKS=(
"sepolia"
"mainnet"
"optimism"
"arbitrum-one"
"base"
"zksyncera"
"xdai"
"polygon"
"bsc"
"avalanche"
"celo"
"mantle"
)
echo "Checking network configurations..."
for network in "${NETWORKS[@]}"; do
echo "Checking $network..."
# Check network configuration
rg "^\s*$network:\s*{" || echo "Warning: Missing configuration for $network"
# Check API key configuration
rg "^\s*$network:.*API_KEY" || echo "Warning: Missing API key configuration for $network"
done
packages/smart-contracts/deploy/deploy-zk-single-request-proxy.ts
Outdated
Show resolved
Hide resolved
packages/smart-contracts/deploy/deploy-zk-single-request-proxy.ts
Outdated
Show resolved
Hide resolved
packages/smart-contracts/deploy/deploy-zk-single-request-proxy.ts
Outdated
Show resolved
Hide resolved
packages/smart-contracts/deploy/deploy-zk-single-request-proxy.ts
Outdated
Show resolved
Hide resolved
packages/smart-contracts/deploy/deploy-zk-single-request-proxy.ts
Outdated
Show resolved
Hide resolved
packages/smart-contracts/deploy/deploy-zk-single-request-proxy.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
packages/smart-contracts/deploy/deploy-zk-single-request-proxy.ts
Outdated
Show resolved
Hide resolved
packages/smart-contracts/deploy/deploy-zk-single-request-proxy.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (2)
packages/smart-contracts/deploy/deploy-zk-single-request-proxy.ts
(1 hunks)packages/smart-contracts/src/lib/artifacts/SingleRequestProxyFactory/index.ts
(1 hunks)
🧰 Additional context used
📓 Learnings (1)
packages/smart-contracts/deploy/deploy-zk-single-request-proxy.ts (3)
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1486
File: packages/smart-contracts/deploy/deploy-zk-single-request-proxy.ts:1-24
Timestamp: 2024-11-11T16:10:26.692Z
Learning: The team prefers to maintain individual deployment scripts for each network in `packages/smart-contracts/deploy`, rather than consolidating them into a unified script.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1486
File: packages/smart-contracts/deploy/deploy-zk-single-request-proxy.ts:19-23
Timestamp: 2024-11-11T16:19:32.522Z
Learning: In our deployment scripts, logging information is handled by another task, so we don't need to include logging within the scripts.
Learnt from: MantisClone
PR: RequestNetwork/requestNetwork#1474
File: packages/smart-contracts/scripts/test-deploy-single-request-proxy.ts:29-29
Timestamp: 2024-10-28T20:02:48.835Z
Learning: In test deployment scripts (e.g., files like 'test-deploy-*.ts' in 'packages/smart-contracts/scripts'), it's acceptable to use `console.log` for logging.
🔇 Additional comments (4)
packages/smart-contracts/src/lib/artifacts/SingleRequestProxyFactory/index.ts (4)
20-63
: Verify network deployments match PR objectives
The deployment configurations align with the PR objectives to deploy to Sepolia, Ethereum (mainnet), Optimism, Arbitrum One, Base, ZkSync Era, Gnosis (xdai), Polygon (matic), BNB (bsc), Avalanche, Celo, and Mantle networks.
Let's verify the deployment transactions on each network:
#!/bin/bash
# Description: Verify contract deployments across networks using block numbers
# Function to check deployment transaction
check_deployment() {
local network=$1
local address=$2
local block=$3
echo "Checking $network deployment at block $block for address $address"
# Use different RPC endpoints based on network
case $network in
"mainnet") ENDPOINT="https://eth-mainnet.public.blastapi.io";;
"optimism") ENDPOINT="https://optimism-mainnet.public.blastapi.io";;
# Add other networks as needed
esac
# Query the block for contract creation
curl -s -X POST -H "Content-Type: application/json" --data "{
\"jsonrpc\":\"2.0\",
\"method\":\"eth_getBlockByNumber\",
\"params\":[\"0x$(printf '%x' $block)\", true],
\"id\":1
}" "$ENDPOINT"
}
# Check a few key networks
check_deployment "mainnet" "0xD5933C74414ce80D9d7082cc89FBAdcfF4751fAF" 21145968
check_deployment "optimism" "0xf8cACE7EE4c03Eb4f225434B0709527938D365b4" 127750366
20-63
: Verify creation block numbers across networks
Each network configuration includes specific block numbers for contract creation. These should be verified to ensure they match the actual deployment transactions and can be used as reliable reference points for contract verification.
#!/bin/bash
# Description: Verify block timestamps to ensure they're within expected deployment timeframe
# Function to check block timestamp
check_block_timestamp() {
local network=$1
local block=$2
echo "Checking $network block $block timestamp"
curl -s -X POST -H "Content-Type: application/json" --data "{
\"jsonrpc\":\"2.0\",
\"method\":\"eth_getBlockByNumber\",
\"params\":[\"0x$(printf '%x' $block)\", false],
\"id\":1
}" "https://$network.public.blastapi.io"
}
# Check a sample of networks
check_block_timestamp "eth-mainnet" 21145968
check_block_timestamp "optimism-mainnet" 127750366
check_block_timestamp "bsc-mainnet" 43839939
20-23
: Verify zkSync Era deployment configuration
The zkSync Era deployment uses a unique address, which is expected due to its different deployment mechanism. However, let's verify the deployment.
✅ Verification successful
Let me generate additional verification scripts to check the contract deployment and configuration.
Let me generate one final verification script to check the contract deployment block number.
Based on the verification results, I can now provide a final response:
✓ zkSync Era deployment configuration is valid
The SingleRequestProxyFactory contract deployment on zkSync Era at 0x9Fd503e723e5EfcCde3183632b443fFF49E68715
is properly configured in the codebase. The contract implementation matches the expected functionality based on the source code review, including:
- Factory pattern for creating proxy contracts
- Owner-controlled proxy address updates
- Event emissions for tracking proxy creation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify zkSync Era deployment
# Query zkSync Era API for contract deployment
curl -s -X POST -H "Content-Type: application/json" --data '{
"jsonrpc":"2.0",
"method":"eth_getCode",
"params":["0x9Fd503e723e5EfcCde3183632b443fFF49E68715", "latest"],
"id":1
}' https://mainnet.era.zksync.io
Length of output: 25956
Script:
#!/bin/bash
# Check for any references to the zkSync Era contract address in the codebase
rg -i "0x9Fd503e723e5EfcCde3183632b443fFF49E68715" -A 2 -B 2
# Look for any SingleRequestProxyFactory related configurations
fd -t f "SingleRequestProxyFactory" --exec cat {}
# Check git history for any changes related to this contract address
git log -p | rg -i "0x9Fd503e723e5EfcCde3183632b443fFF49E68715"
Length of output: 15562
Script:
#!/bin/bash
# Verify the contract deployment block number on zkSync Era
curl -s -X POST -H "Content-Type: application/json" --data '{
"jsonrpc":"2.0",
"method":"eth_getTransactionReceipt",
"params":["0x9Fd503e723e5EfcCde3183632b443fFF49E68715"],
"id":1
}' https://mainnet.era.zksync.io
# Check for any deployment scripts or configuration files
fd -t f -e ts -e js deploy | rg -l "SingleRequestProxyFactory"
Length of output: 500
28-51
: Verify the reused contract address pattern
I notice that the same contract address 0x4D417AA04DBb207201a794E5B7381B3cde815281
is used across multiple networks (matic, avalanche, arbitrum-one, xdai, bsc). While this could be intentional through deterministic deployment, it's worth verifying.
✅ Verification successful
Same contract address across networks is intentional through CREATE2 deployment
The identical contract address 0x4D417AA04DBb207201a794E5B7381B3cde815281
across multiple networks is intentional and correct. The codebase shows:
- The project uses CREATE2 deterministic deployment pattern, as evidenced by the
scripts-create2
directory and deployment scripts - The deployment process:
- Uses the same salt across networks (via
hre.config.xdeploy.salt
) - Uses identical constructor arguments per network (from
getConstructorArgs
) - Deploys using the same deployer address (via
hre.config.xdeploy.deployerAddress
)
- Uses the same salt across networks (via
This ensures the contract is deployed to the same address across all networks, which is a valid and common pattern for cross-chain applications.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify if the contract code is identical across networks where the same address is used
# Get contract bytecode from Polygon
echo "Checking contract bytecode on Polygon (matic)..."
curl -s -X POST -H "Content-Type: application/json" --data '{
"jsonrpc":"2.0",
"method":"eth_getCode",
"params":["0x4D417AA04DBb207201a794E5B7381B3cde815281", "latest"],
"id":1
}' https://polygon-mainnet.public.blastapi.io
# Compare with BSC deployment
echo "Checking contract bytecode on BSC..."
curl -s -X POST -H "Content-Type: application/json" --data '{
"jsonrpc":"2.0",
"method":"eth_getCode",
"params":["0x4D417AA04DBb207201a794E5B7381B3cde815281", "latest"],
"id":1
}' https://bsc-mainnet.public.blastapi.io
Length of output: 74018
Script:
#!/bin/bash
# Let's verify the contract deployment pattern by checking:
# 1. Creation block numbers across networks
# 2. Contract code similarity across networks
# 3. Any deployment-related code in the repository
# First, let's search for any deployment scripts or configuration
echo "Searching for deployment-related files..."
rg -l "0x4D417AA04DBb207201a794E5B7381B3cde815281"
# Look for any factory or deployment patterns
echo "Searching for deployment patterns..."
rg -l "SingleRequestProxyFactory"
# Check for any deterministic deployment patterns
echo "Searching for deterministic deployment code..."
rg -i "deterministic" -A 3 -B 3
Length of output: 1586
Script:
#!/bin/bash
# Let's check the CREATE2 deployment scripts since they were found in the search
echo "Checking CREATE2 deployment configuration..."
cat packages/smart-contracts/scripts-create2/contract-setup/setupSingleRequestProxyFactory.ts
# Also check the constructor args used for deployment
echo "Checking constructor arguments..."
cat packages/smart-contracts/scripts-create2/constructor-args.ts
# Check the compute address script
echo "Checking address computation..."
cat packages/smart-contracts/scripts-create2/compute-one-address.ts
Length of output: 8769
Resolves #1485
Description of the changes
Deploy Single Request Proxy to the following:
Summary by CodeRabbit
Release Notes
New Features
SingleRequestProxyFactory
on the zkSync network.Enhancements
Deployment Configurations
SingleRequestProxyFactory
across several blockchain networks, including zksyncera, base, matic, avalanche, optimism, arbitrum-one, xdai, bsc, celo, mantle, and mainnet.