Skip to content

Commit

Permalink
feat: replace getAllProposals as getProposal
Browse files Browse the repository at this point in the history
  • Loading branch information
@iavl
iavl committed Mar 28, 2024
1 parent ce64166 commit dd5d5c2
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 72 deletions.
5 changes: 2 additions & 3 deletions script/Deploy.s.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,8 @@ contract Deploy is Deployer {
ProxyAdminMultiSig multiSig = new ProxyAdminMultiSig(cfg.getOwners(), cfg.getThreshold());

// check states
(uint256 threshold, uint256 ownersCount, address[] memory owners) = multiSig.getWalletDetail();
require(threshold == cfg.getThreshold(), "Threshold mismatch");
require(ownersCount == owners.length, "Owners count mismatch");
require(multiSig.getThreshold() == cfg.getThreshold(), "Threshold mismatch");
address[] memory owners = multiSig.getOwners();
for (uint256 i = 0; i < owners.length; i++) {
require(multiSig.isOwner(owners[i]), "Owners mismatch");
}
Expand Down
26 changes: 10 additions & 16 deletions src/ProxyAdminMultiSig.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {Const} from "./libraries/Const.sol";
import {Events} from "./libraries/Events.sol";
import {IProxy} from "./interfaces/IProxy.sol";
import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";

contract ProxyAdminMultiSig is IErrors {
using EnumerableSet for EnumerableSet.AddressSet;
Expand Down Expand Up @@ -163,24 +162,19 @@ contract ProxyAdminMultiSig is IErrors {
}
}

/// @dev returns all proposals
function getAllProposals(uint256 offset, uint256 limit) external view returns (Proposal[] memory results) {
if (offset >= _proposalCount) return results;

uint256 len = Math.min(limit, _proposalCount - offset);
/// @dev returns a proposal
function getProposal(uint256 proposalId) external view returns (Proposal memory) {
return _proposals[proposalId];
}

results = new Proposal[](len);
for (uint256 i = offset; i < offset + len; i++) {
// plus 1 because proposalId starts from 1
results[i - offset] = _proposals[i + 1];
}
/// @dev returns the threshold of multi-sig wallet
function getThreshold() external view returns (uint256) {
return _threshold;
}

/// @dev returns wallet detail
function getWalletDetail() external view returns (uint256 threshold, uint256 ownersCount, address[] memory owners) {
threshold = _threshold;
ownersCount = _ownersCount;
owners = _owners.values();
/// @dev returns the owners of multi-sig wallet
function getOwners() external view returns (address[] memory) {
return _owners.values();
}

/// @dev get proposal count
Expand Down
66 changes: 13 additions & 53 deletions test/ProxyAdminMultiSig.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ contract MultiSigTest is IErrors, Test, Utils {
assertEq(multiSig.getProposalCount(), 1);

// check proposal status
_checkAllProposal(proposalId, proxy, Const.CHANGE_ADMIN, dan, "", 0, new address[](0), Const.STATUS_DELETED);
_checkProposal(proposalId, proxy, Const.CHANGE_ADMIN, dan, "", 0, new address[](0), Const.STATUS_DELETED);
}

function testDeleteProposalFail() public {
Expand Down Expand Up @@ -369,7 +369,7 @@ contract MultiSigTest is IErrors, Test, Utils {
multiSig.executeProposal(proposalId);

assertEq(_getImplementation(proxy), implementationV2);
_checkAllProposal(
_checkProposal(
proposalId,
proxy,
Const.TYPE_UPGRADE,
Expand Down Expand Up @@ -418,7 +418,7 @@ contract MultiSigTest is IErrors, Test, Utils {
multiSig.executeProposal(proposalId);

assertEq(_getImplementation(proxy), implementationV2);
_checkAllProposal(
_checkProposal(
proposalId,
proxy,
Const.TYPE_UPGRADE_AND_CALL,
Expand Down Expand Up @@ -453,7 +453,7 @@ contract MultiSigTest is IErrors, Test, Utils {
// check the admin has changed
assertEq(_getAdmin(proxy), dan);
assertEq(_getImplementation(proxy), implementationV1);
_checkAllProposal(proposalId, proxy, Const.CHANGE_ADMIN, dan, "", 2, array(alice, bob), Const.STATUS_EXECUTED);
_checkProposal(proposalId, proxy, Const.CHANGE_ADMIN, dan, "", 2, array(alice, bob), Const.STATUS_EXECUTED);
}

function testExecuteMultipleProposals() public {
Expand Down Expand Up @@ -486,7 +486,7 @@ contract MultiSigTest is IErrors, Test, Utils {
// check executed status
assertEq(_getImplementation(proxy), implementationV2);
assertEq(_getAdmin(proxy), dan);
_checkAllProposal(
_checkProposal(
1,
proxy,
Const.TYPE_UPGRADE,
Expand All @@ -496,7 +496,7 @@ contract MultiSigTest is IErrors, Test, Utils {
array(alice, charlie),
Const.STATUS_EXECUTED
);
_checkAllProposal(2, proxy, Const.CHANGE_ADMIN, dan, "", 2, array(alice, charlie), Const.STATUS_EXECUTED);
_checkProposal(2, proxy, Const.CHANGE_ADMIN, dan, "", 2, array(alice, charlie), Const.STATUS_EXECUTED);
}

function testExecuteProposalFail() public {
Expand Down Expand Up @@ -528,45 +528,6 @@ contract MultiSigTest is IErrors, Test, Utils {
multiSig.executeProposal(proposalId);
}

function testGetAllProposals() public {
vm.prank(alice);
multiSig.propose(proxy, Const.TYPE_UPGRADE, implementationV2, "");
vm.prank(bob);
multiSig.propose(proxy, Const.CHANGE_ADMIN, dan, "");
vm.prank(charlie);
multiSig.propose(
proxy,
Const.TYPE_UPGRADE_AND_CALL,
implementationV2,
abi.encodeWithSelector(UpgradeV2.increment.selector)
);

ProxyAdminMultiSig.Proposal[] memory proposals = multiSig.getAllProposals(0, 100);
assertEq(proposals.length, 3);
assertEq(multiSig.getAllProposals(0, 3).length, 3);
assertEq(multiSig.getAllProposals(1, 1).length, 1);
assertEq(multiSig.getAllProposals(1, 100).length, 2);

vm.prank(alice);
multiSig.approveProposal(1);
vm.prank(bob);
multiSig.approveProposal(1);
assertEq(multiSig.getAllProposals(0, 100).length, 3);

vm.prank(alice);
multiSig.deleteProposal(3);

vm.prank(alice);
multiSig.deleteProposal(2);
assertEq(multiSig.getAllProposals(0, 100).length, 3);
}

function testGetAllProposalsFail() public view {
// offset >= proposalCount returns nothing
ProxyAdminMultiSig.Proposal[] memory proposals = multiSig.getAllProposals(2, 2);
assertEq(proposals.length, 0);
}

function testGetPendingProposals() public {
assertEq(multiSig.getPendingProposals().length, 0);

Expand Down Expand Up @@ -602,9 +563,8 @@ contract MultiSigTest is IErrors, Test, Utils {
) internal view {
// get pending proposals and all proposals
ProxyAdminMultiSig.Proposal[] memory pendingProposals = multiSig.getPendingProposals();
ProxyAdminMultiSig.Proposal[] memory allProposals = multiSig.getAllProposals(proposalId - 1, 1);
// get proposal by _proposalId
ProxyAdminMultiSig.Proposal memory proposal = allProposals[proposalId - 1];
ProxyAdminMultiSig.Proposal memory proposal = multiSig.getProposal(proposalId);
// check if this id is in pending list
bool exist = false;
for (uint256 i = 0; i < pendingProposals.length; i++) {
Expand All @@ -623,7 +583,7 @@ contract MultiSigTest is IErrors, Test, Utils {
assertEq(proposal.status, status);
}

function _checkAllProposal(
function _checkProposal(
uint256 proposalId,
address proxy_,
string memory proposalType,
Expand All @@ -633,8 +593,7 @@ contract MultiSigTest is IErrors, Test, Utils {
address[] memory approvals,
string memory status
) internal view {
ProxyAdminMultiSig.Proposal[] memory proposals = multiSig.getAllProposals(proposalId - 1, 1);
ProxyAdminMultiSig.Proposal memory p = proposals[0];
ProxyAdminMultiSig.Proposal memory p = multiSig.getProposal(proposalId);
assertEq(p.proxy, proxy_);
assertEq(p.proposalType, proposalType);
assertEq(p.newAdminOrImplementation, adminOrImplementation);
Expand All @@ -645,9 +604,10 @@ contract MultiSigTest is IErrors, Test, Utils {
}

function _checkWalletDetail(uint256 threshold_, uint256 ownersCount_, address[] memory owners_) internal view {
(uint256 threshold, uint256 ownersCount, address[] memory owners) = multiSig.getWalletDetail();
assertEq(threshold, threshold_);
assertEq(ownersCount, ownersCount_);
assertEq(multiSig.getThreshold(), threshold_);

address[] memory owners = multiSig.getOwners();
assertEq(owners.length, ownersCount_);
assertEq(owners, owners_);
}

Expand Down

0 comments on commit dd5d5c2

Please sign in to comment.