Skip to content

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

Merged
Zitzak merged 3 commits intodevfrom
sc-848-task-copy-fee-adaption-from-topos-to-dev
Dec 5, 2024
Merged

Sc 848 task copy fee adaption from topos to dev#700
Zitzak merged 3 commits intodevfrom
sc-848-task-copy-fee-adaption-from-topos-to-dev

Conversation

@FHieser
Copy link
Contributor

@FHieser FHieser commented Dec 5, 2024

Abstracts the collection of the projectfee into its own function
Directly copied from the topos branch #667

@FHieser FHieser requested a review from Zitzak December 5, 2024 09:51
@linear
Copy link

linear bot commented Dec 5, 2024

@FHieser
Copy link
Contributor Author

FHieser commented Dec 5, 2024

@Zitzak could you pls rereview

@Zitzak Zitzak merged commit 1634fcf into dev Dec 5, 2024
@Zitzak Zitzak deleted the sc-848-task-copy-fee-adaption-from-topos-to-dev branch December 5, 2024 17:27
@marvinkruse
Copy link
Member

Just saw this was merged without me being a reviewer, three things:

(1) Why is the downstream contract not version upgraded?
If BondingCurveBase_v1 changes, the version of all contracts inheriting it also needs to change, i.e. FM_BC_Bancor_Redeeming_VirtualSupply_v1 and FM_BC_Restricted_Bancor_Redeeming_VirtualSupply_v1, as the code of these also changed subsequently, speaking from a contract standpoint and it not the same anymore, so it needs changing.

(2) The _projectFeeCollected function Natspec should have @notice, not just @dev

(3) The name of the merged commit is not as we agreed.

Please mark me as a reviewer for any PRs that go into dev until we are all up to speed on using the Inverter Standard, otherwise it's just double work. (@FHieser @Zitzak)

@marvinkruse
Copy link
Member

This merge-to-dev-commit should be reverted, or did you guys already rebase too many other PRs?

0xNuggan pushed a commit that referenced this pull request Dec 6, 2024
* Abstract projectFeeCollected

* Create tests

* Adapt emit test
@Zitzak
Copy link
Collaborator

Zitzak commented Dec 6, 2024

I did merge it into the feature branch for the External price funding manager. However if you would like to change it than please go ahead and I force push it to the feature branch @marvinkruse

FHieser added a commit that referenced this pull request Dec 19, 2024
* chore: adds notes md

* chore: adapts PP Simple

* chore: adapts PP Streaming

* tmp

* chore: adapts recurring pc

* chore: adapts staking lm

* chore: adapts payment router

* chore: compiles

* fix: `ERC20PaymentClientBaseV1Test > testAddPaymentOrders`

* fix: `ERC20PaymentClientBaseV1Test > testAddPaymentOrder`

* fix: `PP_SimpleV1Test` > `test_ValidPaymentOrder`

* fix: `PP_StreamingV1Test` > `test_processPayments`

* fix: `PP_StreamingV1Test`

* chore: removes console logs

* fix: `LM_PC_RecurringPayments_v1`

* fix: `LM_PC_Staking_v1Test`

* chore: fix outstanding tests

* fix: event in pp template

* chore: removes todo

* chore: cleanup I

* fix: always sets cliff flag in payment router

* chore: cleanup II

* chore: fix commented tests

* chore: cleanup III

* chore: adds default values to streaming pp

* chore: renames `original_` => `exposed_`

* chore: uses internal setters for default vals

* chore: sets flags manually in recurring payments

* chore: adds validation for stream time config

* chore: fmt

* chore: turn state vars private

* chore: getter functions in staking and kpi module

* chore: adapt state vars to coding standards

* chore: adds internal validator fn for staking module

* chore: make fmt

* chore: shift flags by one

* chore: fmt

* Feat: Add Wrapper for Mint and Transfer Functions

* Make the issue tokens functionality abstract

* Adapt all inherited contracts

* Revert Mock override

Shouldnt have done that in the first place

* Adapt Tests

* Rename issueTokens to distributeIssuanceToken

* Make distribute collateral functionality abstract

* Rename parameter

* Make naming uniform

* Adapt all inherited contracts

* adapt spelling mistake

* Adapt tests

* Cleanup

* Fix test

* Address Comments: Natspec

* Cleanup

* Rename _distributeIssuanceToken to _handleIssuanceTokenAfterBuy

* Rename _distributeCollateralToken to _handleCollateralTokenAfterSell

* Update FM_BC_Bancor_Redeeming_VirtualSupply_v1.sol

* Update BondingCurveBase_v1.sol

* Update Sections

* Rename Function

* Adapt sections in mocks

* rename _handleIssuanceTokenAfterBuy

* Update comments

* Rename to _collateralTokenAmount

* Update src/modules/fundingManager/bondingCurve/abstracts/BondingCurveBase_v1.sol

Co-authored-by: Marvin Kruse <marvinkruse@users.noreply.github.com>

* Rename _issuanceMintAmount

* Update src/modules/fundingManager/bondingCurve/abstracts/RedeemingBondingCurveBase_v1.sol

Co-authored-by: Marvin Kruse <marvinkruse@users.noreply.github.com>

* Adapt version

---------

Co-authored-by: Marvin <43185740+Zitzak@users.noreply.github.com>
Co-authored-by: Marvin Kruse <marvinkruse@users.noreply.github.com>

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

* Abstract projectFeeCollected

* Create tests

* Adapt emit test

* convert flags to bytes32

* setter for default times in Streaming_PP + tests

* test bugfix

* refactor flags into PaymentClientBase

* Adaptations to repo CoC

* more CoC update

