[REVIEW] Feat: BondingCurveFundingManager + Fixes + CleanUp by marvinkruse · Pull Request #350 · InverterNetwork/contracts

marvinkruse · 2023-10-06T12:16:41Z

⚠️ This PR merges from dev into the main branch. ⚠️
At the time of creating this PR (until it is merged), the main branch contains the latest state that has received a security review. Anything in dev has been reviewed internally, but not received a security review.

➡️ Process
This PR will receive an external security review. Additionally, at least one of our developers will review it as well, ideally someone who didn't work on it. Please raise any findings/questions directly within this PR, either as a code comment, referencing the corresponding line or as a general comment to the discussion. Please mark comments that are resolved as resolved.

📋 Contents
This PR contains all of the changes of the current dev branch, which is mainly the BondingCurveFundingManager (+ corresponding contracts), fixes to prior security reviews and general cleanups and fixes.

📄 Notes from BondingCurve PRs (PR1 and PR2)

Most base contract (BondingCurveFundingManagerBase) implements functions concerning the issuing (buying) of tokens, which every bonding curve funding manager must have. It also defines an abstract function _issueTokensFormulaWrapper which must be used in downstream contracts to define through which formula price is calculated

The next base contract (RedeemingBondingCurveFundingMangerBase) implements the functions concerning the redemption of tokens back for their collateral. It also defines an abstract function _redeemTokensFormulaWrapper which must be used in downstream contracts to define through which formula price is calculated. The reason to have two of these functions is because the Bancor contract has a different interface for calculating issuing tokens vs redeeming tokens

There are 2 more abstract contracts which introduce a virtual issuing token supply (VirtualTokenSupplyBase) and a virtual collateral supply (VirtualCollateralSupplyBase). The would be considered even more specialised in terms of functionality and therefor would be added more in a compositing fashion instead of strict inheritence

The final contract has all the features above and is called BancorVirtualSupplyBondingCurveFundingManger

* 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

* use _triggerFor correctly * pre-commit

* 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>

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

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

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

Remove Virtual Supply token and collateral 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

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

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.

* Restructure base e2e * Cleanup * Create StreamingPaymentsLifecycle.sol * pre-commit * after merge fix * Merge update * pre-commit * Merge fix * pre-commit

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.

* 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

* 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

#330) * Seperate Modifier into notClaimed and notLocked * pre-commit * Review Fixes

* 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>

…ctor 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>

* 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>

bugfixes in the contracts are marked with @review tag

* bugfix: solve address collition in BC * Remove fuzzing from testSupportsInterface --------- Co-authored-by: Zitzak <marvin88gr@gmail.com>

…nding-manager

* Remove LibInterface * Change to ERC165Checker * Fix Test

…nding-manager

…r350-bonding-curve-funding-manager SC-147: Security Review - #350 BondingCurveFndgMgr issues

* Fix amountPaid for removedpayments * Move up for Checks-Effects-Interactions Pattern

* correct findings * pr review fixes * address collision fix

ben-kaufman · 2024-03-04T23:52:21Z

src/modules/paymentProcessor/StreamingPaymentProcessor.sol

