Sablier payroll

[andreivladbrg]

Later edit: This discussion is outdated, it still has relevant concepts but new updates have been added. Will open a new one as soon as I have new version completed.

---

New features

• Create streams for an indefinite period of time
• Top ups, which are public (you can ask a friend to deposit money for you instead)
• Withdraw funds from the stream as a sender, if available
• No deposit is required at creation time, basically the sender collects "debt"
• No deposit amount limits

What is different

I will start off by showing the PayrollStream struct.

Data types

https://github.com/sablierhq/v2-payroll/blob/93378d4d444be88d154ae42b908895d5551352c3/src/libraries/DataTypes.sol#L12-L18

As you can see, this struct is pretty different from the linear one. The stream is continuous without having a stop time; the sender will just accumulate debt if he will not cancel it.

• The amountPerSecond represents the amount of tokens (given that we are using PRBMath, we will normalize it to 18 decimals) that the recipient receives every second. This be calculated at the UI level and will be passed in the create function.
• The balance represents the available tokens in stream.
• The lastUpdate represents the time reference for the streamed amount calculation.

The stream creation and the deposit are two different operations in the payroll contract.

Create function

https://github.com/sablierhq/v2-payroll/blob/93378d4d444be88d154ae42b908895d5551352c3/src/interfaces/ISablierV2Payroll.sol#L113-L132

No deposit and time arguments.

Deposit function

https://github.com/sablierhq/v2-payroll/blob/93378d4d444be88d154ae42b908895d5551352c3/src/interfaces/ISablierV2Payroll.sol#L157-L168

You can't deposit money into the contract if there is no stream created beforehand.

To make this easy for the front-end clients, we will implement such methods as createAndDeposit and depositAll.

Withdraw sender function

Kinda a downstream functionality, but the amount per second is not affected.

https://github.com/sablierhq/v2-payroll/blob/93378d4d444be88d154ae42b908895d5551352c3/src/interfaces/ISablierV2Payroll.sol#L196-L207

How it works

When a stream is created, the balance is set to 0 and the lastUpdate to block.timestamp.

https://github.com/sablierhq/v2-payroll/blob/93378d4d444be88d154ae42b908895d5551352c3/src/SablierV2Payroll.sol#L346-L354

Every time a deposit is made, the balance is increased by the given amount.

Every time the recipient withdraws money the balance is decreased by the given amount and the lastUpdate is updated to the block.timestamp.

The withdrawable amount calculation

If no deposits were made, the function returns 0. If the calculated streamed amount is greater than the balance, the returned value is balance. In all other cases, the withdrawable amount is the calculated streamed amount.

The streamed amount calculation

https://github.com/sablierhq/v2-payroll/blob/93378d4d444be88d154ae42b908895d5551352c3/src/SablierV2Payroll.sol#L121-L137

The returnable amount calculation

The maximum amount available for the sender to withdraw.

https://github.com/sablierhq/v2-payroll/blob/93378d4d444be88d154ae42b908895d5551352c3/src/SablierV2Payroll.sol#L96-L101

Helper function for sender's debt calculation (if any)

https://github.com/sablierhq/v2-payroll/blob/93378d4d444be88d154ae42b908895d5551352c3/src/SablierV2Payroll.sol#L109-L113

Invariants

• balance >= withdrawable amount
• lastUpdate <= block.timestamp

Idea: a method that changes the amountPerSecond without deleting the stream entity.

[PaulRBerg]

Kudos for this, @andreivladbrg. The depth and clarity of this proposal is a hallmark of how far you've progressed in your journey to becoming a Solidity developer.

Use Case and Positioning

I agree that the model proposed here would better fit the payroll use case than the time-bound approach from v2-core. Talking about this reminds me of @razgraf's observation that "not all streams are created equal", which becomes more relevant by the day. Razvan captured a lot of truth with that meme.

We are confronted with a crossroads with our product positioning. We either:

1. Keep the v2-core nomenclature unaltered, and continue to refer to the time-bound version of Sablier as the canonical "Sablier V2" release; OR we
2. Refactor the v2-core nomenclature into "v2-vesting", i.e. rename "SablierV2Linear" to "SablierV2VestingLinear", etc.

Both approaches have pros and cons, however, I am becoming slightly more in favor of the second approach (for reasons I will spell out in a separate post).

