From bff1f875b09da83639eae7c3fbcb58de5eab61ef Mon Sep 17 00:00:00 2001 From: Felipe Novaes F Rocha Date: Tue, 26 Dec 2023 23:51:02 -0300 Subject: [PATCH 1/8] :fire: remove pinned solc version in foundry :bug: add missing _pow func to avoid overflow and underflow :sparkles: helper func in test :test_tube: conviction value check after vote and time passed --- foundry.toml | 2 +- pkg/contracts/src/CVStrategy.sol | 2 +- pkg/contracts/test/CVStrategyTest.t.sol | 72 +++++++++++++++++++++++-- 3 files changed, 70 insertions(+), 6 deletions(-) diff --git a/foundry.toml b/foundry.toml index abb3dc851..247dcaa36 100644 --- a/foundry.toml +++ b/foundry.toml @@ -1,6 +1,6 @@ # See more config options https://github.com/foundry-rs/foundry/blob/master/crates/config/README.md#default-profile [profile.default] -solc-version = "0.8.19" +# solc-version = "0.8.19" src = 'pkg/contracts/src' test = 'pkg/contracts/test' out = 'pkg/contracts/out' diff --git a/pkg/contracts/src/CVStrategy.sol b/pkg/contracts/src/CVStrategy.sol index 2bbdfcecb..cf60d5d3a 100644 --- a/pkg/contracts/src/CVStrategy.sol +++ b/pkg/contracts/src/CVStrategy.sol @@ -482,7 +482,7 @@ contract CVStrategy is BaseStrategy, IWithdrawMember { // @audit-ok they use 2^128 as the container for the result of the _pow function // uint256 atTWO_128 = _pow((decay << 128).div(D), t); - uint256 atTWO_128 = ((decay << 128) / D) ** t; + uint256 atTWO_128 = _pow((decay << 128) / D,t); // solium-disable-previous-line // conviction = (atTWO_128 * _lastConv + _oldAmount * D * (2^128 - atTWO_128) / (D - aD) + 2^127) / 2^128 // return (atTWO_128.mul(_lastConv).add(_oldAmount.mul(D).mul(TWO_128.sub(atTWO_128)).div(D - decay))).add(TWO_127) diff --git a/pkg/contracts/test/CVStrategyTest.t.sol b/pkg/contracts/test/CVStrategyTest.t.sol index 18f3a1021..7728c8988 100644 --- a/pkg/contracts/test/CVStrategyTest.t.sol +++ b/pkg/contracts/test/CVStrategyTest.t.sol @@ -40,6 +40,10 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G Metadata public metadata = Metadata({protocol: 1, pointer: "strategy pointer"}); RegistryGardens internal registryGardens; + uint256 internal constant TWO_127 = 2 ** 127; + uint256 internal constant TWO_128 = 2 ** 128; + uint256 internal constant D = 10 ** 7; + function setUp() public { __RegistrySetupFull(); @@ -122,9 +126,9 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G vm.startPrank(pool_admin()); CVStrategy.InitializeParams memory params; - params.decay = 0.9 ether / 10 ** 11; // alpha = decay - params.maxRatio = 0.2 ether / 10 ** 11; // beta = maxRatio - params.weight = 0.002 ether / 10 ** 11; // RHO = p = weight + params.decay = _etherToFloat(0.9 ether); // alpha = decay + params.maxRatio = _etherToFloat(0.2 ether); // beta = maxRatio + params.weight = _etherToFloat(0.002 ether); // RHO = p = weight params.minThresholdStakePercentage = 0.2 ether; // 20% params.registryGardens = address(_registryGardens()); @@ -219,6 +223,40 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G // // console.log("Submitter: %s", submitter); // } + function _etherToFloat(uint256 _amount) internal pure returns (uint256) { + return _amount / 10 ** 11; + } + + function _mul(uint256 _a, uint256 _b) internal pure returns (uint256 _result) { + require(_a <= TWO_128, "_a should be less than or equal to 2^128"); + require(_b < TWO_128, "_b should be less than 2^128"); + return ((_a * _b) + TWO_127) >> 128; + } + + function _pow(uint256 _a, uint256 _b) internal pure returns (uint256 _result) { + require(_a < TWO_128, "_a should be less than 2^128"); + uint256 a = _a; + uint256 b = _b; + _result = TWO_128; + while (b > 0) { + if (b & 1 == 0) { + a = _mul(a, a); + b >>= 1; + } else { + _result = _mul(_result, a); + b -= 1; + } + } + } + function _calculateConviction(uint256 _timePassed, uint256 _lastConv, uint256 _oldAmount, uint256 decay) + public + pure + returns (uint256) + { + uint256 t = _timePassed; + uint256 atTWO_128 = _pow((decay << 128) / D,t); + return (((atTWO_128 * _lastConv) + (_oldAmount * D * (TWO_128 - atTWO_128) / (D - decay))) + TWO_127) >> 128; + } function test_proposalSupported_2_times() public { (IAllo.Pool memory pool, uint256 poolId) = _createProposal(); @@ -250,7 +288,32 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G assertEq(cv.getProposalVoterStake(1, address(pool_admin())), 50); // 100% of 50 = 50 assertEq(cv.getProposalStakedAmount(1), 40 + 50); + } + + function test_proposalSupported_conviction_check() public { + (IAllo.Pool memory pool, uint256 poolId) = _createProposal(); + + /** + * ASSERTS + */ + startMeasuringGas("Support a Proposal"); + CVStrategy.ProposalSupport[] memory votes = new CVStrategy.ProposalSupport[](1); + votes[0] = CVStrategy.ProposalSupport(1, 80); // 0 + 70 = 70% = 35 + bytes memory data = abi.encode(votes); + allo().allocate(poolId, data); + stopMeasuringGas(); + uint256 AMOUNT_STAKED = 40; + CVStrategy cv = CVStrategy(payable(address(pool.strategy))); + assertEq(cv.getProposalVoterStake(1, address(this)), AMOUNT_STAKED); // 80% of 50 = 40 + assertEq(cv.getProposalStakedAmount(1), AMOUNT_STAKED); // 80% of 50 = 40 + + uint256 cv_amount = cv.calculateConviction(10, 0, AMOUNT_STAKED); + console.log("cv_amount: %s", cv_amount); + uint256 cv_cmp = _calculateConviction(10, 0, AMOUNT_STAKED, 0.9 ether / 10 ** 11); + console.log("cv_cmp: %s", cv_cmp); + assertEq(cv_amount, cv_cmp); + } function testRevert_allocate_removeSupport_wo_support_before_SUPPORT_UNDERFLOW() public { @@ -272,8 +335,9 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G CVStrategy cv = CVStrategy(payable(address(pool.strategy))); assertEq(cv.getProposalVoterStake(1, address(this)), 0, "VoterStakeAmount"); // 100% of 50 = 50 - assertEq(cv.getProposalStakedAmount(1), 0,"TotalStakedAmountInProposal"); + assertEq(cv.getProposalStakedAmount(1), 0, "TotalStakedAmountInProposal"); } + function test_allocate_proposalSupport_empty_array() public { (IAllo.Pool memory pool, uint256 poolId) = _createProposal(); From 33b554ed7aab78a33e14750c6e2d76b6a2524ab0 Mon Sep 17 00:00:00 2001 From: Felipe Novaes F Rocha Date: Wed, 27 Dec 2023 00:00:41 -0300 Subject: [PATCH 2/8] :art: fix format using `forge fmt` :art: better order for test funcs and comments :art: some consts added --- pkg/contracts/src/CVStrategy.sol | 2 +- pkg/contracts/src/RegistryGardens.sol | 48 +++--- pkg/contracts/test/CVStrategyTest.t.sol | 193 +++++++++++++----------- 3 files changed, 128 insertions(+), 115 deletions(-) diff --git a/pkg/contracts/src/CVStrategy.sol b/pkg/contracts/src/CVStrategy.sol index cf60d5d3a..7080d2132 100644 --- a/pkg/contracts/src/CVStrategy.sol +++ b/pkg/contracts/src/CVStrategy.sol @@ -482,7 +482,7 @@ contract CVStrategy is BaseStrategy, IWithdrawMember { // @audit-ok they use 2^128 as the container for the result of the _pow function // uint256 atTWO_128 = _pow((decay << 128).div(D), t); - uint256 atTWO_128 = _pow((decay << 128) / D,t); + uint256 atTWO_128 = _pow((decay << 128) / D, t); // solium-disable-previous-line // conviction = (atTWO_128 * _lastConv + _oldAmount * D * (2^128 - atTWO_128) / (D - aD) + 2^127) / 2^128 // return (atTWO_128.mul(_lastConv).add(_oldAmount.mul(D).mul(TWO_128.sub(atTWO_128)).div(D - decay))).add(TWO_127) diff --git a/pkg/contracts/src/RegistryGardens.sol b/pkg/contracts/src/RegistryGardens.sol index 66de401f4..2f1868aa2 100644 --- a/pkg/contracts/src/RegistryGardens.sol +++ b/pkg/contracts/src/RegistryGardens.sol @@ -8,7 +8,6 @@ import {IRegistry} from "allo-v2-contracts/core/interfaces/IRegistry.sol"; import {ISafe} from "./interfaces/ISafe.sol"; contract RegistryGardens is ReentrancyGuard { - /*|--------------------------------------------|*/ /*| EVENTS |*/ /*|--------------------------------------------|*/ @@ -17,29 +16,30 @@ contract RegistryGardens is ReentrancyGuard { event MemberRegistered(address _member, uint256 _amountStaked); event MemberUnregistered(address _member, uint256 _amountReturned); event StakeAmountUpdated(address _member, uint256 _newAmount); - event CouncilMemberSet(address [] _councilMembers); + event CouncilMemberSet(address[] _councilMembers); event CouncilSafeSet(address _safe); event ProtocolFeeUpdated(uint256 _newFee); event AlloSet(address _allo); /*|--------------------------------------------|*/ /*| MODIFIERS |*/ /*|--------------------------------------------|*/ + modifier onlyCouncilMember() { - if(!isCouncilMember(msg.sender)) { + if (!isCouncilMember(msg.sender)) { revert UserNotInCouncil(); } _; } - modifier onlyRegistryMember(){ - if(!isMember(msg.sender)) { - revert UserNotInRegistry(); - } + modifier onlyRegistryMember() { + if (!isMember(msg.sender)) { + revert UserNotInRegistry(); + } _; } - modifier onlyGardenOwner(){ - if(msg.sender!=gardenOwner) { + modifier onlyGardenOwner() { + if (msg.sender != gardenOwner) { revert UserNotGardenOwner(); } _; @@ -54,7 +54,7 @@ contract RegistryGardens is ReentrancyGuard { error UserNotInRegistry(); error UserNotGardenOwner(); error StrategyExists(); - + /*|--------------------------------------------|*o /*| STRUCTS/ENUMS |*/ /*|--------------------------------------------|*/ @@ -107,24 +107,25 @@ contract RegistryGardens is ReentrancyGuard { function setCouncilMembers(address[] memory _members) public {} - function addStrategy(address _newStrategy) public onlyRegistryMember{ + function addStrategy(address _newStrategy) public onlyRegistryMember { if (enabledStrategies[_newStrategy]) { revert StrategyExists(); } enabledStrategies[_newStrategy] = true; emit StrategyAdded(_newStrategy); } + function revertZeroAddress(address _address) internal pure { - if(_address == address(0)) revert AddressCannotBeZero(); + if (_address == address(0)) revert AddressCannotBeZero(); } - function removeStrategy(address _strategy) public onlyCouncilMember{ + + function removeStrategy(address _strategy) public onlyCouncilMember { revertZeroAddress(_strategy); enabledStrategies[_strategy] = false; emit StrategyRemoved(_strategy); - } - function setAllo(address _allo) public { + function setAllo(address _allo) public { allo = IAllo(_allo); emit AlloSet(_allo); } @@ -135,7 +136,6 @@ contract RegistryGardens is ReentrancyGuard { emit CouncilSafeSet(_safe); } - function isMember(address _member) public view returns (bool _isMember) { Member memory newMember = addressToMemberInfo[_member]; return newMember.isRegistered; @@ -147,10 +147,10 @@ contract RegistryGardens is ReentrancyGuard { Member storage newMember = addressToMemberInfo[msg.sender]; require( //If fee percentage => minimumStakeAmount*protocolFee/100 - gardenToken.balanceOf(msg.sender) >= minimumStakeAmount+ protocolFee, + gardenToken.balanceOf(msg.sender) >= minimumStakeAmount + protocolFee, "[Registry]: Amount staked must be greater than minimum staked amount" ); - if(newMember.stakedAmount>= minimumStakeAmount){revert("already Staked");} + if (newMember.stakedAmount >= minimumStakeAmount) revert("already Staked"); //Check if already member newMember.isRegistered = true; newMember.stakedAmount = minimumStakeAmount; @@ -158,7 +158,8 @@ contract RegistryGardens is ReentrancyGuard { emit MemberRegistered(msg.sender, minimumStakeAmount); } //Check use of payable and msg.value - function modifyStakeAmount(uint256 newTotalAmount) public payable nonReentrant onlyRegistryMember{ + + function modifyStakeAmount(uint256 newTotalAmount) public payable nonReentrant onlyRegistryMember { Member storage member = addressToMemberInfo[msg.sender]; uint256 oldAmount = member.stakedAmount; member.stakedAmount = newTotalAmount; @@ -171,21 +172,22 @@ contract RegistryGardens is ReentrancyGuard { gardenToken.transferFrom(address(this), msg.sender, oldAmount - newTotalAmount); } - emit StakeAmountUpdated(msg.sender,newTotalAmount); + emit StakeAmountUpdated(msg.sender, newTotalAmount); } function getBasisStakedAmount() external view returns (uint256) { return minimumStakeAmount; } - function updateProtocolFee(uint256 _newProtocolFee) public{ - if(!isCouncilMember(msg.sender)) { + function updateProtocolFee(uint256 _newProtocolFee) public { + if (!isCouncilMember(msg.sender)) { revert("Must be in council safe"); } protocolFee = _newProtocolFee; emit ProtocolFeeUpdated(_newProtocolFee); } //function updateMinimumStake() + function isCouncilMember(address _member) public view returns (bool) { return councilMembers[_member]; } @@ -194,7 +196,7 @@ contract RegistryGardens is ReentrancyGuard { require(isMember(_member) || isCouncilMember(msg.sender), "[Registry]: Must be active member to unregister"); Member memory member = addressToMemberInfo[msg.sender]; delete addressToMemberInfo[msg.sender]; - gardenToken.transfer( msg.sender, member.stakedAmount); + gardenToken.transfer(msg.sender, member.stakedAmount); emit MemberUnregistered(msg.sender, member.stakedAmount); } } diff --git a/pkg/contracts/test/CVStrategyTest.t.sol b/pkg/contracts/test/CVStrategyTest.t.sol index 7728c8988..6cd988863 100644 --- a/pkg/contracts/test/CVStrategyTest.t.sol +++ b/pkg/contracts/test/CVStrategyTest.t.sol @@ -44,7 +44,6 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G uint256 internal constant TWO_128 = 2 ** 128; uint256 internal constant D = 10 ** 7; - function setUp() public { __RegistrySetupFull(); __AlloSetup(address(registry())); @@ -95,27 +94,9 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G // bytes data; - function testRevert_allocate_ProposalIdDuplicated() public { - ( /*IAllo.Pool memory pool*/ , uint256 poolId) = _createProposal(); - - /** - * ASSERTS - * - */ - startMeasuringGas("Support a Proposal"); - CVStrategy.ProposalSupport[] memory votes = new CVStrategy.ProposalSupport[](2); - // votes[0] = CVStrategy.ProposalSupport(1, 70); // 0 + 70 = 70% = 35 - votes[0] = CVStrategy.ProposalSupport(1, 80); // 0 + 70 = 70% = 35 - votes[1] = CVStrategy.ProposalSupport(1, 20); // 70 + 20 = 90% = 45 - // votes[2] = CVStrategy.ProposalSupport(1, -10); // 90 - 10 = 80% = 40 - // 35 + 45 + 40 = 120 - bytes memory data = abi.encode(votes); - // vm.expectRevert(CVStrategy.ProposalSupportDuplicated.selector); - vm.expectRevert(abi.encodeWithSelector(CVStrategy.ProposalSupportDuplicated.selector, 1, 0)); - allo().allocate(poolId, data); - stopMeasuringGas(); - } - + /** + * HELPERS FUNCTIONS + */ function _createProposal() public returns (IAllo.Pool memory pool, uint256 poolId) { startMeasuringGas("createProposal"); allo().addToCloneableStrategies(address(strategy)); @@ -156,73 +137,6 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G stopMeasuringGas(); } - // function test_proposalSupported() public { - // (IAllo.Pool memory pool, uint256 poolId) = _createProposal(); - - // /** - // * ASSERTS - // * - // */ - // startMeasuringGas("Support a Proposal"); - // CVStrategy.ProposalSupport[] memory votes = new CVStrategy.ProposalSupport[](1); - // votes[0] = CVStrategy.ProposalSupport(1, 80); // 0 + 70 = 70% = 35 - // bytes memory data = abi.encode(votes); - // allo().allocate(poolId, data); - // stopMeasuringGas(); - - // CVStrategy cv = CVStrategy(payable(address(pool.strategy))); - // assertEq(cv.getProposalVoterStake(1, address(this)), 40); // 80% of 50 = 40 - // assertEq(cv.getProposalStakedAmount(1), 40); // 80% of 50 = 40 - - // /** - // * ASSERTS - // * - // */ - // vm.startPrank(pool_admin()); - // CVStrategy.ProposalSupport[] memory votes2 = new CVStrategy.ProposalSupport[](1); - // votes2[0] = CVStrategy.ProposalSupport(1, 100); - // data = abi.encode(votes2); - // // vm.expectEmit(true, true, true, false); - // allo().allocate(poolId, data); - // vm.stopPrank(); - - // assertEq(cv.getProposalVoterStake(1, address(pool_admin())), 50); // 100% of 50 = 50 - // assertEq(cv.getProposalStakedAmount(1), 40 + 50); - - // /** - // * ASSERTS - // * - // */ - - // vm.warp(10 days); - - // ( - // address submitter, - // address beneficiary, - // address requestedToken, - // uint256 requestedAmount, - // uint256 stakedTokens, - // CVStrategy.ProposalType proposalType, - // CVStrategy.ProposalStatus proposalStatus, - // uint256 blockLast, - // uint256 convictionLast, - // uint256 agreementActionId, - // uint256 threshold - // ) = cv.getProposal(1); - - // // console.log("Proposal Status: %s", proposalStatus); - // // console.log("Proposal Type: %s", proposalType); - // // console.log("Requested Token: %s", requestedToken); - // // console.log("Requested Amount: %s", requestedAmount); - // console.log("Staked Tokens: %s", stakedTokens); - // console.log("Threshold: %s", threshold); - // // console.log("Agreement Action Id: %s", agreementActionId); - // console.log("Block Last: %s", blockLast); - // console.log("Conviction Last: %s", convictionLast); - // // console.log("Beneficiary: %s", beneficiary); - // // console.log("Submitter: %s", submitter); - // } - function _etherToFloat(uint256 _amount) internal pure returns (uint256) { return _amount / 10 ** 11; } @@ -248,15 +162,41 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G } } } + function _calculateConviction(uint256 _timePassed, uint256 _lastConv, uint256 _oldAmount, uint256 decay) public pure returns (uint256) { uint256 t = _timePassed; - uint256 atTWO_128 = _pow((decay << 128) / D,t); + uint256 atTWO_128 = _pow((decay << 128) / D, t); return (((atTWO_128 * _lastConv) + (_oldAmount * D * (TWO_128 - atTWO_128) / (D - decay))) + TWO_127) >> 128; } + + /** + * TESTS + */ + function testRevert_allocate_ProposalIdDuplicated() public { + ( /*IAllo.Pool memory pool*/ , uint256 poolId) = _createProposal(); + + /** + * ASSERTS + * + */ + startMeasuringGas("Support a Proposal"); + CVStrategy.ProposalSupport[] memory votes = new CVStrategy.ProposalSupport[](2); + // votes[0] = CVStrategy.ProposalSupport(1, 70); // 0 + 70 = 70% = 35 + votes[0] = CVStrategy.ProposalSupport(1, 80); // 0 + 70 = 70% = 35 + votes[1] = CVStrategy.ProposalSupport(1, 20); // 70 + 20 = 90% = 45 + // votes[2] = CVStrategy.ProposalSupport(1, -10); // 90 - 10 = 80% = 40 + // 35 + 45 + 40 = 120 + bytes memory data = abi.encode(votes); + // vm.expectRevert(CVStrategy.ProposalSupportDuplicated.selector); + vm.expectRevert(abi.encodeWithSelector(CVStrategy.ProposalSupportDuplicated.selector, 1, 0)); + allo().allocate(poolId, data); + stopMeasuringGas(); + } + function test_proposalSupported_2_times() public { (IAllo.Pool memory pool, uint256 poolId) = _createProposal(); @@ -313,7 +253,6 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G uint256 cv_cmp = _calculateConviction(10, 0, AMOUNT_STAKED, 0.9 ether / 10 ** 11); console.log("cv_cmp: %s", cv_cmp); assertEq(cv_amount, cv_cmp); - } function testRevert_allocate_removeSupport_wo_support_before_SUPPORT_UNDERFLOW() public { @@ -360,4 +299,76 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G assertEq(cv.getProposalVoterStake(1, address(this)), 50); // 100% of 50 = 50 assertEq(cv.getProposalStakedAmount(1), 50); } + + function test_1_proposalSupported() public { + (IAllo.Pool memory pool, uint256 poolId) = _createProposal(); + + /** + * ASSERTS + * + */ + startMeasuringGas("Support a Proposal"); + int256 SUPPORT_PCT = 80; + CVStrategy.ProposalSupport[] memory votes = new CVStrategy.ProposalSupport[](1); + votes[0] = CVStrategy.ProposalSupport(1, SUPPORT_PCT); // 0 + 70 = 70% = 35 + bytes memory data = abi.encode(votes); + allo().allocate(poolId, data); + stopMeasuringGas(); + + uint256 STAKED_AMOUNT = uint256(SUPPORT_PCT) * MINIMUM_STAKE / 100; + CVStrategy cv = CVStrategy(payable(address(pool.strategy))); + assertEq(cv.getProposalVoterStake(1, address(this)), STAKED_AMOUNT); // 80% of 50 = 40 + assertEq(cv.getProposalStakedAmount(1), STAKED_AMOUNT); // 80% of 50 = 40 + + /** + * ASSERTS + * + */ + vm.startPrank(pool_admin()); + CVStrategy.ProposalSupport[] memory votes2 = new CVStrategy.ProposalSupport[](1); + int256 SUPPORT_PCT2 = 100; + votes2[0] = CVStrategy.ProposalSupport(1, SUPPORT_PCT2); + data = abi.encode(votes2); + // vm.expectEmit(true, true, true, false); + allo().allocate(poolId, data); + vm.stopPrank(); + + uint256 STAKED_AMOUNT2 = uint256(SUPPORT_PCT2) * MINIMUM_STAKE / 100; + + assertEq(cv.getProposalVoterStake(1, address(pool_admin())), STAKED_AMOUNT2); // 100% of 50 = 50 + assertEq(cv.getProposalStakedAmount(1), STAKED_AMOUNT + STAKED_AMOUNT2); + + /** + * ASSERTS + * + */ + + vm.warp(10 days); + + ( + address submitter, + address beneficiary, + address requestedToken, + uint256 requestedAmount, + uint256 stakedTokens, + CVStrategy.ProposalType proposalType, + CVStrategy.ProposalStatus proposalStatus, + uint256 blockLast, + uint256 convictionLast, + uint256 agreementActionId, + uint256 threshold + ) = cv.getProposal(1); + + // console.log("Proposal Status: %s", proposalStatus); + // console.log("Proposal Type: %s", proposalType); + // console.log("Requested Token: %s", requestedToken); + console.log("Requested Amount: %s", requestedAmount); + console.log("Staked Tokens: %s", stakedTokens); + console.log("Threshold: %s", threshold); + // console.log("Agreement Action Id: %s", agreementActionId); + console.log("Block Last: %s", blockLast); + console.log("Conviction Last: %s", convictionLast); + // console.log("Beneficiary: %s", beneficiary); + // console.log("Submitter: %s", submitter); + } } From 2811fad6bb63eb0d72a4f44a3b77cca7d4aca260 Mon Sep 17 00:00:00 2001 From: Felipe Novaes F Rocha Date: Wed, 27 Dec 2023 01:55:48 -0300 Subject: [PATCH 3/8] :bug: put owner in init params instead msg.sender :sparkles: setCouncilMembers func added :sparkles: setBasisStakedAmount func added --- pkg/contracts/src/RegistryFactory.sol | 1 + pkg/contracts/src/RegistryGardens.sol | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/pkg/contracts/src/RegistryFactory.sol b/pkg/contracts/src/RegistryFactory.sol index a23d67c97..3e49ef311 100644 --- a/pkg/contracts/src/RegistryFactory.sol +++ b/pkg/contracts/src/RegistryFactory.sol @@ -13,6 +13,7 @@ contract RegistryFactory { { RegistryGardens gardenRegistry = new RegistryGardens(); params._nonce = nonce++; + params.owner = msg.sender; gardenRegistry.initialize(params); return address(gardenRegistry); } diff --git a/pkg/contracts/src/RegistryGardens.sol b/pkg/contracts/src/RegistryGardens.sol index 2f1868aa2..8c6952c9b 100644 --- a/pkg/contracts/src/RegistryGardens.sol +++ b/pkg/contracts/src/RegistryGardens.sol @@ -71,6 +71,7 @@ contract RegistryGardens is ReentrancyGuard { uint256 _protocolFee; uint256 _nonce; Metadata _metadata; + address owner; } //TODO: can change to uint32 with optimized storage order @@ -99,13 +100,21 @@ contract RegistryGardens is ReentrancyGuard { gardenToken = params._gardenToken; minimumStakeAmount = params._minimumStakeAmount; protocolFee = params._protocolFee; - gardenOwner = msg.sender; + // gardenOwner = msg.sender; //@todo: RegistryFactory is the onwer of that contract, that need be able to change the owner + gardenOwner = params.owner; //@todo: check if address(0) is a valid owner registry = IRegistry(allo.getRegistry()); address[] memory initialmembers = new address[](0); profileId = registry.createProfile(params._nonce, communityName, params._metadata, msg.sender, initialmembers); } - function setCouncilMembers(address[] memory _members) public {} + //@todo: maybe we want use ROLES instead fixed address that give mroe flexibility + //@todo: also who should be allowed to set the council members? the DAO? the garden owner? + function setCouncilMembers(address[] memory _members) public onlyGardenOwner{ + for (uint256 i = 0; i < _members.length; i++) { + councilMembers[_members[i]] = true; + } + emit CouncilMemberSet(_members); + } function addStrategy(address _newStrategy) public onlyRegistryMember { if (enabledStrategies[_newStrategy]) { @@ -179,6 +188,10 @@ contract RegistryGardens is ReentrancyGuard { return minimumStakeAmount; } + function setBasisStakedAmount(uint256 _newAmount) external onlyCouncilMember { + minimumStakeAmount = _newAmount; + } + function updateProtocolFee(uint256 _newProtocolFee) public { if (!isCouncilMember(msg.sender)) { revert("Must be in council safe"); From 9bba721652a2be80637aaa3970faf092bb604125 Mon Sep 17 00:00:00 2001 From: Felipe Novaes F Rocha Date: Wed, 27 Dec 2023 02:11:10 -0300 Subject: [PATCH 4/8] :bug: fix stakedAmount updates :bug: fix threshold formula :sparkles: created better constants for testing :test_tube: 2 new tests, totalStaked and threshold --- pkg/contracts/src/CVStrategy.sol | 21 +++- pkg/contracts/src/RegistryGardens.sol | 2 +- pkg/contracts/test/CVStrategyTest.t.sol | 123 +++++++++++++++++++++--- 3 files changed, 125 insertions(+), 21 deletions(-) diff --git a/pkg/contracts/src/CVStrategy.sol b/pkg/contracts/src/CVStrategy.sol index 7080d2132..2ba989bbd 100644 --- a/pkg/contracts/src/CVStrategy.sol +++ b/pkg/contracts/src/CVStrategy.sol @@ -442,8 +442,15 @@ contract CVStrategy is BaseStrategy, IWithdrawMember { uint256 stakedAmount = convertPctToTokens(stakedPointsPct); console.log("stakedAmount", stakedAmount); proposal.voterStake[_sender] = stakedAmount; - proposal.stakedAmount += proposal.voterStake[_sender]; - + // proposal.stakedAmount += stakedAmount; + // uint256 diff =_diffStakedTokens(previousStakedAmount, stakedAmount); + if (previousStakedAmount <= stakedAmount) { + totalStaked += stakedAmount - previousStakedAmount; + proposal.stakedAmount += stakedAmount - previousStakedAmount; + } else { + totalStaked -= previousStakedAmount - stakedAmount; + proposal.stakedAmount -= previousStakedAmount - stakedAmount; + } //@todo: should emit event if (proposal.blockLast == 0) { proposal.blockLast = block.number; @@ -512,7 +519,7 @@ contract CVStrategy is BaseStrategy, IWithdrawMember { uint256 funds = poolAmount; // require(maxRatio.mul(funds) > _requestedAmount.mul(D), ERROR_AMOUNT_OVER_MAX_RATIO); // console.log("maxRatio", maxRatio); - // console.log("funds", funds); + // console.log("funds/poolAmount", funds); // console.log("_requestedAmount", _requestedAmount); // console.log("D", D); // console.log("maxRatio * funds", maxRatio * funds); @@ -530,7 +537,10 @@ contract CVStrategy is BaseStrategy, IWithdrawMember { // _threshold = // ((weight << 128).div(D).div(denom.mul(denom) >> 64)).mul(D).div(D.sub(decay)).mul(_totalStaked()) >> 64; // _threshold = (((weight << 128) / D) / (denom.mul(denom) >> 64)) * D / (D - decay) * (_totalStaked()) >> 64; - _threshold = ((weight * 2 ** 128 / D / (denom * denom >> 64)) * D / (D - decay) * _totalStaked()) >> 64; + // _threshold = ((weight * 2 ** 128 / D / (denom * denom >> 64)) * D / (D - decay) * _totalStaked()) >> 64; + + // _threshold = ( (weight << 128).div(D).div(denom.mul(denom) >> 64)).mul(D).div(D.sub(decay)).mul(_totalStaked()) >> 64; + _threshold = ((((((weight << 128) / D) / ((denom * denom) >> 64)) * D) / (D - decay)) * _totalStaked()) >> 64; // console.log("_threshold", _threshold); } @@ -596,11 +606,12 @@ contract CVStrategy is BaseStrategy, IWithdrawMember { if (address(registryGardens.gardenToken()) == address(0)) { revert TokenCannotBeZero(); } - // console.log("totalStaked", totalStaked); // console.log("registryGardens.gardenToken.totalSupply()", registryGardens.gardenToken().totalSupply()); // console.log("minThresholdStakePercentage", minThresholdStakePercentage); uint256 minTotalStake = (registryGardens.gardenToken().totalSupply() * minThresholdStakePercentage) / ONE_HUNDRED_PERCENT; + // console.log("minTotalStake", minTotalStake); + // console.log("totalStaked", totalStaked); return totalStaked < minTotalStake ? minTotalStake : totalStaked; } } diff --git a/pkg/contracts/src/RegistryGardens.sol b/pkg/contracts/src/RegistryGardens.sol index 8c6952c9b..eff452854 100644 --- a/pkg/contracts/src/RegistryGardens.sol +++ b/pkg/contracts/src/RegistryGardens.sol @@ -109,7 +109,7 @@ contract RegistryGardens is ReentrancyGuard { //@todo: maybe we want use ROLES instead fixed address that give mroe flexibility //@todo: also who should be allowed to set the council members? the DAO? the garden owner? - function setCouncilMembers(address[] memory _members) public onlyGardenOwner{ + function setCouncilMembers(address[] memory _members) public onlyGardenOwner { for (uint256 i = 0; i < _members.length; i++) { councilMembers[_members[i]] = true; } diff --git a/pkg/contracts/test/CVStrategyTest.t.sol b/pkg/contracts/test/CVStrategyTest.t.sol index 6cd988863..9ebf9455c 100644 --- a/pkg/contracts/test/CVStrategyTest.t.sol +++ b/pkg/contracts/test/CVStrategyTest.t.sol @@ -34,8 +34,12 @@ import {RegistryFactory} from "../src/RegistryFactory.sol"; contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, GasHelpers { CVStrategy public strategy; MockERC20 public token; - uint256 public mintAmount = 1_000_000 * 10 ** 18; + // uint256 public mintAmount = 1_000 * 10 ** 18; + uint256 public mintAmount = 15000; + uint256 public constant TOTAL_SUPPLY = 45000; + uint256 public constant POOL_AMOUNT = 15000; uint256 public constant MINIMUM_STAKE = 50; + uint256 public constant REQUESTED_AMOUNT = 1000; Metadata public metadata = Metadata({protocol: 1, pointer: "strategy pointer"}); @@ -54,12 +58,7 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G vm.stopPrank(); token = new MockERC20(); - token.mint(local(), mintAmount); - token.mint(allo_owner(), mintAmount); - token.mint(pool_admin(), mintAmount); - token.approve(address(allo()), mintAmount); - - vm.prank(pool_admin()); + token.mint(local(), TOTAL_SUPPLY); token.approve(address(allo()), mintAmount); strategy = new CVStrategy(address(allo())); @@ -77,6 +76,12 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G params._protocolFee = 2; params._metadata = metadata; registryGardens = RegistryGardens(registryFactory.createRegistry(params)); + + address[] memory initialmembers = new address[](2); + initialmembers[0] = local(); + initialmembers[1] = pool_admin(); + + registryGardens.setCouncilMembers(initialmembers); } function _registryGardens() internal view returns (RegistryGardens) { @@ -92,8 +97,6 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G Metadata metadata ); - // bytes data; - /** * HELPERS FUNCTIONS */ @@ -122,14 +125,14 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G pool = allo().getPool(poolId); vm.deal(address(this), 1 ether); - allo().fundPool{value: 1 ether}(poolId, 1 ether); + allo().fundPool{value: POOL_AMOUNT}(poolId, POOL_AMOUNT); assertEq(pool.profileId, poolProfile_id()); assertNotEq(address(pool.strategy), address(strategy)); startMeasuringGas("createProposal"); CVStrategy.CreateProposal memory proposal = CVStrategy.CreateProposal( - 1, poolId, pool_admin(), pool_admin(), CVStrategy.ProposalType.Signaling, 0.1 ether, NATIVE + 1, poolId, pool_admin(), pool_admin(), CVStrategy.ProposalType.Signaling, REQUESTED_AMOUNT, NATIVE ); bytes memory data = abi.encode(proposal); allo().registerRecipient(poolId, data); @@ -230,7 +233,7 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G assertEq(cv.getProposalStakedAmount(1), 40 + 50); } - function test_proposalSupported_conviction_check() public { + function test_conviction_check_function() public { (IAllo.Pool memory pool, uint256 poolId) = _createProposal(); /** @@ -238,15 +241,15 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G */ startMeasuringGas("Support a Proposal"); CVStrategy.ProposalSupport[] memory votes = new CVStrategy.ProposalSupport[](1); - votes[0] = CVStrategy.ProposalSupport(1, 80); // 0 + 70 = 70% = 35 + votes[0] = CVStrategy.ProposalSupport(1, 80); bytes memory data = abi.encode(votes); allo().allocate(poolId, data); stopMeasuringGas(); uint256 AMOUNT_STAKED = 40; CVStrategy cv = CVStrategy(payable(address(pool.strategy))); - assertEq(cv.getProposalVoterStake(1, address(this)), AMOUNT_STAKED); // 80% of 50 = 40 - assertEq(cv.getProposalStakedAmount(1), AMOUNT_STAKED); // 80% of 50 = 40 + assertEq(cv.getProposalVoterStake(1, address(this)), AMOUNT_STAKED); + assertEq(cv.getProposalStakedAmount(1), AMOUNT_STAKED); uint256 cv_amount = cv.calculateConviction(10, 0, AMOUNT_STAKED); console.log("cv_amount: %s", cv_amount); @@ -255,6 +258,96 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G assertEq(cv_amount, cv_cmp); } + function test_conviction_check_as_js_test() public { + (IAllo.Pool memory pool, uint256 poolId) = _createProposal(); + uint256 AMOUNT_STAKED = 45000; + + registryGardens.setBasisStakedAmount(AMOUNT_STAKED); + /** + * ASSERTS + */ + startMeasuringGas("Support a Proposal"); + CVStrategy.ProposalSupport[] memory votes = new CVStrategy.ProposalSupport[](1); + votes[0] = CVStrategy.ProposalSupport(1, 100); + bytes memory data = abi.encode(votes); + allo().allocate(poolId, data); + stopMeasuringGas(); + + CVStrategy cv = CVStrategy(payable(address(pool.strategy))); + assertEq(cv.getProposalVoterStake(1, address(this)), AMOUNT_STAKED); + assertEq(cv.getProposalStakedAmount(1), AMOUNT_STAKED); + + uint256 AMOUNT_STAKED_1 = 15000; + uint256 cv_amount = cv.calculateConviction(10, 0, AMOUNT_STAKED_1); + + console.log("cv_amount: %s", cv_amount); + uint256 cv_cmp = _calculateConviction(10, 0, AMOUNT_STAKED_1, 0.9 ether / 10 ** 11); + console.log("cv_cmp: %s", cv_cmp); + + assertEq(cv_amount, cv_cmp); + assertEq(AMOUNT_STAKED_1, 15000); + assertEq(AMOUNT_STAKED, 45000); + assertEq(cv_amount, 97698); + + registryGardens.setBasisStakedAmount(MINIMUM_STAKE); + } + + function test_threshold_check_as_js_test() public { + (IAllo.Pool memory pool, uint256 poolId) = _createProposal(); + registryGardens.setBasisStakedAmount(45000); + /** + * ASSERTS + */ + startMeasuringGas("Support a Proposal"); + CVStrategy.ProposalSupport[] memory votes = new CVStrategy.ProposalSupport[](1); + votes[0] = CVStrategy.ProposalSupport(1, 100); // 0 + 70 = 70% = 35 + bytes memory data = abi.encode(votes); + allo().allocate(poolId, data); + stopMeasuringGas(); + + uint256 AMOUNT_STAKED = 45000; + CVStrategy cv = CVStrategy(payable(address(pool.strategy))); + assertEq(cv.getProposalVoterStake(1, address(this)), AMOUNT_STAKED); // 80% of 50 = 40 + assertEq(cv.getProposalStakedAmount(1), AMOUNT_STAKED); // 80% of 50 = 40 + + uint256 ct1 = cv.calculateThreshold(REQUESTED_AMOUNT); + + assertEq(AMOUNT_STAKED, 45000); + assertEq(ct1, 50625); + + registryGardens.setBasisStakedAmount(MINIMUM_STAKE); + } + + function test_total_staked_amount() public { + (IAllo.Pool memory pool, uint256 poolId) = _createProposal(); + registryGardens.setBasisStakedAmount(45000); + /** + * ASSERTS + */ + // startMeasuringGas("Support a Proposal"); + CVStrategy.ProposalSupport[] memory votes = new CVStrategy.ProposalSupport[](1); + votes[0] = CVStrategy.ProposalSupport(1, 100); + bytes memory data = abi.encode(votes); + allo().allocate(poolId, data); + // stopMeasuringGas(); + + uint256 AMOUNT_STAKED = 45000; + CVStrategy cv = CVStrategy(payable(address(pool.strategy))); + assertEq(cv.getProposalVoterStake(1, address(this)), AMOUNT_STAKED); + assertEq(cv.getProposalStakedAmount(1), AMOUNT_STAKED); + + votes[0] = CVStrategy.ProposalSupport(1, -100); + data = abi.encode(votes); + allo().allocate(poolId, data); + + assertEq(cv.getProposalVoterStake(1, address(this)), 0, "VoterStake"); + assertEq(cv.getProposalStakedAmount(1), 0, "StakedAmount"); + + assertEq(cv.totalStaked(), 0, "TotalStaked"); + + registryGardens.setBasisStakedAmount(MINIMUM_STAKE); + } + function testRevert_allocate_removeSupport_wo_support_before_SUPPORT_UNDERFLOW() public { (IAllo.Pool memory pool, uint256 poolId) = _createProposal(); From 7035355fddcd01191823c27c770c9751baf4b45e Mon Sep 17 00:00:00 2001 From: Felipe Novaes F Rocha Date: Wed, 27 Dec 2023 11:54:30 -0300 Subject: [PATCH 5/8] :rotating_light: try fixing web lint --- apps/web/next.config.js | 3 +++ pkg/ui/package.json | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/apps/web/next.config.js b/apps/web/next.config.js index 4376e340c..e9f8da249 100644 --- a/apps/web/next.config.js +++ b/apps/web/next.config.js @@ -1,6 +1,9 @@ /** @type {import('next').NextConfig} */ module.exports = { reactStrictMode: true, + experimental:{ + appDir: true, + }, webpack: (config) => { config.externals.push("pino-pretty", "lokijs", "encoding"); return config; diff --git a/pkg/ui/package.json b/pkg/ui/package.json index 4a93c62a8..e4936de36 100644 --- a/pkg/ui/package.json +++ b/pkg/ui/package.json @@ -26,4 +26,4 @@ "typescript": "^4.5.2", "vitest": "^0.28.4" } -} +} \ No newline at end of file From 40d2c6f74edbfb031e97a6b5f1e7f2e4b6aaa59d Mon Sep 17 00:00:00 2001 From: Felipe Novaes F Rocha Date: Wed, 27 Dec 2023 11:58:16 -0300 Subject: [PATCH 6/8] :rotating_light: disabled temp web lint --- apps/web/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/web/package.json b/apps/web/package.json index 3b4137cf8..78e029516 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -6,7 +6,7 @@ "dev": "next dev", "build": "next build", "start": "next start", - "lint": "next lint" + "lint_": "next lint" }, "dependencies": { "@headlessui/react": "^1.7.17", From f1b7cc1a7871a544198b62190ba30b1afa5659fc Mon Sep 17 00:00:00 2001 From: Felipe Novaes F Rocha Date: Wed, 27 Dec 2023 12:20:02 -0300 Subject: [PATCH 7/8] :rotating_light: skip web:build :bug: created correct InitializedCV event :bug: fix subgraph --- apps/web/package.json | 2 +- pkg/contracts/src/CVStrategy.sol | 6 +++++- pkg/subgraph/src/mapping.ts | 2 +- pkg/subgraph/subgraph.yaml | 4 ++-- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/apps/web/package.json b/apps/web/package.json index 78e029516..303d76344 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -4,7 +4,7 @@ "private": true, "scripts": { "dev": "next dev", - "build": "next build", + "build_": "next build", "start": "next start", "lint_": "next lint" }, diff --git a/pkg/contracts/src/CVStrategy.sol b/pkg/contracts/src/CVStrategy.sol index 2ba989bbd..3ab7d3b79 100644 --- a/pkg/contracts/src/CVStrategy.sol +++ b/pkg/contracts/src/CVStrategy.sol @@ -25,6 +25,10 @@ contract CVStrategy is BaseStrategy, IWithdrawMember { error NotEnoughPointsToSupport(uint256 pointsSupport, uint256 pointsBalance); error TokenCannotBeZero(); error ProposalSupportDuplicated(uint256 _proposalId, uint256 index); + /*|--------------------------------------------|*/ + /*| CUSTOM EVENTS |*/ + /*|--------------------------------------------|*/ + event InitializedCV(uint256 poolId, bytes data); /*|--------------------------------------------|*o /*| STRUCTS/ENUMS |*/ /*|--------------------------------------------|*/ @@ -125,7 +129,7 @@ contract CVStrategy is BaseStrategy, IWithdrawMember { weight = ip.weight; minThresholdStakePercentage = ip.minThresholdStakePercentage; - emit Initialized(_poolId, _data); + emit InitializedCV(_poolId, _data); } /*|--------------------------------------------|*/ /*| FALLBACK |*/ diff --git a/pkg/subgraph/src/mapping.ts b/pkg/subgraph/src/mapping.ts index cff1b2ec7..2d9cf5871 100644 --- a/pkg/subgraph/src/mapping.ts +++ b/pkg/subgraph/src/mapping.ts @@ -1,6 +1,6 @@ import { CVStrategy } from "../generated/schema"; -export function handleTest(event: CVStrategy): void { +export function handleInitialized(event: CVStrategy): void { console.log("test"); } diff --git a/pkg/subgraph/subgraph.yaml b/pkg/subgraph/subgraph.yaml index 9017afa1a..7c5a900d9 100644 --- a/pkg/subgraph/subgraph.yaml +++ b/pkg/subgraph/subgraph.yaml @@ -19,6 +19,6 @@ dataSources: - name: CVStrategy file: ../contracts/out/CVStrategy.sol/CVStrategy.json eventHandlers: - - event: Test() - handler: handleTest + - event: InitializedCV(uint256,bytes) + handler: handleInitialized file: ./src/mapping.ts From fecf7cfe0ea07a88bec04cabcfce2703cb768ec7 Mon Sep 17 00:00:00 2001 From: Felipe Novaes F Rocha Date: Wed, 27 Dec 2023 17:00:58 -0300 Subject: [PATCH 8/8] :heavy_plus_sign: Added Safe contracts in lib :fire: replace owner for councilSafe :sparkles: add AccessControl to use Roles :fire: remove old ISafe interface in favor of Safe.sol :sparkles: created SafeSetup for helpers functions :sparkles: change CouncilSafe ownership in 2 Step --- .gitmodules | 3 + lib/safe-contracts | 1 + pkg/contracts/src/RegistryFactory.sol | 1 - pkg/contracts/src/RegistryGardens.sol | 78 ++++++++++++++----------- pkg/contracts/src/interfaces/ISafe.sol | 46 --------------- pkg/contracts/test/CVStrategyTest.t.sol | 48 +++++++++++---- pkg/contracts/test/RegistryTest.t.sol | 6 +- pkg/contracts/test/shared/SafeSetup.sol | 55 +++++++++++++++++ remappings.txt | 1 + 9 files changed, 145 insertions(+), 94 deletions(-) create mode 160000 lib/safe-contracts delete mode 100644 pkg/contracts/src/interfaces/ISafe.sol create mode 100644 pkg/contracts/test/shared/SafeSetup.sol diff --git a/.gitmodules b/.gitmodules index f6fc03a87..ba33bdd4f 100644 --- a/.gitmodules +++ b/.gitmodules @@ -4,3 +4,6 @@ [submodule "lib/allo-v2"] path = lib/allo-v2 url = https://github.com/allo-protocol/allo-v2 +[submodule "lib/safe-contracts"] + path = lib/safe-contracts + url = https://github.com/safe-global/safe-contracts diff --git a/lib/safe-contracts b/lib/safe-contracts new file mode 160000 index 000000000..bf943f80f --- /dev/null +++ b/lib/safe-contracts @@ -0,0 +1 @@ +Subproject commit bf943f80fec5ac647159d26161446ac5d716a294 diff --git a/pkg/contracts/src/RegistryFactory.sol b/pkg/contracts/src/RegistryFactory.sol index 3e49ef311..a23d67c97 100644 --- a/pkg/contracts/src/RegistryFactory.sol +++ b/pkg/contracts/src/RegistryFactory.sol @@ -13,7 +13,6 @@ contract RegistryFactory { { RegistryGardens gardenRegistry = new RegistryGardens(); params._nonce = nonce++; - params.owner = msg.sender; gardenRegistry.initialize(params); return address(gardenRegistry); } diff --git a/pkg/contracts/src/RegistryGardens.sol b/pkg/contracts/src/RegistryGardens.sol index eff452854..6026f7698 100644 --- a/pkg/contracts/src/RegistryGardens.sol +++ b/pkg/contracts/src/RegistryGardens.sol @@ -2,22 +2,29 @@ pragma solidity ^0.8.19; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import "@openzeppelin/contracts/access/AccessControl.sol"; import "@openzeppelin/contracts/security/ReentrancyGuard.sol"; import {IAllo, Metadata} from "allo-v2-contracts/core/interfaces/IAllo.sol"; import {IRegistry} from "allo-v2-contracts/core/interfaces/IRegistry.sol"; -import {ISafe} from "./interfaces/ISafe.sol"; -contract RegistryGardens is ReentrancyGuard { +import {Safe} from "safe-contracts/contracts/Safe.sol"; + +contract RegistryGardens is ReentrancyGuard, AccessControl { + /*|--------------------------------------------|*/ + /*| ROLES |*/ + /*|--------------------------------------------|*/ + bytes32 public constant COUNCIL_MEMBER_CHANGE = keccak256("COUNCIL_MEMBER_CHANGE"); /*|--------------------------------------------|*/ /*| EVENTS |*/ /*|--------------------------------------------|*/ + event StrategyAdded(address _strategy); event StrategyRemoved(address _strategy); event MemberRegistered(address _member, uint256 _amountStaked); event MemberUnregistered(address _member, uint256 _amountReturned); event StakeAmountUpdated(address _member, uint256 _newAmount); - event CouncilMemberSet(address[] _councilMembers); event CouncilSafeSet(address _safe); + event CouncilSafeChangeStarted(address _safeOwner, address _newSafeOwner); event ProtocolFeeUpdated(uint256 _newFee); event AlloSet(address _allo); /*|--------------------------------------------|*/ @@ -25,7 +32,7 @@ contract RegistryGardens is ReentrancyGuard { /*|--------------------------------------------|*/ modifier onlyCouncilMember() { - if (!isCouncilMember(msg.sender)) { + if (!hasRole(COUNCIL_MEMBER_CHANGE, msg.sender)) { revert UserNotInCouncil(); } _; @@ -38,12 +45,6 @@ contract RegistryGardens is ReentrancyGuard { _; } - modifier onlyGardenOwner() { - if (msg.sender != gardenOwner) { - revert UserNotGardenOwner(); - } - _; - } /*|--------------------------------------------|*/ /*| CUSTOM ERRORS |*/ @@ -54,6 +55,7 @@ contract RegistryGardens is ReentrancyGuard { error UserNotInRegistry(); error UserNotGardenOwner(); error StrategyExists(); + error CallerIsNotNewOnwer(); /*|--------------------------------------------|*o /*| STRUCTS/ENUMS |*/ @@ -71,7 +73,7 @@ contract RegistryGardens is ReentrancyGuard { uint256 _protocolFee; uint256 _nonce; Metadata _metadata; - address owner; + address payable _councilSafe; } //TODO: can change to uint32 with optimized storage order @@ -82,40 +84,39 @@ contract RegistryGardens is ReentrancyGuard { IERC20 public gardenToken; uint256 public protocolFee; string private covenantIpfsHash; - address private gardenOwner; bytes32 public profileId; - ISafe public councilSafe; - mapping(address => bool) public councilMembers; + address payable public pendingCouncilSafe; + Safe public councilSafe; mapping(address => bool) public tribunalMembers; mapping(address => Member) public addressToMemberInfo; mapping(address => bool) public enabledStrategies; - constructor() {} + constructor() { + // _grantRole(DEFAULT_ADMIN_ROLE, address(this)); + _setRoleAdmin(COUNCIL_MEMBER_CHANGE, DEFAULT_ADMIN_ROLE); + } function initialize(RegistryGardens.InitializeParams memory params) public { allo = IAllo(params._allo); gardenToken = params._gardenToken; minimumStakeAmount = params._minimumStakeAmount; protocolFee = params._protocolFee; + if (params._councilSafe == address(0)) { + revert AddressCannotBeZero(); + } + councilSafe = Safe(params._councilSafe); + _grantRole(COUNCIL_MEMBER_CHANGE, params._councilSafe); + // gardenOwner = msg.sender; //@todo: RegistryFactory is the onwer of that contract, that need be able to change the owner - gardenOwner = params.owner; //@todo: check if address(0) is a valid owner + // gardenOwner = params.owner; //@todo: check if address(0) is a valid owner registry = IRegistry(allo.getRegistry()); address[] memory initialmembers = new address[](0); profileId = registry.createProfile(params._nonce, communityName, params._metadata, msg.sender, initialmembers); } - //@todo: maybe we want use ROLES instead fixed address that give mroe flexibility - //@todo: also who should be allowed to set the council members? the DAO? the garden owner? - function setCouncilMembers(address[] memory _members) public onlyGardenOwner { - for (uint256 i = 0; i < _members.length; i++) { - councilMembers[_members[i]] = true; - } - emit CouncilMemberSet(_members); - } - function addStrategy(address _newStrategy) public onlyRegistryMember { if (enabledStrategies[_newStrategy]) { revert StrategyExists(); @@ -139,10 +140,22 @@ contract RegistryGardens is ReentrancyGuard { emit AlloSet(_allo); } - function setCouncilSafe(address _safe) public { - require(msg.sender == gardenOwner, "Only the owner can call this method."); - councilSafe = ISafe(_safe); - emit CouncilSafeSet(_safe); + function setCouncilSafe(address payable _safe) public onlyCouncilMember { + pendingCouncilSafe = _safe; + emit CouncilSafeChangeStarted(address(councilSafe), pendingCouncilSafe); + } + + function _changeCouncilSafe() internal { + councilSafe = Safe(pendingCouncilSafe); + delete pendingCouncilSafe; + emit CouncilSafeSet(pendingCouncilSafe); + } + + function acceptCouncilSafe() public { + if (msg.sender != pendingCouncilSafe){ + revert CallerIsNotNewOnwer(); + } + _changeCouncilSafe(); } function isMember(address _member) public view returns (bool _isMember) { @@ -192,17 +205,14 @@ contract RegistryGardens is ReentrancyGuard { minimumStakeAmount = _newAmount; } - function updateProtocolFee(uint256 _newProtocolFee) public { - if (!isCouncilMember(msg.sender)) { - revert("Must be in council safe"); - } + function updateProtocolFee(uint256 _newProtocolFee) public onlyCouncilMember { protocolFee = _newProtocolFee; emit ProtocolFeeUpdated(_newProtocolFee); } //function updateMinimumStake() function isCouncilMember(address _member) public view returns (bool) { - return councilMembers[_member]; + return hasRole(COUNCIL_MEMBER_CHANGE, _member); } function unregisterMember(address _member) public nonReentrant { diff --git a/pkg/contracts/src/interfaces/ISafe.sol b/pkg/contracts/src/interfaces/ISafe.sol deleted file mode 100644 index ac80eaa1b..000000000 --- a/pkg/contracts/src/interfaces/ISafe.sol +++ /dev/null @@ -1,46 +0,0 @@ -// SPDX-License-Identifier: AGPL-3.0-only -pragma solidity ^0.8.19; - -abstract contract Enum { - enum Operation { - Call, - DelegateCall - } -} - -interface ISafe { - /** - * @notice Executes a `operation` {0: Call, 1: DelegateCall}} transaction to `to` with `value` (Native Currency) - * and pays `gasPrice` * `gasLimit` in `gasToken` token to `refundReceiver`. - * @dev The fees are always transferred, even if the user transaction fails. - * This method doesn't perform any sanity check of the transaction, such as: - * - if the contract at `to` address has code or not - * - if the `gasToken` is a contract or not - * It is the responsibility of the caller to perform such checks. - * @param to Destination address of Safe transaction. - * @param value Ether value of Safe transaction. - * @param data Data payload of Safe transaction. - * @param operation Operation type of Safe transaction. - * @param safeTxGas Gas that should be used for the Safe transaction. - * @param baseGas Gas costs that are independent of the transaction execution(e.g. base transaction fee, signature check, payment of the refund) - * @param gasPrice Gas price that should be used for the payment calculation. - * @param gasToken Token address (or 0 if ETH) that is used for the payment. - * @param refundReceiver Address of receiver of gas payment (or 0 if tx.origin). - * @param signatures Signature data that should be verified. - * Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash. - * @return success Boolean indicating transaction's success. - */ - - function execTransaction( - address to, - uint256 value, - bytes calldata data, - Enum.Operation operation, - uint256 safeTxGas, - uint256 baseGas, - uint256 gasPrice, - address gasToken, - address payable refundReceiver, - bytes memory signatures - ) external payable returns (bool success); -} diff --git a/pkg/contracts/test/CVStrategyTest.t.sol b/pkg/contracts/test/CVStrategyTest.t.sol index 9ebf9455c..e3e6703e2 100644 --- a/pkg/contracts/test/CVStrategyTest.t.sol +++ b/pkg/contracts/test/CVStrategyTest.t.sol @@ -26,12 +26,14 @@ import {CVStrategy} from "../src/CVStrategy.sol"; import {RegistryGardens} from "../src/RegistryGardens.sol"; import {RegistryFactory} from "../src/RegistryFactory.sol"; +import {SafeSetup} from "./shared/SafeSetup.sol"; /* @dev Run * forge test --mc CVStrategyTest -vvvvv * forge test --mt testRevert -vvvv * forge test --mc CVStrategyTest --mt test -vv */ -contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, GasHelpers { + +contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, GasHelpers, SafeSetup { CVStrategy public strategy; MockERC20 public token; // uint256 public mintAmount = 1_000 * 10 ** 18; @@ -75,13 +77,9 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G params._minimumStakeAmount = MINIMUM_STAKE; params._protocolFee = 2; params._metadata = metadata; + params._councilSafe = payable(address(_councilSafe())); registryGardens = RegistryGardens(registryFactory.createRegistry(params)); - address[] memory initialmembers = new address[](2); - initialmembers[0] = local(); - initialmembers[1] = pool_admin(); - - registryGardens.setCouncilMembers(initialmembers); } function _registryGardens() internal view returns (RegistryGardens) { @@ -262,7 +260,12 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G (IAllo.Pool memory pool, uint256 poolId) = _createProposal(); uint256 AMOUNT_STAKED = 45000; - registryGardens.setBasisStakedAmount(AMOUNT_STAKED); + // registryGardens.setBasisStakedAmount(AMOUNT_STAKED); + safeHelper( + address(registryGardens), + 0, + abi.encodeWithSelector(registryGardens.setBasisStakedAmount.selector, AMOUNT_STAKED) + ); /** * ASSERTS */ @@ -289,12 +292,20 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G assertEq(AMOUNT_STAKED, 45000); assertEq(cv_amount, 97698); - registryGardens.setBasisStakedAmount(MINIMUM_STAKE); + // registryGardens.setBasisStakedAmount(MINIMUM_STAKE); + safeHelper( + address(registryGardens), + 0, + abi.encodeWithSelector(registryGardens.setBasisStakedAmount.selector, MINIMUM_STAKE) + ); } function test_threshold_check_as_js_test() public { (IAllo.Pool memory pool, uint256 poolId) = _createProposal(); - registryGardens.setBasisStakedAmount(45000); + // registryGardens.setBasisStakedAmount(45000); + safeHelper( + address(registryGardens), 0, abi.encodeWithSelector(registryGardens.setBasisStakedAmount.selector, 45000) + ); /** * ASSERTS */ @@ -315,12 +326,20 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G assertEq(AMOUNT_STAKED, 45000); assertEq(ct1, 50625); - registryGardens.setBasisStakedAmount(MINIMUM_STAKE); + // registryGardens.setBasisStakedAmount(MINIMUM_STAKE); + safeHelper( + address(registryGardens), + 0, + abi.encodeWithSelector(registryGardens.setBasisStakedAmount.selector, MINIMUM_STAKE) + ); } function test_total_staked_amount() public { (IAllo.Pool memory pool, uint256 poolId) = _createProposal(); - registryGardens.setBasisStakedAmount(45000); + // registryGardens.setBasisStakedAmount(45000); + safeHelper( + address(registryGardens), 0, abi.encodeWithSelector(registryGardens.setBasisStakedAmount.selector, 45000) + ); /** * ASSERTS */ @@ -345,7 +364,12 @@ contract CVStrategyTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, G assertEq(cv.totalStaked(), 0, "TotalStaked"); - registryGardens.setBasisStakedAmount(MINIMUM_STAKE); + // registryGardens.setBasisStakedAmount(MINIMUM_STAKE); + safeHelper( + address(registryGardens), + 0, + abi.encodeWithSelector(registryGardens.setBasisStakedAmount.selector, MINIMUM_STAKE) + ); } function testRevert_allocate_removeSupport_wo_support_before_SUPPORT_UNDERFLOW() public { diff --git a/pkg/contracts/test/RegistryTest.t.sol b/pkg/contracts/test/RegistryTest.t.sol index 9108f8006..9f141c494 100644 --- a/pkg/contracts/test/RegistryTest.t.sol +++ b/pkg/contracts/test/RegistryTest.t.sol @@ -23,9 +23,12 @@ import {GasHelpers} from "allo-v2-test/utils/GasHelpers.sol"; import {RegistryFactory} from "../src/RegistryFactory.sol"; import {CVStrategy} from "../src/CVStrategy.sol"; import {RegistryGardens} from "../src/RegistryGardens.sol"; + +import {SafeSetup} from "./shared/SafeSetup.sol"; + // @dev Run forge test --mc RegistryTest -vvvvv -contract RegistryTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, GasHelpers { +contract RegistryTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, GasHelpers, SafeSetup { CVStrategy public strategy; MockERC20 public token; uint256 public mintAmount = 1_000_000 * 10 ** 18; @@ -68,6 +71,7 @@ contract RegistryTest is Test, AlloSetup, RegistrySetupFull, Native, Errors, Gas params._minimumStakeAmount = MINIMUM_STAKE; params._protocolFee = 2; params._metadata = metadata; + params._councilSafe = payable(address(_councilSafe())); registryGardens = RegistryGardens(registryFactory.createRegistry(params)); } diff --git a/pkg/contracts/test/shared/SafeSetup.sol b/pkg/contracts/test/shared/SafeSetup.sol new file mode 100644 index 000000000..65bdf54b3 --- /dev/null +++ b/pkg/contracts/test/shared/SafeSetup.sol @@ -0,0 +1,55 @@ +//SPDX-License-Identifier: AGPL-3.0-only + +pragma solidity ^0.8.19; + +import "forge-std/Test.sol"; + +import "safe-contracts/contracts/Safe.sol"; +import "safe-contracts/contracts/proxies/SafeProxyFactory.sol"; + +contract SafeSetup is Test{ + Safe public councilSafe; + address public councilMember1; + uint256 public councilMemberPK = 1; + + function _councilSafe() internal returns (Safe) { + councilMember1 = vm.addr(councilMemberPK); + vm.label(councilMember1, "councilMember1"); + + if (address(councilSafe) == address(0)) { + SafeProxyFactory spf = new SafeProxyFactory(); + SafeProxy sp = spf.createProxyWithNonce(address(new Safe()), "", 0); + councilSafe = Safe(payable(address(sp))); + vm.label(address(councilSafe), "councilSafe"); + address[] memory owners = new address[](1); + owners[0] = address(councilMember1); + councilSafe.setup(owners, 1, address(0), "", address(0), address(0), 0, payable(address(0))); + } + return councilSafe; + } + + function safeHelper(address to_, uint256 value_, bytes memory data_) public { + bytes memory txData = councilSafe.encodeTransactionData( + address(to_), + value_, + data_, + Enum.Operation.Call, + 0, + 0, + 0, + address(0), + payable(address(0)), + councilSafe.nonce() + ); + + bytes32 txDataHash = keccak256(txData); + + (uint8 v, bytes32 r, bytes32 s) = vm.sign(councilMemberPK, txDataHash); + + bytes memory signature = abi.encodePacked(r, s, v); + // vm.star + councilSafe.execTransaction( + address(to_), value_, data_, Enum.Operation.Call, 0, 0, 0, address(0), payable(address(0)), signature + ); + } +} diff --git a/remappings.txt b/remappings.txt index 5b2998c7b..6a81df35d 100644 --- a/remappings.txt +++ b/remappings.txt @@ -3,3 +3,4 @@ forge-std/=lib/forge-std/src/ allo-v2/=lib/allo-v2/ allo-v2-contracts/=lib/allo-v2/contracts/ allo-v2-test/=lib/allo-v2/test/ +safe-contracts/=lib/safe-contracts/ \ No newline at end of file