Conversation
- updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed
Collaborator
Author
|
No we didn't, but the OZ contract need to be compiled with |
FHieser
approved these changes
Jan 16, 2024
Member
|
I think upgrading the Solidity version makes sense, as we wanted to do it anyway at some point. It makes sense to just do it in one go now, imho. One brief discussion maybe to have now is whether we want to go with a higher version directly. Ideally, for launch, we would like to have a version that has been out for a few weeks/months so the risk of undetected bugs in that version is as low as possible, but not a version that is too old. Even the latest version |
marvinkruse
approved these changes
Jan 19, 2024
Collaborator
Author
|
Updating to |
The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet
Zitzak
force-pushed
the
openZeppelinV5
branch
from
9b61c0c to
fd06b46
Compare
Collaborator
Author
|
@marvinkruse, talked with @FHieser and he agreed with your suggestion of updating the version. It is included in the last commit |
Contributor
|
Sounds good to me! Let's go with |
marvinkruse
changed the title
0xNuggan
pushed a commit
that referenced
this pull request
Apr 12, 2024
* [DO NOT MERGE] OpenZeppelin V5 update (#401) * chore: update OZ lib to V5 * chore: update openzeppelin to v5 - updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed * chore: update Solidity version 0.8.19 -> 0.8.20 * format: format code * chore: fix CI and update version The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet * Delete .gas-snapshot * Update .gitignore * Change TestsupportsInterface to only check immediate Iterfaces --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: FHieser <felix_hieser@web.de> * chore: update CI to include testing scripts * Update ci.yml (#421) * fix: solved address collition in bc test * SC-122: Implement MultiCall and MetaTransaction Support (#405) * chore: update OZ lib to V5 * chore: update openzeppelin to v5 - updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed * chore: update Solidity version 0.8.19 -> 0.8.20 * format: format code * 394 metatransactions dev (#399) * Modify ModuleManager to enact ERC2771 * Intermediate Push I need to check something and pushing makes most sense he * Fix inheritance to enable build * Expose isTrustedForwarder in Orchestrator * Implement modified ERC2771 to Module * Fix Tests A lot of modules havent been properly initialized in the tests. The fix targets that by actually using the proxy Clone implementation and then properly initializing them * Add Module Test * Implement MinimalForwarder * Fix MakeFile * Create E2ETest * Add specific Role section in E2E Test * Remove todo * Marvin G Fix * Foundryup formatting * Fix after openZepplelin Update * Create Transaction forwarder * Update IModule.sol * Change to Transaction forwarder * chore: fix CI and update version The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet * WIP * Fixing tests * Cleanup * Add Multicall Section into E2ETest * Cleanup * Merge fix * Fix based on comments * Delete .gas-snapshot * Update .gitignore * Change TestsupportsInterface to only check immediate Iterfaces * Fix merge * Create DeployAndSetUpTransactionForwarder.s.sol * Add Transaction Forwarder Script to deployment scripts --------- Co-authored-by: Zitzak <marvin88gr@gmail.com> * SC-140: Implement version and intervention mechanism in beacon (#418) * Rename Contracts * Refactor Beacons and add Version function * Make it buildable * Fix test * Cleanup * Update InverterBeacon.t.sol * Update InverterBeacon.t.sol * add BeaconProxy Test * Clarify Outcommented * Fix Beacon Upgrade Script * Update UpgradeBeacon.s.sol * Add Intervention Mechanism to InverterBeacon * Make it buildable * PR comment fix * Add tests * Add Interface * Cleanup * Move and update Beacon e2e * Update InverterBeaconE2E.t.sol * Cleanup * Adapt naming * Fix after merge * Fix inheritance * Remove beacon implementation assumption * update _contextSuffixLength * Remove test for empty beacon implementation * Let Module inherit from ERC2771ContextUpgradeable * Fix init in Module test * Fix tests --------- Co-authored-by: Marvin <43185740+Zitzak@users.noreply.github.com> Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: FHieser <felix_hieser@web.de> Co-authored-by: Zitzak <marvin88gr@gmail.com>
marvinkruse
added a commit
that referenced
this pull request
Apr 12, 2024
* [DO NOT MERGE] OpenZeppelin V5 update (#401) * chore: update OZ lib to V5 * chore: update openzeppelin to v5 - updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed * chore: update Solidity version 0.8.19 -> 0.8.20 * format: format code * chore: fix CI and update version The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet * Delete .gas-snapshot * Update .gitignore * Change TestsupportsInterface to only check immediate Iterfaces --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: FHieser <felix_hieser@web.de> * chore: update CI to include testing scripts * Update ci.yml (#421) * fix: solved address collition in bc test * SC-122: Implement MultiCall and MetaTransaction Support (#405) * chore: update OZ lib to V5 * chore: update openzeppelin to v5 - updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed * chore: update Solidity version 0.8.19 -> 0.8.20 * format: format code * 394 metatransactions dev (#399) * Modify ModuleManager to enact ERC2771 * Intermediate Push I need to check something and pushing makes most sense he * Fix inheritance to enable build * Expose isTrustedForwarder in Orchestrator * Implement modified ERC2771 to Module * Fix Tests A lot of modules havent been properly initialized in the tests. The fix targets that by actually using the proxy Clone implementation and then properly initializing them * Add Module Test * Implement MinimalForwarder * Fix MakeFile * Create E2ETest * Add specific Role section in E2E Test * Remove todo * Marvin G Fix * Foundryup formatting * Fix after openZepplelin Update * Create Transaction forwarder * Update IModule.sol * Change to Transaction forwarder * chore: fix CI and update version The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet * WIP * Fixing tests * Cleanup * Add Multicall Section into E2ETest * Cleanup * Merge fix * Fix based on comments * Delete .gas-snapshot * Update .gitignore * Change TestsupportsInterface to only check immediate Iterfaces * Fix merge * Create DeployAndSetUpTransactionForwarder.s.sol * Add Transaction Forwarder Script to deployment scripts --------- Co-authored-by: Zitzak <marvin88gr@gmail.com> * SC-140: Implement version and intervention mechanism in beacon (#418) * Rename Contracts * Refactor Beacons and add Version function * Make it buildable * Fix test * Cleanup * Update InverterBeacon.t.sol * Update InverterBeacon.t.sol * add BeaconProxy Test * Clarify Outcommented * Fix Beacon Upgrade Script * Update UpgradeBeacon.s.sol * Add Intervention Mechanism to InverterBeacon * Make it buildable * PR comment fix * Add tests * Add Interface * Cleanup * Move and update Beacon e2e * Update InverterBeaconE2E.t.sol * Cleanup * Adapt naming * Fix after merge * Fix inheritance * Remove beacon implementation assumption * update _contextSuffixLength * Remove test for empty beacon implementation * Let Module inherit from ERC2771ContextUpgradeable * Fix init in Module test * Fix tests * Fix: Revert merging issue --------- Co-authored-by: Marvin <43185740+Zitzak@users.noreply.github.com> Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: FHieser <felix_hieser@web.de> Co-authored-by: Zitzak <marvin88gr@gmail.com>
marvinkruse
pushed a commit
that referenced
this pull request
Apr 12, 2024
* chore: update OZ lib to V5 * chore: update openzeppelin to v5 - updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed * chore: update Solidity version 0.8.19 -> 0.8.20 * format: format code * chore: fix CI and update version The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet * Delete .gas-snapshot * Update .gitignore * Change TestsupportsInterface to only check immediate Iterfaces --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: FHieser <felix_hieser@web.de>
marvinkruse
added a commit
that referenced
this pull request
Apr 12, 2024
* Rename Contributor to paymentReceiver (#309) * Rename PaymentClient and proposal (#310) * Rename to ERC20PaymentClient * Revert "Rename to ERC20PaymentClient" This reverts commit ac66c53. * Rename to ERC20PaymentClient * Rename Proposal to Orchestrator * rename configdata to configData * rename dependencydata to dependencyData * Fix / Pre-commit * 318 code review recurringpaymentmanager (#321) * use _triggerFor correctly * pre-commit * Initial commit (#325) * 292 authorization cleanup (#313) * first batch of restructuring * move roles to module * remove moduleManagerRoles * make roleAuthorizer default * fix singlevote tests * fix some todos * more merge fixes * fix singlevotegovernor tests * authorizer housekeeping * add missing tests * further cleanup * format * fix code review issues * add singleVoteLifecycle (not finished)) * finish e2e * fix scripts * update lcov * remove comment * expand authorization error description * update tests * Removal of local unused varaiables foundry throws errors if variables are not in use * icebox MilestoneManager (#326) * remove MilestoneManager * delete commented --------- Co-authored-by: FHieser <felix_hieser@web.de> Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> * feat(BondingCurveFundingManagerBase): Add base features The base contract enables the basic functionalitiy of issuing token which every bonding curve must have. It makes use of a abstract function for the calculation which should be implemented in downstream contracts * feat(RedeemingBondingCurveFundingManager): Add redeem functionalities This base contract adds configuration function for a redeeming bonding curve functionality as well as abastract function for calculating token values to be implemented in an downstream contract * feat(VirtualSupply): implements virtual token and collateral supply * feat(BancorVirtualSupplyBondingCurveFundingManger): Inherits from the base contracts to enable issuing and redeeming of tokens on a bonding curve, calculated through the Bancor formula, based on virtual supplies of the issuing token and collateral * feat:BancorVirtualSupplyBondingCurveFundingManager Remove Virtual Supply token and collateral interfaces * feat: add getter func and errors to interfaces The getter function got added as per PR review request. Next to this errors got added to the interface for over/underflow when setting the virtual supply * feat: add setter func to VirtualSupply contract The setter function has been implemented as abstract function so the upstream contract has to implement it. Because the getter functions got added to the interfaces, both contract now implement the interface. Lastly some input validation got added * test: virtual supply token contracts * feat: add Bancor contracts to repo All contracts have added comments for clarification what has been changed, including links to the original files. An ignore for the formula directory got added to the foundry.toml file to prevent the formatter from creating to big of a diff, in case somebody would like to check our statement of the changes made. * chore: update modifier after rebase * e2e test streamingPayments (#307) * Restructure base e2e * Cleanup * Create StreamingPaymentsLifecycle.sol * pre-commit * after merge fix * Merge update * pre-commit * Merge fix * pre-commit * chore: remove collateral token address The fundingManager infrastructure currently only works with one tokens. For that reason it is unneeded to facilitate for multi collateral token possibility, rather build for what is required now. * Refactor PaymentClient to be logicModule base (#312) * Move PaymentClient to LogicModule * Refactor PaymentClient to be base of logicModules * Move test position * Update comments * Add testing for internal functions * pre-commit * Review Fixes * pre-commit * Fix merge * pre-commit * pre-commit * 315 code review results bountymanager (#328) * Update OnlyRole for updateClaimContributors * Add notClaimed Modifiers * remove ensureTokenBalance * Add front run safeguard * Remove indexed parameter * Change naming * Refactor for easier usage * Removal of unnecessary code * pre-commit * pre-commit * 329 bountymanager bounties should be able to be claimed multiple times (#330) * Seperate Modifier into notClaimed and notLocked * pre-commit * Review Fixes * Feat/setup contracts bonding curve (#323) * feat(BondingCurveFundingManagerBase): Add base features The base contract enables the basic functionalitiy of issuing token which every bonding curve must have. It makes use of a abstract function for the calculation which should be implemented in downstream contracts * feat(RedeemingBondingCurveFundingManager): Add redeem functionalities This base contract adds configuration function for a redeeming bonding curve functionality as well as abastract function for calculating token values to be implemented in an downstream contract * feat(VirtualSupply): implements virtual token and collateral supply * feat(BancorVirtualSupplyBondingCurveFundingManger): Inherits from the base contracts to enable issuing and redeeming of tokens on a bonding curve, calculated through the Bancor formula, based on virtual supplies of the issuing token and collateral * feat:BancorVirtualSupplyBondingCurveFundingManager Remove Virtual Supply token and collateral interfaces * feat: add getter func and errors to interfaces The getter function got added as per PR review request. Next to this errors got added to the interface for over/underflow when setting the virtual supply * feat: add setter func to VirtualSupply contract The setter function has been implemented as abstract function so the upstream contract has to implement it. Because the getter functions got added to the interfaces, both contract now implement the interface. Lastly some input validation got added * test: virtual supply token contracts * feat: add Bancor contracts to repo All contracts have added comments for clarification what has been changed, including links to the original files. An ignore for the formula directory got added to the foundry.toml file to prevent the formatter from creating to big of a diff, in case somebody would like to check our statement of the changes made. * chore: update modifier after rebase --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> * Remove _ensureTokenBalance checks when adding payment Orders and refactor triggerFor() (#332) * add paymentorders individually in _triggerFor * remove tokenBalance checks when adding payment Orders * Remove artificial limit of ensureTokenBalance * Simplify trigger() Reverted change of commit commit f86b1a5 * downgrade again * pre-commit --------- Co-authored-by: FHieser <felix_hieser@web.de> * Authorizer codereview fixes (#331) * first batch of restructuring * move roles to module * remove moduleManagerRoles * make roleAuthorizer default * fix singlevote tests * fix some todos * more merge fixes * fix singlevotegovernor tests * authorizer housekeeping * add missing tests * further cleanup * format * fix code review issues * add singleVoteLifecycle (not finished)) * finish e2e * fix scripts * update lcov * remove comment * correct documentation typo * more review fixes * allow for token gated global roles * remove selfManaged logic * first steps to change role format * partial fixes in tests * forge update * fix test setup funcs * fix rest of tests * add missing test + cleanup * Felix review fixes * fix lib version bug * solve merge conflicts * Update .gitmodules --------- Co-authored-by: FHieser <felix_hieser@web.de> * chore: WIP * Add VirtualToken/Collateralbase tests * address merge issues * set up testing skeleton * BondingCurveManagerBase Test + small bugfixes bugfixes in the contracts are marked with @review tag * RedeemingBondingCurve test skeleton + minor cleanup * small corrections expands the redeemingFundingManager, some formatting and implements a stopgap solution for stackTooDeep error * bump Base Bonding Curve test coverage to 100% * Rename test file It was messing up the coverage table (ò_ó) * finish RedeemingBC tests + bugfixes * bump coverage to 100% in RedeemBC * add new scripts for the bountyManager (#339) * add new scripts * format * WIP Bancor tests * format + fix typo * WIP BancorBondingCurve Tests * format + substitute msg.sender * add checks to setters (#337) * add checks to setters * add tests * decimalConversion tests + bugfix * bancor tests v1 ready * add scripts + e2e and bump coverage * Issue #341 Create Constants.sol file for the scripts (#343) * created constants for addBountyManagerClaim.s.sol * added constants from deployTokenAuthorizerAndSwitch to constants file * setTokenGatedRole * replaced all constants in scripts * ran forge fmt * chore: review bugfixes and add getter for Bancor Funding Manager What has been done? - Reviewd bug fixes and removed the comments - Add getter functions as suggested by comment - Add tests for getter functions * chore: remove comment of idempotence Why? Thought process here is that it saves gas to not needlessly execute a tx. If the PR reviewers would like a different practice then I'm happy to change the code, because it might be personal pref * chore: remove bugfix comments of Redeeming and Base BCFM Reviewd all the comments and agreed. Removed the comments * docs: add documentation for max computational amount Bancor BCFM The Bancor Formula contract has a maximum uint value it can process for issuing and redeeming, 10^26 and 10^38 respectively. When a larger value is used for calculations, the contract will revert. These values leave enough room for most cases but the edge cases. Edge cases could include tokens that have very little value or tokens with high decimals. For those cases it might be necessary to split buy amounts over multiple transaction. Because we don't want to modify the Aragon formula contract we decided to make sure to inform the user and dev of this fact by means of a natspec documentation * WIP * emit bugfix * Update RedeemingB_CurveFundingManagerBase.t.sol * Update BondingCurveFundingManagerBase.t.sol * removed token from orchestrator * token removed from orchestrator * renamed proposal -> orchestrator * review fixes review fixes - make base bonding curve FundingManager -rename virtualBuy/Sell orders - fix burn bug - add check for edge case in testing - other smaller stuff * feat: add events to bonding curve funding manager Adds all the events and tests for: - BondingCurveFundingManagerBase - RedeemingBondingCurveFundingMangerBase - VirtualCollateralSupplyBase - VirtualTokenSupplyBase - BancorVirtualSupplyBondingCurveFundingManager * Completed IFundingManager cleanup (#345) * Completed IFundingManager cleanup * dummy commit to re-trigger the tests * forge fmt * Docs: Update contribution guidelines (#340) * Update CONTRIBUTING.md * Docs: Add reviewing to workflow of main * Docs: Add section on our dev guidelines * fix: add decimal input validation * docs: add comment clarifying abstract functions * chore: add events and remove todo comments * remove todo * rename roles (#347) see issue #335 description Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> * resolve merge issues * make fmt * address edge case in ElasticReceiptToken * removed the << >> symbols * initial commit * corrected the token() call * Restructered init function of BancorVirtualSupplyBondingCurveFundingManger and introduced the accepted token variable in the contract * refactoring to resolve stack-too-deep. should work as intended now * Add extra E2E tests (#349) * setup new E2E tests * clean up E2E tests * rename roles * address review * Implemented Pablo's PR review * forge fmt * Security review PR #350: resolving comments (#362) * fix(gh-356): add view modifier to functions for gas optimization The following function are set to view to optimize gas: - _issueTokensFormulaWrapper - _issueTokens - _redeemTokensFormulaWrapper - _redeemTokens * docs(gh-357): update comments for clarity and misspelling * perf(gh-359): Remove unnecessary payable modifier Removed payable modifer because the function will not receive Ether, and removing them makes for gas optimization * perf(gh-358): remove unnecessary input validation for fee percentage The function _calculateFeeDeductedDepositAmount can only be called from a function which has already done an input validation. For that reason the value can never be bigger than BPS. The check is unnecessary. Removed the test which check for the input validation * refactor(gh-360): Rename function sellOrder(for) & buyOrder(for) Because there is not really an order placed but instead tokens are being bought and sold on the bonding curve, the word "order" can be misleading. The functions have been renamed for clarity * refactor(gh-361): remove unnecessary function _issueTokens & _redeemTokens * refactor(gh-374): move token decimal input validation to implementation The issuance token decimal input validation was done within the BondingCurveFundingManagerBase. It made more sense to move this to the implementation contract. In addition to the refactor a check was added within the BancorVirtualSupplyBondingCurveFundingManager for tokens to revert tokens decimals < 7 to ensure no destructive precision loss when using PPM for calculations. * fix: address pr review comments - Fix typo in function comment - Fix branching test tree comment - Fix test to include vm.assume for == 7 * Omega security review #350 : Additional fixes (#372) * remove unused imports remove unused import from SingleVoteGovernor * add zero check in virtual supply contracts with tests * make virtual sub and add unchecked * Block ownerless deployment on setup - more extensive comment - added tests * format * correction in VirtualCollateralSupplyBase * Issue template for security reviews (#379) * Issue template for security reviews * fix thump up emoji * Orchestrator security review (#378) * Update Orchestrator.sol * Update lcov.info * E2E test restructuring (#377) * Renaming and reorganizing E2E test - unifying naming structure - creating folders * E2E test refactors First round of refactorings RoleAuthorizerE2E done. Should serve as example for the rest finsha Authorizers refactor and small subdivision changes add setUp func + moduleConfigurations to all tests add template to E2ERegistry Finish rest of easy refactors Also deleted RegisterModuleAtFactory.t. sol because it was duplicating the setUp tests in the E2EModuleRegsitry Orchestrator E2E refactor * clean up imports + small final refactor * add template + pre-commit * Uncomment template so linter doesn't complain * remove TODOs * change file format for template tests were failing * new fmt in foundry --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: FHieser <felix_hieser@web.de> * lcov removal from make pre-commit (#391) * Update Makefile * Foundry Update fmt * Update .gitignore * 376 Create E2E Test for MetadataManager (#392) * Move E2E test to the according folder * Complement Interface * Update MetadataManager.sol * Create E2E Test for MetadataManager * Small fix * EIP 165 implementation (#371) * Removed verifyAddressIsxyz from Orchestrator.sol * created a separate library for fetching interfaces * Included all interfaces in the interfaceID Library * implemented erc165 in orchestratorFactory * Navigating a pretty complex web of inheritances to implement ERC165 * Added the supportsInterface function to all relevant solidity files * does not work right now. inheritances are very messy * This compiles. need to figure out a way to do _supportsInterface calls to addresses * Fully integrated eip-165 into the inverter contracts * forge fmt * added supportsInterface function to the AuthorizerMock contract * Fixed the failing tests (hopefully) by implementing the supportsInterface functions in them. Maybe I need to introduce them in the interface itself * implemented felix's optimisation * switched from named return values * refactored position of supportsInterface * refactored stuff * supportsInterface test introduced in unit tests - 1 * added eip165 tests - 2 * eip165 tests -3 * added eip165 tests to authorizer modules * tests for eip165 completed * fixed cases of collision of random interface Ids being the same as actually supported ids * reverted the runs * fixed a typo * removed the scope of randomized IDs to avoid collisions with all the other supported interfaceIDs * renamed randomInterfaceId to invalidInterfaceId * Make supportsInterface more compact * fmt * Integrate supportInterface fuzzing * Add dependency check --------- Co-authored-by: FHieser <felix_hieser@web.de> * [Bugfix] correct metadata name and makefile links (#400) * correct makefile links and metadata * format * Update SetupToyOrchestratorScript.s.sol * Update DeploymentScript.s.sol * Update DeploymentScript.s.sol * Create UpgradeBeacon.s.sol * Foundry update --------- Co-authored-by: FHieser <felix_hieser@web.de> Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> * Omega security review #350 : Remaining bonding curve comments (#385) * perf(gh-380): add Orchestrator token decimals to implementaion state The Orchestrator token decimals got called several times within the implementation which are external calls. To optimize for gas, the decimals now get stored in the state on fundingmanager init * fix(gh-381): add state to track transaction fee for BC The transaction fee for buying and selling now gets subtracted from the virtual balances that we store into a seperate variable. The function calculating the amount subtracted by the fee now also returns the fee amount, and has been renamed to reflect this change. This commit includes the adapted tests * Issue template for security reviews (#379) * Issue template for security reviews * fix thump up emoji * feat: add static price getter for buying and selling A static price is needed to display within the UI as reference. The formula to calculate the static price is taken from the Aragon contracts to ensure it works with the formula contract as well. Test have been added for the new feature * feat: add slippage functionalitiy through minAmountOut parameter Function that calcuate the amount out are added to the contract so that the UI can call them, and use the output as minAmountOut parameter in the functions. This parameter function as slippage in case a big order gets bought before the users tx, or the tx gets frontrun This commit includes the adapted tests * Static Price Test (#397) * WIP * WIP * format * correct typo * add fuzzing test to static price calculation --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: 0xNuggan <82726722+0xNuggan@users.noreply.github.com> --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: 0xNuggan <82726722+0xNuggan@users.noreply.github.com> * Remove testStaticPriceWithFuzzing (#403) * 366 security review 350 erc20paymentclient findings (#386) * Fix #350 (comment) * Adapting _ensureTokenAllowance * Add internal token tracking * Split ERC20PaymentClient into Access and Mock * Restructuring ModuleTest to include OrchestratorMock * testEnsureTokenBalance correctly * finish PaymentClient tests * Test for AmountPaid in PaymentProcessors * pre-commit * Update Natspec * Delete lcov.info * Update TokenGatedRoleAuthorizer.t.sol * Update BancorVirtualSupplyBondingCurveFundingManager.t.sol * Update BancorVirtualSupplyBondingCurveFundingManager.t.sol * Update ERC165 test * update interfaceid test * Additional events (#404) * preliminary steps * add additional events * event testing * Authorizer event emission test * all emissions added to tests * NATSPEC event description * bugfix * foundryup + format * review fixes * bugFix: subtract fee amount from collateral for sellOrder * chore: remove unused function * fix: add check to prevent address collision * Sc 156 testing bug fix (#417) * bugfix: solve address collition in BC * Remove fuzzing from testSupportsInterface --------- Co-authored-by: Zitzak <marvin88gr@gmail.com> * SC-148: Security Review - Interface lib ERC165Checker (#410) * Remove LibInterface * Change to ERC165Checker * Fix Test * Update ERC20PaymentClient.sol (#413) * Security review pr#350 payment processor (#425) * Fix amountPaid for removedpayments * Move up for Checks-Effects-Interactions Pattern * Security review part two fixes (#420) * correct findings * pr review fixes * address collision fix * Bonding curve extra scripts (#355) * Investable Workstreams Scripts script part 1 weird revert eror script part 2 fscripts run corectly * bugfix + update values * post-merge fix * Create giveBountyManagerRolesToAddress.s.sol * Update giveBountyManagerRolesToAddress.s.sol * fmt * review fixes * Pass the structs * chore: fix review comment (#440) * Completed IFundingManager cleanup (#345) * Completed IFundingManager cleanup * dummy commit to re-trigger the tests * forge fmt * Docs: Update contribution guidelines (#340) * Update CONTRIBUTING.md * Docs: Add reviewing to workflow of main * Docs: Add section on our dev guidelines * rename roles (#347) see issue #335 description Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> * chore: remove collateral token address The fundingManager infrastructure currently only works with one tokens. For that reason it is unneeded to facilitate for multi collateral token possibility, rather build for what is required now. * chore: WIP * Add VirtualToken/Collateralbase tests * address merge issues * set up testing skeleton * BondingCurveManagerBase Test + small bugfixes bugfixes in the contracts are marked with @review tag * RedeemingBondingCurve test skeleton + minor cleanup * small corrections expands the redeemingFundingManager, some formatting and implements a stopgap solution for stackTooDeep error * bump Base Bonding Curve test coverage to 100% * Rename test file It was messing up the coverage table (ò_ó) * finish RedeemingBC tests + bugfixes * bump coverage to 100% in RedeemBC * WIP Bancor tests * format + fix typo * WIP BancorBondingCurve Tests * format + substitute msg.sender * decimalConversion tests + bugfix * bancor tests v1 ready * add scripts + e2e and bump coverage * chore: review bugfixes and add getter for Bancor Funding Manager What has been done? - Reviewd bug fixes and removed the comments - Add getter functions as suggested by comment - Add tests for getter functions * chore: remove comment of idempotence Why? Thought process here is that it saves gas to not needlessly execute a tx. If the PR reviewers would like a different practice then I'm happy to change the code, because it might be personal pref * chore: remove bugfix comments of Redeeming and Base BCFM Reviewd all the comments and agreed. Removed the comments * docs: add documentation for max computational amount Bancor BCFM The Bancor Formula contract has a maximum uint value it can process for issuing and redeeming, 10^26 and 10^38 respectively. When a larger value is used for calculations, the contract will revert. These values leave enough room for most cases but the edge cases. Edge cases could include tokens that have very little value or tokens with high decimals. For those cases it might be necessary to split buy amounts over multiple transaction. Because we don't want to modify the Aragon formula contract we decided to make sure to inform the user and dev of this fact by means of a natspec documentation * WIP * emit bugfix * Update RedeemingB_CurveFundingManagerBase.t.sol * Update BondingCurveFundingManagerBase.t.sol * review fixes review fixes - make base bonding curve FundingManager -rename virtualBuy/Sell orders - fix burn bug - add check for edge case in testing - other smaller stuff * feat: add events to bonding curve funding manager Adds all the events and tests for: - BondingCurveFundingManagerBase - RedeemingBondingCurveFundingMangerBase - VirtualCollateralSupplyBase - VirtualTokenSupplyBase - BancorVirtualSupplyBondingCurveFundingManager * fix: add decimal input validation * docs: add comment clarifying abstract functions * chore: add events and remove todo comments * remove todo * resolve merge issues * make fmt * address edge case in ElasticReceiptToken * initial commit * corrected the token() call * Restructered init function of BancorVirtualSupplyBondingCurveFundingManger and introduced the accepted token variable in the contract * refactoring to resolve stack-too-deep. should work as intended now * Add extra E2E tests (#349) * setup new E2E tests * clean up E2E tests * rename roles * address review * Implemented Pablo's PR review * forge fmt * Security review PR #350: resolving comments (#362) * fix(gh-356): add view modifier to functions for gas optimization The following function are set to view to optimize gas: - _issueTokensFormulaWrapper - _issueTokens - _redeemTokensFormulaWrapper - _redeemTokens * docs(gh-357): update comments for clarity and misspelling * perf(gh-359): Remove unnecessary payable modifier Removed payable modifer because the function will not receive Ether, and removing them makes for gas optimization * perf(gh-358): remove unnecessary input validation for fee percentage The function _calculateFeeDeductedDepositAmount can only be called from a function which has already done an input validation. For that reason the value can never be bigger than BPS. The check is unnecessary. Removed the test which check for the input validation * refactor(gh-360): Rename function sellOrder(for) & buyOrder(for) Because there is not really an order placed but instead tokens are being bought and sold on the bonding curve, the word "order" can be misleading. The functions have been renamed for clarity * refactor(gh-361): remove unnecessary function _issueTokens & _redeemTokens * refactor(gh-374): move token decimal input validation to implementation The issuance token decimal input validation was done within the BondingCurveFundingManagerBase. It made more sense to move this to the implementation contract. In addition to the refactor a check was added within the BancorVirtualSupplyBondingCurveFundingManager for tokens to revert tokens decimals < 7 to ensure no destructive precision loss when using PPM for calculations. * fix: address pr review comments - Fix typo in function comment - Fix branching test tree comment - Fix test to include vm.assume for == 7 * Omega security review #350 : Additional fixes (#372) * remove unused imports remove unused import from SingleVoteGovernor * add zero check in virtual supply contracts with tests * make virtual sub and add unchecked * Block ownerless deployment on setup - more extensive comment - added tests * format * correction in VirtualCollateralSupplyBase * Issue template for security reviews (#379) * Issue template for security reviews * fix thump up emoji * Orchestrator security review (#378) * Update Orchestrator.sol * Update lcov.info * E2E test restructuring (#377) * Renaming and reorganizing E2E test - unifying naming structure - creating folders * E2E test refactors First round of refactorings RoleAuthorizerE2E done. Should serve as example for the rest finsha Authorizers refactor and small subdivision changes add setUp func + moduleConfigurations to all tests add template to E2ERegistry Finish rest of easy refactors Also deleted RegisterModuleAtFactory.t. sol because it was duplicating the setUp tests in the E2EModuleRegsitry Orchestrator E2E refactor * clean up imports + small final refactor * add template + pre-commit * Uncomment template so linter doesn't complain * remove TODOs * change file format for template tests were failing * new fmt in foundry --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: FHieser <felix_hieser@web.de> * lcov removal from make pre-commit (#391) * Update Makefile * Foundry Update fmt * Update .gitignore * 376 Create E2E Test for MetadataManager (#392) * Move E2E test to the according folder * Complement Interface * Update MetadataManager.sol * Create E2E Test for MetadataManager * Small fix * EIP 165 implementation (#371) * Removed verifyAddressIsxyz from Orchestrator.sol * created a separate library for fetching interfaces * Included all interfaces in the interfaceID Library * implemented erc165 in orchestratorFactory * Navigating a pretty complex web of inheritances to implement ERC165 * Added the supportsInterface function to all relevant solidity files * does not work right now. inheritances are very messy * This compiles. need to figure out a way to do _supportsInterface calls to addresses * Fully integrated eip-165 into the inverter contracts * forge fmt * added supportsInterface function to the AuthorizerMock contract * Fixed the failing tests (hopefully) by implementing the supportsInterface functions in them. Maybe I need to introduce them in the interface itself * implemented felix's optimisation * switched from named return values * refactored position of supportsInterface * refactored stuff * supportsInterface test introduced in unit tests - 1 * added eip165 tests - 2 * eip165 tests -3 * added eip165 tests to authorizer modules * tests for eip165 completed * fixed cases of collision of random interface Ids being the same as actually supported ids * reverted the runs * fixed a typo * removed the scope of randomized IDs to avoid collisions with all the other supported interfaceIDs * renamed randomInterfaceId to invalidInterfaceId * Make supportsInterface more compact * fmt * Integrate supportInterface fuzzing * Add dependency check --------- Co-authored-by: FHieser <felix_hieser@web.de> * [Bugfix] correct metadata name and makefile links (#400) * correct makefile links and metadata * format * Update SetupToyOrchestratorScript.s.sol * Update DeploymentScript.s.sol * Update DeploymentScript.s.sol * Create UpgradeBeacon.s.sol * Foundry update --------- Co-authored-by: FHieser <felix_hieser@web.de> Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> * Omega security review #350 : Remaining bonding curve comments (#385) * perf(gh-380): add Orchestrator token decimals to implementaion state The Orchestrator token decimals got called several times within the implementation which are external calls. To optimize for gas, the decimals now get stored in the state on fundingmanager init * fix(gh-381): add state to track transaction fee for BC The transaction fee for buying and selling now gets subtracted from the virtual balances that we store into a seperate variable. The function calculating the amount subtracted by the fee now also returns the fee amount, and has been renamed to reflect this change. This commit includes the adapted tests * Issue template for security reviews (#379) * Issue template for security reviews * fix thump up emoji * feat: add static price getter for buying and selling A static price is needed to display within the UI as reference. The formula to calculate the static price is taken from the Aragon contracts to ensure it works with the formula contract as well. Test have been added for the new feature * feat: add slippage functionalitiy through minAmountOut parameter Function that calcuate the amount out are added to the contract so that the UI can call them, and use the output as minAmountOut parameter in the functions. This parameter function as slippage in case a big order gets bought before the users tx, or the tx gets frontrun This commit includes the adapted tests * Static Price Test (#397) * WIP * WIP * format * correct typo * add fuzzing test to static price calculation --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: 0xNuggan <82726722+0xNuggan@users.noreply.github.com> --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: 0xNuggan <82726722+0xNuggan@users.noreply.github.com> * Remove testStaticPriceWithFuzzing (#403) * 366 security review 350 erc20paymentclient findings (#386) * Fix #350 (comment) * Adapting _ensureTokenAllowance * Add internal token tracking * Split ERC20PaymentClient into Access and Mock * Restructuring ModuleTest to include OrchestratorMock * testEnsureTokenBalance correctly * finish PaymentClient tests * Test for AmountPaid in PaymentProcessors * pre-commit * Update Natspec * Delete lcov.info * Update TokenGatedRoleAuthorizer.t.sol * Update BancorVirtualSupplyBondingCurveFundingManager.t.sol * Update BancorVirtualSupplyBondingCurveFundingManager.t.sol * Update ERC165 test * update interfaceid test * Additional events (#404) * preliminary steps * add additional events * event testing * Authorizer event emission test * all emissions added to tests * NATSPEC event description * bugfix * foundryup + format * review fixes * bugFix: subtract fee amount from collateral for sellOrder * chore: remove unused function * fix: add check to prevent address collision * Sc 156 testing bug fix (#417) * bugfix: solve address collition in BC * Remove fuzzing from testSupportsInterface --------- Co-authored-by: Zitzak <marvin88gr@gmail.com> * SC-148: Security Review - Interface lib ERC165Checker (#410) * Remove LibInterface * Change to ERC165Checker * Fix Test * Update ERC20PaymentClient.sol (#413) * Security review pr#350 payment processor (#425) * Fix amountPaid for removedpayments * Move up for Checks-Effects-Interactions Pattern * Security review part two fixes (#420) * correct findings * pr review fixes * address collision fix * Bonding curve extra scripts (#355) * Investable Workstreams Scripts script part 1 weird revert eror script part 2 fscripts run corectly * bugfix + update values * post-merge fix * Create giveBountyManagerRolesToAddress.s.sol * Update giveBountyManagerRolesToAddress.s.sol * fmt * review fixes * Pass the structs * chore: fix review comment (#440) * Feat: MetaTx/Multicall + Intervention Mechanism + OZ v5 * [DO NOT MERGE] OpenZeppelin V5 update (#401) * chore: update OZ lib to V5 * chore: update openzeppelin to v5 - updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed * chore: update Solidity version 0.8.19 -> 0.8.20 * format: format code * chore: fix CI and update version The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet * Delete .gas-snapshot * Update .gitignore * Change TestsupportsInterface to only check immediate Iterfaces --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: FHieser <felix_hieser@web.de> * chore: update CI to include testing scripts * Update ci.yml (#421) * fix: solved address collition in bc test * SC-122: Implement MultiCall and MetaTransaction Support (#405) * chore: update OZ lib to V5 * chore: update openzeppelin to v5 - updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed * chore: update Solidity version 0.8.19 -> 0.8.20 * format: format code * 394 metatransactions dev (#399) * Modify ModuleManager to enact ERC2771 * Intermediate Push I need to check something and pushing makes most sense he * Fix inheritance to enable build * Expose isTrustedForwarder in Orchestrator * Implement modified ERC2771 to Module * Fix Tests A lot of modules havent been properly initialized in the tests. The fix targets that by actually using the proxy Clone implementation and then properly initializing them * Add Module Test * Implement MinimalForwarder * Fix MakeFile * Create E2ETest * Add specific Role section in E2E Test * Remove todo * Marvin G Fix * Foundryup formatting * Fix after openZepplelin Update * Create Transaction forwarder * Update IModule.sol * Change to Transaction forwarder * chore: fix CI and update version The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet * WIP * Fixing tests * Cleanup * Add Multicall Section into E2ETest * Cleanup * Merge fix * Fix based on comments * Delete .gas-snapshot * Update .gitignore * Change TestsupportsInterface to only check immediate Iterfaces * Fix merge * Create DeployAndSetUpTransactionForwarder.s.sol * Add Transaction Forwarder Script to deployment scripts --------- Co-authored-by: Zitzak <marvin88gr@gmail.com> * SC-140: Implement version and intervention mechanism in beacon (#418) * Rename Contracts * Refactor Beacons and add Version function * Make it buildable * Fix test * Cleanup * Update InverterBeacon.t.sol * Update InverterBeacon.t.sol * add BeaconProxy Test * Clarify Outcommented * Fix Beacon Upgrade Script * Update UpgradeBeacon.s.sol * Add Intervention Mechanism to InverterBeacon * Make it buildable * PR comment fix * Add tests * Add Interface * Cleanup * Move and update Beacon e2e * Update InverterBeaconE2E.t.sol * Cleanup * Adapt naming * Fix after merge * Fix inheritance * Remove beacon implementation assumption * update _contextSuffixLength * Remove test for empty beacon implementation * Let Module inherit from ERC2771ContextUpgradeable * Fix init in Module test * Fix tests * Fix: Revert merging issue --------- Co-authored-by: Marvin <43185740+Zitzak@users.noreply.github.com> Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: FHieser <felix_hieser@web.de> Co-authored-by: Zitzak <marvin88gr@gmail.com> * Fix: Merge issue resolved * Fix: Merge conflict issue --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: Rahul Saxena <rahulsaxena.pro@gmail.com> Co-authored-by: FHieser <felix_hieser@web.de> Co-authored-by: Zitzak <marvin88gr@gmail.com> Co-authored-by: Marvin <43185740+Zitzak@users.noreply.github.com> Co-authored-by: Marvin Kruse <marvinkruse@users.noreply.github.com>
marvinkruse
pushed a commit
that referenced
this pull request
Apr 12, 2024
* chore: update OZ lib to V5 * chore: update openzeppelin to v5 - updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed * chore: update Solidity version 0.8.19 -> 0.8.20 * format: format code * chore: fix CI and update version The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet * Delete .gas-snapshot * Update .gitignore * Change TestsupportsInterface to only check immediate Iterfaces --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: FHieser <felix_hieser@web.de>
marvinkruse
added a commit
that referenced
this pull request
Apr 14, 2024
* [DO NOT MERGE] OpenZeppelin V5 update (#401) * chore: update OZ lib to V5 * chore: update openzeppelin to v5 - updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed * chore: update Solidity version 0.8.19 -> 0.8.20 * format: format code * chore: fix CI and update version The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet * Delete .gas-snapshot * Update .gitignore * Change TestsupportsInterface to only check immediate Iterfaces --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: FHieser <felix_hieser@web.de> * chore: update CI to include testing scripts * Update ci.yml (#421) * fix: solved address collition in bc test * SC-122: Implement MultiCall and MetaTransaction Support (#405) * chore: update OZ lib to V5 * chore: update openzeppelin to v5 - updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed * chore: update Solidity version 0.8.19 -> 0.8.20 * format: format code * 394 metatransactions dev (#399) * Modify ModuleManager to enact ERC2771 * Intermediate Push I need to check something and pushing makes most sense he * Fix inheritance to enable build * Expose isTrustedForwarder in Orchestrator * Implement modified ERC2771 to Module * Fix Tests A lot of modules havent been properly initialized in the tests. The fix targets that by actually using the proxy Clone implementation and then properly initializing them * Add Module Test * Implement MinimalForwarder * Fix MakeFile * Create E2ETest * Add specific Role section in E2E Test * Remove todo * Marvin G Fix * Foundryup formatting * Fix after openZepplelin Update * Create Transaction forwarder * Update IModule.sol * Change to Transaction forwarder * chore: fix CI and update version The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet * WIP * Fixing tests * Cleanup * Add Multicall Section into E2ETest * Cleanup * Merge fix * Fix based on comments * Delete .gas-snapshot * Update .gitignore * Change TestsupportsInterface to only check immediate Iterfaces * Fix merge * Create DeployAndSetUpTransactionForwarder.s.sol * Add Transaction Forwarder Script to deployment scripts --------- Co-authored-by: Zitzak <marvin88gr@gmail.com> * SC-140: Implement version and intervention mechanism in beacon (#418) * Rename Contracts * Refactor Beacons and add Version function * Make it buildable * Fix test * Cleanup * Update InverterBeacon.t.sol * Update InverterBeacon.t.sol * add BeaconProxy Test * Clarify Outcommented * Fix Beacon Upgrade Script * Update UpgradeBeacon.s.sol * Add Intervention Mechanism to InverterBeacon * Make it buildable * PR comment fix * Add tests * Add Interface * Cleanup * Move and update Beacon e2e * Update InverterBeaconE2E.t.sol * Cleanup * Adapt naming * Fix after merge * Fix ModuleTest bug * Create digest helper function for meta tx forwarder (#430) * Create digest creation function * Cleanup * Add getDigest Test * add _ to getStructHash() * Update deployment metadata * typo fixes * format + change active addresses * Fix: Revamp scripts & re-add to CI (#419) * Fix: Workflow File Syntax * Chore: Update VSCode settings - Adding the git.detectSubmodule setting to the config. This tells your local VSCode to ignore the submodules in the "git" tab. Which makes it a bit easier to work with, as we will only touch the submodules via forge commands anyways. - Update remote compiler version to 0.8.23 as well (just in case) * Chore: Add install cmd to Makefile * Fix: Old Wording in Makefile * Add basic run functions * Update ci.yml * Swap scripts to higher position * Update ci.yml * Test removing the override of the clean command * Split make pre-test into pre-test and pre-script * Update Makefile * Update Env Variables in make file * Fix typo * Quick test * Revert "Quick test" This reverts commit 192f71e. * Change comments * Update ci.yml to source the env from dev.env * Test if set dev command was wrong * Update ci.yml * Update ci.yml * Test setting env in make file directly * Load directly from the dev.env file * Create DeployRecurringPaymentManager.s.sol * Simplify Transaction Forwarder Script * Update DeployAndSetUpBeacon.s.sol * Fix comments * Update DeploymentScript.s.sol Add missing scripts and todo comments for @Zitzak * fix: DeploymentScript * Remove Beacon from Factories * Small Fixes * Fix merge * Remove todos * Update ci.yml * Revert removal of make clean * Change metadata * Fix: Clarify comment * Chore: Update yml file to use make commands * Tests: Add check for bc funding manager in tests --------- Co-authored-by: FHieser <felix_hieser@web.de> Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: Zitzak <marvin88gr@gmail.com> * CI: Enable CI on certain Branches & all PRs * Fix: Enable <18 Decimal Tokens as Collateral in Bonding Curve (#435) * change decimals to 6 * working draft some test are still failing because of excessive rejections * formatting * bugfix * fix vm.assume rejections * review fixes modified the getVirtual... functions to return the value in the native decimal amount instead of the normalized one and added some tests * add normalization to virtualSupply this should help set stuff up for external BC tokens * changes after discussion with marvin + corresponding tests Ended up moving the place where the decimals get converted, and adapted all tests * remove remaining question * format the online editor merge messed the format up * remove stray comments * Feat: Add Governance Contract (#436) * Create IGovernanceContract.sol * Create GovernanceContract.sol * Add Natspec * Refactoring and improvements * Create Tests * Add more detail to test * Add Governance to dev.env * Update ModuleFactoryMock.sol * Update ModuleFactory.t.sol * Remove comment * Remove comments * Update Module Factory to check for governanceContract ownership * Add Governance to E2ETests * Update InverterBeaconE2E.t.sol * Delete ModuleTest_Template.txt * Fix Tests * Fix to a buildable state in scripts * Create DeployGovernanceContract.s.sol * Fix description * Remove validNewVersion and add setImplementation in Constructor * Update Makefile * Fix typo * Update DeploymentScript.s.sol * Rename to governor contract * Correct error naming and set minimum Timelock Period to 48h * Revert "Delete ModuleTest_Template.txt" This reverts commit a62d07c. * Fix based on recommondations * Test for beacon not owned by Governor * Make Governance Implementation a proxy in scripts and e2e * accessible instead of accessable * Fix typos * Adapt naming * Adapt naming * Change naming * Adapt naming * Remove Ownable dependency * Use AccessControlUpgradeable instead of AccessControl * Adapt naming * Forgot the R in Governor * Fix inheritance * Remove beacon implementation assumption * update _contextSuffixLength * Remove test for empty beacon implementation * Let Module inherit from ERC2771ContextUpgradeable * Fix init in Module test * Fix tests * Add batched role grant/revoke (#444) * Add batched functions * Clean up AuthorizerMock * Update tests * Feat: Allow zero minor version for first implementation (#446) * Feat: Allow minor version to be zero Note: Only possible during intialization, which is why the modifier (used for any subsequent version) only allows versions greater than zero/the last version. * Test: Adapt tests to cover zero version flow * Test: Change tests to all start at minor version zero * Scripts: Change deployment scripts to start with minor version zero * Feat: Allow minor version to be zero Note: Only possible during intialization, which is why the modifier (used for any subsequent version) only allows versions greater than zero/the last version. * Test: Adapt tests to cover zero version flow * Test: Change tests to all start at minor version zero * Scripts: Change deployment scripts to start with minor version zero * [DO NOT MERGE] OpenZeppelin V5 update (#401) * chore: update OZ lib to V5 * chore: update openzeppelin to v5 - updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed * chore: update Solidity version 0.8.19 -> 0.8.20 * format: format code * chore: fix CI and update version The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet * Delete .gas-snapshot * Update .gitignore * Change TestsupportsInterface to only check immediate Iterfaces --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: FHieser <felix_hieser@web.de> * chore: update CI to include testing scripts * Update ci.yml (#421) * SC-122: Implement MultiCall and MetaTransaction Support (#405) * chore: update OZ lib to V5 * chore: update openzeppelin to v5 - updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed * chore: update Solidity version 0.8.19 -> 0.8.20 * format: format code * 394 metatransactions dev (#399) * Modify ModuleManager to enact ERC2771 * Intermediate Push I need to check something and pushing makes most sense he * Fix inheritance to enable build * Expose isTrustedForwarder in Orchestrator * Implement modified ERC2771 to Module * Fix Tests A lot of modules havent been properly initialized in the tests. The fix targets that by actually using the proxy Clone implementation and then properly initializing them * Add Module Test * Implement MinimalForwarder * Fix MakeFile * Create E2ETest * Add specific Role section in E2E Test * Remove todo * Marvin G Fix * Foundryup formatting * Fix after openZepplelin Update * Create Transaction forwarder * Update IModule.sol * Change to Transaction forwarder * chore: fix CI and update version The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet * WIP * Fixing tests * Cleanup * Add Multicall Section into E2ETest * Cleanup * Merge fix * Fix based on comments * Delete .gas-snapshot * Update .gitignore * Change TestsupportsInterface to only check immediate Iterfaces * Fix merge * Create DeployAndSetUpTransactionForwarder.s.sol * Add Transaction Forwarder Script to deployment scripts --------- Co-authored-by: Zitzak <marvin88gr@gmail.com> * SC-140: Implement version and intervention mechanism in beacon (#418) * Rename Contracts * Refactor Beacons and add Version function * Make it buildable * Fix test * Cleanup * Update InverterBeacon.t.sol * Update InverterBeacon.t.sol * add BeaconProxy Test * Clarify Outcommented * Fix Beacon Upgrade Script * Update UpgradeBeacon.s.sol * Add Intervention Mechanism to InverterBeacon * Make it buildable * PR comment fix * Add tests * Add Interface * Cleanup * Move and update Beacon e2e * Update InverterBeaconE2E.t.sol * Cleanup * Adapt naming * Fix after merge * Fix ModuleTest bug * Create digest helper function for meta tx forwarder (#430) * Create digest creation function * Cleanup * Add getDigest Test * add _ to getStructHash() * Update deployment metadata * typo fixes * format + change active addresses * Fix: Revamp scripts & re-add to CI (#419) * Fix: Workflow File Syntax * Chore: Update VSCode settings - Adding the git.detectSubmodule setting to the config. This tells your local VSCode to ignore the submodules in the "git" tab. Which makes it a bit easier to work with, as we will only touch the submodules via forge commands anyways. - Update remote compiler version to 0.8.23 as well (just in case) * Chore: Add install cmd to Makefile * Fix: Old Wording in Makefile * Add basic run functions * Update ci.yml * Swap scripts to higher position * Update ci.yml * Test removing the override of the clean command * Split make pre-test into pre-test and pre-script * Update Makefile * Update Env Variables in make file * Fix typo * Quick test * Revert "Quick test" This reverts commit 192f71e. * Change comments * Update ci.yml to source the env from dev.env * Test if set dev command was wrong * Update ci.yml * Update ci.yml * Test setting env in make file directly * Load directly from the dev.env file * Create DeployRecurringPaymentManager.s.sol * Simplify Transaction Forwarder Script * Update DeployAndSetUpBeacon.s.sol * Fix comments * Update DeploymentScript.s.sol Add missing scripts and todo comments for @Zitzak * fix: DeploymentScript * Remove Beacon from Factories * Small Fixes * Fix merge * Remove todos * Update ci.yml * Revert removal of make clean * Change metadata * Fix: Clarify comment * Chore: Update yml file to use make commands * Tests: Add check for bc funding manager in tests --------- Co-authored-by: FHieser <felix_hieser@web.de> Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: Zitzak <marvin88gr@gmail.com> * CI: Enable CI on certain Branches & all PRs * Fix: Enable <18 Decimal Tokens as Collateral in Bonding Curve (#435) * change decimals to 6 * working draft some test are still failing because of excessive rejections * formatting * bugfix * fix vm.assume rejections * review fixes modified the getVirtual... functions to return the value in the native decimal amount instead of the normalized one and added some tests * add normalization to virtualSupply this should help set stuff up for external BC tokens * changes after discussion with marvin + corresponding tests Ended up moving the place where the decimals get converted, and adapted all tests * remove remaining question * format the online editor merge messed the format up * remove stray comments * Feat: Add Governance Contract (#436) * Create IGovernanceContract.sol * Create GovernanceContract.sol * Add Natspec * Refactoring and improvements * Create Tests * Add more detail to test * Add Governance to dev.env * Update ModuleFactoryMock.sol * Update ModuleFactory.t.sol * Remove comment * Remove comments * Update Module Factory to check for governanceContract ownership * Add Governance to E2ETests * Update InverterBeaconE2E.t.sol * Delete ModuleTest_Template.txt * Fix Tests * Fix to a buildable state in scripts * Create DeployGovernanceContract.s.sol * Fix description * Remove validNewVersion and add setImplementation in Constructor * Update Makefile * Fix typo * Update DeploymentScript.s.sol * Rename to governor contract * Correct error naming and set minimum Timelock Period to 48h * Revert "Delete ModuleTest_Template.txt" This reverts commit a62d07c. * Fix based on recommondations * Test for beacon not owned by Governor * Make Governance Implementation a proxy in scripts and e2e * accessible instead of accessable * Fix typos * Adapt naming * Adapt naming * Change naming * Adapt naming * Remove Ownable dependency * Use AccessControlUpgradeable instead of AccessControl * Adapt naming * Forgot the R in Governor * Add batched role grant/revoke (#444) * Add batched functions * Clean up AuthorizerMock * Update tests * Feat: Allow zero minor version for first implementation (#446) * Feat: Allow minor version to be zero Note: Only possible during intialization, which is why the modifier (used for any subsequent version) only allows versions greater than zero/the last version. * Test: Adapt tests to cover zero version flow * Test: Change tests to all start at minor version zero * Scripts: Change deployment scripts to start with minor version zero * Feat: Allow minor version to be zero Note: Only possible during intialization, which is why the modifier (used for any subsequent version) only allows versions greater than zero/the last version. * Test: Adapt tests to cover zero version flow * Test: Change tests to all start at minor version zero * Scripts: Change deployment scripts to start with minor version zero * [DO NOT MERGE] OpenZeppelin V5 update (#401) * chore: update OZ lib to V5 * chore: update openzeppelin to v5 - updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed * chore: update Solidity version 0.8.19 -> 0.8.20 * format: format code * chore: fix CI and update version The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet * Delete .gas-snapshot * Update .gitignore * Change TestsupportsInterface to only check immediate Iterfaces --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: FHieser <felix_hieser@web.de> * chore: update CI to include testing scripts * Update ci.yml (#421) * SC-122: Implement MultiCall and MetaTransaction Support (#405) * chore: update OZ lib to V5 * chore: update openzeppelin to v5 - updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed * chore: update Solidity version 0.8.19 -> 0.8.20 * format: format code * 394 metatransactions dev (#399) * Modify ModuleManager to enact ERC2771 * Intermediate Push I need to check something and pushing makes most sense he * Fix inheritance to enable build * Expose isTrustedForwarder in Orchestrator * Implement modified ERC2771 to Module * Fix Tests A lot of modules havent been properly initialized in the tests. The fix targets that by actually using the proxy Clone implementation and then properly initializing them * Add Module Test * Implement MinimalForwarder * Fix MakeFile * Create E2ETest * Add specific Role section in E2E Test * Remove todo * Marvin G Fix * Foundryup formatting * Fix after openZepplelin Update * Create Transaction forwarder * Update IModule.sol * Change to Transaction forwarder * chore: fix CI and update version The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet * WIP * Fixing tests * Cleanup * Add Multicall Section into E2ETest * Cleanup * Merge fix * Fix based on comments * Delete .gas-snapshot * Update .gitignore * Change TestsupportsInterface to only check immediate Iterfaces * Fix merge * Create DeployAndSetUpTransactionForwarder.s.sol * Add Transaction Forwarder Script to deployment scripts --------- Co-authored-by: Zitzak <marvin88gr@gmail.com> * SC-140: Implement version and intervention mechanism in beacon (#418) * Rename Contracts * Refactor Beacons and add Version function * Make it buildable * Fix test * Cleanup * Update InverterBeacon.t.sol * Update InverterBeacon.t.sol * add BeaconProxy Test * Clarify Outcommented * Fix Beacon Upgrade Script * Update UpgradeBeacon.s.sol * Add Intervention Mechanism to InverterBeacon * Make it buildable * PR comment fix * Add tests * Add Interface * Cleanup * Move and update Beacon e2e * Update InverterBeaconE2E.t.sol * Cleanup * Adapt naming * Fix after merge * Create digest helper function for meta tx forwarder (#430) * Create digest creation function * Cleanup * Add getDigest Test * add _ to getStructHash() * typo fixes * format + change active addresses * Fix: Revamp scripts & re-add to CI (#419) * Fix: Workflow File Syntax * Chore: Update VSCode settings - Adding the git.detectSubmodule setting to the config. This tells your local VSCode to ignore the submodules in the "git" tab. Which makes it a bit easier to work with, as we will only touch the submodules via forge commands anyways. - Update remote compiler version to 0.8.23 as well (just in case) * Chore: Add install cmd to Makefile * Fix: Old Wording in Makefile * Add basic run functions * Update ci.yml * Swap scripts to higher position * Update ci.yml * Test removing the override of the clean command * Split make pre-test into pre-test and pre-script * Update Makefile * Update Env Variables in make file * Fix typo * Quick test * Revert "Quick test" This reverts commit 192f71e. * Change comments * Update ci.yml to source the env from dev.env * Test if set dev command was wrong * Update ci.yml * Update ci.yml * Test setting env in make file directly * Load directly from the dev.env file * Create DeployRecurringPaymentManager.s.sol * Simplify Transaction Forwarder Script * Update DeployAndSetUpBeacon.s.sol * Fix comments * Update DeploymentScript.s.sol Add missing scripts and todo comments for @Zitzak * fix: DeploymentScript * Remove Beacon from Factories * Small Fixes * Fix merge * Remove todos * Update ci.yml * Revert removal of make clean * Change metadata * Fix: Clarify comment * Chore: Update yml file to use make commands * Tests: Add check for bc funding manager in tests --------- Co-authored-by: FHieser <felix_hieser@web.de> Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: Zitzak <marvin88gr@gmail.com> * Feat: Add Governance Contract (#436) * Create IGovernanceContract.sol * Create GovernanceContract.sol * Add Natspec * Refactoring and improvements * Create Tests * Add more detail to test * Add Governance to dev.env * Update ModuleFactoryMock.sol * Update ModuleFactory.t.sol * Remove comment * Remove comments * Update Module Factory to check for governanceContract ownership * Add Governance to E2ETests * Update InverterBeaconE2E.t.sol * Delete ModuleTest_Template.txt * Fix Tests * Fix to a buildable state in scripts * Create DeployGovernanceContract.s.sol * Fix description * Remove validNewVersion and add setImplementation in Constructor * Update Makefile * Fix typo * Update DeploymentScript.s.sol * Rename to governor contract * Correct error naming and set minimum Timelock Period to 48h * Revert "Delete ModuleTest_Template.txt" This reverts commit a62d07c. * Fix based on recommondations * Test for beacon not owned by Governor * Make Governance Implementation a proxy in scripts and e2e * accessible instead of accessable * Fix typos * Adapt naming * Adapt naming * Change naming * Adapt naming * Remove Ownable dependency * Use AccessControlUpgradeable instead of AccessControl * Adapt naming * Forgot the R in Governor * FIx: Merge conflicts * Fix merge issue: Reworked deployment script * Fix Merge issue: ERC2771ContextUpgradeable Inheritance * Fix scripts on a basic level * Add to make scripts and reorder scripts * cleanup * Fix: Remove hardcoded decimals from RebasingFM tests --------- Co-authored-by: Marvin <43185740+Zitzak@users.noreply.github.com> Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: FHieser <felix_hieser@web.de> Co-authored-by: Zitzak <marvin88gr@gmail.com> Co-authored-by: 0xNuggan <82726722+0xNuggan@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What has been done?
OZErrors.soland tests using it do work with custom errorsNotes for the review:
Address.isContracthas been removed, which was used inBeacon.sol. Reason being its misleading nature, which you can read more about within their PR. To replace it, I went with implementing the logic that the function used to implement. We might want to update this in the future for a better check