This is a really important decision we have to make since, because based on it we may have to refactor a significant portion of the v2-core repo. For instance, we should rename the structs to VestingLinearStream, etc.

Spec Review

Generally speaking, I like it. Below I will respond to the bits where I think improvements can be made, or where I disagree.

Create streams for an indefinite period of time

It would be better to refer to these streams as "open-ended", or "infinite", or "streams that run in perpetuity".

"indefinite period of time" might also be interpreted as a time-bound stream whose stop time is unknown.

Top ups, which are public (you can ask a friend to deposit money for you instead)

Vitalik agrees that this would be useful!

I will start off by showing the PayrollStream struct.

I think you forgot to include the recipient in the struct definition.

uint256 amountPerSecond;

Suggested alternative names for this: ratePerSecond or flowPerSecond.

uint64 lastUpdate;

Suggested alternative names for this: lastUpdateTime or lastUpdatedAt or lastWithdrawTime or lastWithdrawnAt.

Using the word "withdraw" would clarify that this variable gets updated only at withdrawal time, and not at deposit time.

given that we are using PRBMath, we will normalize it to 18 decimals

This isn't necessary if the result has the same format as one of the factors (e.g. if the result is a token amount and one of the operands is also a token amount, both following the same number of decimals). The calculation can be made without normalizing those operands to have 18 decimals since the factors cancel out.

An analogous scenario is the quotient.mul(depositAmount) calculation performed in getWithdrawableAmount (see associated issue sablier-labs/v2-core#11 for more details).

function create(
address recipient,
address sender,

Let's swap the order of these two arguments, to keep the create function consistent with v2-core.

function withdrawSender(uint256 streamId, uint256 amount) external; 

I would rename this to claimBack or something else (TBD what) to avoid any confounding with the recipient's withdraw function.

The maximum amount available for the sender to withdraw.

Again, to avoid any potential confusion, I would rephrase this to say "get back" or "claim back" at the end.

int256 balance = int256(getBalance(streamId)); 
int256 streamedAmount = int256(getStreamedAmount(streamId));

This is an unsafe casting❗️

As explained here:

This will throw an exception (in Solidity 0.8+) for values between 2^255-1 (int max value) and 2^256-1 (uint max value)

Idea: a method that changes the amountPerSecond without deleting the stream entity.

Yes, this would definitely be helpful.

Spec Questions

1. Should these payroll streams be NFTs, too?
2. Could you explain what depositAll does?
3. Could you explain what you meant by "kinda a downstream functionality"?

Planning

In terms of planning.

Repo Organization

I am not so sure that the implementation for this should live in a separate repo. Doing this might make sense as a stopgap solution, until we sort out our product positioning and ship v2-core (regardless of how it will be called at the time of launch), but in the long haul I would be in favor of keeping the payroll contract in the same repo.

The rationale behind this suggestion is twofold:

1. Keep maintenance costs low. The more repos we have, the more difficult it becomes to keep them in sync, e.g. to keep the config files uniform across all repos (I can speak from personal experience trying to maintain a dozen or so open-source projects, this can get exhausting over time).
2. Minimize integration costs for third-party developers. It's easier to document one repo rather than N repos.

However, I agree that a bit of separation between the vesting contracts and the payroll contract would be helpful. To accomplish this, we may want to turn the v2-core repo into a monorepo (at the appropriate time). The Optimism monorepo would be a good example to follow.

Sense of Where We Are

It's been a while since we have shipped something at Sablier, so I would classify our go-to-market speed as one of our top priorities now.

I love this payroll contract proposal (and product idea!), but trying to develop and include it in our initial V2 release would set us back months. It's still an open question if the payroll functionality should be kept in the same UI, or a separate UI should be built for it. Furthermore, it's yet another open question if it would be worth it put effort into an EVM-based smart contract that offers open-ended streams the way they are defined here, or whether we should fully focus on the V3 rollup.

Therefore, I suggest we keep your proposal in the backlog and in the back of our minds, and come back to this when we launch V2.

[andreivladbrg]

Can't believe it's been almost 2 years since the initial version 🤯 time flies

Looking forward for the release 🚀🎉

cc @sablier-labs/solidity

[PaulRBerg]

Congrats! Impressive work.