Skip to content

Commit

Permalink
chore: rewrite transferOwnership script to use Foundry (#158)
Browse files Browse the repository at this point in the history
## Description

The transferOwnership script has been ported to Foundry, with feature
parity, including tests.

There was the need for a few workarounds in the tests, mostly because
`prank` and `broadcast` aren't compatible with each other and because
our proxy is based on Solidity 0.7 and its code isn't directly available
in this repo.

## Test Plan

New tests.
I also deployed a new set of contracts on Sepolia and used the script to
change the owner and manager. You can see the two resulting transactions
for the
[manager](https://sepolia.etherscan.io/tx/0xe4d4499c66f696c671374ffc3f7255dc05fc09b9643af85e4c0e4243919f044a#eventlog)
and the
[owner](https://sepolia.etherscan.io/tx/0x42a592e04a3a76463b51bff67f38a7195875400e41b7b59737ac8dd41dcb45a7#eventlog).

## Related Issues

Closes #102
Closes #103
  • Loading branch information
fedgiac authored Jun 7, 2024
1 parent b4d64fd commit 76e2363
Show file tree
Hide file tree
Showing 16 changed files with 387 additions and 264 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ jobs:
run: |
forge --version
if [ "${{ matrix.profile }}" == "solc-0.7.6" ]; then
forge build --sizes --use 0.7.6 --skip 'test/*'
forge build --sizes --use 0.7.6 --skip 'test/*' --skip 'script/*'
else
forge build --sizes
fi
Expand Down
7 changes: 4 additions & 3 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ build/
coverage/
deployments/*/solcInputs
deployments/localhost/
lib/
/lib/
node_modules/
coverage.json
yarn-error.log
Expand All @@ -15,9 +15,10 @@ cache/
out/

# Ignores development broadcast logs
!/broadcast
/broadcast/*/31337/
/broadcast/**/dry-run/
# Ignores logs of some scripts
/broadcast/TransferOwnership.s.sol/

# Dotenv file
.env
.env
18 changes: 16 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,24 @@ We recommend to never sign orders of this form and, if developing a contract tha

There is a dedicated script to change the owner of the authenticator proxy.

Usage and parameters can be seen by running:
The following parameters can be set:

```sh
yarn hardhat transfer-ownership --help
export ETH_RPC_URL='https://rpc.url.example.com'
export NEW_OWNER=0x1111111111111111111111111111111111111111
export RESET_MANAGER=true # true if the new owner should also become the manager, false otherwise
```

To test run the script from a specific owner (sender):

```sh
forge script script/TransferOwnership.s.sol:TransferOwnership --rpc-url "$ETH_RPC_URL" --sender 0xcA771eda0c70aA7d053aB1B25004559B918FE662
```

To actually execute the transaction:

```sh
forge script script/TransferOwnership.s.sol:TransferOwnership --rpc-url "$ETH_RPC_URL" --private-key 0x0000000000000000000000000000000000000000000000000000000000000001 --broadcast
```

## Releases
Expand Down
2 changes: 2 additions & 0 deletions foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ via_ir = false
optimizer = true
optimizer_runs = 1000000

fs_permissions = [{ access = "read", path = "./networks.json"}]

[fmt]
ignore = [
# We don't want to change the formatting of our main contracts until the
Expand Down
10 changes: 3 additions & 7 deletions hardhat.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import type { HttpNetworkUserConfig } from "hardhat/types";
import type { MochaOptions } from "mocha";
import yargs from "yargs";

import { setupTasks } from "./src/tasks";

const argv = yargs
.option("network", {
type: "string",
Expand Down Expand Up @@ -65,25 +63,23 @@ switch (MOCHA_CONF) {
case undefined:
break;
case "coverage":
// End to end and task tests are skipped because:
// End to end tests are skipped because:
// - coverage tool does not play well with proxy deployment with
// hardhat-deploy
// - coverage compiles without optimizer and, unlike Waffle, hardhat-deploy
// strictly enforces the contract size limits from EIP-170
mocha.grep = /^(?!E2E|Task)/;
mocha.grep = /^(?!E2E)/;
// Note: unit is Wei, not GWei. This is a workaround to make the coverage
// tool work with the London hardfork.
initialBaseFeePerGas = 1;
break;
case "ignored in coverage":
mocha.grep = /^E2E|Task/;
mocha.grep = /^E2E/;
break;
default:
throw new Error("Invalid MOCHA_CONF");
}

setupTasks();

export default {
mocha,
paths: {
Expand Down
126 changes: 126 additions & 0 deletions script/TransferOwnership.s.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
// SPDX-License-Identifier: LGPL-3.0-or-later
pragma solidity ^0.8.26;

import {console} from "forge-std/Script.sol";

import {GPv2AllowListAuthentication} from "../src/contracts/GPv2AllowListAuthentication.sol";

import {ERC173, ERC165} from "./interfaces/ERC173.sol";
import {NetworksJson} from "./lib/NetworksJson.sol";

contract TransferOwnership is NetworksJson {
// Required input
string private constant INPUT_ENV_NEW_OWNER = "NEW_OWNER";
string private constant INPUT_ENV_RESET_MANAGER = "RESET_MANAGER";
// Optional input
string private constant INPUT_ENV_AUTHENTICATOR_PROXY = "AUTHENTICATOR_PROXY";

NetworksJson internal networksJson;

struct ScriptParams {
address newOwner;
bool resetManager;
ERC173 authenticatorProxy;
}

constructor() {
networksJson = new NetworksJson();
}

function run() public virtual {
ScriptParams memory params = paramsFromEnv();
runWith(params);
}

function runWith(ScriptParams memory params) public {
console.log(string.concat("Using account ", vm.toString(msg.sender)));

checkIsProxy(address(params.authenticatorProxy));

address owner = params.authenticatorProxy.owner();
if (owner != msg.sender) {
revert(string.concat("Account does NOT match current owner ", vm.toString(owner)));
}

GPv2AllowListAuthentication authenticator = GPv2AllowListAuthentication(address(params.authenticatorProxy));

// Make sure to reset the manager BEFORE transferring ownership, or else
// we will not be able to do it once we lose permissions.
if (params.resetManager) {
console.log(
string.concat(
"Setting new solver manager from ",
vm.toString(authenticator.manager()),
" to ",
vm.toString(params.newOwner)
)
);
vm.broadcast(msg.sender);
authenticator.setManager(params.newOwner);
console.log("Set new solver manager account.");
}

console.log(
string.concat(
"Setting new authenticator proxy owner from ", vm.toString(owner), " to ", vm.toString(params.newOwner)
)
);
vm.broadcast(msg.sender);
params.authenticatorProxy.transferOwnership(params.newOwner);
console.log("Set new owner of the authenticator proxy.");
}

function paramsFromEnv() internal view returns (ScriptParams memory) {
address newOwner = vm.envAddress(INPUT_ENV_NEW_OWNER);
bool resetManager = vm.envBool(INPUT_ENV_RESET_MANAGER);

address authenticatorProxy;
try vm.envAddress(INPUT_ENV_AUTHENTICATOR_PROXY) returns (address env) {
authenticatorProxy = env;
} catch {
try networksJson.addressOf("GPv2AllowListAuthentication_Proxy") returns (address addr) {
authenticatorProxy = addr;
} catch {
revert(
string.concat(
"Could not find default authenticator address in file ",
networksJson.PATH(),
" for network with chain id ",
vm.toString(block.chainid),
". Export variable ",
INPUT_ENV_AUTHENTICATOR_PROXY,
" to manually specify a non-standard address for the authenticator."
)
);
}
}

return ScriptParams({
newOwner: newOwner,
resetManager: resetManager,
authenticatorProxy: ERC173(authenticatorProxy)
});
}

function checkIsProxy(address candidate) internal view {
if (address(candidate).code.length == 0) {
revert(string.concat("No code at target authenticator proxy ", vm.toString(address(candidate)), "."));
}

bool isERC173;
try ERC165(candidate).supportsInterface(type(ERC173).interfaceId) returns (bool isERC173_) {
isERC173 = isERC173_;
} catch {
isERC173 = false;
}
if (!isERC173) {
revert(
string.concat(
"Not a valid proxy contract: target address ",
vm.toString(address(candidate)),
" does not support the ERC173 interface."
)
);
}
}
}
30 changes: 30 additions & 0 deletions script/interfaces/ERC173.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// SPDX-License-Identifier: LGPL-3.0
pragma solidity >=0.7.6 <0.9.0;

// Copied from:
// <https://eips.ethereum.org/EIPS/eip-173#specification>

/// @title ERC-173 Contract Ownership Standard
/// Note: the ERC-165 identifier for this interface is 0x7f5828d0
interface ERC173 {
/// @dev This emits when ownership of a contract changes.
event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);

/// @notice Get the address of the owner
/// @return The address of the owner.
function owner() external view returns (address);

/// @notice Set the address of the new owner of the contract
/// @dev Set _newOwner to address(0) to renounce any ownership.
/// @param _newOwner The address of the new owner of the contract
function transferOwnership(address _newOwner) external;
}

interface ERC165 {
/// @notice Query if a contract implements an interface
/// @param interfaceID The interface identifier, as specified in ERC-165
/// @dev Interface identification is specified in ERC-165.
/// @return `true` if the contract implements `interfaceID` and
/// `interfaceID` is not 0xffffffff, `false` otherwise
function supportsInterface(bytes4 interfaceID) external view returns (bool);
}
18 changes: 18 additions & 0 deletions script/lib/NetworksJson.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// SPDX-License-Identifier: LGPL-3.0-or-later
pragma solidity ^0.8.26;

import {Script} from "forge-std/Script.sol";

contract NetworksJson is Script {
string public constant PATH = "./networks.json";

function addressOf(string memory contractName) public view returns (address) {
return addressByChainId(contractName, block.chainid);
}

function addressByChainId(string memory contractName, uint256 chainId) public view returns (address) {
string memory networksJson = vm.readFile(PATH);
return
vm.parseJsonAddress(networksJson, string.concat(".", contractName, ".", vm.toString(chainId), ".address"));
}
}
5 changes: 0 additions & 5 deletions src/tasks/index.ts

This file was deleted.

91 changes: 0 additions & 91 deletions src/tasks/transferOwnership.ts

This file was deleted.

Loading

0 comments on commit 76e2363

Please sign in to comment.