Skip to content
This repository has been archived by the owner on Apr 27, 2022. It is now read-only.

[playground] Upgradeable authenticator: split proxy ownership and authenticator ownership #379

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 13 additions & 7 deletions src/contracts/GPv2AllowListAuthentication.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,26 @@ import "./interfaces/GPv2Authentication.sol";

/// @title Gnosis Protocol v2 Access Control Contract
/// @author Gnosis Developers
contract GPv2AllowListAuthentication is
GPv2Authentication,
StorageAccessible,
OwnableUpgradeable
{
contract GPv2AllowListAuthentication is GPv2Authentication, StorageAccessible {
using EnumerableSet for EnumerableSet.AddressSet;
address public manager;

EnumerableSet.AddressSet private solvers;

function addSolver(address solver) external onlyOwner {
function setManager(address _manager) public {
manager = _manager;
}

modifier onlyManager() {
require(manager == msg.sender, "caller is not the manager");
_;
}

function addSolver(address solver) external onlyManager {
solvers.add(solver);
}

function removeSolver(address solver) external onlyOwner {
function removeSolver(address solver) external onlyManager {
solvers.remove(solver);
}

Expand Down
5 changes: 2 additions & 3 deletions src/deploy/001_authenticator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,8 @@ const deployAuthenticator: DeployFunction = async function ({
gasLimit: 2000000,
deterministicDeployment: SALT,
log: true,
// proxy: {
// owner: owner,
// },
proxy: "setManager",
args: [owner],
});
};

Expand Down
3 changes: 2 additions & 1 deletion test/AllowListStorageReader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ describe("GPv2AllowListAuthentication", () => {
);

reader = await AllowListStorageReader.deploy();
authenticator = await GPv2AllowListAuthentication.connect(owner).deploy();
authenticator = await GPv2AllowListAuthentication.deploy();
await authenticator.setManager(owner.address);
allowListReader = new AllowListReader(authenticator, reader);
});

Expand Down
9 changes: 5 additions & 4 deletions test/GPv2AllowListAuthenticator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@ describe("GPv2AllowListAuthentication", () => {
);

authenticator = await GPv2AllowListAuthentication.deploy();
await authenticator.setManager(owner.address);
});

describe("constructor", () => {
it("should set the owner", async () => {
expect(await authenticator.owner()).to.equal(owner.address);
expect(await authenticator.manager()).to.equal(owner.address);
});
it("deployer is not the owner", async () => {
expect(await authenticator.owner()).not.to.be.equal(deployer.address);
expect(await authenticator.manager()).not.to.be.equal(deployer.address);
});
});

Expand All @@ -33,7 +34,7 @@ describe("GPv2AllowListAuthentication", () => {
it("should not allow non-owner to add solver", async () => {
await expect(
authenticator.connect(nonOwner).addSolver(solver.address),
).to.be.revertedWith("caller is not the owner");
).to.be.revertedWith("caller is not the manager");
});
});

Expand All @@ -46,7 +47,7 @@ describe("GPv2AllowListAuthentication", () => {
it("should not allow non-owner to remove solver", async () => {
await expect(
authenticator.connect(nonOwner).removeSolver(solver.address),
).to.be.revertedWith("caller is not the owner");
).to.be.revertedWith("caller is not the manager");
});
});

Expand Down