sablier-labs / v2-core

⏳ Core smart contracts of the Sablier V2 token distribution protocol
https://sablier.com
Other
311 stars 47 forks source link

Override default fee in Airstreams with two mappings #1040

Closed PaulRBerg closed 2 weeks ago

PaulRBerg commented 1 month ago

Due to our plans to introduce Sablier Pro, it would be helpful to implement a mechanism to override the default fee that gets set as immutable in the MerkleLockup campaign.

Spec:

Note: the 'user' in this context refers to the campaign creator. You are welcome to find a better name.

Related:

smol-ninja commented 1 month ago

Question: why do we need two sets? Isn't feesByUser enough? I am assuming the goal here is to allow admin to set different fee for some users. That can be achieved by setting fee through feesByUser and use that to store in the campaigns created by them.

Question: will there be only one special fee? By reading "user has a special fee and if yes, use that as the campaign fee instead of the default fee", it seems like there are two fees: default and special. Does that mean custom fee cant be set for each user if required?

smol-ninja commented 1 month ago

Pending answers to above questions, I will work on the PR that introduces these changes and will merge it into https://github.com/sablier-labs/v2-core/pull/1038.

andreivladbrg commented 1 month ago

Isn't feesByUser enough? I am assuming the goal here is to allow admin to set different fee for some users. That can be achieved by setting fee through feesByUser and use that to store in the campaigns created by them.

The idea behind the second mapping is to allow for a fee of 0 (zero) in cases where the default fee is greater than zero.

We will have a sablierFee, which is the default fee. If some users (future campaign admins) are configured, we will allow them to have special fees (possibly with a discount or even no fee).

Here’s how it should look:

// If not configured, the default is zero. This means we can't tell if it was intentionally set to zero 
// or if it was never configured. That's why we need the second mapping.
mapping(address => uint256) public feesByUser; 
mapping(address => bool) public isUserSet; 
uint256 public defaultSablierFee = 0.0001e18;

function createMerkle(address admin) public {
    uint256 _sablierFee;

    if (isUserSet[admin]) {
        _sablierFee = feesByUser[admin];
    } else {
        _sablierFee = defaultSablierFee;
    }

    new SablierMerkle(admin, _sablierFee);
}

Does that mean custom fee cant be set for each user if required

I’m not sure I fully understand the question, but I’ll put it this way: the idea is to have specific clients who would benefit from the best rates.

Please lmk if everything is clear, and if you have any suggestions for a better version

PaulRBerg commented 1 month ago

why do we need two sets? Isn't feesByUser enough?

No, because the default value of a Solidity mapping is zero, so it is impossible to differentiate between a default value of zero and an actual fee we want to set to zero for a particular user.

will there be only one special fee?

No. Different users may have different special fees. Some users will be granted a zero fee, while others may pay more than the default fee.

This feature is about giving us and users more optionality.

smol-ninja commented 1 month ago

@andreivladbrg thanks for explaining. Yes its clear to me know. I didn't consider the scenario of zero fee.

razgraf commented 1 month ago

Instead of changes to the code itself, what if we simply use a separate factory where the fee is set to 0% (either hardcoded or linked to a separate Comptroller). This saves lines of code, extra code that has to be tested and audited etc.

Based on the address they're using or the token itself, we "allowlist" them from the UI to use one factory over the other.

smol-ninja commented 1 month ago

To @razgraf, are you proposing the following structure?

  1. One factory contract with default claim fee
  2. One factory contract with 0 claim fee
  3. That means no custom fee
  4. We would trust users won't bypass our fees by interacting directly with the 0 fee factory contract. And if they do so, our UI would still help their recipients to claim airstreams because the UI would not know which factory contract creator used to create campaign.

The current implementation in https://github.com/sablier-labs/v2-core/pull/1038 allows us to set a custom fee for a given user which could be 0 as well, otherwise default fee applies. This also mean, that user cannot bypass our fee system by interacting directly with the contract which in your approach would be feasible.

Comments?

PaulRBerg commented 1 month ago

Yeas as Shub pointed out, having a separate factory won't work as well as a custom fee mechanism in the same factory.

  1. Users and integrators could use the zero-fee factory to skirt around Sablier's fees
  2. We couldn't set custom non-zero fees
razgraf commented 1 month ago

Good points re. bypassing, was mostly thinking senders would be lazy and not do it. Let's go with the custom system then!