+        uint stillReleasable =
+            releasableForSpecificWalletId(client, paymentReceiver, walletId);
+        //In case there is still something to be released
+        if (stillReleasable > 0) {


This checks if the amount ready to be released is more than 0, but since when removing a payment you first claim what's ready it will always be 0 in that case. What you should check and mark as amount paid is the total amount minus the already released amount (except the amount in unclaimable).

Has been addressed

Looks better, but you forgot to add the unclaimable amount in the calculation since you still have to reserve that amount.

So to my understanding, and @FHieser, I would love for you to double check me here, this function is meant to "free up" all the payment that is still held in the contract after a claim has been removed.

As the full amount is represented by ..._salary, and all that is claimable is represented by ..._released and it has been claimed before this function call as you stated in your first comment, so what is left is the unclaimable amount.

This entry for unclaimable amount has already been removed from the paymentProcessor, so to "remove" any claim to this amount from the paymentClient we flag the unclaimable amount as paid within the paymentClient.

I don't see it removed aywhere though, and if you remove it here then the user will not be able to retry and claim it

Ah ok, I think I see what you mean, but I think it's still a problem. The user should be able to claim the unclaimable funds even if the payment is no longer active (since that part belongs to the user already). I think you should either change the code or add a function for that so users can just retry to claim their unclaimable funds even if the payment is no longer active.

So there is one intended case whereby the stream of tokens should be able to be canceled in which means the user should not be able to access the claim anymore.

The other case would be that there is an accounting error somewhere, in which case remainingReleasable has a value when it should't, signalling that the user still has something unclaimed.

I've messaged @FHieser to give some input here as well on how to handle the latter case

So we discussed the issue during our sync and you are right, the unclaimableAmount which represents failed transfer would be removed as well. This is an edge case but it is not intended behaviour so we will need to adapt the code.

Due to our time constraints and the fact that there is a lot of code in this PR which we need in main for protocol audit, we decided to go ahead and merge this even though the edge case is not taken care of. I've created an issue for this and we will refactor the code this coming work to make sure that the code works as expected.
@ben-kaufman

Here is the link #445

The issue has been created in Linear Jfyi

* 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

* 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

* 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

* 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>

* 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

* 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>

* Fix InverterNetwork/contracts#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

* Rename Contributor to paymentReceiver (#309) * Rename PaymentClient and proposal (#310) * Rename to ERC20PaymentClient * Revert "Rename to ERC20PaymentClient" This reverts commit ac66c536332390db963d8d81ea5a137ef8dc67e6. * 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 f86b1a5e65a911349bd2cc89a3ef115ce7821ace * 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 InverterNetwork/contracts#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) --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: Rahul Saxena <rahulsaxena.pro@gmail.com> Co-authored-by: 0xNuggan <82726722+0xNuggan@users.noreply.github.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>

* Fix InverterNetwork/contracts#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

* Rename Contributor to paymentReceiver (#309) * Rename PaymentClient and proposal (#310) * Rename to ERC20PaymentClient * Revert "Rename to ERC20PaymentClient" This reverts commit ac66c536332390db963d8d81ea5a137ef8dc67e6. * 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 f86b1a5e65a911349bd2cc89a3ef115ce7821ace * 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 InverterNetwork/contracts#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) --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: Rahul Saxena <rahulsaxena.pro@gmail.com> Co-authored-by: 0xNuggan <82726722+0xNuggan@users.noreply.github.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>

FHieser and others added 30 commits August 1, 2023 10:14

Rename Contributor to paymentReceiver (#309)

3201685

318 code review recurringpaymentmanager (#321)

e024a7c

* use _triggerFor correctly * pre-commit

Initial commit (#325)

310bedf

feat(BondingCurveFundingManagerBase): Add base features

95eb4ae

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

74c4a45

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

34d8014

feat(BancorVirtualSupplyBondingCurveFundingManger):

6c2e09f

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

b2c6a99

Remove Virtual Supply token and collateral interfaces

feat: add getter func and errors to interfaces

b5f1f58

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

test: virtual supply token contracts

c6c546a

chore: update modifier after rebase

04b2d24

e2e test streamingPayments (#307)

d152075

* 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

fe46334

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.

329 bountymanager bounties should be able to be claimed multiple times (

5a93940

#330) * Seperate Modifier into notClaimed and notLocked * pre-commit * Review Fixes

chore: WIP

6bcb23b

Add VirtualToken/Collateralbase tests

02fad0d

Merge branch 'main' into test/add_test_for_bondingCurve_contract_setup

a1964ac

address merge issues

b367c93

set up testing skeleton

8fa0880

BondingCurveManagerBase Test + small bugfixes

02c37d1

bugfixes in the contracts are marked with @review tag

RedeemingBondingCurve test skeleton + minor cleanup

6a83c0e

Zitzak and others added 9 commits February 9, 2024 12:18

bugFix: subtract fee amount from collateral for sellOrder

d4f3e00

chore: remove unused function

e590c87

fix: add check to prevent address collision

39578b4

Sc 156 testing bug fix (#417)

8379da7

* bugfix: solve address collition in BC * Remove fuzzing from testSupportsInterface --------- Co-authored-by: Zitzak <marvin88gr@gmail.com>

Merge branch 'dev' into sc-147-security-review-pr350-bonding-curve-fu…

bf2a0cc

…nding-manager

SC-148: Security Review - Interface lib ERC165Checker (#410)

0bcc2f2

* Remove LibInterface * Change to ERC165Checker * Fix Test

Update ERC20PaymentClient.sol (#413)

baed969

Merge branch 'dev' into sc-147-security-review-pr350-bonding-curve-fu…

5aa24f9

…nding-manager

Merge pull request #414 from InverterNetwork/sc-147-security-review-p…

cd7b9c4

…r350-bonding-curve-funding-manager SC-147: Security Review - #350 BondingCurveFndgMgr issues

FHieser mentioned this pull request Feb 28, 2024

Security review pr#350 payment processor #425

Merged

FHieser and others added 2 commits February 28, 2024 14:35

Security review pr#350 payment processor (#425)

8004142

* Fix amountPaid for removedpayments * Move up for Checks-Effects-Interactions Pattern

Security review part two fixes (#420)

a09dd12

* correct findings * pr review fixes * address collision fix

marvinkruse mentioned this pull request Feb 29, 2024

[SC-191] Security Review - MetaTx/Multicall, Intervention + OZv5 #427

Closed

ben-kaufman reviewed Mar 4, 2024

View reviewed changes

0xNuggan and others added 2 commits March 14, 2024 15:15

chore: fix review comment (#440)

a9b8925

0xNuggan approved these changes Apr 12, 2024

View reviewed changes

Zitzak approved these changes Apr 12, 2024

View reviewed changes

marvinkruse merged commit 82740e8 into main Apr 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[REVIEW] Feat: BondingCurveFundingManager + Fixes + CleanUp#350

[REVIEW] Feat: BondingCurveFundingManager + Fixes + CleanUp#350
marvinkruse merged 111 commits intomainfrom
dev

marvinkruse commented Oct 6, 2023 •

edited

Loading

Uh oh!

ben-kaufman Mar 4, 2024

Uh oh!

Zitzak Apr 10, 2024

Uh oh!

ben-kaufman Apr 11, 2024

Uh oh!

Zitzak Apr 11, 2024 •

edited

Loading

Uh oh!

ben-kaufman Apr 11, 2024

Uh oh!

ben-kaufman Apr 11, 2024

Uh oh!

Zitzak Apr 11, 2024

Uh oh!

Zitzak Apr 11, 2024 •

edited

Loading

Uh oh!

FHieser Apr 12, 2024

Uh oh!

Zitzak Apr 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Conversation

marvinkruse commented Oct 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zitzak Apr 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zitzak Apr 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

marvinkruse commented Oct 6, 2023 •

edited

Loading

Zitzak Apr 11, 2024 •

edited

Loading

Zitzak Apr 11, 2024 •

edited

Loading