* even more CoC

* Update tests

* Explainer text

* refactor IERC20PaymentClientBase errors + bugfix

* abstract functions out + update tests

* address review comments

* add init tests to inheritng modules

* typo fix

* Apply comment suggestions from code review

Co-authored-by: Marvin Kruse <marvinkruse@users.noreply.github.com>

* adapt access mock function naming

* Adapt naming of internal variable

* Adapt comments

* Adapt function naming

* Fmt

* Copy over lib subprojects from dev

* Update src/modules/logicModule/LM_PC_RecurringPayments_v1.sol

Co-authored-by: Marvin Kruse <marvinkruse@users.noreply.github.com>

---------

Co-authored-by: Felix Hieser <47272854+FHieser@users.noreply.github.com>
Co-authored-by: Marvin <43185740+Zitzak@users.noreply.github.com>
Co-authored-by: Marvin Kruse <marvinkruse@users.noreply.github.com>
Co-authored-by: 0xNuggan <82726722+0xNuggan@users.noreply.github.com>
Co-authored-by: FHieser <felix_hieser@web.de>
0xNuggan pushed a commit that referenced this pull request Apr 8, 2025
* Abstract projectFeeCollected

* Create tests

* Adapt emit test
0xNuggan added a commit that referenced this pull request Apr 8, 2025
* chore: adds notes md

* chore: adapts PP Simple

* chore: adapts PP Streaming

* tmp

* chore: adapts recurring pc

* chore: adapts staking lm

* chore: adapts payment router

* chore: compiles

* fix: `ERC20PaymentClientBaseV1Test > testAddPaymentOrders`

* fix: `ERC20PaymentClientBaseV1Test > testAddPaymentOrder`

* fix: `PP_SimpleV1Test` > `test_ValidPaymentOrder`

* fix: `PP_StreamingV1Test` > `test_processPayments`

* fix: `PP_StreamingV1Test`

* chore: removes console logs

* fix: `LM_PC_RecurringPayments_v1`

* fix: `LM_PC_Staking_v1Test`

* chore: fix outstanding tests

* fix: event in pp template

* chore: removes todo

* chore: cleanup I

* fix: always sets cliff flag in payment router

* chore: cleanup II

* chore: fix commented tests

* chore: cleanup III

* chore: adds default values to streaming pp

* chore: renames `original_` => `exposed_`

* chore: uses internal setters for default vals

* chore: sets flags manually in recurring payments

* chore: adds validation for stream time config

* chore: fmt

* chore: turn state vars private

* chore: getter functions in staking and kpi module

* chore: adapt state vars to coding standards

* chore: adds internal validator fn for staking module

* chore: make fmt

* chore: shift flags by one

* chore: fmt

* Feat: Add Wrapper for Mint and Transfer Functions

* Make the issue tokens functionality abstract

* Adapt all inherited contracts

* Revert Mock override

Shouldnt have done that in the first place

* Adapt Tests

* Rename issueTokens to distributeIssuanceToken

* Make distribute collateral functionality abstract

* Rename parameter

* Make naming uniform

* Adapt all inherited contracts

* adapt spelling mistake

* Adapt tests

* Cleanup

* Fix test

* Address Comments: Natspec

* Cleanup

* Rename _distributeIssuanceToken to _handleIssuanceTokenAfterBuy

* Rename _distributeCollateralToken to _handleCollateralTokenAfterSell

* Update FM_BC_Bancor_Redeeming_VirtualSupply_v1.sol

* Update BondingCurveBase_v1.sol

* Update Sections

* Rename Function

* Adapt sections in mocks

* rename _handleIssuanceTokenAfterBuy

* Update comments

* Rename to _collateralTokenAmount

* Update src/modules/fundingManager/bondingCurve/abstracts/BondingCurveBase_v1.sol

Co-authored-by: Marvin Kruse <marvinkruse@users.noreply.github.com>

* Rename _issuanceMintAmount

* Update src/modules/fundingManager/bondingCurve/abstracts/RedeemingBondingCurveBase_v1.sol

Co-authored-by: Marvin Kruse <marvinkruse@users.noreply.github.com>

* Adapt version

---------

Co-authored-by: Marvin <43185740+Zitzak@users.noreply.github.com>
Co-authored-by: Marvin Kruse <marvinkruse@users.noreply.github.com>

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

* Abstract projectFeeCollected

* Create tests

* Adapt emit test

* convert flags to bytes32

* setter for default times in Streaming_PP + tests

* test bugfix

* refactor flags into PaymentClientBase

* Adaptations to repo CoC

* more CoC update

* even more CoC

* Update tests

* Explainer text

* refactor IERC20PaymentClientBase errors + bugfix

* abstract functions out + update tests

* address review comments

* add init tests to inheritng modules

* typo fix

* Apply comment suggestions from code review

Co-authored-by: Marvin Kruse <marvinkruse@users.noreply.github.com>

* adapt access mock function naming

* Adapt naming of internal variable

* Adapt comments

* Adapt function naming

* Fmt

* Copy over lib subprojects from dev

* Update src/modules/logicModule/LM_PC_RecurringPayments_v1.sol

Co-authored-by: Marvin Kruse <marvinkruse@users.noreply.github.com>

---------

Co-authored-by: Felix Hieser <47272854+FHieser@users.noreply.github.com>
Co-authored-by: Marvin <43185740+Zitzak@users.noreply.github.com>
Co-authored-by: Marvin Kruse <marvinkruse@users.noreply.github.com>
Co-authored-by: 0xNuggan <82726722+0xNuggan@users.noreply.github.com>
Co-authored-by: FHieser <felix_hieser@web.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants