Skip to content
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.

Sign up for GitHub

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

Contract deploy: record L2 contract addresses #1667

Merged
merged 2 commits into from
Nov 29, 2023
Merged

Contract deploy: record L2 contract addresses #1667

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5ea9597
Contract deploy: record L2 contract addresses
BedrockSquirrel Nov 27, 2023
b1d12d8
Merge remote-tracking branch 'origin/main' into matt/record-l2-contra…
BedrockSquirrel Nov 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions contracts/deployment_scripts/bridge/001_deploy_bridge.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) {

// get management contract and write the L1 bridge address to it
const mgmtContract = (await hre.ethers.getContractFactory('ManagementContract')).attach(mgmtContractAddress)
const tx = await mgmtContract.SetImportantContractAddress("L1Bridge", layer1BridgeDeployment.address);
const receipt = await tx.wait();
const recordL1AddressTx = await mgmtContract.SetImportantContractAddress("L1Bridge", layer1BridgeDeployment.address);
const receipt = await recordL1AddressTx.wait();
if (receipt.status !== 1) {
console.log("Failed to set L1BridgeAddress in management contract");
}
Expand All @@ -53,6 +53,12 @@ const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) {
log: true,
}, "setRemoteBridge", layer2BridgeDeployment.address);

const recordL2AddressTx = await mgmtContract.SetImportantContractAddress("L2Bridge", layer2BridgeDeployment.address);
const receipt2 = await recordL2AddressTx.wait();
if (receipt2.status !== 1) {
console.log("Failed to set L2BridgeAddress in management contract");
}
console.log(`L2BridgeAddress=${layer2BridgeDeployment.address}`)
console.log(` Bridge deployed with from L1 address=${accountsL1.deployer} L2 Address=${accountsL2.deployer}`);
};

Expand Down
2 changes: 1 addition & 1 deletion contracts/deployment_scripts/messenger/layer1/001_deploy_cross_chain_messenger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) {
if (receipt.status !== 1) {
console.log("Failed to set L1CrossChainMessenger in management contract");
}
console.log(`L1CrossChainMessenger=${crossChainDeployment.address}`)
console.log(`L1CrossChainMessenger=${crossChainDeployment.address}`);
};

export default func;
Expand Down
16 changes: 14 additions & 2 deletions contracts/deployment_scripts/messenger/layer2/001_deploy_cross_chain_messenger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@ import {DeployFunction} from 'hardhat-deploy/types';
const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) {
const {
deployments,
getNamedAccounts
getNamedAccounts,
companionNetworks,
} = hre;
// Use the contract addresses from the management contract deployment.
const mgmtContractAddress = process.env.MGMT_CONTRACT_ADDRESS!!
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The non-null assertion operator !! is used without checking if MGMT_CONTRACT_ADDRESS is actually set, which could lead to runtime errors if the environment variable is not provided. Consider adding a check to ensure the variable is set before using it.

- const mgmtContractAddress = process.env.MGMT_CONTRACT_ADDRESS!!
+ const mgmtContractAddress = process.env.MGMT_CONTRACT_ADDRESS;
+ if (!mgmtContractAddress) {
+     throw new Error('MGMT_CONTRACT_ADDRESS is not set');
+ }
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.

Suggested change
const mgmtContractAddress = process.env.MGMT_CONTRACT_ADDRESS!!
const mgmtContractAddress = process.env.MGMT_CONTRACT_ADDRESS;
if (!mgmtContractAddress) {
throw new Error('MGMT_CONTRACT_ADDRESS is not set');
}


// Get the prefunded L2 deployer account to use for deploying.
const {deployer} = await getNamedAccounts();
Expand All @@ -25,11 +28,20 @@ const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) {
console.log(`Beginning deploy of cross chain messenger`);

// Deploy the L2 Cross chain messenger and use the L2 bus for validation
await deployments.deploy('CrossChainMessenger', {
const crossChainDeployment = await deployments.deploy('CrossChainMessenger', {
from: deployer,
args: [ busAddress ],
log: true,
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no error handling for the deployment of CrossChainMessenger. If the deployment fails, the script will continue to execute, potentially leading to misleading logs or further errors. Consider adding error handling after the deployment attempt.


// get L1 management contract and write the cross chain messenger address to it
const mgmtContract = (await hre.ethers.getContractFactory('ManagementContract')).attach(mgmtContractAddress);
const tx = await mgmtContract.SetImportantContractAddress("L2CrossChainMessenger", crossChainDeployment.address);
const receipt = await tx.wait();
if (receipt.status !== 1) {
console.log("Failed to set L2CrossChainMessenger in management contract");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message in the console.log statement could be more informative by including the transaction hash or additional details to aid in debugging.

- console.log("Failed to set L2CrossChainMessenger in management contract");
+ console.log(`Failed to set L2CrossChainMessenger in management contract, transaction hash: ${tx.hash}`);
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.

Suggested change
console.log("Failed to set L2CrossChainMessenger in management contract");
console.log(`Failed to set L2CrossChainMessenger in management contract, transaction hash: ${tx.hash}`);

}
console.log(`L2CrossChainMessenger=${crossChainDeployment.address}`);
};

export default func;
Expand Down
Loading