Remove SablierFees from this repo

[smol-ninja]

I would suggest removing SablierFees from this repo. Adminable, Batch, and NoDelegateCall are quite standard and can be expected to be used by external projects without any modification.

On the other hand, collectFees is a very Sablier-specific function and can have a different implementation. For example, it has an entirely different implementation in the Airdrops repo. Similarly, even though it has the same implementation in Lockup and Flow, we cannot be 100% certain that it will remain like that forever, whereas we can say the same about rest of the utility contracts.

Therefore, I propose removing SablierFees from this repo and keeping the local implementation in each repo.

cc @sablier-labs/solidity

[andreivladbrg]

i don't agree with this

are quite standard and can be expected to be used by external projects without any modification.

this repo is not meant for this purpose: being used by external projects, but for moving the common logic between our protocols
i would focus on being used by external projects.

For example, it has an entirely different implementation in the Airdrops repo. Similarly, even though it has the same implementation in Lockup and Flow,

but our current state of agreement is to have the same system of fees (collectFees) for all protocols, right?

we cannot be 100% certain that it will remain like that forever, whereas we can say the same about rest of the utility contracts

ofc, we can’t be 100% sure, but that’s a problem we should address when we face it. for now (given all 3 will use it), it makes sense to keep it here

Therefore, I propose removing SablierFees from this repo and keeping the local implementation in each repo.

it seems you changed your mind about your view here 😁

[smol-ninja]

but our current state of agreement is to have the same system of fees (collectFees) for all protocols, right?

Airdrops has a different implementation for collectFees. So, its not entirely common among "all" but only common between Lockup and Flow.

this repo is not meant for this purpose

OK fair enough. But given that the repo contains very generic function, why not? People can use Sablier Utilities to inherit Adminable, Batch and NoDelegate function.

it seems you changed your mind about your view sablier-labs/lockup#1125 (comment)

Yes I have because airdrops dont have the same implementation. So that changed my mind about collectFees.

[andreivladbrg]

Airdrops has a different implementation for collectFees

oh, you are right

But given that the repo contains very generic function, why not? People can use Sablier Utilities to inherit Adminable, Batch and NoDelegate function.

ofc, it can be used by others. what i’m saying is not just to build the contracts and include only the ones that are “generic”
but rather to prioritize making changes that would favor us.

to make the contract more general-purpose, we could name it FeesCollector

[smol-ninja]

I have no argument against your second point. I understood it.

oh, you are right

But what about this? Since collectFees has different implementation in Airdrops, should we remove it from Utilities contract for maximum differentiation instead of inheriting it in Flow and Lockup but not in Airdrops?

I am still in favour of removing it.

[PaulRBerg]

I am in favor of removing SablierFees from this repo because of YAGNI.

We will 100% change our fee structure as a result of user feedback over the coming months, and the resulting fee structure might not be the same between contracts. Thus, sharing the fee implementation at this stage is overkill.

[andreivladbrg]

ok, so it’s a majority vote for removing the fees. will remove them.

[andreivladbrg]

818b26e