From 2080f9ef57b712bb1ae34b269fb1e46f0a23e5e1 Mon Sep 17 00:00:00 2001 From: orenyodfat Date: Sun, 18 Oct 2020 12:20:55 +0300 Subject: [PATCH] DAOToken is none upgradable proxy (#799) * test externaltoken(which is native token) workaround * DAOToken is none upgradable * tests * tests --- contracts/schemes/UpgradeScheme.sol | 9 +++- contracts/utils/DAOFactory.sol | 32 ++++++++++-- test/contributionreward.js | 36 ++++++++++++-- test/schemefactory.js | 9 ++-- test/upgradescheme.js | 76 ++++++++++++++++------------- 5 files changed, 115 insertions(+), 47 deletions(-) diff --git a/contracts/schemes/UpgradeScheme.sol b/contracts/schemes/UpgradeScheme.sol index 14650f19..a023cdc1 100644 --- a/contracts/schemes/UpgradeScheme.sol +++ b/contracts/schemes/UpgradeScheme.sol @@ -33,6 +33,8 @@ contract UpgradeScheme is VotingMachineCallbacks, ProposalExecuteInterface { event ProposalDeleted(address indexed _avatar, bytes32 indexed _proposalId); + event UpgradedContracts(address indexed _avatar, address[] _upgradedContracts); + // Details of a voting proposal: struct Proposal { uint64[3] packageVersion; @@ -91,14 +93,19 @@ contract UpgradeScheme is VotingMachineCallbacks, ProposalExecuteInterface { ).getImplementation(contractName); Controller controller = Controller(avatar.owner()); - controller.genericCall( + (bool success,) = controller.genericCall( contractsToUpgrade[i], abi.encodeWithSignature("upgradeTo(address)", updatedImp), 0 ); + if (!success) { + contractsToUpgrade[i] = address(0); + } } + emit UpgradedContracts(address(avatar), contractsToUpgrade); } + delete organizationProposals[_proposalId]; emit ProposalDeleted(address(avatar), _proposalId); emit ProposalExecuted(address(avatar), _proposalId, _decision == 1); diff --git a/contracts/utils/DAOFactory.sol b/contracts/utils/DAOFactory.sol index dadf9d10..1968e681 100644 --- a/contracts/utils/DAOFactory.sol +++ b/contracts/utils/DAOFactory.sol @@ -6,6 +6,7 @@ pragma experimental ABIEncoderV2; import "../registry/App.sol"; import "../registry/ImplementationDirectory.sol"; import "@daostack/upgrades/contracts/upgradeability/AdminUpgradeabilityProxy.sol"; +import "@daostack/upgrades/contracts/upgradeability/UpgradeabilityProxy.sol"; import "../libs/BytesLib.sol"; import "../controller/Controller.sol"; import "../libs/Bytes32ToStr.sol"; @@ -164,6 +165,32 @@ contract DAOFactory is Initializable { delete locks[address(_avatar)]; } + /** + * @dev createNonUpgradableInstance creates a new proxy for the given contract and forwards a function call to it. + * This is useful to initialize the proxied contract. + * @param _packageVersion of the instance. + * @param _contractName Name of the contract. + * @param _data Data to send as msg.data to the corresponding implementation to initialize the proxied contract. + * It should include the signature and the parameters of the function to be called, as described in + * https://solidity.readthedocs.io/en/v0.4.24/abi-spec.html#function-selector-and-argument-encoding. + * This parameter is optional, if no data is given the initialization call to proxied contract will be skipped. + * @return Address of the new proxy. + */ + function createNonUpgradableInstance(uint64[3] memory _packageVersion, + string memory _contractName, + bytes memory _data) + public + payable + returns (UpgradeabilityProxy) { + uint64[3] memory version = getPackageVersion(_packageVersion); + address implementation = getImplementation(version, _contractName); + /* solhint-disable */ + UpgradeabilityProxy proxy = (new UpgradeabilityProxy){value:msg.value}(implementation, _data); + /* solhint-enable */ + emit ProxyCreated(address(proxy), implementation, _contractName, version); + return proxy; + } + /** * @dev Creates a new proxy for the given contract and forwards a function call to it. * This is useful to initialize the proxied contract. @@ -298,8 +325,8 @@ contract DAOFactory is Initializable { "_founderlength != _foundersTokenAmount.length"); require(_founders.length == _foundersReputationAmount.length, "_founderlength != _foundersReputationAmount.length"); - AdminUpgradeabilityProxy nativeToken = - createInstance(_packageVersion, "DAOToken", address(this), _tokenInitData); + UpgradeabilityProxy nativeToken = + createNonUpgradableInstance(_packageVersion, "DAOToken", _tokenInitData); AdminUpgradeabilityProxy nativeReputation = createInstance(_packageVersion, "Reputation", address(this), abi.encodeWithSignature("initialize(address)", address(this))); @@ -311,7 +338,6 @@ contract DAOFactory is Initializable { address(nativeToken), address(nativeReputation), address(this))); - nativeToken.changeAdmin(address(avatar)); nativeReputation.changeAdmin(address(avatar)); avatar.changeAdmin(address(avatar)); // Mint token and reputation for founders: diff --git a/test/contributionreward.js b/test/contributionreward.js index d771b7d3..3110879c 100644 --- a/test/contributionreward.js +++ b/test/contributionreward.js @@ -4,13 +4,12 @@ const ERC20Mock = artifacts.require('./test/ERC20Mock.sol'); const Avatar = artifacts.require("./Avatar.sol"); const Redeemer = artifacts.require("./Redeemer.sol"); - - class ContributionRewardParams { constructor() { } } + const checkRedeemedPeriods = async function( testSetup, proposalId, @@ -70,7 +69,7 @@ const setupContributionReward = async function( return contributionRewardParams; }; -const setup = async function (accounts,genesisProtocol = false,tokenAddress=0) { +const setup = async function (accounts,genesisProtocol = false,tokenAddress=helpers.NULL_ADDRESS) { var testSetup = new helpers.TestSetup(); registration = await helpers.registerImplementation(); testSetup.standardTokenMock = await ERC20Mock.new(accounts[1],100); @@ -86,6 +85,7 @@ const setup = async function (accounts,genesisProtocol = false,tokenAddress=0) { genesisProtocol, tokenAddress); var permissions = "0x00000000"; + [testSetup.org,tx] = await helpers.setupOrganizationWithArraysDAOFactory(testSetup.proxyAdmin, accounts, registration, @@ -963,4 +963,34 @@ contract('ContributionReward', accounts => { helpers.assertVMException(ex); } }); + + it("execute proposeContributionReward send externalToken(which is nativeToken) ", async function() { + var testSetup = await setup(accounts); + //give some nativ tokens to organization avatar + await testSetup.org.token.transfer(testSetup.org.avatar.address,30,{from:accounts[0]}); + var reputationReward = 0; + var nativeTokenReward = 0; + var ethReward = 0; + var externalTokenReward = 12; + var periodLength = 50; + var numberOfPeriods = 1; + //send some ether to the org avatar + var otherAvatar = await Avatar.new(); + await otherAvatar.initialize('otheravatar', helpers.NULL_ADDRESS, helpers.NULL_ADDRESS , accounts[0]); + + var tx = await testSetup.contributionReward.proposeContributionReward( + web3.utils.asciiToHex("description"), + reputationReward, + [nativeTokenReward,ethReward,externalTokenReward,periodLength,numberOfPeriods], + testSetup.org.token.address, + otherAvatar.address + ); + //Vote with reputation to trigger execution + var proposalId = await helpers.getValueFromLogs(tx, '_proposalId',1); + await testSetup.contributionRewardParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); + await helpers.increaseTime(periodLength+1); + await testSetup.contributionReward.redeem(proposalId,[false,false,false,true]); + var tokens = await testSetup.org.token.balanceOf(otherAvatar.address); + assert.equal(tokens.toNumber(),externalTokenReward); + }); }); diff --git a/test/schemefactory.js b/test/schemefactory.js index 3327c828..771361c0 100644 --- a/test/schemefactory.js +++ b/test/schemefactory.js @@ -114,23 +114,22 @@ contract('SchemeFactory', accounts => { it("execute proposeScheme and execute -yes - permissions== 0x0000001f", async function() { var testSetup = await setup(accounts); - var initdata = await new web3.eth.Contract(registration.schemeFactory.abi) + var initdata = await new web3.eth.Contract(registration.genericScheme.abi) .methods .initialize(testSetup.org.avatar.address, testSetup.schemeFactoryParams.votingMachine.absoluteVote.address, [0,0,0,0,0,0,0,0,0,0,0], helpers.NULL_ADDRESS, testSetup.schemeFactoryParams.votingMachine.params, - registration.daoFactory.address) + testSetup.org.token.address) .encodeABI(); - var tx = await testSetup.schemeFactory.proposeScheme( + tx = await testSetup.schemeFactory.proposeScheme( [0,1,0], - 'SchemeFactory', + 'GenericScheme', initdata, "0x0000001f", helpers.NULL_ADDRESS, helpers.NULL_HASH); - //Vote with reputation to trigger execution var proposalId = await helpers.getValueFromLogs(tx, '_proposalId',1); tx = await testSetup.schemeFactoryParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); diff --git a/test/upgradescheme.js b/test/upgradescheme.js index f69fac81..1efd73d5 100644 --- a/test/upgradescheme.js +++ b/test/upgradescheme.js @@ -217,7 +217,6 @@ contract('UpgradeScheme', function(accounts) { await helpers.registrationAddVersionToPackege(registration,[0, 1, 1]); let avatarProxy = await AdminUpgradeabilityProxy.at(testSetup.org.avatar.address); - let tokenProxy = await AdminUpgradeabilityProxy.at(testSetup.org.token.address); let reputationProxy = await AdminUpgradeabilityProxy.at(testSetup.org.reputation.address); let oldImpAddress = await testSetup.registration.packageInstance.getContract([0,1,0]); @@ -227,10 +226,6 @@ contract('UpgradeScheme', function(accounts) { await avatarProxy.implementation.call({from: testSetup.org.avatar.address}), await oldImp.getImplementation("Avatar") ); - assert.equal( - await tokenProxy.implementation.call({from: testSetup.org.avatar.address}), - await oldImp.getImplementation("DAOToken") - ); assert.equal( await reputationProxy.implementation.call({from: testSetup.org.avatar.address}), await oldImp.getImplementation("Reputation") @@ -238,8 +233,8 @@ contract('UpgradeScheme', function(accounts) { var tx = await testSetup.upgradeScheme.proposeUpgrade( [0, 1, 1], - [web3.utils.fromAscii("Avatar"),web3.utils.fromAscii("DAOToken"),web3.utils.fromAscii("Reputation")], - [testSetup.org.avatar.address, testSetup.org.token.address, testSetup.org.reputation.address], + [web3.utils.fromAscii("Avatar"),web3.utils.fromAscii("Reputation")], + [testSetup.org.avatar.address,testSetup.org.reputation.address], helpers.NULL_HASH ); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); @@ -255,10 +250,6 @@ contract('UpgradeScheme', function(accounts) { await avatarProxy.implementation.call({from: testSetup.org.avatar.address}), await newImp.getImplementation("Avatar") ); - assert.equal( - await tokenProxy.implementation.call({from: testSetup.org.avatar.address}), - await newImp.getImplementation("DAOToken") - ); assert.equal( await reputationProxy.implementation.call({from: testSetup.org.avatar.address}), await newImp.getImplementation("Reputation") @@ -270,7 +261,6 @@ contract('UpgradeScheme', function(accounts) { await helpers.registrationAddVersionToPackege(registration,[0, 1, 1]); let avatarProxy = await AdminUpgradeabilityProxy.at(testSetup.org.avatar.address); - let tokenProxy = await AdminUpgradeabilityProxy.at(testSetup.org.token.address); let reputationProxy = await AdminUpgradeabilityProxy.at(testSetup.org.reputation.address); let oldImpAddress = await testSetup.registration.packageInstance.getContract([0,1,0]); @@ -280,10 +270,7 @@ contract('UpgradeScheme', function(accounts) { await avatarProxy.implementation.call({from: testSetup.org.avatar.address}), await oldImp.getImplementation("Avatar") ); - assert.equal( - await tokenProxy.implementation.call({from: testSetup.org.avatar.address}), - await oldImp.getImplementation("DAOToken") - ); + assert.equal( await reputationProxy.implementation.call({from: testSetup.org.avatar.address}), await oldImp.getImplementation("Reputation") @@ -291,9 +278,9 @@ contract('UpgradeScheme', function(accounts) { let contractsNames = []; let contractsToUpgrade = []; - for (let i = 0; i < 20; i++) { - contractsNames.push(web3.utils.fromAscii("Avatar"),web3.utils.fromAscii("DAOToken"),web3.utils.fromAscii("Reputation")); - contractsToUpgrade.push(testSetup.org.avatar.address, testSetup.org.token.address, testSetup.org.reputation.address); + for (let i = 0; i < 30; i++) { + contractsNames.push(web3.utils.fromAscii("Avatar"),web3.utils.fromAscii("Reputation")); + contractsToUpgrade.push(testSetup.org.avatar.address, testSetup.org.reputation.address); } var tx = await testSetup.upgradeScheme.proposeUpgrade( @@ -315,10 +302,7 @@ contract('UpgradeScheme', function(accounts) { await avatarProxy.implementation.call({from: testSetup.org.avatar.address}), await newImp.getImplementation("Avatar") ); - assert.equal( - await tokenProxy.implementation.call({from: testSetup.org.avatar.address}), - await newImp.getImplementation("DAOToken") - ); + assert.equal( await reputationProxy.implementation.call({from: testSetup.org.avatar.address}), await newImp.getImplementation("Reputation") @@ -331,7 +315,6 @@ contract('UpgradeScheme', function(accounts) { await helpers.registrationAddVersionToPackege(registration,[0, 1, 1]); let avatarProxy = await AdminUpgradeabilityProxy.at(testSetup.org.avatar.address); - let tokenProxy = await AdminUpgradeabilityProxy.at(testSetup.org.token.address); let reputationProxy = await AdminUpgradeabilityProxy.at(testSetup.org.reputation.address); let oldImpAddress = await testSetup.registration.packageInstance.getContract([0,1,0]); @@ -341,10 +324,7 @@ contract('UpgradeScheme', function(accounts) { await avatarProxy.implementation.call({from: testSetup.org.avatar.address}), await oldImp.getImplementation("Avatar") ); - assert.equal( - await tokenProxy.implementation.call({from: testSetup.org.avatar.address}), - await oldImp.getImplementation("DAOToken") - ); + assert.equal( await reputationProxy.implementation.call({from: testSetup.org.avatar.address}), await oldImp.getImplementation("Reputation") @@ -352,9 +332,9 @@ contract('UpgradeScheme', function(accounts) { let contractsNames = []; let contractsToUpgrade = []; - for (let i = 0; i < 20; i++) { - contractsNames.push(web3.utils.fromAscii("Avatar"),web3.utils.fromAscii("DAOToken"),web3.utils.fromAscii("Reputation")); - contractsToUpgrade.push(testSetup.org.avatar.address, testSetup.org.token.address, testSetup.org.reputation.address); + for (let i = 0; i < 30; i++) { + contractsNames.push(web3.utils.fromAscii("Avatar"),web3.utils.fromAscii("Reputation")); + contractsToUpgrade.push(testSetup.org.avatar.address, testSetup.org.reputation.address); } var tx = await testSetup.upgradeScheme.proposeUpgrade( @@ -376,10 +356,6 @@ contract('UpgradeScheme', function(accounts) { await avatarProxy.implementation.call({from: testSetup.org.avatar.address}), await newImp.getImplementation("Avatar") ); - assert.equal( - await tokenProxy.implementation.call({from: testSetup.org.avatar.address}), - await newImp.getImplementation("DAOToken") - ); assert.equal( await reputationProxy.implementation.call({from: testSetup.org.avatar.address}), await newImp.getImplementation("Reputation") @@ -405,4 +381,34 @@ contract('UpgradeScheme', function(accounts) { }); + it("cannot upgrade DAOToken", async function() { + var testSetup = await setup(accounts); + + await helpers.registrationAddVersionToPackege(registration,[0, 1, 1]); + + var tx = await testSetup.upgradeScheme.proposeUpgrade( + [0, 1, 1], + [web3.utils.fromAscii("Avatar"),web3.utils.fromAscii("DAOToken"),web3.utils.fromAscii("Reputation")], + [testSetup.org.avatar.address, testSetup.org.token.address, testSetup.org.reputation.address], + helpers.NULL_HASH); + + var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); + tx = await testSetup.upgradeSchemeParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); + await testSetup.upgradeScheme.getPastEvents('UpgradedContracts', { + fromBlock: tx.blockNumber, + toBlock: 'latest' + }) + .then(function(events){ + assert.equal(events[0].event,"UpgradedContracts"); + for (var i = 0;i< events[0].args._upgradedContracts.length;i++) { + if (events[0].args._upgradedContracts[i] === testSetup.org.token.address) { + assert.equal(false,true,"cannot upgrade daotoken"); + } + } + }); + //check organizationsProposals after execution + var organizationProposal = await testSetup.upgradeScheme.organizationProposals(proposalId); + assert.equal(organizationProposal,false); + }); + });