From efa59523351aa42c71a8c2c8cff6b260f1ee4c5c Mon Sep 17 00:00:00 2001 From: Felipe Forbeck Date: Mon, 17 May 2021 11:40:06 -0300 Subject: [PATCH 1/2] can not join the dao using 0x0 addr --- README.md | 2 + contracts/core/DaoRegistry.sol | 6 +- migrations/2_deploy_contracts.js | 13 +- package-lock.json | 2 +- package.json | 10 +- test/adapters/distribute.test.js | 3 + test/adapters/onboarding.test.js | 34 +++- test/core/registry.test.js | 8 + utils/DeploymentUtil.js | 271 ++++++++++++++++++------------- 9 files changed, 229 insertions(+), 120 deletions(-) diff --git a/README.md b/README.md index 11b1b890b..9cee14ccd 100644 --- a/README.md +++ b/README.md @@ -99,6 +99,8 @@ You can find more information about the purpose of each access flag at [DAO Regi Added the following environment variables to your local .env file: ``` +# The DAO Owner ETH Address (0x...) in the target network +DAO_OWNER_ADDR= # The Infura API Key is used to communicate with the Ethereum blockchain INFURA_KEY= # The mnemonic that generates you account diff --git a/contracts/core/DaoRegistry.sol b/contracts/core/DaoRegistry.sol index 5c931ad85..4d8c57c3e 100644 --- a/contracts/core/DaoRegistry.sol +++ b/contracts/core/DaoRegistry.sol @@ -211,6 +211,8 @@ contract DaoRegistry is MemberGuard, AdapterGuard { public hasAccess(this, AclFlag.NEW_MEMBER) { + require(memberAddress != address(0x0), "invalid member address"); + Member storage member = members[memberAddress]; if (!getFlag(member.flags, uint8(MemberFlag.EXISTS))) { member.flags = setFlag( @@ -539,8 +541,8 @@ contract DaoRegistry is MemberGuard, AdapterGuard { * @param addr The address to look up */ function isMember(address addr) public view returns (bool) { - address memberAddr = memberAddressesByDelegatedKey[addr]; - return getMemberFlag(memberAddr, MemberFlag.EXISTS); + address memberAddress = memberAddressesByDelegatedKey[addr]; + return getMemberFlag(memberAddress, MemberFlag.EXISTS); } /** diff --git a/migrations/2_deploy_contracts.js b/migrations/2_deploy_contracts.js index fa49a5fa9..1686ffc24 100644 --- a/migrations/2_deploy_contracts.js +++ b/migrations/2_deploy_contracts.js @@ -13,15 +13,15 @@ const truffleImports = require("../utils/TruffleUtil.js"); require("dotenv").config(); -module.exports = async (deployer, network) => { +module.exports = async (deployer, network, accounts) => { let dao; const deployFunction = truffleImports.deployFunctionFactory(deployer); if (network === "ganache") { - dao = await deployGanacheDao(deployFunction, network); + dao = await deployGanacheDao(deployFunction, network, accounts); } else if (network === "rinkeby") { dao = await deployRinkebyDao(deployFunction, network); } else if (network === "test" || network === "coverage") { - dao = await deployTestDao(deployFunction, network); + dao = await deployTestDao(deployFunction, network, accounts); } if (dao) { @@ -38,7 +38,7 @@ module.exports = async (deployer, network) => { } }; -async function deployTestDao(deployFunction, network) { +async function deployTestDao(deployFunction, network, accounts) { let { dao } = await deployDao({ ...truffleImports, deployFunction, @@ -56,6 +56,7 @@ async function deployTestDao(deployFunction, network) { couponCreatorAddress: "0x7D8cad0bbD68deb352C33e80fccd4D8e88b4aBb8", offchainAdmin: "0xedC10CFA90A135C41538325DD57FDB4c7b88faf7", daoName: process.env.DAO_NAME, + owner: accounts[0], }); return dao; } @@ -77,12 +78,13 @@ async function deployRinkebyDao(deployFunction, network) { maxExternalTokens: 100, couponCreatorAddress: process.env.COUPON_CREATOR_ADDR, daoName: process.env.DAO_NAME, + owner: process.env.DAO_OWNER_ADDR, offchainAdmin: "0xedC10CFA90A135C41538325DD57FDB4c7b88faf7", }); return dao; } -async function deployGanacheDao(deployFunction, network) { +async function deployGanacheDao(deployFunction, network, accounts) { let { dao } = await deployDao({ ...truffleImports, deployFunction, @@ -99,6 +101,7 @@ async function deployGanacheDao(deployFunction, network) { maxExternalTokens: 100, couponCreatorAddress: process.env.COUPON_CREATOR_ADDR, daoName: process.env.DAO_NAME, + owner: accounts[0], offchainAdmin: "0xedC10CFA90A135C41538325DD57FDB4c7b88faf7", }); return dao; diff --git a/package-lock.json b/package-lock.json index e46d3c243..9877daf6e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "tribute-contracts", - "version": "0.0.1", + "version": "1.0.0-rc2", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index f1eaad6be..15f036b5f 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,15 @@ "version": "1.0.0-rc2", "description": "A new modular DAO framework, inspired by the Moloch smart contracts", "main": "truffle-config.js", - "keywords": ["dao", "framework", "smart-contract", "solidity", "modular", "moloch", "ethereum"], + "keywords": [ + "dao", + "framework", + "smart-contract", + "solidity", + "modular", + "moloch", + "ethereum" + ], "directories": { "test": "test" }, diff --git a/test/adapters/distribute.test.js b/test/adapters/distribute.test.js index d9e1664f2..bf94c865a 100644 --- a/test/adapters/distribute.test.js +++ b/test/adapters/distribute.test.js @@ -53,6 +53,8 @@ const { const { onboardingNewMember } = require("../../utils/TestUtils.js"); const daoOwner = accounts[2]; +const daoCreator = accounts[9]; + const proposalCounter = proposalIdGenerator().generator; function getProposalCounter() { @@ -63,6 +65,7 @@ describe("Adapter - Distribute", () => { before("deploy dao", async () => { const { dao, adapters, extensions } = await deployDefaultDao({ owner: daoOwner, + creator: daoCreator, }); this.dao = dao; this.adapters = adapters; diff --git a/test/adapters/onboarding.test.js b/test/adapters/onboarding.test.js index fb1a055ee..60ced6487 100644 --- a/test/adapters/onboarding.test.js +++ b/test/adapters/onboarding.test.js @@ -50,6 +50,8 @@ const { const { checkBalance, isMember } = require("../../utils/TestUtils.js"); const daoOwner = accounts[0]; +const delegatedKey = accounts[9]; + const proposalCounter = proposalIdGenerator().generator; function getProposalCounter() { @@ -60,6 +62,7 @@ describe("Adapter - Onboarding", () => { before("deploy dao", async () => { const { dao, adapters, extensions } = await deployDefaultDao({ owner: daoOwner, + creator: delegatedKey, }); this.dao = dao; this.adapters = adapters; @@ -425,7 +428,7 @@ describe("Adapter - Onboarding", () => { }); it("should be possible to update delegate key and the member continues as an active member", async () => { - const delegateKey = accounts[2]; + const delegateKey = accounts[9]; const dao = this.dao; const bank = this.extensions.bank; const daoRegistryAdapter = this.adapters.daoRegistryAdapter; @@ -433,7 +436,7 @@ describe("Adapter - Onboarding", () => { expect(await isMember(bank, daoOwner)).equal(true); expect(await dao.isMember(delegateKey)).equal(true); // use the dao to check delegatedKeys - const newDelegatedKey = accounts[9]; + const newDelegatedKey = accounts[5]; await daoRegistryAdapter.updateDelegateKey(dao.address, newDelegatedKey, { from: daoOwner, gasPrice: toBN("0"), @@ -518,4 +521,31 @@ describe("Adapter - Onboarding", () => { "address already taken as delegated key" ); }); + + it("should not be possible to onboard a member with a zero address", async () => { + const applicant = "0x0000000000000000000000000000000000000000"; + const dao = this.dao; + const onboarding = this.adapters.onboarding; + const voting = this.adapters.voting; + + const proposalId = getProposalCounter(); + await expectRevert( + onboarding.submitProposal( + dao.address, + proposalId, + applicant, + UNITS, + unitPrice.mul(toBN(3)).add(remaining), + [], + { + from: daoOwner, + gasPrice: toBN("0"), + } + ), + "invalid member address." + ); + + let isMember = await dao.isMember(applicant); + expect(isMember).equal(false); + }); }); diff --git a/test/core/registry.test.js b/test/core/registry.test.js index ba432020f..0708512e6 100644 --- a/test/core/registry.test.js +++ b/test/core/registry.test.js @@ -103,4 +103,12 @@ describe("Core - Registry", () => { "adapterId must not be empty" ); }); + + it("should not be possible to a zero address be considered a member", async () => { + let registry = await DaoRegistry.new(); + let isMember = await registry.isMember( + "0x0000000000000000000000000000000000000000" + ); + expect(isMember).equal(false); + }); }); diff --git a/utils/DeploymentUtil.js b/utils/DeploymentUtil.js index 140db3da8..ef487d515 100644 --- a/utils/DeploymentUtil.js +++ b/utils/DeploymentUtil.js @@ -25,12 +25,13 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -const { UNITS, LOOT, ETH_TOKEN, sha3, toBN } = require("./ContractUtil"); +const { UNITS, LOOT, sha3, toBN } = require("./ContractUtil"); const deployDao = async (options) => { const { deployFunction, owner, + creator, deployTestTokens, finalize, DaoRegistry, @@ -68,8 +69,9 @@ const deployDao = async (options) => { const { bankAddress } = pastEvent.returnValues; const bankExtension = await BankExtension.at(bankAddress); - const creator = await dao.getMemberAddress(1); - await dao.addExtension(sha3("bank"), bankExtension.address, creator); + await dao.addExtension(sha3("bank"), bankExtension.address, owner, { + from: owner, + }); await nftFactory.createNFTCollection(); pastEvent = undefined; @@ -80,7 +82,9 @@ const deployDao = async (options) => { const { nftCollAddress } = pastEvent.returnValues; const nftExtension = await NFTExtension.at(nftCollAddress); - await dao.addExtension(sha3("nft"), nftCollAddress, creator); + await dao.addExtension(sha3("nft"), nftCollAddress, owner, { + from: owner, + }); const extensions = { bank: bankExtension, nft: nftExtension }; @@ -188,7 +192,8 @@ const configureOffchainVoting = async ({ await daoFactory.updateAdapter( dao.address, - entryDao("voting", offchainVoting, {}) + entryDao("voting", offchainVoting, {}), + { from: owner } ); await dao.setAclToExtensionForAdapter( @@ -198,9 +203,16 @@ const configureOffchainVoting = async ({ ADD_TO_BALANCE: true, SUB_FROM_BALANCE: true, INTERNAL_TRANSFER: true, - }).flags + }).flags, + { from: owner } + ); + await offchainVoting.configureDao( + dao.address, + votingPeriod, + gracePeriod, + 10, + { from: owner } ); - await offchainVoting.configureDao(dao.address, votingPeriod, gracePeriod, 10); return { offchainVoting, snapshotProposalContract, handleBadReporterAdapter }; }; @@ -339,6 +351,7 @@ const addDefaultAdapters = async ({ dao, options, daoFactory, nftAddr }) => { const nftExtension = await NFTExtension.at(nftExtAddr); await configureDao({ + owner: options.owner, daoFactory, dao, ragequit, @@ -383,6 +396,7 @@ const addDefaultAdapters = async ({ dao, options, daoFactory, nftAddr }) => { }; const configureDao = async ({ + owner, daoFactory, dao, ragequit, @@ -409,98 +423,118 @@ const configureDao = async ({ gracePeriod, couponCreatorAddress, }) => { - await daoFactory.addAdapters(dao.address, [ - entryDao("voting", voting, {}), - entryDao("configuration", configuration, { - SUBMIT_PROPOSAL: true, - SET_CONFIGURATION: true, - }), - entryDao("ragequit", ragequit, {}), - entryDao("guildkick", guildkick, { - SUBMIT_PROPOSAL: true, - }), - entryDao("managing", managing, { - SUBMIT_PROPOSAL: true, - REPLACE_ADAPTER: true, - }), - entryDao("financing", financing, { - SUBMIT_PROPOSAL: true, - }), - entryDao("onboarding", onboarding, { - SUBMIT_PROPOSAL: true, - UPDATE_DELEGATE_KEY: true, - NEW_MEMBER: true, - }), - entryDao("coupon-onboarding", couponOnboarding, { - SUBMIT_PROPOSAL: false, - ADD_TO_BALANCE: true, - UPDATE_DELEGATE_KEY: false, - NEW_MEMBER: true, - }), - entryDao("daoRegistry", daoRegistryAdapter, { - UPDATE_DELEGATE_KEY: true, - }), - entryDao("nft", nftAdapter, {}), - entryDao("bank", bankAdapter, {}), - entryDao("tribute", tribute, { - SUBMIT_PROPOSAL: true, - NEW_MEMBER: true, - }), - entryDao("tribute-nft", tributeNFT, { - SUBMIT_PROPOSAL: true, - NEW_MEMBER: true, - }), - entryDao("distribute", distribute, { - SUBMIT_PROPOSAL: true, - }), - ]); + await daoFactory.addAdapters( + dao.address, + [ + entryDao("voting", voting, {}), + entryDao("configuration", configuration, { + SUBMIT_PROPOSAL: true, + SET_CONFIGURATION: true, + }), + entryDao("ragequit", ragequit, {}), + entryDao("guildkick", guildkick, { + SUBMIT_PROPOSAL: true, + }), + entryDao("managing", managing, { + SUBMIT_PROPOSAL: true, + REPLACE_ADAPTER: true, + }), + entryDao("financing", financing, { + SUBMIT_PROPOSAL: true, + }), + entryDao("onboarding", onboarding, { + SUBMIT_PROPOSAL: true, + UPDATE_DELEGATE_KEY: true, + NEW_MEMBER: true, + }), + entryDao("coupon-onboarding", couponOnboarding, { + SUBMIT_PROPOSAL: false, + ADD_TO_BALANCE: true, + UPDATE_DELEGATE_KEY: false, + NEW_MEMBER: true, + }), + entryDao("daoRegistry", daoRegistryAdapter, { + UPDATE_DELEGATE_KEY: true, + }), + entryDao("nft", nftAdapter, {}), + entryDao("bank", bankAdapter, {}), + entryDao("tribute", tribute, { + SUBMIT_PROPOSAL: true, + NEW_MEMBER: true, + }), + entryDao("tribute-nft", tributeNFT, { + SUBMIT_PROPOSAL: true, + NEW_MEMBER: true, + }), + entryDao("distribute", distribute, { + SUBMIT_PROPOSAL: true, + }), + ], + { + from: owner, + } + ); - await daoFactory.configureExtension(dao.address, bankExtension.address, [ - entryBank(ragequit, { - INTERNAL_TRANSFER: true, - SUB_FROM_BALANCE: true, - ADD_TO_BALANCE: true, - }), - entryBank(guildkick, { - INTERNAL_TRANSFER: true, - SUB_FROM_BALANCE: true, - ADD_TO_BALANCE: true, - }), - entryBank(bankAdapter, { - WITHDRAW: true, - SUB_FROM_BALANCE: true, - UPDATE_TOKEN: true, - }), - entryBank(onboarding, { - ADD_TO_BALANCE: true, - }), - entryBank(couponOnboarding, { - ADD_TO_BALANCE: true, - }), - entryBank(financing, { - ADD_TO_BALANCE: true, - SUB_FROM_BALANCE: true, - }), - entryBank(tribute, { - ADD_TO_BALANCE: true, - REGISTER_NEW_TOKEN: true, - }), - entryBank(distribute, { - INTERNAL_TRANSFER: true, - }), - entryBank(tributeNFT, { - ADD_TO_BALANCE: true, - }), - ]); + await daoFactory.configureExtension( + dao.address, + bankExtension.address, + [ + entryBank(ragequit, { + INTERNAL_TRANSFER: true, + SUB_FROM_BALANCE: true, + ADD_TO_BALANCE: true, + }), + entryBank(guildkick, { + INTERNAL_TRANSFER: true, + SUB_FROM_BALANCE: true, + ADD_TO_BALANCE: true, + }), + entryBank(bankAdapter, { + WITHDRAW: true, + SUB_FROM_BALANCE: true, + UPDATE_TOKEN: true, + }), + entryBank(onboarding, { + ADD_TO_BALANCE: true, + }), + entryBank(couponOnboarding, { + ADD_TO_BALANCE: true, + }), + entryBank(financing, { + ADD_TO_BALANCE: true, + SUB_FROM_BALANCE: true, + }), + entryBank(tribute, { + ADD_TO_BALANCE: true, + REGISTER_NEW_TOKEN: true, + }), + entryBank(distribute, { + INTERNAL_TRANSFER: true, + }), + entryBank(tributeNFT, { + ADD_TO_BALANCE: true, + }), + ], + { + from: owner, + } + ); - await daoFactory.configureExtension(dao.address, nftExtension.address, [ - entryNft(tributeNFT, { - COLLECT_NFT: true, - }), - entryNft(nftAdapter, { - COLLECT_NFT: true, - }), - ]); + await daoFactory.configureExtension( + dao.address, + nftExtension.address, + [ + entryNft(tributeNFT, { + COLLECT_NFT: true, + }), + entryNft(nftAdapter, { + COLLECT_NFT: true, + }), + ], + { + from: owner, + } + ); await onboarding.configureDao( dao.address, @@ -508,7 +542,10 @@ const configureDao = async ({ unitPrice, nbUnits, maxChunks, - tokenAddr + tokenAddr, + { + from: owner, + } ); await onboarding.configureDao( @@ -517,19 +554,39 @@ const configureDao = async ({ unitPrice, nbUnits, maxChunks, - tokenAddr + tokenAddr, + { + from: owner, + } ); - await couponOnboarding.configureDao(dao.address, couponCreatorAddress, UNITS); - await voting.configureDao(dao.address, votingPeriod, gracePeriod); - await tribute.configureDao(dao.address, UNITS); - await tribute.configureDao(dao.address, LOOT); - await tributeNFT.configureDao(dao.address); + await couponOnboarding.configureDao( + dao.address, + couponCreatorAddress, + UNITS, + { + from: owner, + } + ); + + await voting.configureDao(dao.address, votingPeriod, gracePeriod, { + from: owner, + }); + await tribute.configureDao(dao.address, UNITS, { + from: owner, + }); + await tribute.configureDao(dao.address, LOOT, { + from: owner, + }); + await tributeNFT.configureDao(dao.address, { + from: owner, + }); }; const cloneDao = async ({ identityDao, owner, + creator, deployFunction, DaoRegistry, DaoFactory, @@ -541,11 +598,7 @@ const cloneDao = async ({ owner ); - if (owner) { - await daoFactory.createDao(name, ETH_TOKEN, { from: owner }); - } else { - await daoFactory.createDao(name, ETH_TOKEN); - } + await daoFactory.createDao(name, creator ? creator : owner, { from: owner }); // checking the gas usaged to clone a contract let pastEvents = await daoFactory.getPastEvents(); From 5d3251f24d8e3ccbdf5277b13c29f623cd3e3fbe Mon Sep 17 00:00:00 2001 From: Felipe Forbeck Date: Mon, 17 May 2021 14:41:41 -0300 Subject: [PATCH 2/2] Update test/core/registry.test.js Co-authored-by: Jarrel deLottinville --- test/core/registry.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/registry.test.js b/test/core/registry.test.js index 0708512e6..b187efd85 100644 --- a/test/core/registry.test.js +++ b/test/core/registry.test.js @@ -104,7 +104,7 @@ describe("Core - Registry", () => { ); }); - it("should not be possible to a zero address be considered a member", async () => { + it("should not be possible for a zero address to be considered a member", async () => { let registry = await DaoRegistry.new(); let isMember = await registry.isMember( "0x0000000000000000000000000000000000000000"