Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Update wallet details #2

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

feat: Update wallet details #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4fd82fa
add TODOs
Fooooooooooox Sep 10, 2022
1a83229
feat: add new owner
Fooooooooooox Sep 10, 2022
e1ac1d8
faet: add delete owner
Fooooooooooox Sep 11, 2022
b04a5f5
fix: fix bug can't delete first one
Fooooooooooox Sep 11, 2022
d162f7c
add tests for add/delete owners
Fooooooooooox Sep 11, 2022
892bf71
feat update threshold
Fooooooooooox Sep 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions contracts/ProxyAdminMultisig.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ contract ProxyAdminMultisig {
);
event Upgrade(address target, address implementation);
event ChangeAdmin(address target, address newAdmin);
event AddOwner(address);
event DeleteOwner(address);

modifier onlyMember() {
require(owners[msg.sender] != address(0), "NotOwner");
Expand Down Expand Up @@ -81,6 +83,51 @@ contract ProxyAdminMultisig {
emit Setup(msg.sender, _owners, ownersCount, threshold);
}

function addOwner(address _newOwner) external onlyMember {
require(owners[_newOwner] == address(0), "OwnerExists");
require(_newOwner != address(0) && _newOwner != Constants.SENTINEL_OWNER, "InvalidOwner");

address[] memory _ownersArr = _getOwners();
address _lastOne = _ownersArr[_ownersArr.length - 1];
owners[_lastOne] = _newOwner;
owners[_newOwner] = Constants.SENTINEL_OWNER;
ownersCount += 1;

emit AddOwner(_newOwner);
}

function deleteOwner(address _targetOwner) external onlyMember {
require(owners[_targetOwner] != address(0), "OwnerNotExists");
require(
_targetOwner != address(0) && _targetOwner != Constants.SENTINEL_OWNER,
"InvalidOwner"
);

address _followingOne = owners[_targetOwner];
address[] memory _ownersArr = _getOwners();
_ownersArr[_ownersArr.length - 1] = Constants.SENTINEL_OWNER;
for (uint256 i = 0; i < _ownersArr.length; i++) {
address owner = _ownersArr[i];
if (owners[owner] == _targetOwner) {
address _previousOne = owner;
owners[_previousOne] = _followingOne;
// delete target owner
owners[_targetOwner] = address(0);
ownersCount -= 1;
break;
}
}

emit DeleteOwner(_targetOwner);
}

function updateThreshold(uint256 _newThreshold) external onlyMember{
address[] memory _owners = _getOwners();
require(_newThreshold > 0, "ThresholdIsZero");
require(_newThreshold <= _owners.length, "ThresholdExceedsOwnersCount");
threshold = _newThreshold;
}

function propose(
address target,
string calldata proposalType,
Expand Down
105 changes: 105 additions & 0 deletions test/ProxyAdminMultisig.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ interface DumbEmitterEvents {
);
event Upgrade(address target, address implementation);
event ChangeAdmin(address target, address newAdmin);
event AddOwner(address);
event DeleteOwner(address);
}

contract MultisigTest is DumbEmitterEvents, Test, Utils {
Expand All @@ -44,6 +46,7 @@ contract MultisigTest is DumbEmitterEvents, Test, Utils {
address public daniel = address(0x4444);
address[] public ownersArr2 = [alice, bob];
address[] public ownersArr3 = [alice, bob, charlie];
address[] public ownersArr4 = [alice, bob, charlie, daniel];
address[] public replicatedOwners = [alice, alice];
address[] public zeroOwners = [alice, address(0x0)];
address[] public sentinelOwners = [alice, address(0x1)];
Expand Down Expand Up @@ -352,6 +355,108 @@ contract MultisigTest is DumbEmitterEvents, Test, Utils {
assertEq(_proposal.status, _status);
}

function testAddOwner() public {
_checkWalletDetail(2, 3, ownersArr3);
assert(!proxyAdminMultisig.isOwner(daniel));
vm.prank(alice);
expectEmit(CheckTopic1 | CheckTopic2 | CheckTopic3 | CheckData);
emit AddOwner(daniel);
proxyAdminMultisig.addOwner(daniel);
_checkWalletDetail(2, 4, ownersArr4);
assert(proxyAdminMultisig.isOwner(daniel));

// add a previously deleted owner
vm.prank(alice);
proxyAdminMultisig.deleteOwner(charlie);
address[] memory wallet3 = new address[](3);
wallet3[0] = alice;
wallet3[1] = bob;
wallet3[2] = daniel;
_checkWalletDetail(2, 3, wallet3);
assert(!proxyAdminMultisig.isOwner(charlie));
vm.prank(alice);
proxyAdminMultisig.addOwner(charlie);
address[] memory wallet4 = new address[](4);
wallet4[0] = alice;
wallet4[1] = bob;
wallet4[2] = daniel;
wallet4[3] = charlie;
_checkWalletDetail(2, 4, wallet4);
assert(proxyAdminMultisig.isOwner(charlie));
}

function testAddOwnerFail() public {
vm.expectRevert(abi.encodePacked("NotOwner"));
proxyAdminMultisig.addOwner(daniel);

vm.expectRevert(abi.encodePacked("InvalidOwner"));
vm.startPrank(alice);
proxyAdminMultisig.addOwner(address(0x0));

vm.expectRevert(abi.encodePacked("OwnerExists"));
proxyAdminMultisig.addOwner(Constants.SENTINEL_OWNER);

vm.expectRevert(abi.encodePacked("OwnerExists"));
proxyAdminMultisig.addOwner(alice);
}

function testDeleteOwner() public {
// alice delete bob
assert(proxyAdminMultisig.isOwner(bob));
vm.prank(alice);
expectEmit(CheckTopic1 | CheckTopic2 | CheckTopic3 | CheckData);
emit DeleteOwner(bob);
proxyAdminMultisig.deleteOwner(bob);
assert(!proxyAdminMultisig.isOwner(bob));
address[] memory wallet2 = new address[](2);
wallet2[0] = alice;
wallet2[1] = charlie;
_checkWalletDetail(2, 2, wallet2);

// alice delete charlie
assert(proxyAdminMultisig.isOwner(charlie));
vm.prank(alice);
proxyAdminMultisig.deleteOwner(charlie);
assert(!proxyAdminMultisig.isOwner(charlie));
address[] memory wallet1 = new address[](1);
wallet1[0] = alice;
_checkWalletDetail(2, 1, wallet1);

// add new owner then delete
vm.prank(alice);
proxyAdminMultisig.addOwner(daniel);
wallet2[1] = daniel;
_checkWalletDetail(2, 2, wallet2);
assert(proxyAdminMultisig.isOwner(daniel));
vm.prank(alice);
proxyAdminMultisig.deleteOwner(daniel);
assert(!proxyAdminMultisig.isOwner(daniel));
_checkWalletDetail(2, 1, wallet1);
}

function testUpdateThreshold() public {
vm.prank(alice);
proxyAdminMultisig.updateThreshold(3);
_checkWalletDetail(3, 3, ownersArr3);
}

function testUpdateThresholdFail() public {
// only owners can update threshold
vm.prank(daniel);
vm.expectRevert(abi.encodePacked("NotOwner"));
proxyAdminMultisig.updateThreshold(3);

// threshold can't be 0
vm.prank(alice);
vm.expectRevert(abi.encodePacked("ThresholdIsZero"));
proxyAdminMultisig.updateThreshold(0);

// threshold can't > ownerCount
vm.prank(alice);
vm.expectRevert(abi.encodePacked("ThresholdExceedsOwnersCount"));
proxyAdminMultisig.updateThreshold(4);
}

function _checkAllProposal(
uint256 _proposalId,
address _target,
Expand Down