From 968cc6bfd9cb155debeeecd2fd096e623c59fbb8 Mon Sep 17 00:00:00 2001 From: Nenad Date: Wed, 19 Jul 2023 08:05:08 +0200 Subject: [PATCH 01/12] Add missing functions to IAccountsDonationMatch --- .../core/accounts/interfaces/IAccountsDonationMatch.sol | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/contracts/core/accounts/interfaces/IAccountsDonationMatch.sol b/contracts/core/accounts/interfaces/IAccountsDonationMatch.sol index 7c89c4961..f02941af5 100644 --- a/contracts/core/accounts/interfaces/IAccountsDonationMatch.sol +++ b/contracts/core/accounts/interfaces/IAccountsDonationMatch.sol @@ -1,6 +1,12 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.16; +import {AccountMessages} from "../message.sol"; + interface IAccountsDonationMatch { - function depositDonationMatchErC20(uint32 id, address token, uint256 amount) external; + function depositDonationMatchERC20(uint32 id, address token, uint256 amount) external; + + function withdrawDonationMatchERC20(uint32 id, address recipient, uint256 amount) external; + + function setupDonationMatch(uint32 id, AccountMessages.DonationMatch memory details) external; } From 7a60a40e04a741f1f63c4939ffb1634dbb2e06dd Mon Sep 17 00:00:00 2001 From: Nenad Date: Wed, 19 Jul 2023 08:05:28 +0200 Subject: [PATCH 02/12] Fix typos in AccountsDonationMatch (ErC20 -> ERC20) --- .../accounts/facets/AccountsDonationMatch.sol | 5 +- .../donation-match/DonationMatch.sol | 2 +- .../donation-match/DonationMatchCharity.sol | 2 +- .../subdao-token/SubDaoToken.sol | 2 +- test/core/accounts/AccountsDonationMatch.ts | 87 +++++++++++++++++++ 5 files changed, 92 insertions(+), 6 deletions(-) create mode 100644 test/core/accounts/AccountsDonationMatch.ts diff --git a/contracts/core/accounts/facets/AccountsDonationMatch.sol b/contracts/core/accounts/facets/AccountsDonationMatch.sol index 01f1e1907..90cda55f0 100644 --- a/contracts/core/accounts/facets/AccountsDonationMatch.sol +++ b/contracts/core/accounts/facets/AccountsDonationMatch.sol @@ -31,7 +31,7 @@ contract AccountsDonationMatch is ReentrancyGuardFacet, IAccountsEvents, IAccoun * @param id Endowment ID * @param amount Amount of DAOToken to deposit */ - function depositDonationMatchErC20(uint32 id, address token, uint256 amount) public { + function depositDonationMatchERC20(uint32 id, address token, uint256 amount) public { AccountStorage.State storage state = LibAccounts.diamondStorage(); AccountStorage.Endowment storage tempEndowment = state.ENDOWMENTS[id]; @@ -57,7 +57,7 @@ contract AccountsDonationMatch is ReentrancyGuardFacet, IAccountsEvents, IAccoun * @param recipient Recipient address * @param amount Amount of DAOToken to withdraw */ - function withdrawDonationMatchErC20(uint32 id, address recipient, uint256 amount) public { + function withdrawDonationMatchERC20(uint32 id, address recipient, uint256 amount) public { AccountStorage.State storage state = LibAccounts.diamondStorage(); AccountStorage.Endowment storage tempEndowment = state.ENDOWMENTS[id]; @@ -134,7 +134,6 @@ contract AccountsDonationMatch is ReentrancyGuardFacet, IAccountsEvents, IAccoun // TODO: add donation match address?? : state.ENDOWMENTS[id].donationMatchContract = donationMatch; - // Shouldn't this be emitted from contracts/normalized_endowment/donation-match/DonationMatch.sol > initialize ? IDonationMatchEmitter(registrar_config.donationMatchEmitter).initializeDonationMatch( id, donationMatch, diff --git a/contracts/normalized_endowment/donation-match/DonationMatch.sol b/contracts/normalized_endowment/donation-match/DonationMatch.sol index 302572676..3a54019d1 100644 --- a/contracts/normalized_endowment/donation-match/DonationMatch.sol +++ b/contracts/normalized_endowment/donation-match/DonationMatch.sol @@ -131,7 +131,7 @@ contract DonationMatch is IDonationMatching, Storage, Initializable { "Approve failed" ); - IAccountsDonationMatch(registrar_config.accountsContract).depositDonationMatchErC20( + IAccountsDonationMatch(registrar_config.accountsContract).depositDonationMatchERC20( endowmentId, token, endowmentAmount diff --git a/contracts/normalized_endowment/donation-match/DonationMatchCharity.sol b/contracts/normalized_endowment/donation-match/DonationMatchCharity.sol index 0926ec274..653c66595 100644 --- a/contracts/normalized_endowment/donation-match/DonationMatchCharity.sol +++ b/contracts/normalized_endowment/donation-match/DonationMatchCharity.sol @@ -124,7 +124,7 @@ contract DonationMatchCharity is IDonationMatching, Storage, Initializable, Reen IERC20Burnable(token).safeApprove(registrar_config.accountsContract, endowmentAmount); - IAccountsDonationMatch(registrar_config.accountsContract).depositDonationMatchErC20( + IAccountsDonationMatch(registrar_config.accountsContract).depositDonationMatchERC20( endowmentId, token, endowmentAmount diff --git a/contracts/normalized_endowment/subdao-token/SubDaoToken.sol b/contracts/normalized_endowment/subdao-token/SubDaoToken.sol index d832711d7..24e8c8ffe 100644 --- a/contracts/normalized_endowment/subdao-token/SubDaoToken.sol +++ b/contracts/normalized_endowment/subdao-token/SubDaoToken.sol @@ -85,7 +85,7 @@ contract SubDaoToken is ISubDaoToken, Storage, ContinuousToken { require(IERC20(address(this)).approve(accountscontract, endowmentAmount), "Approve failed"); - IAccountsDonationMatch(accountscontract).depositDonationMatchErC20( + IAccountsDonationMatch(accountscontract).depositDonationMatchERC20( endowmentId, address(this), endowmentAmount diff --git a/test/core/accounts/AccountsDonationMatch.ts b/test/core/accounts/AccountsDonationMatch.ts new file mode 100644 index 000000000..0f597d86b --- /dev/null +++ b/test/core/accounts/AccountsDonationMatch.ts @@ -0,0 +1,87 @@ +import {FakeContract, smock} from "@defi-wonderland/smock"; +import {SignerWithAddress} from "@nomiclabs/hardhat-ethers/signers"; +import {expect, use} from "chai"; +import {BigNumber} from "ethers"; +import hre from "hardhat"; +import { + DEFAULT_CHARITY_ENDOWMENT, + DEFAULT_REGISTRAR_CONFIG, + DEFAULT_STRATEGY_SELECTOR, +} from "test/utils"; +import { + AccountsDonationMatch, + AccountsDonationMatch__factory, + Registrar, + Registrar__factory, + TestFacetProxyContract, +} from "typechain-types"; +import {LibAccounts} from "typechain-types/contracts/core/accounts/facets/AccountsDonationMatch"; +import {RegistrarStorage} from "typechain-types/contracts/core/registrar/Registrar"; +import {AccountStorage} from "typechain-types/contracts/test/accounts/TestFacetProxyContract"; +import {genWallet, getSigners} from "utils"; +import {deployFacetAsProxy} from "./utils/deployTestFacet"; + +use(smock.matchers); + +describe("AccountsDonationMatch", function () { + const {ethers} = hre; + + const accountId = 1; + + let accOwner: SignerWithAddress; + let proxyAdmin: SignerWithAddress; + let endowOwner: SignerWithAddress; + + let facet: AccountsDonationMatch; + let state: TestFacetProxyContract; + + let endowment: AccountStorage.EndowmentStruct; + let treasuryAddress: string; + + let registrarFake: FakeContract; + + before(async function () { + const signers = await getSigners(hre); + accOwner = signers.apTeam1; + proxyAdmin = signers.proxyAdmin; + endowOwner = signers.deployer; + treasuryAddress = signers.apTeam2.address; + + endowment = {...DEFAULT_CHARITY_ENDOWMENT, owner: endowOwner.address}; + }); + + beforeEach(async function () { + let Facet = new AccountsDonationMatch__factory(accOwner); + let facetImpl = await Facet.deploy(); + state = await deployFacetAsProxy(hre, accOwner, proxyAdmin, facetImpl.address); + + registrarFake = await smock.fake(new Registrar__factory(), { + address: genWallet().address, + }); + + const config: RegistrarStorage.ConfigStruct = { + ...DEFAULT_REGISTRAR_CONFIG, + treasury: treasuryAddress, + }; + registrarFake.queryConfig.returns(config); + + await state.setEndowmentDetails(accountId, endowment); + await state.setConfig({ + owner: accOwner.address, + version: "1", + networkName: "Polygon", + registrarContract: registrarFake.address, + nextAccountId: accountId + 1, + maxGeneralCategoryId: 1, + subDao: ethers.constants.AddressZero, + earlyLockedWithdrawFee: {bps: 1000, payoutAddress: ethers.constants.AddressZero}, + reentrancyGuardLocked: false, + }); + + facet = AccountsDonationMatch__factory.connect(state.address, endowOwner); + }); + + describe("upon depositDonationMatchERC20", () => { + it("reverts "); + }); +}); From 3d00c0f271f9617a15b285acc61a92e4fdbc8de7 Mon Sep 17 00:00:00 2001 From: Nenad Date: Wed, 19 Jul 2023 08:10:19 +0200 Subject: [PATCH 03/12] Add/fix comments --- .../accounts/facets/AccountsDonationMatch.sol | 7 +++--- .../interfaces/IAccountsDonationMatch.sol | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/contracts/core/accounts/facets/AccountsDonationMatch.sol b/contracts/core/accounts/facets/AccountsDonationMatch.sol index 90cda55f0..31926aa9d 100644 --- a/contracts/core/accounts/facets/AccountsDonationMatch.sol +++ b/contracts/core/accounts/facets/AccountsDonationMatch.sol @@ -17,10 +17,8 @@ import {ProxyContract} from "../../proxy.sol"; import {IAccountsDonationMatch} from "../interfaces/IAccountsDonationMatch.sol"; /** - * @title AccountsDeployContract - * @notice This contract is used to deploy contracts from accounts diamond - * @dev Created so that deploying facets (which call this) don't have size conflicts - * @dev Is always going to be called by address(this) + * @title AccountsDonationMatch + * @dev This contract is used to manage donation match tokens */ contract AccountsDonationMatch is ReentrancyGuardFacet, IAccountsEvents, IAccountsDonationMatch { using SafeERC20 for IERC20; @@ -29,6 +27,7 @@ contract AccountsDonationMatch is ReentrancyGuardFacet, IAccountsEvents, IAccoun * @notice Deposit DAOToken(or Halo) to the endowment and store its balance * @dev Function manages reserve token sent by donation matching contract * @param id Endowment ID + * @param token DAOToken or HALO address * @param amount Amount of DAOToken to deposit */ function depositDonationMatchERC20(uint32 id, address token, uint256 amount) public { diff --git a/contracts/core/accounts/interfaces/IAccountsDonationMatch.sol b/contracts/core/accounts/interfaces/IAccountsDonationMatch.sol index f02941af5..92fc0930e 100644 --- a/contracts/core/accounts/interfaces/IAccountsDonationMatch.sol +++ b/contracts/core/accounts/interfaces/IAccountsDonationMatch.sol @@ -3,10 +3,34 @@ pragma solidity ^0.8.16; import {AccountMessages} from "../message.sol"; +/** + * @title AccountsDonationMatch + * @dev This contract is used to manage donation match tokens + */ interface IAccountsDonationMatch { + /** + * @notice Deposit DAOToken(or Halo) to the endowment and store its balance + * @dev Function manages reserve token sent by donation matching contract + * @param id Endowment ID + * @param token DAOToken or HALO address + * @param amount Amount of DAOToken to deposit + */ function depositDonationMatchERC20(uint32 id, address token, uint256 amount) external; + /** + * @notice Withdraw DAOToken(or Halo) from the endowment + * @dev Function manages reserve token sent by donation matching contract + * @param id Endowment ID + * @param recipient Recipient address + * @param amount Amount of DAOToken to withdraw + */ function withdrawDonationMatchERC20(uint32 id, address recipient, uint256 amount) external; + /** + * @notice This function creates a donation match contract for an endowment + * @dev creates a donation match contract for an endowment based on parameters (performs donation matching for contract against USDC) + * @param id The id of the endowment + * @param details The details of the donation match contract + */ function setupDonationMatch(uint32 id, AccountMessages.DonationMatch memory details) external; } From 7e734f2025beeff9715008dffaf85f46cc8edfd7 Mon Sep 17 00:00:00 2001 From: Nenad Date: Wed, 19 Jul 2023 08:28:26 +0200 Subject: [PATCH 04/12] Add tests for depositDonationMatchERC20 --- test/core/accounts/AccountsDonationMatch.ts | 70 +++++++++++++++------ 1 file changed, 51 insertions(+), 19 deletions(-) diff --git a/test/core/accounts/AccountsDonationMatch.ts b/test/core/accounts/AccountsDonationMatch.ts index 0f597d86b..3e340e752 100644 --- a/test/core/accounts/AccountsDonationMatch.ts +++ b/test/core/accounts/AccountsDonationMatch.ts @@ -11,11 +11,12 @@ import { import { AccountsDonationMatch, AccountsDonationMatch__factory, + DummyERC20, + DummyERC20__factory, Registrar, Registrar__factory, TestFacetProxyContract, } from "typechain-types"; -import {LibAccounts} from "typechain-types/contracts/core/accounts/facets/AccountsDonationMatch"; import {RegistrarStorage} from "typechain-types/contracts/core/registrar/Registrar"; import {AccountStorage} from "typechain-types/contracts/test/accounts/TestFacetProxyContract"; import {genWallet, getSigners} from "utils"; @@ -26,7 +27,7 @@ use(smock.matchers); describe("AccountsDonationMatch", function () { const {ethers} = hre; - const accountId = 1; + const endowId = 1; let accOwner: SignerWithAddress; let proxyAdmin: SignerWithAddress; @@ -35,9 +36,8 @@ describe("AccountsDonationMatch", function () { let facet: AccountsDonationMatch; let state: TestFacetProxyContract; - let endowment: AccountStorage.EndowmentStruct; - let treasuryAddress: string; - + let daoTokenFake: FakeContract; + let haloTokenFake: FakeContract; let registrarFake: FakeContract; before(async function () { @@ -45,43 +45,75 @@ describe("AccountsDonationMatch", function () { accOwner = signers.apTeam1; proxyAdmin = signers.proxyAdmin; endowOwner = signers.deployer; - treasuryAddress = signers.apTeam2.address; - - endowment = {...DEFAULT_CHARITY_ENDOWMENT, owner: endowOwner.address}; }); beforeEach(async function () { - let Facet = new AccountsDonationMatch__factory(accOwner); - let facetImpl = await Facet.deploy(); + const Facet = new AccountsDonationMatch__factory(accOwner); + const facetImpl = await Facet.deploy(); state = await deployFacetAsProxy(hre, accOwner, proxyAdmin, facetImpl.address); - registrarFake = await smock.fake(new Registrar__factory(), { - address: genWallet().address, - }); + facet = AccountsDonationMatch__factory.connect(state.address, endowOwner); + + registrarFake = await smock.fake(new Registrar__factory()); + daoTokenFake = await smock.fake(new DummyERC20__factory()); + haloTokenFake = await smock.fake(new DummyERC20__factory()); const config: RegistrarStorage.ConfigStruct = { ...DEFAULT_REGISTRAR_CONFIG, - treasury: treasuryAddress, + haloToken: haloTokenFake.address, }; registrarFake.queryConfig.returns(config); - await state.setEndowmentDetails(accountId, endowment); + const endowment: AccountStorage.EndowmentStruct = { + ...DEFAULT_CHARITY_ENDOWMENT, + owner: endowOwner.address, + daoToken: daoTokenFake.address, + }; + await state.setEndowmentDetails(endowId, endowment); await state.setConfig({ owner: accOwner.address, version: "1", networkName: "Polygon", registrarContract: registrarFake.address, - nextAccountId: accountId + 1, + nextAccountId: endowId + 1, maxGeneralCategoryId: 1, subDao: ethers.constants.AddressZero, earlyLockedWithdrawFee: {bps: 1000, payoutAddress: ethers.constants.AddressZero}, reentrancyGuardLocked: false, }); - - facet = AccountsDonationMatch__factory.connect(state.address, endowOwner); }); describe("upon depositDonationMatchERC20", () => { - it("reverts "); + it("reverts if the token is neither HALO nor DAOToken", async () => { + await expect( + facet.depositDonationMatchERC20(endowId, genWallet().address, 10) + ).to.be.revertedWith("Invalid Token"); + }); + + it("passes when token used is HALO", async () => { + const prevBalance = await state.getDaoTokenBalance(endowId); + const amount = BigNumber.from(10); + + haloTokenFake.transferFrom.returns(true); + + await expect(facet.depositDonationMatchERC20(endowId, haloTokenFake.address, amount)) + .to.emit(facet, "DonationDeposited") + .withArgs(endowId, haloTokenFake.address, amount); + + expect(await state.getDaoTokenBalance(endowId)).to.equal(prevBalance.add(amount)); + }); + + it("passes when token used is DAOToken", async () => { + const prevBalance = await state.getDaoTokenBalance(endowId); + const amount = BigNumber.from(10); + + daoTokenFake.transferFrom.returns(true); + + await expect(facet.depositDonationMatchERC20(endowId, daoTokenFake.address, amount)) + .to.emit(facet, "DonationDeposited") + .withArgs(endowId, daoTokenFake.address, amount); + + expect(await state.getDaoTokenBalance(endowId)).to.equal(prevBalance.add(amount)); + }); }); }); From f6308bc67fcd02a2f7bf31b02ae7a61e3244b093 Mon Sep 17 00:00:00 2001 From: Nenad Date: Wed, 19 Jul 2023 08:39:32 +0200 Subject: [PATCH 05/12] Add withdrawDonationMatchERC20 tests --- .../accounts/facets/AccountsDonationMatch.sol | 4 +- test/core/accounts/AccountsDonationMatch.ts | 53 +++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/contracts/core/accounts/facets/AccountsDonationMatch.sol b/contracts/core/accounts/facets/AccountsDonationMatch.sol index 31926aa9d..dddd18e58 100644 --- a/contracts/core/accounts/facets/AccountsDonationMatch.sol +++ b/contracts/core/accounts/facets/AccountsDonationMatch.sol @@ -61,9 +61,9 @@ contract AccountsDonationMatch is ReentrancyGuardFacet, IAccountsEvents, IAccoun AccountStorage.Endowment storage tempEndowment = state.ENDOWMENTS[id]; - require(amount > 0, "amount should be grater than 0"); + require(amount > 0, "amount should be greater than 0"); - require(msg.sender == tempEndowment.owner, "UnAuthorized"); + require(msg.sender == tempEndowment.owner, "Unauthorized"); require(state.DAOTOKENBALANCE[id] >= amount, "Insufficient amount"); diff --git a/test/core/accounts/AccountsDonationMatch.ts b/test/core/accounts/AccountsDonationMatch.ts index 3e340e752..da9f39198 100644 --- a/test/core/accounts/AccountsDonationMatch.ts +++ b/test/core/accounts/AccountsDonationMatch.ts @@ -32,6 +32,7 @@ describe("AccountsDonationMatch", function () { let accOwner: SignerWithAddress; let proxyAdmin: SignerWithAddress; let endowOwner: SignerWithAddress; + let user: SignerWithAddress; let facet: AccountsDonationMatch; let state: TestFacetProxyContract; @@ -45,6 +46,7 @@ describe("AccountsDonationMatch", function () { accOwner = signers.apTeam1; proxyAdmin = signers.proxyAdmin; endowOwner = signers.deployer; + user = signers.apTeam2; }); beforeEach(async function () { @@ -116,4 +118,55 @@ describe("AccountsDonationMatch", function () { expect(await state.getDaoTokenBalance(endowId)).to.equal(prevBalance.add(amount)); }); }); + + describe("upon withdrawDonationMatchERC20", () => { + it("reverts if amount is 0 (zero)", async () => { + await expect( + facet.withdrawDonationMatchERC20(endowId, genWallet().address, 0) + ).to.be.revertedWith("amount should be greater than 0"); + }); + + it("reverts if sender is not the endowment owner", async () => { + await expect( + facet.connect(user).withdrawDonationMatchERC20(endowId, genWallet().address, 10) + ).to.be.revertedWith("Unauthorized"); + }); + + it("reverts if endowment's DAO token balance is smaller than the requested amount", async () => { + // current DAO token balance is not set, which makes it 0 (zero) by default + await expect( + facet.withdrawDonationMatchERC20(endowId, genWallet().address, 10) + ).to.be.revertedWith("Insufficient amount"); + }); + + it("passes if DAO token balance is equal to the requested amount", async () => { + const amount = 10; + const recipient = genWallet().address; + + await state.setDaoTokenBalance(endowId, amount); + + daoTokenFake.transfer.whenCalledWith(recipient, amount).returns(true); + + await expect(facet.withdrawDonationMatchERC20(endowId, recipient, amount)) + .to.emit(facet, "DonationWithdrawn") + .withArgs(endowId, recipient, daoTokenFake.address, amount); + + expect(await state.getDaoTokenBalance(endowId)).to.equal(0); + }); + + it("passes if DAO token balance is greater than the requested amount", async () => { + const amount = 10; + const recipient = genWallet().address; + + await state.setDaoTokenBalance(endowId, amount + 1); + + daoTokenFake.transfer.whenCalledWith(recipient, amount).returns(true); + + await expect(facet.withdrawDonationMatchERC20(endowId, recipient, amount)) + .to.emit(facet, "DonationWithdrawn") + .withArgs(endowId, recipient, daoTokenFake.address, amount); + + expect(await state.getDaoTokenBalance(endowId)).to.equal(1); + }); + }); }); From 6aa49b0550a6999875b09da131de344198365601 Mon Sep 17 00:00:00 2001 From: Nenad Date: Wed, 19 Jul 2023 08:43:25 +0200 Subject: [PATCH 06/12] Add comment --- contracts/core/accounts/facets/AccountsDonationMatch.sol | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/contracts/core/accounts/facets/AccountsDonationMatch.sol b/contracts/core/accounts/facets/AccountsDonationMatch.sol index dddd18e58..0d5455a31 100644 --- a/contracts/core/accounts/facets/AccountsDonationMatch.sol +++ b/contracts/core/accounts/facets/AccountsDonationMatch.sol @@ -18,7 +18,7 @@ import {IAccountsDonationMatch} from "../interfaces/IAccountsDonationMatch.sol"; /** * @title AccountsDonationMatch - * @dev This contract is used to manage donation match tokens + * @dev This contract is used to manage donation match tokens and to setup a donation match contract for an endowment */ contract AccountsDonationMatch is ReentrancyGuardFacet, IAccountsEvents, IAccountsDonationMatch { using SafeERC20 for IERC20; @@ -86,7 +86,6 @@ contract AccountsDonationMatch is ReentrancyGuardFacet, IAccountsEvents, IAccoun AccountStorage.State storage state = LibAccounts.diamondStorage(); AccountStorage.Endowment memory tempEndowment = state.ENDOWMENTS[id]; - // AccountStorage.Config memory tempConfig = state.config; require(msg.sender == tempEndowment.owner, "Unauthorized"); From 2dc1366d4a37cccd5143e6823723dc5f70d2512f Mon Sep 17 00:00:00 2001 From: Nenad Date: Wed, 19 Jul 2023 08:54:06 +0200 Subject: [PATCH 07/12] Improve error messages --- .../accounts/facets/AccountsDonationMatch.sol | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/contracts/core/accounts/facets/AccountsDonationMatch.sol b/contracts/core/accounts/facets/AccountsDonationMatch.sol index 0d5455a31..6da61233f 100644 --- a/contracts/core/accounts/facets/AccountsDonationMatch.sol +++ b/contracts/core/accounts/facets/AccountsDonationMatch.sol @@ -89,25 +89,31 @@ contract AccountsDonationMatch is ReentrancyGuardFacet, IAccountsEvents, IAccoun require(msg.sender == tempEndowment.owner, "Unauthorized"); - require(tempEndowment.owner != address(0), "AD E02"); //A DAO does not exist yet for this Endowment. Please set that up first. - require(tempEndowment.donationMatchContract == address(0), "AD E03"); // A Donation Match contract already exists for this Endowment + require(tempEndowment.owner != address(0), "A DAO does not exist yet for this Endowment"); + require( + tempEndowment.donationMatchContract == address(0), + "A Donation Match contract already exists for this Endowment" + ); - require(details.data.uniswapFactory != address(0), "Invalid Data"); - require(details.data.poolFee != 0, "Invalid Data"); + require(details.data.uniswapFactory != address(0), "Invalid UniswapFactory address"); + require(details.data.poolFee != 0, "Invalid pool fee"); RegistrarStorage.Config memory registrar_config = IRegistrar(state.config.registrarContract) .queryConfig(); - require(registrar_config.donationMatchContract != address(0), "AD E04"); // No implementation for donation matching contract - require(registrar_config.usdcAddress != address(0), "AD E05"); // Invalid Registrar Data + require( + registrar_config.donationMatchContract != address(0), + "Missing implementation for donation matching contract" + ); + require(registrar_config.usdcAddress != address(0), "Missing USDC address in Registrar"); address inputtoken; if (details.enumData == AccountMessages.DonationMatchEnum.HaloTokenReserve) { - require(registrar_config.haloToken != address(0), "AD E05"); // Invalid Registrar Data + require(registrar_config.haloToken != address(0), "Invalid HALO address in Registrar"); inputtoken = registrar_config.haloToken; } else { - require(details.data.reserveToken != address(0), "Invalid Data"); + require(details.data.reserveToken != address(0), "Invalid reserve token address"); inputtoken = details.data.reserveToken; } From fa3622423110400a5f03a4a03d65c7b0686db44b Mon Sep 17 00:00:00 2001 From: Nenad Date: Wed, 19 Jul 2023 08:58:50 +0200 Subject: [PATCH 08/12] Remove redundant tempEndowment.owner != address(0) check --- contracts/core/accounts/facets/AccountsDonationMatch.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/core/accounts/facets/AccountsDonationMatch.sol b/contracts/core/accounts/facets/AccountsDonationMatch.sol index 6da61233f..95ac296a3 100644 --- a/contracts/core/accounts/facets/AccountsDonationMatch.sol +++ b/contracts/core/accounts/facets/AccountsDonationMatch.sol @@ -89,7 +89,6 @@ contract AccountsDonationMatch is ReentrancyGuardFacet, IAccountsEvents, IAccoun require(msg.sender == tempEndowment.owner, "Unauthorized"); - require(tempEndowment.owner != address(0), "A DAO does not exist yet for this Endowment"); require( tempEndowment.donationMatchContract == address(0), "A Donation Match contract already exists for this Endowment" From 70af824c432daf2e236324f4eb0a9aa3f55ea114 Mon Sep 17 00:00:00 2001 From: Nenad Date: Wed, 19 Jul 2023 09:02:58 +0200 Subject: [PATCH 09/12] Fix USDC error message --- contracts/core/accounts/facets/AccountsDonationMatch.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/core/accounts/facets/AccountsDonationMatch.sol b/contracts/core/accounts/facets/AccountsDonationMatch.sol index 95ac296a3..72132e259 100644 --- a/contracts/core/accounts/facets/AccountsDonationMatch.sol +++ b/contracts/core/accounts/facets/AccountsDonationMatch.sol @@ -104,7 +104,7 @@ contract AccountsDonationMatch is ReentrancyGuardFacet, IAccountsEvents, IAccoun registrar_config.donationMatchContract != address(0), "Missing implementation for donation matching contract" ); - require(registrar_config.usdcAddress != address(0), "Missing USDC address in Registrar"); + require(registrar_config.usdcAddress != address(0), "Invalid USDC address in Registrar"); address inputtoken; if (details.enumData == AccountMessages.DonationMatchEnum.HaloTokenReserve) { From 9f2b8e12b2e8d436ff547b7554010dd37399540e Mon Sep 17 00:00:00 2001 From: Nenad Date: Wed, 19 Jul 2023 09:31:20 +0200 Subject: [PATCH 10/12] Add comment --- contracts/core/accounts/facets/AccountsDonationMatch.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/core/accounts/facets/AccountsDonationMatch.sol b/contracts/core/accounts/facets/AccountsDonationMatch.sol index 72132e259..a9eff4212 100644 --- a/contracts/core/accounts/facets/AccountsDonationMatch.sol +++ b/contracts/core/accounts/facets/AccountsDonationMatch.sol @@ -109,7 +109,6 @@ contract AccountsDonationMatch is ReentrancyGuardFacet, IAccountsEvents, IAccoun address inputtoken; if (details.enumData == AccountMessages.DonationMatchEnum.HaloTokenReserve) { require(registrar_config.haloToken != address(0), "Invalid HALO address in Registrar"); - inputtoken = registrar_config.haloToken; } else { require(details.data.reserveToken != address(0), "Invalid reserve token address"); @@ -131,6 +130,7 @@ contract AccountsDonationMatch is ReentrancyGuardFacet, IAccountsEvents, IAccoun registrar_config.donationMatchEmitter ); + // TODO: might be a good idea to create this contract using a factory, will improve testability as well address donationMatch = address( new ProxyContract(registrar_config.donationMatchContract, registrar_config.proxyAdmin, data) ); From c8ce97e49178c9c7b9237c7f7cf85a4fc150ae78 Mon Sep 17 00:00:00 2001 From: Nenad Date: Wed, 19 Jul 2023 09:41:09 +0200 Subject: [PATCH 11/12] Add setupDonationMatch test cases --- test/core/accounts/AccountsDonationMatch.ts | 168 +++++++++++++++++++- test/utils/helpers/types.ts | 5 + 2 files changed, 172 insertions(+), 1 deletion(-) diff --git a/test/core/accounts/AccountsDonationMatch.ts b/test/core/accounts/AccountsDonationMatch.ts index da9f39198..07ecd5637 100644 --- a/test/core/accounts/AccountsDonationMatch.ts +++ b/test/core/accounts/AccountsDonationMatch.ts @@ -7,10 +7,15 @@ import { DEFAULT_CHARITY_ENDOWMENT, DEFAULT_REGISTRAR_CONFIG, DEFAULT_STRATEGY_SELECTOR, + DonationMatchEnum, } from "test/utils"; import { AccountsDonationMatch, AccountsDonationMatch__factory, + DonationMatch, + DonationMatchEmitter, + DonationMatchEmitter__factory, + DonationMatch__factory, DummyERC20, DummyERC20__factory, Registrar, @@ -21,6 +26,8 @@ import {RegistrarStorage} from "typechain-types/contracts/core/registrar/Registr import {AccountStorage} from "typechain-types/contracts/test/accounts/TestFacetProxyContract"; import {genWallet, getSigners} from "utils"; import {deployFacetAsProxy} from "./utils/deployTestFacet"; +import {AccountMessages} from "typechain-types/contracts/core/accounts/facets/AccountsDonationMatch"; +import {DonationMatchStorage} from "typechain-types/contracts/normalized_endowment/donation-match/DonationMatchEmitter"; use(smock.matchers); @@ -29,6 +36,8 @@ describe("AccountsDonationMatch", function () { const endowId = 1; + const usdcAddress = genWallet().address; + let accOwner: SignerWithAddress; let proxyAdmin: SignerWithAddress; let endowOwner: SignerWithAddress; @@ -38,6 +47,8 @@ describe("AccountsDonationMatch", function () { let state: TestFacetProxyContract; let daoTokenFake: FakeContract; + let donationMatchFake: FakeContract; + let donationMatchEmitterFake: FakeContract; let haloTokenFake: FakeContract; let registrarFake: FakeContract; @@ -56,13 +67,21 @@ describe("AccountsDonationMatch", function () { facet = AccountsDonationMatch__factory.connect(state.address, endowOwner); - registrarFake = await smock.fake(new Registrar__factory()); daoTokenFake = await smock.fake(new DummyERC20__factory()); + donationMatchFake = await smock.fake(new DonationMatch__factory()); + donationMatchEmitterFake = await smock.fake( + new DonationMatchEmitter__factory() + ); haloTokenFake = await smock.fake(new DummyERC20__factory()); + registrarFake = await smock.fake(new Registrar__factory()); const config: RegistrarStorage.ConfigStruct = { ...DEFAULT_REGISTRAR_CONFIG, + donationMatchContract: donationMatchFake.address, + donationMatchEmitter: donationMatchEmitterFake.address, haloToken: haloTokenFake.address, + proxyAdmin: proxyAdmin.address, + usdcAddress, }; registrarFake.queryConfig.returns(config); @@ -169,4 +188,151 @@ describe("AccountsDonationMatch", function () { expect(await state.getDaoTokenBalance(endowId)).to.equal(1); }); }); + + describe("upon setupDonationMatch", () => { + let details: AccountMessages.DonationMatchStruct; + + beforeEach(() => { + details = { + enumData: DonationMatchEnum.HaloTokenReserve, + data: { + poolFee: 5, + reserveToken: ethers.constants.AddressZero, + uniswapFactory: genWallet().address, + }, + }; + }); + + it("reverts if sender is not the endowment owner", async () => { + await expect(facet.connect(user).setupDonationMatch(endowId, details)).to.be.revertedWith( + "Unauthorized" + ); + }); + + it("reverts if a donation match contract has already been created for an endowment", async () => { + const prevEndow = await state.getEndowmentDetails(endowId); + const endowWithDonMatch: AccountStorage.EndowmentStruct = { + ...prevEndow, + donationMatchContract: genWallet().address, + }; + await state.setEndowmentDetails(endowId, endowWithDonMatch); + + await expect(facet.setupDonationMatch(endowId, details)).to.be.revertedWith( + "A Donation Match contract already exists for this Endowment" + ); + }); + + it("reverts if the passed uniswapFactory address is a zero address", async () => { + details.data.uniswapFactory = ethers.constants.AddressZero; + + await expect(facet.setupDonationMatch(endowId, details)).to.be.revertedWith( + "Invalid UniswapFactory address" + ); + }); + + it("reverts if the passed pool fee is 0 (zero)", async () => { + details.data.poolFee = 0; + + await expect(facet.setupDonationMatch(endowId, details)).to.be.revertedWith( + "Invalid pool fee" + ); + }); + + it("reverts if there is no donation matching contract implementation in the Registrar", async () => { + const prevConfig = await registrarFake.queryConfig(); + const config: RegistrarStorage.ConfigStruct = { + ...prevConfig, + donationMatchContract: ethers.constants.AddressZero, + }; + registrarFake.queryConfig.returns(config); + + await expect(facet.setupDonationMatch(endowId, details)).to.be.revertedWith( + "Missing implementation for donation matching contract" + ); + }); + + it("reverts if there is no USDC address in the Registrar", async () => { + const prevConfig = await registrarFake.queryConfig(); + const config: RegistrarStorage.ConfigStruct = { + ...prevConfig, + usdcAddress: ethers.constants.AddressZero, + }; + registrarFake.queryConfig.returns(config); + + await expect(facet.setupDonationMatch(endowId, details)).to.be.revertedWith( + "Invalid USDC address in Registrar" + ); + }); + + it("reverts if there is no HALO address in the Registrar", async () => { + const prevConfig = await registrarFake.queryConfig(); + const config: RegistrarStorage.ConfigStruct = { + ...prevConfig, + haloToken: ethers.constants.AddressZero, + }; + registrarFake.queryConfig.returns(config); + + await expect(facet.setupDonationMatch(endowId, details)).to.be.revertedWith( + "Invalid HALO address in Registrar" + ); + }); + + it("reverts if the passed reserveToken address is a zero address", async () => { + details.enumData = DonationMatchEnum.Cw20TokenReserve; + details.data.reserveToken = ethers.constants.AddressZero; + + await expect(facet.setupDonationMatch(endowId, details)).to.be.revertedWith( + "Invalid reserve token address" + ); + }); + + it("passes when setting up HALO as the reserve token", async () => { + await expect(facet.setupDonationMatch(endowId, details)).to.emit( + facet, + "DonationMatchCreated" + ); + + const endow = await state.getEndowmentDetails(endowId); + expect(endow.donationMatchContract).to.not.equal(ethers.constants.AddressZero); + + const expectedConfig: DonationMatchStorage.ConfigStruct = { + poolFee: details.data.poolFee, + uniswapFactory: details.data.uniswapFactory, + registrarContract: registrarFake.address, + reserveToken: haloTokenFake.address, + usdcAddress, + }; + expect(donationMatchEmitterFake.initializeDonationMatch).to.have.been.calledWith( + endowId, + endow.donationMatchContract, + expectedConfig + ); + }); + + it("passes when setting up some custom reserve token", async () => { + details.enumData = DonationMatchEnum.Cw20TokenReserve; + details.data.reserveToken = genWallet().address; + + await expect(facet.setupDonationMatch(endowId, details)).to.emit( + facet, + "DonationMatchCreated" + ); + + const endow = await state.getEndowmentDetails(endowId); + expect(endow.donationMatchContract).to.not.equal(ethers.constants.AddressZero); + + const expectedConfig: DonationMatchStorage.ConfigStruct = { + poolFee: details.data.poolFee, + uniswapFactory: details.data.uniswapFactory, + registrarContract: registrarFake.address, + reserveToken: details.data.reserveToken, + usdcAddress, + }; + expect(donationMatchEmitterFake.initializeDonationMatch).to.have.been.calledWith( + endowId, + endow.donationMatchContract, + expectedConfig + ); + }); + }); }); diff --git a/test/utils/helpers/types.ts b/test/utils/helpers/types.ts index 26ef2e92c..5ccf69fd2 100644 --- a/test/utils/helpers/types.ts +++ b/test/utils/helpers/types.ts @@ -56,3 +56,8 @@ export enum FeeTypes { EarlyLockedWithdrawCharity, EarlyLockedWithdrawNormal, } + +export enum DonationMatchEnum { + HaloTokenReserve, + Cw20TokenReserve, +} From 5ee0c5538ce9511e22054371f36e17c87a13e68a Mon Sep 17 00:00:00 2001 From: Nenad Date: Wed, 19 Jul 2023 09:42:28 +0200 Subject: [PATCH 12/12] Rename Cw20TokenReserve -> ERC20TokenReserve --- contracts/core/accounts/message.sol | 2 +- test/core/accounts/AccountsDonationMatch.ts | 4 ++-- test/utils/helpers/types.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/core/accounts/message.sol b/contracts/core/accounts/message.sol index 211d24b74..487a88eef 100644 --- a/contracts/core/accounts/message.sol +++ b/contracts/core/accounts/message.sol @@ -160,7 +160,7 @@ library AccountMessages { enum DonationMatchEnum { HaloTokenReserve, - Cw20TokenReserve + ERC20TokenReserve } struct DonationMatchData { diff --git a/test/core/accounts/AccountsDonationMatch.ts b/test/core/accounts/AccountsDonationMatch.ts index 07ecd5637..69383580f 100644 --- a/test/core/accounts/AccountsDonationMatch.ts +++ b/test/core/accounts/AccountsDonationMatch.ts @@ -278,7 +278,7 @@ describe("AccountsDonationMatch", function () { }); it("reverts if the passed reserveToken address is a zero address", async () => { - details.enumData = DonationMatchEnum.Cw20TokenReserve; + details.enumData = DonationMatchEnum.ERC20TokenReserve; details.data.reserveToken = ethers.constants.AddressZero; await expect(facet.setupDonationMatch(endowId, details)).to.be.revertedWith( @@ -310,7 +310,7 @@ describe("AccountsDonationMatch", function () { }); it("passes when setting up some custom reserve token", async () => { - details.enumData = DonationMatchEnum.Cw20TokenReserve; + details.enumData = DonationMatchEnum.ERC20TokenReserve; details.data.reserveToken = genWallet().address; await expect(facet.setupDonationMatch(endowId, details)).to.emit( diff --git a/test/utils/helpers/types.ts b/test/utils/helpers/types.ts index 5ccf69fd2..79e217cc7 100644 --- a/test/utils/helpers/types.ts +++ b/test/utils/helpers/types.ts @@ -59,5 +59,5 @@ export enum FeeTypes { export enum DonationMatchEnum { HaloTokenReserve, - Cw20TokenReserve, + ERC20TokenReserve, }