Skip to content

Commit

Permalink
fix(ct): add proposers to the ChugSplashManager
Browse files Browse the repository at this point in the history
  • Loading branch information
sam-goldman committed Dec 3, 2022
1 parent d7fff20 commit 40c7bfb
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/grumpy-ways-rhyme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@chugsplash/contracts': patch
---

Adds proposers to the ChugSplashManager
59 changes: 58 additions & 1 deletion packages/contracts/contracts/ChugSplashManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,22 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable {
*/
event OwnerWithdrewETH(address indexed owner, uint256 amount);

/**
* @notice Emitted when the owner of this contract adds a new proposer.
*
* @param proposer Address of the proposer that was added.
* @param proposer Address of the owner.
*/
event ProposerAdded(address indexed proposer, address indexed owner);

/**
* @notice Emitted when the owner of this contract removes an existing proposer.
*
* @param proposer Address of the proposer that was removed.
* @param proposer Address of the owner.
*/
event ProposerRemoved(address indexed proposer, address indexed owner);

/**
* @notice Emitted when ETH is deposited in this contract
*/
Expand Down Expand Up @@ -203,6 +219,13 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable {
*/
mapping(string => address) public implementations;

/**
* @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;

/**
* @notice Mapping of bundle IDs to bundle state.
*/
Expand Down Expand Up @@ -331,7 +354,8 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable {
}

/**
* @notice Lets anyone propose a new ChugSplash bundle to be approved.
* @notice Propose a new ChugSplash bundle to be approved. Only callable by the owner of this
* contract or a proposer.
*
* @param _bundleRoot Root of the bundle's merkle tree.
* @param _bundleSize Number of elements in the bundle's tree.
Expand All @@ -342,6 +366,11 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable {
uint256 _bundleSize,
string memory _configUri
) public {
require(
msg.sender == owner() || proposers[msg.sender] == true,
"ChugSplashManager: caller must be proposer or owner"
);

bytes32 bundleId = computeBundleId(_bundleRoot, _bundleSize, _configUri);
ChugSplashBundleState storage bundle = _bundles[bundleId];

Expand Down Expand Up @@ -798,6 +827,34 @@ contract ChugSplashManager is OwnableUpgradeable, ReentrancyGuardUpgradeable {
registry.announce("OwnerWithdrewETH");
}

/**
* @notice Allows the owner of this contract to add a proposer.
*
* @param _proposer Address of the proposer to add.
*/
function addProposer(address _proposer) external onlyOwner {
require(proposers[_proposer] == false, "ChugSplashManager: proposer was already added");

proposers[_proposer] = true;

emit ProposerAdded(_proposer, msg.sender);
registry.announce("ProposerAdded");
}

/**
* @notice Allows the owner of this contract to remove a proposer.
*
* @param _proposer Address of the proposer to remove.
*/
function removeProposer(address _proposer) external onlyOwner {
require(proposers[_proposer] == true, "ChugSplashManager: proposer was already removed");

proposers[_proposer] = false;

emit ProposerRemoved(_proposer, msg.sender);
registry.announce("ProposerRemoved");
}

/**
* @notice Allows anyone to send ETH to this contract.
*/
Expand Down
2 changes: 1 addition & 1 deletion packages/contracts/src/ifaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export const DefaultAdapterArtifact = require('../artifacts/contracts/adapters/D
export const ProxyArtifact = require('../artifacts/contracts/libraries/Proxy.sol/Proxy.json')
export const DeterministicProxyOwnerArtifact = require('../artifacts/contracts/DeterministicProxyOwner.sol/DeterministicProxyOwner.json')

export const buildInfo = require('../artifacts/build-info/e2689fa8b28e57eb1a58836e6f354f84.json')
export const buildInfo = require('../artifacts/build-info/d862807b4a352c50f5edd2644c7696f0.json')

export const ChugSplashRegistryABI = ChugSplashRegistryArtifact.abi
export const ChugSplashBootLoaderABI = ChugSplashBootLoaderArtifact.abi
Expand Down
90 changes: 88 additions & 2 deletions packages/contracts/test/ChugSplashManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ contract ChugSplashManager_Test is Test {

event OwnerWithdrewETH(address indexed owner, uint256 amount);

event ProposerAdded(address indexed proposer, address indexed owner);

event ProposerRemoved(address indexed proposer, address indexed owner);

event ETHDeposited(address indexed from, uint256 indexed amount);

bytes32 constant EIP1967_IMPLEMENTATION_KEY =
Expand All @@ -85,6 +89,7 @@ contract ChugSplashManager_Test is Test {
bytes32 bundleId = 0xacdebe08df0d7c9650dfca6191bc88c5a50eb099b18d5cfa663e6f0f11165e2b;
bytes32 bundleRoot = 0xbe5e67004de3ec5f536e2b5d1ccb2d6d924956a55a866706e97b671f33a8b648;

address proposer = address(64);
address owner = address(128);
address nonOwner = address(256);
address executor1 = address(512);
Expand Down Expand Up @@ -221,17 +226,34 @@ contract ChugSplashManager_Test is Test {
assertEq(manager.getSelectedExecutor(bundleId), executor1);
}

function test_proposeChugSplashBundle_revert_notProposerOrOwner() external {
vm.expectRevert("ChugSplashManager: caller must be proposer or owner");
vm.prank(executor1);
manager.proposeChugSplashBundle(bundleRoot, bundleSize, configUri);
}

// proposeChugSplashBundle:
// - reverts if bundle's status is not `EMPTY`
function test_proposeChugSplashBundle_revert_nonEmpty() external {
vm.startPrank(owner);
manager.proposeChugSplashBundle(bundleRoot, bundleSize, configUri);
vm.expectRevert("ChugSplashManager: bundle already exists");
manager.proposeChugSplashBundle(bundleRoot, bundleSize, configUri);
}

function test_proposeChugSplashBundle_success_proposer() external {
vm.prank(owner);
manager.addProposer(proposer);
test_proposeChugSplashBundle_success(proposer);
}

function test_proposeChugSplashBundle_success_owner() external {
test_proposeChugSplashBundle_success(owner);
}

// proposeChugSplashBundle:
// - updates bundles mapping
function test_proposeChugSplashBundle_success() external {
function test_proposeChugSplashBundle_success(address _caller) internal {
vm.expectEmit(true, true, true, true);
emit ChugSplashBundleProposed(bundleId, bundleRoot, bundleSize, configUri);
vm.expectCall(
Expand All @@ -242,6 +264,7 @@ contract ChugSplashManager_Test is Test {
)
);

vm.prank(_caller);
manager.proposeChugSplashBundle(bundleRoot, bundleSize, configUri);
ChugSplashBundleState memory bundle = manager.bundles(bundleId);
assertEq(uint8(bundle.status), uint8(ChugSplashBundleStatus.PROPOSED));
Expand Down Expand Up @@ -383,7 +406,7 @@ contract ChugSplashManager_Test is Test {

// We add 1 here to account for the Proxy deployment that occurs before the implementation deployment.
uint256 implementationDeploymentNonce = 1 + vm.getNonce(address(manager));

address implementationAddress = computeCreateAddress(address(manager), implementationDeploymentNonce);
assertEq(implementationAddress.code.length, 0);
uint256 initialTotalDebt = manager.totalDebt();
Expand Down Expand Up @@ -816,6 +839,69 @@ contract ChugSplashManager_Test is Test {
assertEq(Proxy(proxyAddress).admin(), executor1);
}

function test_addProposer_revert_nonOwner() external {
vm.prank(nonOwner);
vm.expectRevert('Ownable: caller is not the owner');
manager.addProposer(proposer);
}

function test_addProposer_revert_alreadyAdded() external {
vm.startPrank(owner);
manager.addProposer(proposer);
vm.expectRevert('ChugSplashManager: proposer was already added');
manager.addProposer(proposer);
}

function test_addProposer_success() external {
assertFalse(manager.proposers(proposer));

vm.expectEmit(true, true, true, true);
emit ProposerAdded(proposer, owner);
vm.expectCall(
address(registry),
abi.encodeCall(
ChugSplashRegistry.announce,
("ProposerAdded")
)
);
vm.prank(owner);
manager.addProposer(proposer);

assertTrue(manager.proposers(proposer));
}

function test_removeProposer_revert_nonOwner() external {
vm.prank(nonOwner);
vm.expectRevert('Ownable: caller is not the owner');
manager.removeProposer(proposer);
}

function test_removeProposer_revert_alreadyRemoved() external {
vm.prank(owner);
vm.expectRevert('ChugSplashManager: proposer was already removed');
manager.removeProposer(proposer);
}

function test_removeProposer_success() external {
vm.startPrank(owner);
manager.addProposer(proposer);

assertTrue(manager.proposers(proposer));

vm.expectEmit(true, true, true, true);
emit ProposerRemoved(proposer, owner);
vm.expectCall(
address(registry),
abi.encodeCall(
ChugSplashRegistry.announce,
("ProposerRemoved")
)
);
manager.removeProposer(proposer);

assertFalse(manager.proposers(proposer));
}

// withdrawOwnerETH:
// - reverts if not called by owner
function test_withdrawOwnerETH_revert_nonOwner() external {
Expand Down

0 comments on commit 40c7bfb

Please sign in to comment.