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

Move PoolController,WithdrawController factory definitions to PoolFactory constructor #101

Merged
merged 1 commit into from
Nov 28, 2022
Merged

Move PoolController,WithdrawController factory definitions to PoolFactory constructor #101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
29 changes: 19 additions & 10 deletions contracts/PoolFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,24 @@ contract PoolFactory is IPoolFactory {
*/
address private _serviceConfiguration;

constructor(address serviceConfiguration) {
/**
* @dev Reference to the WithdrawControllerFactory contract
*/
address private _withdrawControllerFactory;

/**
* @dev Reference to the PoolControllerFactory contract
*/
address private _poolControllerFactory;

constructor(
address serviceConfiguration,
address withdrawControllerFactory,
address poolControllerFactory
) {
_serviceConfiguration = serviceConfiguration;
_withdrawControllerFactory = withdrawControllerFactory;
_poolControllerFactory = poolControllerFactory;
}

/**
Expand All @@ -24,8 +40,6 @@ contract PoolFactory is IPoolFactory {
*/
function createPool(
address liquidityAsset,
address withdrawControllerFactory,
address poolControllerFactory,
IPoolConfigurableSettings calldata settings
) public virtual returns (address poolAddress) {
require(
Expand All @@ -36,11 +50,6 @@ contract PoolFactory is IPoolFactory {
settings.withdrawRequestPeriodDuration > 0,
"PoolFactory: Invalid duration"
);
require(
withdrawControllerFactory != address(0) &&
poolControllerFactory != address(0),
"PoolFactory: Invalid address"
);
if (settings.fixedFee > 0) {
require(
settings.fixedFeeInterval > 0,
Expand All @@ -53,8 +62,8 @@ contract PoolFactory is IPoolFactory {
liquidityAsset,
msg.sender,
_serviceConfiguration,
withdrawControllerFactory,
poolControllerFactory,
_withdrawControllerFactory,
_poolControllerFactory,
settings,
"PerimeterPoolToken",
"PPT"
Expand Down
9 changes: 3 additions & 6 deletions contracts/interfaces/IPoolFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@ interface IPoolFactory {
* @dev Creates a pool's PoolAdmin controller
* @dev Emits `PoolControllerCreated` event.
*/
function createPool(
address,
address,
address,
IPoolConfigurableSettings calldata
) external returns (address);
function createPool(address, IPoolConfigurableSettings calldata)
external
returns (address);
}
26 changes: 20 additions & 6 deletions contracts/permissioned/PermissionedPoolFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,25 @@ contract PermissionedPoolFactory is IPoolFactory {
*/
address private _poolAccessControlFactory;

constructor(address serviceConfiguration, address poolAccessControlFactory)
{
/**
* @dev Reference to the WithdrawControllerFactory contract
*/
address private _withdrawControllerFactory;

/**
* @dev Reference to the PoolControllerFactory contract
*/
address private _poolControllerFactory;

constructor(
address serviceConfiguration,
address withdrawControllerFactory,
address poolControllerFactory,
address poolAccessControlFactory
) {
_serviceConfiguration = serviceConfiguration;
_withdrawControllerFactory = withdrawControllerFactory;
_poolControllerFactory = poolControllerFactory;
_poolAccessControlFactory = poolAccessControlFactory;
}

Expand All @@ -44,8 +60,6 @@ contract PermissionedPoolFactory is IPoolFactory {
*/
function createPool(
address liquidityAsset,
address withdrawControllerFactory,
address poolControllerFactory,
IPoolConfigurableSettings calldata settings
) public override onlyVerifiedPoolAdmin returns (address poolAddress) {
require(
Expand All @@ -57,8 +71,8 @@ contract PermissionedPoolFactory is IPoolFactory {
liquidityAsset,
msg.sender,
address(_serviceConfiguration),
withdrawControllerFactory,
poolControllerFactory,
_withdrawControllerFactory,
_poolControllerFactory,
_poolAccessControlFactory,
settings,
"PerimeterPoolToken",
Expand Down
86 changes: 17 additions & 69 deletions test/PoolFactory.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,6 @@ describe("PoolFactory", () => {
const PoolLib = await ethers.getContractFactory("PoolLib");
const poolLib = await PoolLib.deploy();

const PoolFactory = await ethers.getContractFactory("PoolFactory", {
libraries: {
PoolLib: poolLib.address
}
});
const poolFactory = await PoolFactory.deploy(serviceConfiguration.address);
await poolFactory.deployed();

const withdrawControllerFactory = await deployWithdrawControllerFactory(
poolLib.address,
serviceConfiguration.address
Expand All @@ -45,84 +37,40 @@ describe("PoolFactory", () => {
serviceConfiguration.address
);

const PoolFactory = await ethers.getContractFactory("PoolFactory", {
libraries: {
PoolLib: poolLib.address
}
});
const poolFactory = await PoolFactory.deploy(
serviceConfiguration.address,
withdrawControllerFactory.address,
poolControllerFactory.address
);
await poolFactory.deployed();

return {
poolFactory,
liquidityAsset,
withdrawControllerFactory,
poolControllerFactory
liquidityAsset
};
}

it("reverts if given a zero withdraw window", async () => {
const {
poolFactory,
withdrawControllerFactory,
poolControllerFactory,
liquidityAsset
} = await loadFixture(deployFixture);
const { poolFactory, liquidityAsset } = await loadFixture(deployFixture);

const poolSettings = Object.assign({}, DEFAULT_POOL_SETTINGS, {
withdrawRequestPeriodDuration: 0
});
await expect(
poolFactory.createPool(
liquidityAsset.address,
withdrawControllerFactory.address,
poolControllerFactory.address,
poolSettings
)
).to.be.revertedWith("PoolFactory: Invalid duration");
});

it("reverts if given an invalid pool withdraw controller factory", async () => {
const { poolFactory, poolControllerFactory, liquidityAsset } =
await loadFixture(deployFixture);

const poolSettings = Object.assign({}, DEFAULT_POOL_SETTINGS, {
withdrawRequestPeriodDuration: 0
});
await expect(
poolFactory.createPool(
/* liquidityAsset */ liquidityAsset.address,
"0x0000000000000000000000000000000000000000",
poolControllerFactory.address,
poolSettings
)
).to.be.revertedWith("PoolFactory: Invalid duration");
});

it("reverts if given an invalid pool admin controller factory", async () => {
const { poolFactory, withdrawControllerFactory, liquidityAsset } =
await loadFixture(deployFixture);

const poolSettings = Object.assign({}, DEFAULT_POOL_SETTINGS, {
withdrawRequestPeriodDuration: 0
});
await expect(
poolFactory.createPool(
/* liquidityAsset */ liquidityAsset.address,
withdrawControllerFactory.address,
"0x0000000000000000000000000000000000000000",
poolSettings
)
poolFactory.createPool(liquidityAsset.address, poolSettings)
).to.be.revertedWith("PoolFactory: Invalid duration");
});

it("emits PoolCreated", async () => {
const {
poolFactory,
liquidityAsset,
withdrawControllerFactory,
poolControllerFactory
} = await loadFixture(deployFixture);
const { poolFactory, liquidityAsset } = await loadFixture(deployFixture);

await expect(
poolFactory.createPool(
/* liquidityAsset */ liquidityAsset.address,
withdrawControllerFactory.address,
poolControllerFactory.address,
DEFAULT_POOL_SETTINGS
)
poolFactory.createPool(liquidityAsset.address, DEFAULT_POOL_SETTINGS)
).to.emit(poolFactory, "PoolCreated");
});
});
48 changes: 17 additions & 31 deletions test/permissioned/PermissionedPoolFactory.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,16 @@ describe("PermissionedPoolFactory", () => {
permissionedServiceConfiguration.address
);

const withdrawControllerFactory = await deployWithdrawControllerFactory(
poolLib.address,
permissionedServiceConfiguration.address
);

const poolControllerFactory = await deployPoolControllerFactory(
poolLib.address,
permissionedServiceConfiguration.address
);

// Deploy the PermissionedPoolFactory
const PoolFactory = await ethers.getContractFactory(
"PermissionedPoolFactory",
Expand All @@ -71,6 +81,8 @@ describe("PermissionedPoolFactory", () => {
);
const poolFactory = await PoolFactory.deploy(
permissionedServiceConfiguration.address,
withdrawControllerFactory.address,
poolControllerFactory.address,
poolAccessControlFactory.address
);
await poolFactory.deployed();
Expand All @@ -81,25 +93,13 @@ describe("PermissionedPoolFactory", () => {
);
await tx.wait();

const withdrawControllerFactory = await deployWithdrawControllerFactory(
poolLib.address,
permissionedServiceConfiguration.address
);

const poolControllerFactory = await deployPoolControllerFactory(
poolLib.address,
permissionedServiceConfiguration.address
);

return {
poolFactory,
poolAdminAccessControl,
operator,
otherAccount,
liquidityAsset,
tosAcceptanceRegistry,
withdrawControllerFactory,
poolControllerFactory
tosAcceptanceRegistry
};
}

Expand All @@ -109,9 +109,7 @@ describe("PermissionedPoolFactory", () => {
poolAdminAccessControl,
otherAccount,
liquidityAsset,
tosAcceptanceRegistry,
withdrawControllerFactory,
poolControllerFactory
tosAcceptanceRegistry
} = await loadFixture(deployFixture);

await tosAcceptanceRegistry.connect(otherAccount).acceptTermsOfService();
Expand All @@ -120,12 +118,7 @@ describe("PermissionedPoolFactory", () => {
await expect(
poolFactory
.connect(otherAccount)
.createPool(
liquidityAsset.address,
withdrawControllerFactory.address,
poolControllerFactory.address,
DEFAULT_POOL_SETTINGS
)
.createPool(liquidityAsset.address, DEFAULT_POOL_SETTINGS)
).to.emit(poolFactory, "PoolCreated");
});

Expand All @@ -135,21 +128,14 @@ describe("PermissionedPoolFactory", () => {
poolAdminAccessControl,
otherAccount,
liquidityAsset,
tosAcceptanceRegistry,
withdrawControllerFactory,
poolControllerFactory
tosAcceptanceRegistry
} = await loadFixture(deployFixture);

await tosAcceptanceRegistry.connect(otherAccount).acceptTermsOfService();
await poolAdminAccessControl.allow(otherAccount.getAddress());

await expect(
poolFactory.createPool(
liquidityAsset.address,
withdrawControllerFactory.address,
poolControllerFactory.address,
DEFAULT_POOL_SETTINGS
)
poolFactory.createPool(liquidityAsset.address, DEFAULT_POOL_SETTINGS)
).to.be.revertedWith("caller is not allowed pool admin");
});

Expand Down
Loading