diff --git a/packages/contracts-bedrock/scripts/DeployAuthSystem.s.sol b/packages/contracts-bedrock/scripts/DeployAuthSystem.s.sol index cca7f054afcd..66f5da20b740 100644 --- a/packages/contracts-bedrock/scripts/DeployAuthSystem.s.sol +++ b/packages/contracts-bedrock/scripts/DeployAuthSystem.s.sol @@ -48,7 +48,6 @@ contract DeployAuthSystemInput is CommonBase { } function owners() public view returns (address[] memory) { - // expecting to trigger this require(_owners.length != 0, "DeployAuthSystemInput: owners not set"); return _owners; } @@ -78,31 +77,17 @@ contract DeployAuthSystemOutput is CommonBase { } } -// For all broadcasts in this script we explicitly specify the deployer as `msg.sender` because for -// testing we deploy this script from a test contract. If we provide no argument, the foundry -// default sender would be the broadcaster during test, but the broadcaster needs to be the deployer -// since they are set to the initial proxy admin owner. contract DeployAuthSystem is Script { - // -------- Core Deployment Methods -------- - - // This entrypoint is for end-users to deploy from an input file and write to an output file. - // In this usage, we don't need the input and output contract functionality, so we deploy them - // here and abstract that architectural detail away from the end user. function run(string memory _infile, string memory _outfile) public { - // End-user without file IO, so etch the IO helper contracts. (DeployAuthSystemInput dasi, DeployAuthSystemOutput daso) = etchIOContracts(); - // Load the input file into the input contract. dasi.loadInputFile(_infile); - // Run the deployment script and write outputs to the DeployAuthSystemOutput contract. run(dasi, daso); - // Write the output data to a file. daso.writeOutputFile(_outfile); } - // This entrypoint is useful for testing purposes, as it doesn't use any file I/O. function run(DeployAuthSystemInput _dasi, DeployAuthSystemOutput _daso) public { deploySafe(_dasi, _daso); } @@ -110,9 +95,6 @@ contract DeployAuthSystem is Script { function deploySafe(DeployAuthSystemInput _dasi, DeployAuthSystemOutput _daso) public { address[] memory owners = _dasi.owners(); uint256 threshold = _dasi.threshold(); - // Silence unused variable warnings - owners; - threshold; // TODO: replace with a real deployment. The safe deployment logic is fairly complex, so for the purposes of // this scaffolding PR we'll just etch the code. @@ -126,10 +108,6 @@ contract DeployAuthSystem is Script { _daso.set(_daso.safe.selector, safe); } - // This etches the IO contracts into memory so that we can use them in tests. When using file IO - // we don't need to call this directly, as the `DeployAuthSystem.run(file, file)` entrypoint - // handles it. But when interacting with the script programmatically (e.g. in a Solidity test), - // this must be called. function etchIOContracts() public returns (DeployAuthSystemInput dasi_, DeployAuthSystemOutput daso_) { (dasi_, daso_) = getIOContracts(); vm.etch(address(dasi_), type(DeployAuthSystemInput).runtimeCode); @@ -138,7 +116,6 @@ contract DeployAuthSystem is Script { vm.allowCheatcodes(address(daso_)); } - // This returns the addresses of the IO contracts for this script. function getIOContracts() public view returns (DeployAuthSystemInput dasi_, DeployAuthSystemOutput daso_) { dasi_ = DeployAuthSystemInput(DeployUtils.toIOAddress(msg.sender, "optimism.DeployAuthSystemInput")); daso_ = DeployAuthSystemOutput(DeployUtils.toIOAddress(msg.sender, "optimism.DeployAuthSystemOutput")); diff --git a/packages/contracts-bedrock/test/DeployAuthSystem.t.sol b/packages/contracts-bedrock/test/DeployAuthSystem.t.sol index 2e4a9876dd60..f06e9237cb86 100644 --- a/packages/contracts-bedrock/test/DeployAuthSystem.t.sol +++ b/packages/contracts-bedrock/test/DeployAuthSystem.t.sol @@ -68,13 +68,10 @@ contract DeployAuthSystemOutput_Test is Test { function test_set_succeeds() public { address safeAddr = makeAddr("safe"); - // Ensure the address has code, since it's expected to be a contract vm.etch(safeAddr, hex"01"); - // Set the output data daso.set(daso.safe.selector, safeAddr); - // Compare the test data to the getter method assertEq(safeAddr, address(daso.safe()), "100"); } @@ -95,13 +92,11 @@ contract DeployAuthSystemOutput_Test is Test { function test_writeOutputFile_succeeds() public { string memory root = vm.projectRoot(); - // Use the expected data from the test fixture. string memory expOutPath = string.concat(root, "/test/fixtures/test-deploy-auth-system-out.toml"); string memory expOutToml = vm.readFile(expOutPath); address expSafe = expOutToml.readAddress(".safe"); - // Etch code at each address so the code checks pass when settings values. vm.etch(expSafe, hex"01"); daso.set(daso.safe.selector, expSafe); @@ -110,7 +105,6 @@ contract DeployAuthSystemOutput_Test is Test { daso.writeOutputFile(actOutPath); string memory actOutToml = vm.readFile(actOutPath); - // Clean up before asserting so that we don't leave any files behind. vm.removeFile(actOutPath); assertEq(expOutToml, actOutToml); @@ -142,9 +136,6 @@ contract DeployAuthSystem_Test is Test { } function testFuzz_run_memory_succeeds(bytes32 _seed) public { - // Generate random input values from the seed. This doesn't give us the benefit of the forge - // fuzzer's dictionary, but that's ok because we are just testing that values are set and - // passed correctly. address[] memory _owners = Solarray.addresses( address(uint160(uint256(hash(_seed, 0)))), address(uint160(uint256(hash(_seed, 1)))), @@ -160,10 +151,8 @@ contract DeployAuthSystem_Test is Test { dasi.set(dasi.owners.selector, _owners); dasi.set(dasi.threshold.selector, threshold); - // Run the deployment script. deployAuthSystem.run(dasi, daso); - // Assert inputs were properly passed through to the contract initializers. assertNotEq(address(daso.safe()), address(0), "100"); assertEq(daso.safe().getThreshold(), threshold, "200"); // TODO: the getOwners() method requires iterating over the owners linked list. @@ -187,13 +176,11 @@ contract DeployAuthSystem_Test is Test { string memory actOutToml = vm.readFile(outpath); string memory expOutToml = vm.readFile(string.concat(root, "/test/fixtures/test-deploy-auth-system-out.toml")); - // Clean up before asserting so that we don't leave any files behind. vm.removeFile(outpath); assertEq(expOutToml, actOutToml); } function test_run_NullInput_reverts() public { - // Set default values for all inputs. dasi.set(dasi.owners.selector, defaultOwners); dasi.set(dasi.threshold.selector, defaultThreshold); @@ -204,7 +191,6 @@ contract DeployAuthSystem_Test is Test { deployAuthSystem.run(dasi, daso); vm.store(address(dasi), bytes32(uint256(9)), bytes32(defaultOwnersLength)); - // Zero out the threshold slot slot = zeroOutSlotForSelector(dasi.threshold.selector); vm.expectRevert("DeployAuthSystemInput: threshold not set"); deployAuthSystem.run(dasi, daso);