Skip to content

Commit

Permalink
feat(ct): require that proposers are approved by the project owner
Browse files Browse the repository at this point in the history
  • Loading branch information
sam-goldman committed Mar 16, 2023
1 parent e1f554e commit ac40b0b
Show file tree
Hide file tree
Showing 11 changed files with 108 additions and 23 deletions.
7 changes: 7 additions & 0 deletions .changeset/unlucky-rats-type.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@chugsplash/contracts': minor
'@chugsplash/plugins': minor
'@chugsplash/core': minor
---

Require that proposers are approved by the project owner
2 changes: 1 addition & 1 deletion packages/contracts/contracts/ChugSplashBootLoader.sol
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ contract ChugSplashBootLoader is Initializable {
// deployed.
rootManagerProxy.upgradeToAndCall(
_managerImplementation,
abi.encodeCall(ChugSplashManager.initialize, ("Root Manager", _owner))
abi.encodeCall(ChugSplashManager.initialize, ("Root Manager", _owner, false))
);
// Change the admin of the root ChugSplashManagerProxy to itself, since it will be upgrading
// itself during meta-upgradeability (i.e. ChugSplash upgrading itself).
Expand Down
32 changes: 29 additions & 3 deletions packages/contracts/contracts/ChugSplashManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable {
*/
event ProposerRemoved(address indexed proposer, address indexed owner);

event ToggledManagedProposals(bool isManaged, address indexed owner);

/**
* @notice Emitted when ETH is deposited in this contract
*/
Expand Down Expand Up @@ -251,8 +253,6 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable {

/**
* @notice Maps an address to a boolean indicating if the address is allowed to propose bundles.
* The owner of this contract is the only address that can add or remove proposers from
* this mapping.
*/
mapping(address => bool) public proposers;

Expand All @@ -278,6 +278,8 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable {

uint256 public totalProtocolDebt;

bool public allowManagedProposals;

/**
* @notice Modifier that restricts access to the executor.
*/
Expand Down Expand Up @@ -319,7 +321,12 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable {
* @param _name Name of the project this contract is managing.
* @param _owner Initial owner of this contract.
*/
function initialize(string memory _name, address _owner) public initializer {
function initialize(
string memory _name,
address _owner,
bool _allowManagedProposals
) public initializer {
allowManagedProposals = _allowManagedProposals;
name = _name;

__Ownable_init();
Expand Down Expand Up @@ -412,6 +419,8 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable {
uint256 _numTargets,
string memory _configUri
) public {
require(isProposer(msg.sender), "ChugSplashManager: caller must be proposer");

bytes32 bundleId = computeBundleId(
_actionRoot,
_targetRoot,
Expand Down Expand Up @@ -994,6 +1003,23 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable {
recorder.announce("ProposerAdded");
}

function isProposer(address _addr) public view returns (bool) {
return
(allowManagedProposals && registry.managedProposers(_addr) == true) ||
proposers[_addr] == true ||
_addr == owner();
}

function toggleAllowManagedProposals() external onlyOwner {
allowManagedProposals = !allowManagedProposals;

emit ToggledManagedProposals(allowManagedProposals, msg.sender);
recorder.announceWithData(
"ToggledManagedProposals",
abi.encodePacked(allowManagedProposals)
);
}

/**
* @notice Allows the owner of this contract to remove a proposer.
*
Expand Down
25 changes: 23 additions & 2 deletions packages/contracts/contracts/ChugSplashRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ contract ChugSplashRegistry is Initializable, OwnableUpgradeable {
*/
event ExecutorRemoved(address indexed executor);

event ManagedProposerAdded(address indexed proposer);

event ManagedProposerRemoved(address indexed proposer);

event ProtocolPaymentRecipientAdded(address indexed executor);

event ProtocolPaymentRecipientRemoved(address indexed executor);
Expand All @@ -74,6 +78,8 @@ contract ChugSplashRegistry is Initializable, OwnableUpgradeable {
*/
mapping(address => bool) public executors;

mapping(address => bool) public managedProposers;

mapping(address => bool) public protocolPaymentRecipients;

ChugSplashRecorder public recorder;
Expand Down Expand Up @@ -144,7 +150,7 @@ contract ChugSplashRegistry is Initializable, OwnableUpgradeable {
* @param _name Name of the new ChugSplash project.
* @param _owner Initial owner for the new project.
*/
function register(string memory _name, address _owner) public {
function register(string memory _name, address _owner, bool _allowManagedProposals) public {
require(
address(projects[_name]) == address(0),
"ChugSplashRegistry: name already registered"
Expand All @@ -163,7 +169,7 @@ contract ChugSplashRegistry is Initializable, OwnableUpgradeable {
// deployed.
manager.upgradeToAndCall(
_getManagerImpl(),
abi.encodeCall(ChugSplashManager.initialize, (_name, _owner))
abi.encodeCall(ChugSplashManager.initialize, (_name, _owner, _allowManagedProposals))
);

projects[_name] = ChugSplashManager(payable(address(manager)));
Expand Down Expand Up @@ -195,6 +201,21 @@ contract ChugSplashRegistry is Initializable, OwnableUpgradeable {
emit ExecutorRemoved(_executor);
}

function addManagedProposer(address _proposer) external onlyOwner {
require(managedProposers[_proposer] == false, "ChugSplashRegistry: proposer already added");
managedProposers[_proposer] = true;
emit ManagedProposerAdded(_proposer);
}

function removeManagedProposer(address _proposer) external onlyOwner {
require(
managedProposers[_proposer] == true,
"ChugSplashRegistry: proposer already removed"
);
managedProposers[_proposer] = false;
emit ManagedProposerRemoved(_proposer);
}

function addProtocolPaymentRecipient(address _recipient) external onlyOwner {
require(
protocolPaymentRecipients[_recipient] == false,
Expand Down
19 changes: 15 additions & 4 deletions packages/core/src/tasks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export const chugsplashRegisterAbstractTask = async (
provider: ethers.providers.JsonRpcProvider,
signer: ethers.Signer,
parsedConfig: ParsedChugSplashConfig,
allowManagedProposals: boolean,
owner: string,
silent: boolean,
integration: Integration,
Expand All @@ -92,7 +93,8 @@ export const chugsplashRegisterAbstractTask = async (
signer,
await signer.getAddress(),
parsedConfig.options.projectName,
owner
owner,
allowManagedProposals
)

const networkName = await resolveNetworkName(provider, integration)
Expand Down Expand Up @@ -649,6 +651,7 @@ export const chugsplashDeployAbstractTask = async (
confirm: boolean,
withdraw: boolean,
newOwner: string,
allowManagedProposals: boolean,
artifactPaths: ArtifactPaths,
canonicalConfigPath: string,
deploymentFolder: string,
Expand Down Expand Up @@ -714,7 +717,8 @@ export const chugsplashDeployAbstractTask = async (
signer,
signerAddress,
projectName,
signerAddress
signerAddress,
allowManagedProposals
)
spinner.succeed(`Successfully registered ${projectName}.`)
}
Expand Down Expand Up @@ -1587,15 +1591,22 @@ export const proposeChugSplashBundle = async (
integration: Integration
) => {
const projectName = parsedConfig.options.projectName
const ChugSplashManager = getChugSplashManager(signer, projectName)
const signerAddress = await signer.getAddress()

spinner.start(`Checking if the caller is a proposer...`)

// Throw an error if the caller isn't the project owner or a proposer.
if (!(await ChugSplashManager.isProposer(signerAddress))) {
throw new Error(
`Caller is not a proposer for this project. Caller's address: ${signerAddress}`
)
}

spinner.succeed(`Caller is a proposer.`)

spinner.start(`Proposing ${projectName}...`)

const ChugSplashManager = getChugSplashManager(signer, projectName)

if (remoteExecution) {
await chugsplashCommitAbstractSubtask(
provider,
Expand Down
13 changes: 3 additions & 10 deletions packages/core/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,8 @@ export const registerChugSplashProject = async (
signer: Signer,
signerAddress: string,
projectName: string,
projectOwner: string
projectOwner: string,
allowManagedProposals: boolean
): Promise<boolean> => {
const ChugSplashRegistry = getChugSplashRegistry(signer)

Expand All @@ -226,6 +227,7 @@ export const registerChugSplashProject = async (
await ChugSplashRegistry.register(
projectName,
projectOwner,
allowManagedProposals,
await getGasPriceOverrides(provider)
)
).wait()
Expand Down Expand Up @@ -379,15 +381,6 @@ export const getCurrentChugSplashActionType = (
return bundle.actions[actionsExecuted.toNumber()].action.actionType
}

export const isProposer = async (
provider: providers.Provider,
projectName: string,
address: string
): Promise<boolean> => {
const ChugSplashManager = getChugSplashManagerReadOnly(provider, projectName)
return ChugSplashManager.proposers(address)
}

export const isContractDeployed = async (
address: string,
provider: providers.Provider
Expand Down
7 changes: 5 additions & 2 deletions packages/plugins/foundry-contracts/ChugSplash.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ contract ChugSplash is Script, Test {
bool withdrawFunds = vm.envOr("WITHDRAW_FUNDS", true);
string ipfsUrl = vm.envOr("IPFS_URL", NONE);
bool skipStorageCheck = vm.envOr("SKIP_STORAGE_CHECK", false);
bool allowManagedProposals = vm.envOr("ALLOW_MANAGED_PROPOSALS", false);

string rpcUrl = vm.rpcUrl(network);
string filePath = vm.envOr("DEV_FILE_PATH", string('./node_modules/@chugsplash/plugins/dist/foundry/index.js'));
Expand Down Expand Up @@ -81,7 +82,7 @@ contract ChugSplash is Script, Test {
) public returns (bytes memory) {
(string memory outPath, string memory buildInfoPath) = fetchPaths();

string[] memory cmds = new string[](12);
string[] memory cmds = new string[](13);
cmds[0] = "npx";
cmds[1] = "node";
cmds[2] = filePath;
Expand All @@ -94,6 +95,7 @@ contract ChugSplash is Script, Test {
cmds[9] = outPath;
cmds[10] = buildInfoPath;
cmds[11] = newOwner;
cmds[12] = allowManagedProposals == true ? "true" : "false";

bytes memory result = vm.ffi(cmds);

Expand Down Expand Up @@ -215,7 +217,7 @@ contract ChugSplash is Script, Test {
) public {
(string memory outPath, string memory buildInfoPath) = fetchPaths();

string[] memory cmds = new string[](15);
string[] memory cmds = new string[](16);
cmds[0] = "npx";
cmds[1] = "node";
cmds[2] = filePath;
Expand All @@ -231,6 +233,7 @@ contract ChugSplash is Script, Test {
cmds[12] = newOwner;
cmds[13] = ipfsUrl;
cmds[14] = skipStorageCheck == true ? "true" : "false";
cmds[15] = allowManagedProposals == true ? "true" : "false";

bytes memory result = vm.ffi(cmds);
if (isChugSplashTest) {
Expand Down
4 changes: 4 additions & 0 deletions packages/plugins/src/foundry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const command = args[0]
const outPath = cleanPath(args[6])
const buildInfoPath = cleanPath(args[7])
let owner = args[8]
const allowManagedProposals = args[9] === 'true'

const provider = new ethers.providers.JsonRpcProvider(rpcUrl, network)
const wallet = new ethers.Wallet(privateKey, provider)
Expand Down Expand Up @@ -74,6 +75,7 @@ const command = args[0]
provider,
wallet,
config,
allowManagedProposals,
owner,
silent,
'foundry',
Expand Down Expand Up @@ -239,6 +241,7 @@ const command = args[0]
let newOwner = args[9]
const ipfsUrl = args[10] !== 'none' ? args[10] : ''
const skipStorageCheck = args[11] === 'true'
const allowManagedProposals = args[12] === 'true'

const noCompile = true
const confirm = true
Expand Down Expand Up @@ -300,6 +303,7 @@ const command = args[0]
confirm,
withdrawFunds,
newOwner ?? (await wallet.getAddress()),
allowManagedProposals,
artifactPaths,
canonicalConfigPath,
deploymentFolder,
Expand Down
1 change: 1 addition & 0 deletions packages/plugins/src/hardhat/deployments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ export const deployAllChugSplashConfigs = async (
confirm,
true,
await signer.getAddress(),
false,
artifactPaths,
canonicalConfigPath,
deploymentFolder,
Expand Down
15 changes: 14 additions & 1 deletion packages/plugins/src/hardhat/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ subtask(TASK_CHUGSPLASH_BUNDLE_LOCAL)
export const chugsplashDeployTask = async (
args: {
configPath: string
allowManagedProposals: boolean
newOwner: string
ipfsUrl: string
silent: boolean
Expand All @@ -132,6 +133,7 @@ export const chugsplashDeployTask = async (
) => {
const {
configPath,
allowManagedProposals,
newOwner,
ipfsUrl,
silent,
Expand Down Expand Up @@ -202,6 +204,7 @@ export const chugsplashDeployTask = async (
confirm,
!noWithdraw,
newOwner ?? signerAddress,
allowManagedProposals,
artifactPaths,
canonicalConfigPath,
deploymentFolder,
Expand Down Expand Up @@ -233,6 +236,10 @@ task(TASK_CHUGSPLASH_DEPLOY)
'confirm',
'Automatically confirm contract upgrades. Only applicable if upgrading on a live network.'
)
.addFlag(
'allowManagedProposals',
'Allow the ChugSplash Managed Service to propose deployments and upgrades on your behalf.'
)
.addFlag(
'skipStorageCheck',
"Upgrade your contract(s) without checking for storage layout compatibility. Only use this when confident that the upgrade won't lead to storage layout issues."
Expand All @@ -242,12 +249,13 @@ task(TASK_CHUGSPLASH_DEPLOY)
export const chugsplashRegisterTask = async (
args: {
configPath: string
allowManagedProposals: boolean
owner: string
silent: boolean
},
hre: HardhatRuntimeEnvironment
) => {
const { configPath, silent, owner } = args
const { configPath, silent, owner, allowManagedProposals } = args

const provider = hre.ethers.provider
const signer = hre.ethers.provider.getSigner()
Expand All @@ -273,6 +281,7 @@ export const chugsplashRegisterTask = async (
provider,
signer,
parsedConfig,
allowManagedProposals,
owner,
silent,
'hardhat'
Expand All @@ -282,6 +291,10 @@ export const chugsplashRegisterTask = async (
task(TASK_CHUGSPLASH_REGISTER)
.setDescription('Registers a new ChugSplash project')
.addParam('configPath', 'Path to the ChugSplash config file to propose')
.addFlag(
'allowManagedProposals',
'Allow the ChugSplash Managed Service to propose deployments and upgrades on your behalf.'
)
.addParam('owner', 'Owner of the ChugSplash project')
.addFlag('silent', "Hide all of ChugSplash's output")
.setAction(chugsplashRegisterTask)
Expand Down
Loading

0 comments on commit ac40b0b

Please sign in to comment.