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.

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

TD1320 ImmutableSignedZoneV2 deploy script #215

Merged
merged 8 commits into from
May 13, 2024

Conversation

frankisawesome
Copy link
Contributor

  • Use create3
  • Basic test

ZoneDeploymentArgs memory zoneDeploymentArgs = ZoneDeploymentArgs({
owner: address(0xC606830D8341bc9F5F5Dd7615E9313d2655B505D),
name: "TestImmutableSignedZoneV2",
apiEndpoint: "https://api.sandbox.immutable.com/",
Copy link
Contributor

Choose a reason for hiding this comment

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

In the V1 zone deployment script, we've left the api_endpoint empty. Having the api endpoint info is fine but the script will need to account for the different values in different evnironments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@naveen-imtb this is the test, actual deployments will use env var during deployment

}

function deploy() external {
address signer = vm.envAddress("DEPLOYER_ADDRESS");
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the role of the signer?
Does the msg.sender have no role in this workflow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The signer is the deployer used


/// @dev These are Immutable zkEVM testnet values where necessary
ZoneDeploymentArgs memory zoneDeploymentArgs = ZoneDeploymentArgs({
owner: address(0xC606830D8341bc9F5F5Dd7615E9313d2655B505D),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the owner be defaulted to the deployer? Isn't the plan to then transfer ownership of the contract to the multisig wallet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above, this is just a test value

naveen-imtb
naveen-imtb previously approved these changes May 7, 2024
Signed-off-by: Frank Li <[email protected]>
@frankisawesome frankisawesome enabled auto-merge (squash) May 13, 2024 00:31
@frankisawesome frankisawesome merged commit 9295f9e into main May 13, 2024
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants