Feat: Add Topos Funding Manager + Token Vault by marvinkruse · Pull Request #667 · InverterNetwork/contracts

marvinkruse · 2024-10-08T18:12:22Z

Contains the Topos-related contracts, including:

FM_EXT_TokenVault_v1
FM_BC_BondingSurface_Redeeming_Restricted_Repayer_Seizable_v1
FM_BC_BondingSurface_Redeeming_v1
BondingSurface

Ready to be reviewed by Team Omega, after which it will be merged into dev.

* Create Reservepool Contract * Rename and adapt * Create FM_EXT_ReservePool_v1.t.sol * Rename to TokenVault * Adapt to not send native token * Address comments * Add: - Internal function to modifier - Trailing underscore to parameters - Gherkin to test - Cleaned up tests * Update FM_EXT_TokenVault_v1.t.sol --------- Co-authored-by: Zitzak <marvin88gr@gmail.com> Co-authored-by: Marvin <43185740+Zitzak@users.noreply.github.com>

* Create Reservepool Contract * Rename and adapt * Create FM_EXT_ReservePool_v1.t.sol * Topos FM * remove liquidity vault controller contracts * Rename to TokenVault * Adapt to not send native token * Rename to Token Vault * Address comments * Outcomment Burn tests for now * Create setTokenVault * Fix most Burn functions * AddSpentAllowance to ERC20Issuance * Add _spendAllowance to BondingCurveBase * Add _spendAllowance to BurnIssuanceFor * Fix Dependency Issue * Rename Redeemable to Redeeming * Create FM_BC_BondingSurface_Redeeming_v1.sol * WIP * Add: - Internal function to modifier - Trailing underscore to parameters - Gherkin to test - Cleaned up tests * restructure contracts * Add tag for testing file split * Setup seperate Tests * Update Base BondingSurface test * Rename Mock * Remove Tests that are included in the base contract * Remove parameters from BondingSurface Struct * Go over comments * Address comments * Address Comments * Adapt Naming of events * Adding Restricted to the name * Update comments * resolve comments in implementations * Fix: Finalize moving of error into funding manager base * Fix: Typos in ERC20Issuance * Chore: Start cleanup of FM_BC's * Fix: Resolve failing test Was mismatched with implementation, didn't cover the address(this) case. --------- Co-authored-by: FHieser <felix_hieser@web.de> Co-authored-by: Felix Hieser <47272854+FHieser@users.noreply.github.com> Co-authored-by: Marvin Kruse <marvinkruse@users.noreply.github.com>

omega-audits · 2024-10-13T08:15:15Z

src/modules/fundingManager/bondingCurve/FM_BC_BondingSurface_Redeeming_v1.sol

+        override(BondingCurveBase_v1)
+        returns (uint)
+    {
+        return _issueTokensFormulaWrapper(1);


Here it's calculating the how much you can buy with 1 wei of the collateral token, but in another contract this function calculates the price for a whole issuance token.

Same for getStaticPriceForSelling

@pablo @Zitzak could you take a look here?

So... @0xNuggan rightfully pointed me towards the fact that there is actually a function in the bonding surface which does exactly what we need, calculate the spot price 🤦

So @FHieser the function can redirect to the spot price calculation function in the formula

omega-audits · 2024-10-13T08:15:41Z

src/modules/fundingManager/bondingCurve/FM_BC_BondingSurface_Redeeming_v1.sol

+        );
+
+        // The asset pool must never be empty.
+        if (capitalAvailable - redeemAmount < MIN_RESERVE) {


MIN_RESERVE is always 1 ether, this will not work well for tokens with low decimals.

Good point
we will calculate out of the decimals of the token itself

omega-audits · 2024-10-13T08:16:01Z

src/modules/fundingManager/bondingCurve/FM_BC_BondingSurface_Redeeming_v1.sol

+
+        // The asset pool must never be empty.
+        if (capitalAvailable - redeemAmount < MIN_RESERVE) {
+            redeemAmount = capitalAvailable - MIN_RESERVE;


This will reduce the amount the user gets while still taking the entire issued token when they sell. The issued taken from the user should be reduced as well in this case.

Agree. We are going to add a revert here instead of reducing the amount

omega-audits · 2024-10-13T08:16:25Z

src/modules/fundingManager/bondingCurve/FM_BC_BondingSurface_Redeeming_v1.sol

+        ) {
+            revert InvalidOrchestratorTokenWithdrawAmount();
+        }
+        token().safeTransfer(to, amount);


MIN_RESERVE is not checked here

...undingManager/bondingCurve/FM_BC_BondingSurface_Redeeming_Restricted_Repayer_Seizable_v1.sol

omega-audits · 2024-10-21T10:46:34Z

...undingManager/bondingCurve/FM_BC_BondingSurface_Redeeming_Restricted_Repayer_Seizable_v1.sol

+        override(FM_BC_BondingSurface_Redeeming_v1)
+        isBuyAndSellRestricted
+    {
+        super._sellOrder(_msgSender(), _depositAmount, _minAmountOut);


super is not doing anything here and can be removed

omega-audits · 2024-10-21T11:25:25Z

...undingManager/bondingCurve/FM_BC_BondingSurface_Redeeming_Restricted_Repayer_Seizable_v1.sol

+        }
+
+        // solhint-disable-next-line not-rely-on-time
+        lastSeizeTimestamp = uint64(block.timestamp);


why the casting to uint64?

omega-audits · 2024-10-21T11:41:56Z

...undingManager/bondingCurve/FM_BC_BondingSurface_Redeeming_Restricted_Repayer_Seizable_v1.sol

+
+    /// @dev    Processes project fee by transfer
+    /// @param _workflowFeeAmount The amount of project fee to transfer
+    function _projectFeeCollected(uint _workflowFeeAmount) internal override {


if buyFee is 0, then (I think) also the projectfee is always 0, and so youc an save some gas by not callign the transfer at all

The sell fee should also use this to send out the collateral fee that was collected.
Forgot to add that though 😅 👍

...undingManager/bondingCurve/FM_BC_BondingSurface_Redeeming_Restricted_Repayer_Seizable_v1.sol

omega-audits · 2024-10-21T14:19:03Z

src/modules/fundingManager/bondingCurve/formulas/BondingSurface.sol

+        // 1 / C_a_2
+        uint ca2inv = _inverse(_capitalAvailable);
+
+        return _capitalAvailable - _inverse(BCrM + ca2inv);


Is this correct? The formula in the docstring is:

/// B 1 /// x = (----- * m + -------)^-1 /// C_r C_a_2 ```

@rororowyourboat could you please confirm this?

The formula implementation has been confirmed by the data department

* Add empty BondingSurface test file * Remove tokenVault from init The tokenvault cant be set during init because is created simultaneously to the BondingCurve contract. It has to be added via Dependency Injection * Remove unnecessary overrides * Add and adapt tests * Adapt inheritance of functions * Update Comments * Update Tests * Update fmt * Add event to projectFeeCollected * Address comments of PR --------- Co-authored-by: Zitzak <marvin88gr@gmail.com>

...undingManager/bondingCurve/FM_BC_BondingSurface_Redeeming_Restricted_Repayer_Seizable_v1.sol

* Add empty BondingSurface test file * Remove tokenVault from init The tokenvault cant be set during init because is created simultaneously to the BondingCurve contract. It has to be added via Dependency Injection * Remove unnecessary overrides * Add and adapt tests * Adapt inheritance of functions * Update Comments * Update Tests * Update fmt * Add event to projectFeeCollected * Address comments of PR * Revert if redeemamount reaches Min Reserve * Check for Min Reserve in TransferOrchestrator Token * Add MinReserveCheck to transferRepayment * Rename Modifier * Remove time casting * Enable BuyFee * Use function that can be overriden * Add gap * Make Min_Reserves variable based on Token Decimals * Use SpotPrice to get StaticPrice * adapt based on comments * Update FM_BC_BondingSurface_Redeeming_v1.t.sol --------- Co-authored-by: Zitzak <marvin88gr@gmail.com>

marvinkruse · 2024-11-21T16:28:27Z

src/modules/fundingManager/extensions/FM_EXT_TokenVault_v1.sol

+    "@oz-up/utils/introspection/ERC165Upgradeable.sol";
+
+/**
+ * @title   Inverter Token Vault


I don't really see how this is an extension to a funding manager to be honest, the naming is confusing. It's a contract that in itself has no resemblance to a funding manager and does not interact with it or depend on it in any form.

It provides additional logic to a workflow in the form of a vault that can receive any form of tokens and withdraw them again, governed by the workflow admin. To me that looks more like a logic module, but not an extension, if we judge this according to our definitions.

An extension - to me - is the EXT_VotingRoles_v1 module: it extends the functionality of another module within the same workflow, but wouldn't make sense on its own. This module here can exist on its own, so it doesn't extend anything.

Yes and no:
It definitely has properties of a logicmodule as you suggested, but the point here is to not use the paymentProcessor to transfer tokens, which I would say is one of the main points of a logicmodule.

In regards of it being a extension: For me the VotingRoles argument doesnt work here. Yes the Module extends the functionality of another module, but it also makes sense on its own. It can act as a Controller of other contracts for example and doesnt necessarily have to be linked into our authorizer setup. Broken down its just a voting mechanism contract that can call stuff. The same goes for the Vault. It may work on its own but in this context it extends the functionality of the workflow by providing a Auhtorized setup for storing tokens, that are not included in the classic token flow of the workflow

I would agree, that FM in the naming might be misleading here, but we currently dont have a system to categorize this appropriately yet.

remappings.txt

...undingManager/bondingCurve/FM_BC_BondingSurface_Redeeming_Restricted_Repayer_Seizable_v1.sol

@dev

…de of conduct * Add empty BondingSurface test file * Remove tokenVault from init The tokenvault cant be set during init because is created simultaneously to the BondingCurve contract. It has to be added via Dependency Injection * Remove unnecessary overrides * Add and adapt tests * Adapt inheritance of functions * Update Comments * Update Tests * Update fmt * Add event to projectFeeCollected * Address comments of PR * Revert if redeemamount reaches Min Reserve * Check for Min Reserve in TransferOrchestrator Token * Add MinReserveCheck to transferRepayment * Rename Modifier * Remove time casting * Enable BuyFee * Use function that can be overriden * Add gap * Make Min_Reserves variable based on Token Decimals * Use SpotPrice to get StaticPrice * adapt based on comments * Update FM_BC_BondingSurface_Redeeming_v1.t.sol * WIP push * Rename seizable * Make state internal * Rename Parameters and Return Values * Rename Contracts * Adapt to code of conduct * Adapting Interfaces to have Contract Overview Documentation * Adapt Sectioning * Adapt Mocks and Tests to parameter conventions * Remove Todos/notes * Adapt merge aftermath * Update BondingSurface.sol * Rename to prevent shadow * Adapt remapping * Address comments * Adapt init * Update inverter standard versioning * Delete FM_BC_BondingSurface_RedeemingV1_exposed.sol * Adapt Contract Naming Standards * Fix naming references * Fix test * Update naming reference * Adapt modifier to call internal function * Adapt naming of internal function * update modifier Naming * Adapt tests * Adapt naming internal functions * Adapt Parameters * Adapt naming of function state * Adapt sections * Update Interface Inheritance * Update versioning * Update natspec to be rule compliant * Fix naming * Delete ILiquidityVaultProviderWhitelist_v1.sol * Remove Liquidity Vault Controller Interface * Rename Modifier * Add solmate as lib * Revert "Add solmate as lib" This reverts commit 050b9d2. * Remove comment * Adapt natspec * Apply comment suggestions from code review Co-authored-by: Marvin Kruse <marvinkruse@users.noreply.github.com> * Adapt internal function state * Add . to comments * Adapt spacing in natspec comments * Adapt spacing * Replace @dev with @notice * Adapt comment length * Adapt naming * Address Comments * Adapt Version * Reposition MIN_RESERVE * Adapt sectioning * Update more sectioning * Add title section and increment version --------- Co-authored-by: Zitzak <marvin88gr@gmail.com> Co-authored-by: Marvin Kruse <marvinkruse@users.noreply.github.com>

Zitzak · 2025-01-16T09:48:05Z

Afaik, this audit was done right, or are we waiting for another review from someone? @FHieser @marvinkruse

What still need to happen for sure is the documentation for this contracts ser @FHieser.

…rterNetwork/contracts into feature/ToposFundingManager

FHieser and others added 2 commits October 8, 2024 17:25

Chore: Clean up TokenVault

fe7bc8d

marvinkruse force-pushed the feature/ToposFundingManager branch from cb64317 to fe7bc8d Compare October 8, 2024 18:33

marvinkruse assigned Zitzak Oct 8, 2024

marvinkruse requested review from FHieser, ben-kaufman and jellegerbrandy October 8, 2024 23:57

marvinkruse added the audit a feature needs auditing label Oct 8, 2024

marvinkruse marked this pull request as ready for review October 9, 2024 00:07

FHieser removed their request for review October 15, 2024 08:22

marvinkruse added the topos label Oct 18, 2024

omega-audits reviewed Oct 21, 2024

View reviewed changes

FHieser mentioned this pull request Oct 24, 2024

Sc 828 address omega review comments #680

Merged

Merge branch 'dev' into feature/ToposFundingManager

eff8a27

ben-kaufman reviewed Nov 1, 2024

View reviewed changes

...undingManager/bondingCurve/FM_BC_BondingSurface_Redeeming_Restricted_Repayer_Seizable_v1.sol Outdated Show resolved Hide resolved

FHieser and others added 2 commits November 1, 2024 09:57

Merge branch 'dev' into feature/ToposFundingManager

35a19ce

omega-audits approved these changes Nov 21, 2024

View reviewed changes

jellegerbrandy approved these changes Nov 21, 2024

View reviewed changes

marvinkruse commented Nov 26, 2024

View reviewed changes

FHieser mentioned this pull request Dec 5, 2024

Sc 848 task copy fee adaption from topos to dev #700

Merged

FHieser and others added 5 commits December 17, 2024 08:36

Merge branch 'dev' into feature/ToposFundingManager

79fa7fa

Add before and afterSell functionalities

d40fe94

Add new contracts to scripts

d4b983c

Merge branch 'dev' into feature/ToposFundingManager

10f5229

Rename Files to contain .t

24e4474

FHieser and others added 18 commits January 24, 2025 14:21

Add BondingSurface E2E

ba6eb72

Use ModuleRole

f845b08

Merge branch 'dev' into feature/ToposFundingManager

d372d3d

Merge branch 'feature/ToposFundingManager' of https://github.com/Inve…

9a850d6

…rterNetwork/contracts into feature/ToposFundingManager

Finish e2e tests

59ffba4

Add Documentation

6e5d496

Adapt the Deployment Parameters

3a70d63

Create Contract Template.md

d634c2f

4/5 tests passing

5e06f20

Create BondingSurface.md

bd1742e

Merge branch 'dev' into feature/ToposFundingManager

9520bbc

Fix missing tests

3427344

update comment about eror tolerance

dc6aa93

Merge branch 'dev' into feature/ToposFundingManager

55a6b8d

Merge branch 'dev' into feature/ToposFundingManager

da0974e

Merge branch 'dev' into feature/ToposFundingManager

ff19490

refactor: resolve renaming mismatch in contracts

a646781

fix: resolve RPC issue on tests

fa5a8d8

Zitzak merged commit 7556aad into dev Feb 24, 2025
5 checks passed

Zitzak deleted the feature/ToposFundingManager branch February 24, 2025 18:18

Conversation

marvinkruse commented Oct 8, 2024 • 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 Oct 24, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

Choose a reason for hiding this comment

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FHieser Nov 27, 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Zitzak commented Jan 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

marvinkruse commented Oct 8, 2024 •

edited

Loading

Zitzak Oct 24, 2024 •

edited

Loading

FHieser Nov 27, 2024 •

edited

Loading