Fix: Enable <18 Decimal Tokens as Collateral in Bonding Curve by 0xNuggan · Pull Request #435 · InverterNetwork/contracts

0xNuggan · 2024-04-02T10:03:52Z

changes to core contracts:

padding the virtual collateral to have the same amount of decimals than the issued token.

main changes in tests:

bound the fuzzed input in a way that considers the decimals the token has so we avoid overflows when padding
use the padded version when we are directly calling the formula

some test are still failing because of excessive rejections

marvinkruse · 2024-04-02T10:07:50Z

.../fundingManager/bondingCurveFundingManager/BancorVirtualSupplyBondingCurveFundingManager.sol

        // An input verification is needed here since the Bancor formula, which determines the
        // issucance price, utilizes PPM for its computations. This leads to a precision loss
        // that's too significant to be acceptable for tokens with fewer than 7 decimals.
-        if (_decimals < 7) {
+        if (_decimals < 7 || _decimals > collateralTokenDecimals) {


Is this gonna be a problem for Tokens like USDC and Tether, as they are both 6 decimal tokens?

You are of course completely right, this is what I get for rushing to quickly update the setter before the dev call 😆 It should be as mentioned in the interface. I pushed the fix

Haha, all good! 😂
So the bonding curve (issuance) token decimals need to be at least 7 (for precision) and also need to be at least equal to the collateral tokens decimals, not lower, correct? That's how I understand the fix you pushed now, which looks good to me, just making sure.

Initially, I thought _decimals was for the collateral token, but it's for the issuance token, I think I get it now. Is there a good reason why someone would set the issuance tokens decimals to anything but 18? (just out of curiosity)

Yes, exactly!
Personally, I can't think of a good reason to not have 18 decimals. But if we are going to "plug in" our bonding curve to an external token maybe that token had a good reason for it? So better to keep it open...

marvinkruse · 2024-04-02T10:09:44Z

test/modules/fundingManager/RebasingFundingManager.t.sol


    // Other constants.
-    uint8 private constant DECIMALS = 18;
+    //uint8 private constant DECIMALS = 18;  //Question : Is there any reason to hardcode the decimals for this fundingmanager to 18?


Is this for the collateral token or the issuance token? If it's for the collateral token, then it shouldn't be hardcoded, right?

edit: this might be for the issuance token as well, I think I initially misunderstood.

It's for the token that the bonding curve will issue. But I guess since we are going to make changes anyway for minting from an existing token, we shouldn't hardcode it

0xNuggan · 2024-04-02T12:42:22Z

Thinking a bit more about what @marvinkruse said, we'll probably need to do similar checks for the issuance token once we allow using external contracts for it. But for now we can limit it to the collateral

Zitzak · 2024-04-03T13:13:18Z

.../fundingManager/bondingCurveFundingManager/BancorVirtualSupplyBondingCurveFundingManager.sol

-        _addVirtualCollateralAmount(_depositAmount - feeAmount);
+        uint normalizedDepositAmount = _convertAmountToRequiredDecimal(
+            (_depositAmount - feeAmount),
+            collateralTokenDecimals,


As the amounts now get normalized to 18 decimals, if we indeed use for example USDC with 6 decimals as collateral, and a UI would like to get the virtual supply, don't they get this now in 18 decimals. So the dAPP interfacing with the contract has to know to calculate the value back to it's orginal decimals like this right?

Maybe it makes more sense to always convert it back to the original decimals, i.e. in my example this would be 6. Wdyt @0xNuggan

Zitzak · 2024-04-03T13:16:35Z

test/e2e/fundingManager/BondingCurveFundingManagerE2E.sol

            fundingManager.sell(
-                fundingManager.balanceOf(bob), fundingManager.balanceOf(bob)
+                fundingManager.balanceOf(bob),
+                fundingManager.balanceOf(bob) // Question: What happens with the fee? Shouldn't the amounts be wildly different since they are different tokens?


Unsure what is meant by the questions. Could you elaborate pls.

Zitzak · 2024-04-03T13:50:46Z

...undingManager/bondingCurveFundingManager/BancorVirtualSupplyBondingCurveFundingManager.t.sol

+        supply/collateral of 1/1, the first buy will move the price by a lot, since we will be effectively depositing 1e12 collateral tokens (1 + padding to 18). 
+        This will exceed the delta by a lot, but it IS expected behavior.
+        If we set the initial supply to 1e18 and the initial collateral t 1e12 (what we can call "realistic values"), the delta holds again. But on the ohter hand, we
+        aren't testing on the empty curve, just a "normal" point in it. 


Personally I would go with testing for realistic values. The BCRG is also building a simulation tool which will also (I think) help with defining the edge cases which we should then warn the user for

Zitzak

Only thing I found is the questions around always normalizing it, and how we should handle it for when the virtual supply is retrieved.

modified the getVirtual... functions to return the value in the native decimal amount instead of the normalized one and added some tests

0xNuggan · 2024-04-03T15:45:14Z

After a talk with @Zitzak we decided that external facing function should return the collateral and supply values in the native decimals, instead of the normalized ones. I've changed that and added some tests

this should help set stuff up for external BC tokens

Ended up moving the place where the decimals get converted, and adapted all tests

the online editor merge messed the format up

Zitzak

LGTM 🚀

0xNuggan · 2024-04-08T10:51:53Z

@marvinkruse do you think we can merge this?

marvinkruse

lgtm as well! @0xNuggan

* change decimals to 6 * working draft some test are still failing because of excessive rejections * formatting * bugfix * fix vm.assume rejections * review fixes modified the getVirtual... functions to return the value in the native decimal amount instead of the normalized one and added some tests * add normalization to virtualSupply this should help set stuff up for external BC tokens * changes after discussion with marvin + corresponding tests Ended up moving the place where the decimals get converted, and adapted all tests * remove remaining question * format the online editor merge messed the format up * remove stray comments

@Zitzak

* [DO NOT MERGE] OpenZeppelin V5 update (#401) * chore: update OZ lib to V5 * chore: update openzeppelin to v5 - updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed * chore: update Solidity version 0.8.19 -> 0.8.20 * format: format code * chore: fix CI and update version The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet * Delete .gas-snapshot * Update .gitignore * Change TestsupportsInterface to only check immediate Iterfaces --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: FHieser <felix_hieser@web.de> * chore: update CI to include testing scripts * Update ci.yml (#421) * fix: solved address collition in bc test * SC-122: Implement MultiCall and MetaTransaction Support (#405) * chore: update OZ lib to V5 * chore: update openzeppelin to v5 - updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed * chore: update Solidity version 0.8.19 -> 0.8.20 * format: format code * 394 metatransactions dev (#399) * Modify ModuleManager to enact ERC2771 * Intermediate Push I need to check something and pushing makes most sense he * Fix inheritance to enable build * Expose isTrustedForwarder in Orchestrator * Implement modified ERC2771 to Module * Fix Tests A lot of modules havent been properly initialized in the tests. The fix targets that by actually using the proxy Clone implementation and then properly initializing them * Add Module Test * Implement MinimalForwarder * Fix MakeFile * Create E2ETest * Add specific Role section in E2E Test * Remove todo * Marvin G Fix * Foundryup formatting * Fix after openZepplelin Update * Create Transaction forwarder * Update IModule.sol * Change to Transaction forwarder * chore: fix CI and update version The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet * WIP * Fixing tests * Cleanup * Add Multicall Section into E2ETest * Cleanup * Merge fix * Fix based on comments * Delete .gas-snapshot * Update .gitignore * Change TestsupportsInterface to only check immediate Iterfaces * Fix merge * Create DeployAndSetUpTransactionForwarder.s.sol * Add Transaction Forwarder Script to deployment scripts --------- Co-authored-by: Zitzak <marvin88gr@gmail.com> * SC-140: Implement version and intervention mechanism in beacon (#418) * Rename Contracts * Refactor Beacons and add Version function * Make it buildable * Fix test * Cleanup * Update InverterBeacon.t.sol * Update InverterBeacon.t.sol * add BeaconProxy Test * Clarify Outcommented * Fix Beacon Upgrade Script * Update UpgradeBeacon.s.sol * Add Intervention Mechanism to InverterBeacon * Make it buildable * PR comment fix * Add tests * Add Interface * Cleanup * Move and update Beacon e2e * Update InverterBeaconE2E.t.sol * Cleanup * Adapt naming * Fix after merge * Fix ModuleTest bug * Create digest helper function for meta tx forwarder (#430) * Create digest creation function * Cleanup * Add getDigest Test * add _ to getStructHash() * Update deployment metadata * typo fixes * format + change active addresses * Fix: Revamp scripts & re-add to CI (#419) * Fix: Workflow File Syntax * Chore: Update VSCode settings - Adding the git.detectSubmodule setting to the config. This tells your local VSCode to ignore the submodules in the "git" tab. Which makes it a bit easier to work with, as we will only touch the submodules via forge commands anyways. - Update remote compiler version to 0.8.23 as well (just in case) * Chore: Add install cmd to Makefile * Fix: Old Wording in Makefile * Add basic run functions * Update ci.yml * Swap scripts to higher position * Update ci.yml * Test removing the override of the clean command * Split make pre-test into pre-test and pre-script * Update Makefile * Update Env Variables in make file * Fix typo * Quick test * Revert "Quick test" This reverts commit 192f71e. * Change comments * Update ci.yml to source the env from dev.env * Test if set dev command was wrong * Update ci.yml * Update ci.yml * Test setting env in make file directly * Load directly from the dev.env file * Create DeployRecurringPaymentManager.s.sol * Simplify Transaction Forwarder Script * Update DeployAndSetUpBeacon.s.sol * Fix comments * Update DeploymentScript.s.sol Add missing scripts and todo comments for @Zitzak * fix: DeploymentScript * Remove Beacon from Factories * Small Fixes * Fix merge * Remove todos * Update ci.yml * Revert removal of make clean * Change metadata * Fix: Clarify comment * Chore: Update yml file to use make commands * Tests: Add check for bc funding manager in tests --------- Co-authored-by: FHieser <felix_hieser@web.de> Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: Zitzak <marvin88gr@gmail.com> * CI: Enable CI on certain Branches & all PRs * Fix: Enable <18 Decimal Tokens as Collateral in Bonding Curve (#435) * change decimals to 6 * working draft some test are still failing because of excessive rejections * formatting * bugfix * fix vm.assume rejections * review fixes modified the getVirtual... functions to return the value in the native decimal amount instead of the normalized one and added some tests * add normalization to virtualSupply this should help set stuff up for external BC tokens * changes after discussion with marvin + corresponding tests Ended up moving the place where the decimals get converted, and adapted all tests * remove remaining question * format the online editor merge messed the format up * remove stray comments * Feat: Add Governance Contract (#436) * Create IGovernanceContract.sol * Create GovernanceContract.sol * Add Natspec * Refactoring and improvements * Create Tests * Add more detail to test * Add Governance to dev.env * Update ModuleFactoryMock.sol * Update ModuleFactory.t.sol * Remove comment * Remove comments * Update Module Factory to check for governanceContract ownership * Add Governance to E2ETests * Update InverterBeaconE2E.t.sol * Delete ModuleTest_Template.txt * Fix Tests * Fix to a buildable state in scripts * Create DeployGovernanceContract.s.sol * Fix description * Remove validNewVersion and add setImplementation in Constructor * Update Makefile * Fix typo * Update DeploymentScript.s.sol * Rename to governor contract * Correct error naming and set minimum Timelock Period to 48h * Revert "Delete ModuleTest_Template.txt" This reverts commit a62d07c. * Fix based on recommondations * Test for beacon not owned by Governor * Make Governance Implementation a proxy in scripts and e2e * accessible instead of accessable * Fix typos * Adapt naming * Adapt naming * Change naming * Adapt naming * Remove Ownable dependency * Use AccessControlUpgradeable instead of AccessControl * Adapt naming * Forgot the R in Governor * Fix inheritance * Remove beacon implementation assumption * update _contextSuffixLength * Remove test for empty beacon implementation * Let Module inherit from ERC2771ContextUpgradeable * Fix init in Module test * Fix tests * Add batched role grant/revoke (#444) * Add batched functions * Clean up AuthorizerMock * Update tests * Feat: Allow zero minor version for first implementation (#446) * Feat: Allow minor version to be zero Note: Only possible during intialization, which is why the modifier (used for any subsequent version) only allows versions greater than zero/the last version. * Test: Adapt tests to cover zero version flow * Test: Change tests to all start at minor version zero * Scripts: Change deployment scripts to start with minor version zero * Feat: Allow minor version to be zero Note: Only possible during intialization, which is why the modifier (used for any subsequent version) only allows versions greater than zero/the last version. * Test: Adapt tests to cover zero version flow * Test: Change tests to all start at minor version zero * Scripts: Change deployment scripts to start with minor version zero * [DO NOT MERGE] OpenZeppelin V5 update (#401) * chore: update OZ lib to V5 * chore: update openzeppelin to v5 - updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed * chore: update Solidity version 0.8.19 -> 0.8.20 * format: format code * chore: fix CI and update version The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet * Delete .gas-snapshot * Update .gitignore * Change TestsupportsInterface to only check immediate Iterfaces --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: FHieser <felix_hieser@web.de> * chore: update CI to include testing scripts * Update ci.yml (#421) * SC-122: Implement MultiCall and MetaTransaction Support (#405) * chore: update OZ lib to V5 * chore: update openzeppelin to v5 - updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed * chore: update Solidity version 0.8.19 -> 0.8.20 * format: format code * 394 metatransactions dev (#399) * Modify ModuleManager to enact ERC2771 * Intermediate Push I need to check something and pushing makes most sense he * Fix inheritance to enable build * Expose isTrustedForwarder in Orchestrator * Implement modified ERC2771 to Module * Fix Tests A lot of modules havent been properly initialized in the tests. The fix targets that by actually using the proxy Clone implementation and then properly initializing them * Add Module Test * Implement MinimalForwarder * Fix MakeFile * Create E2ETest * Add specific Role section in E2E Test * Remove todo * Marvin G Fix * Foundryup formatting * Fix after openZepplelin Update * Create Transaction forwarder * Update IModule.sol * Change to Transaction forwarder * chore: fix CI and update version The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet * WIP * Fixing tests * Cleanup * Add Multicall Section into E2ETest * Cleanup * Merge fix * Fix based on comments * Delete .gas-snapshot * Update .gitignore * Change TestsupportsInterface to only check immediate Iterfaces * Fix merge * Create DeployAndSetUpTransactionForwarder.s.sol * Add Transaction Forwarder Script to deployment scripts --------- Co-authored-by: Zitzak <marvin88gr@gmail.com> * SC-140: Implement version and intervention mechanism in beacon (#418) * Rename Contracts * Refactor Beacons and add Version function * Make it buildable * Fix test * Cleanup * Update InverterBeacon.t.sol * Update InverterBeacon.t.sol * add BeaconProxy Test * Clarify Outcommented * Fix Beacon Upgrade Script * Update UpgradeBeacon.s.sol * Add Intervention Mechanism to InverterBeacon * Make it buildable * PR comment fix * Add tests * Add Interface * Cleanup * Move and update Beacon e2e * Update InverterBeaconE2E.t.sol * Cleanup * Adapt naming * Fix after merge * Fix ModuleTest bug * Create digest helper function for meta tx forwarder (#430) * Create digest creation function * Cleanup * Add getDigest Test * add _ to getStructHash() * Update deployment metadata * typo fixes * format + change active addresses * Fix: Revamp scripts & re-add to CI (#419) * Fix: Workflow File Syntax * Chore: Update VSCode settings - Adding the git.detectSubmodule setting to the config. This tells your local VSCode to ignore the submodules in the "git" tab. Which makes it a bit easier to work with, as we will only touch the submodules via forge commands anyways. - Update remote compiler version to 0.8.23 as well (just in case) * Chore: Add install cmd to Makefile * Fix: Old Wording in Makefile * Add basic run functions * Update ci.yml * Swap scripts to higher position * Update ci.yml * Test removing the override of the clean command * Split make pre-test into pre-test and pre-script * Update Makefile * Update Env Variables in make file * Fix typo * Quick test * Revert "Quick test" This reverts commit 192f71e. * Change comments * Update ci.yml to source the env from dev.env * Test if set dev command was wrong * Update ci.yml * Update ci.yml * Test setting env in make file directly * Load directly from the dev.env file * Create DeployRecurringPaymentManager.s.sol * Simplify Transaction Forwarder Script * Update DeployAndSetUpBeacon.s.sol * Fix comments * Update DeploymentScript.s.sol Add missing scripts and todo comments for @Zitzak * fix: DeploymentScript * Remove Beacon from Factories * Small Fixes * Fix merge * Remove todos * Update ci.yml * Revert removal of make clean * Change metadata * Fix: Clarify comment * Chore: Update yml file to use make commands * Tests: Add check for bc funding manager in tests --------- Co-authored-by: FHieser <felix_hieser@web.de> Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: Zitzak <marvin88gr@gmail.com> * CI: Enable CI on certain Branches & all PRs * Fix: Enable <18 Decimal Tokens as Collateral in Bonding Curve (#435) * change decimals to 6 * working draft some test are still failing because of excessive rejections * formatting * bugfix * fix vm.assume rejections * review fixes modified the getVirtual... functions to return the value in the native decimal amount instead of the normalized one and added some tests * add normalization to virtualSupply this should help set stuff up for external BC tokens * changes after discussion with marvin + corresponding tests Ended up moving the place where the decimals get converted, and adapted all tests * remove remaining question * format the online editor merge messed the format up * remove stray comments * Feat: Add Governance Contract (#436) * Create IGovernanceContract.sol * Create GovernanceContract.sol * Add Natspec * Refactoring and improvements * Create Tests * Add more detail to test * Add Governance to dev.env * Update ModuleFactoryMock.sol * Update ModuleFactory.t.sol * Remove comment * Remove comments * Update Module Factory to check for governanceContract ownership * Add Governance to E2ETests * Update InverterBeaconE2E.t.sol * Delete ModuleTest_Template.txt * Fix Tests * Fix to a buildable state in scripts * Create DeployGovernanceContract.s.sol * Fix description * Remove validNewVersion and add setImplementation in Constructor * Update Makefile * Fix typo * Update DeploymentScript.s.sol * Rename to governor contract * Correct error naming and set minimum Timelock Period to 48h * Revert "Delete ModuleTest_Template.txt" This reverts commit a62d07c. * Fix based on recommondations * Test for beacon not owned by Governor * Make Governance Implementation a proxy in scripts and e2e * accessible instead of accessable * Fix typos * Adapt naming * Adapt naming * Change naming * Adapt naming * Remove Ownable dependency * Use AccessControlUpgradeable instead of AccessControl * Adapt naming * Forgot the R in Governor * Add batched role grant/revoke (#444) * Add batched functions * Clean up AuthorizerMock * Update tests * Feat: Allow zero minor version for first implementation (#446) * Feat: Allow minor version to be zero Note: Only possible during intialization, which is why the modifier (used for any subsequent version) only allows versions greater than zero/the last version. * Test: Adapt tests to cover zero version flow * Test: Change tests to all start at minor version zero * Scripts: Change deployment scripts to start with minor version zero * Feat: Allow minor version to be zero Note: Only possible during intialization, which is why the modifier (used for any subsequent version) only allows versions greater than zero/the last version. * Test: Adapt tests to cover zero version flow * Test: Change tests to all start at minor version zero * Scripts: Change deployment scripts to start with minor version zero * [DO NOT MERGE] OpenZeppelin V5 update (#401) * chore: update OZ lib to V5 * chore: update openzeppelin to v5 - updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed * chore: update Solidity version 0.8.19 -> 0.8.20 * format: format code * chore: fix CI and update version The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet * Delete .gas-snapshot * Update .gitignore * Change TestsupportsInterface to only check immediate Iterfaces --------- Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: FHieser <felix_hieser@web.de> * chore: update CI to include testing scripts * Update ci.yml (#421) * SC-122: Implement MultiCall and MetaTransaction Support (#405) * chore: update OZ lib to V5 * chore: update openzeppelin to v5 - updated paths were needed - added constructor arguments for Ownable update - updated OZErrors and the expectRevert that were using them to work with custom errors - replaces Address.isContract with the logic it used, as it has been removed * chore: update Solidity version 0.8.19 -> 0.8.20 * format: format code * 394 metatransactions dev (#399) * Modify ModuleManager to enact ERC2771 * Intermediate Push I need to check something and pushing makes most sense he * Fix inheritance to enable build * Expose isTrustedForwarder in Orchestrator * Implement modified ERC2771 to Module * Fix Tests A lot of modules havent been properly initialized in the tests. The fix targets that by actually using the proxy Clone implementation and then properly initializing them * Add Module Test * Implement MinimalForwarder * Fix MakeFile * Create E2ETest * Add specific Role section in E2E Test * Remove todo * Marvin G Fix * Foundryup formatting * Fix after openZepplelin Update * Create Transaction forwarder * Update IModule.sol * Change to Transaction forwarder * chore: fix CI and update version The CI failed because of imports, which are now corrected. Next to this we decided to set the Solidity version to 0.8.23 because it will be more up-to-date when we launch to mainnet * WIP * Fixing tests * Cleanup * Add Multicall Section into E2ETest * Cleanup * Merge fix * Fix based on comments * Delete .gas-snapshot * Update .gitignore * Change TestsupportsInterface to only check immediate Iterfaces * Fix merge * Create DeployAndSetUpTransactionForwarder.s.sol * Add Transaction Forwarder Script to deployment scripts --------- Co-authored-by: Zitzak <marvin88gr@gmail.com> * SC-140: Implement version and intervention mechanism in beacon (#418) * Rename Contracts * Refactor Beacons and add Version function * Make it buildable * Fix test * Cleanup * Update InverterBeacon.t.sol * Update InverterBeacon.t.sol * add BeaconProxy Test * Clarify Outcommented * Fix Beacon Upgrade Script * Update UpgradeBeacon.s.sol * Add Intervention Mechanism to InverterBeacon * Make it buildable * PR comment fix * Add tests * Add Interface * Cleanup * Move and update Beacon e2e * Update InverterBeaconE2E.t.sol * Cleanup * Adapt naming * Fix after merge * Create digest helper function for meta tx forwarder (#430) * Create digest creation function * Cleanup * Add getDigest Test * add _ to getStructHash() * typo fixes * format + change active addresses * Fix: Revamp scripts & re-add to CI (#419) * Fix: Workflow File Syntax * Chore: Update VSCode settings - Adding the git.detectSubmodule setting to the config. This tells your local VSCode to ignore the submodules in the "git" tab. Which makes it a bit easier to work with, as we will only touch the submodules via forge commands anyways. - Update remote compiler version to 0.8.23 as well (just in case) * Chore: Add install cmd to Makefile * Fix: Old Wording in Makefile * Add basic run functions * Update ci.yml * Swap scripts to higher position * Update ci.yml * Test removing the override of the clean command * Split make pre-test into pre-test and pre-script * Update Makefile * Update Env Variables in make file * Fix typo * Quick test * Revert "Quick test" This reverts commit 192f71e. * Change comments * Update ci.yml to source the env from dev.env * Test if set dev command was wrong * Update ci.yml * Update ci.yml * Test setting env in make file directly * Load directly from the dev.env file * Create DeployRecurringPaymentManager.s.sol * Simplify Transaction Forwarder Script * Update DeployAndSetUpBeacon.s.sol * Fix comments * Update DeploymentScript.s.sol Add missing scripts and todo comments for @Zitzak * fix: DeploymentScript * Remove Beacon from Factories * Small Fixes * Fix merge * Remove todos * Update ci.yml * Revert removal of make clean * Change metadata * Fix: Clarify comment * Chore: Update yml file to use make commands * Tests: Add check for bc funding manager in tests --------- Co-authored-by: FHieser <felix_hieser@web.de> Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: Zitzak <marvin88gr@gmail.com> * Feat: Add Governance Contract (#436) * Create IGovernanceContract.sol * Create GovernanceContract.sol * Add Natspec * Refactoring and improvements * Create Tests * Add more detail to test * Add Governance to dev.env * Update ModuleFactoryMock.sol * Update ModuleFactory.t.sol * Remove comment * Remove comments * Update Module Factory to check for governanceContract ownership * Add Governance to E2ETests * Update InverterBeaconE2E.t.sol * Delete ModuleTest_Template.txt * Fix Tests * Fix to a buildable state in scripts * Create DeployGovernanceContract.s.sol * Fix description * Remove validNewVersion and add setImplementation in Constructor * Update Makefile * Fix typo * Update DeploymentScript.s.sol * Rename to governor contract * Correct error naming and set minimum Timelock Period to 48h * Revert "Delete ModuleTest_Template.txt" This reverts commit a62d07c. * Fix based on recommondations * Test for beacon not owned by Governor * Make Governance Implementation a proxy in scripts and e2e * accessible instead of accessable * Fix typos * Adapt naming * Adapt naming * Change naming * Adapt naming * Remove Ownable dependency * Use AccessControlUpgradeable instead of AccessControl * Adapt naming * Forgot the R in Governor * FIx: Merge conflicts * Fix merge issue: Reworked deployment script * Fix Merge issue: ERC2771ContextUpgradeable Inheritance * Fix scripts on a basic level * Add to make scripts and reorder scripts * cleanup * Fix: Remove hardcoded decimals from RebasingFM tests --------- Co-authored-by: Marvin <43185740+Zitzak@users.noreply.github.com> Co-authored-by: hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: FHieser <felix_hieser@web.de> Co-authored-by: Zitzak <marvin88gr@gmail.com> Co-authored-by: 0xNuggan <82726722+0xNuggan@users.noreply.github.com>

0xNuggan added 2 commits March 26, 2024 10:24

change decimals to 6

05c283a

working draft

8943e3a

some test are still failing because of excessive rejections

marvinkruse reviewed Apr 2, 2024

View reviewed changes

formatting

368f92f

marvinkruse reviewed Apr 2, 2024

View reviewed changes

bugfix

d2c2392

fix vm.assume rejections

a533681

0xNuggan changed the title ~~Weird erc tokens test~~ Make the Bonding Curve work with collateral tokens that have less than 18 decimals Apr 3, 2024

0xNuggan marked this pull request as ready for review April 3, 2024 10:55

0xNuggan requested a review from Zitzak April 3, 2024 10:55

Zitzak reviewed Apr 3, 2024

View reviewed changes

review fixes

11fd1a9

modified the getVirtual... functions to return the value in the native decimal amount instead of the normalized one and added some tests

add normalization to virtualSupply

41e1429

this should help set stuff up for external BC tokens

marvinkruse changed the title ~~Make the Bonding Curve work with collateral tokens that have less than 18 decimals~~ Fix: Make Bonding Curve work with collateral tokens with less than 18 decimals Apr 4, 2024

marvinkruse changed the title ~~Fix: Make Bonding Curve work with collateral tokens with less than 18 decimals~~ Fix: Enable <18 decimal tokens as collateral in Bonding Curve Apr 4, 2024

marvinkruse changed the title ~~Fix: Enable <18 decimal tokens as collateral in Bonding Curve~~ Fix: Enable <18 Decimal Tokens as Collateral in Bonding Curve Apr 4, 2024

0xNuggan added 5 commits April 5, 2024 10:27

changes after discussion with marvin + corresponding tests

54e5840

Ended up moving the place where the decimals get converted, and adapted all tests

remove remaining question

99115dd

Merge branch 'temp_dev' into weird-erc-tokens-test

9ad0c29

format

b406450

the online editor merge messed the format up

remove stray comments

ea3cb0e

Zitzak requested review from Zitzak and marvinkruse April 5, 2024 10:08

Zitzak approved these changes Apr 5, 2024

View reviewed changes

marvinkruse approved these changes Apr 9, 2024

View reviewed changes

Zitzak merged commit ae010a6 into temp_dev Apr 9, 2024

Zitzak deleted the weird-erc-tokens-test branch April 9, 2024 12:50

Conversation

0xNuggan commented Apr 2, 2024

Uh oh!

marvinkruse Apr 2, 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!

0xNuggan Apr 2, 2024

Choose a reason for hiding this comment

Uh oh!

marvinkruse Apr 2, 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!

0xNuggan Apr 2, 2024

Choose a reason for hiding this comment

Uh oh!

marvinkruse Apr 2, 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!

0xNuggan Apr 2, 2024

Choose a reason for hiding this comment

Uh oh!

0xNuggan commented Apr 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Zitzak Apr 3, 2024

Choose a reason for hiding this comment

Uh oh!

Zitzak Apr 3, 2024

Choose a reason for hiding this comment

Uh oh!

Zitzak Apr 3, 2024

Choose a reason for hiding this comment

Uh oh!

Zitzak left a comment

Choose a reason for hiding this comment

Uh oh!

0xNuggan commented Apr 3, 2024

Uh oh!

Zitzak left a comment • 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!

0xNuggan commented Apr 8, 2024

Uh oh!

marvinkruse left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

marvinkruse Apr 2, 2024 •

edited

Loading

marvinkruse Apr 2, 2024 •

edited

Loading

marvinkruse Apr 2, 2024 •

edited

Loading

0xNuggan commented Apr 2, 2024 •

edited

Loading

Zitzak left a comment •

edited

Loading