Address audit findings 3, 5, and 11

[andreivladbrg]

Changes

• refactor: add blockTimestamp param in VestingMath functions
• refactor: add casting to uint256 in calculateStreamedPercentage
• docs: add assumptions in VestingMath function
• docs: improve explanatory comments

Addresses these audit findings:

• https://cantina.xyz/code/52d6aa1e-6aa4-4e73-96ed-6077652cf0f1/findings/3
• https://cantina.xyz/code/52d6aa1e-6aa4-4e73-96ed-6077652cf0f1/findings/5
• https://cantina.xyz/code/52d6aa1e-6aa4-4e73-96ed-6077652cf0f1/findings/11

[smol-ninja]

Looks good @andreivladbrg. Only one comment below:

[smol-ninja]

Nice work Andrei. Left few comments below. I also realized that we do not have anything like the following in calculateLockupTranchedStreamedAmount while we have it in LL and LD calculations.

            // Although the streamed amount should never exceed the deposited amount, this condition is checked
            // without asserting to avoid locking tokens in case of a bug. If this situation occurs, the withdrawn
            // amount is considered to be the streamed amount, and the stream is effectively frozen.
            if (streamedAmount > depositedAmount) {
                return withdrawnAmount;
            }

[smol-ninja]

This if block is redundant. We already have the following logic in this function:

        if (tranches[0].timestamp > blockTimestamp) {
            return 0;
        }

[andreivladbrg]

That’s correct—in our case, this check can be removed. However, I left it intentionally because if someone calls this function with an incorrect set of tranches but the correct start time, it would return 0.

Our example for abort functionality

[smol-ninja]

"start time" does not exist in the context of tranched streams. So, one solution could be to only pass timestamps.end as the input parameter but then I suppose, for the API consistencies, you decided to pass timestamps struct, which is good choice.

How about that, instead of reverting on invalid timestamps.start value, we add it as an assumption i.e. this function does not check for timestamps.start value?

[smol-ninja]

shw from cantina also recommended something similar: https://cantina.xyz/code/52d6aa1e-6aa4-4e73-96ed-6077652cf0f1/findings/5#comment-1583e631-54c2-45b8-aff3-4422bba95142

[andreivladbrg]

I also realized that we do not have anything like the following in calculateLockupTranchedStreamedAmount while we have it in LL and LD calculations.

Correct. The reason we have these checks in linear/dynamic is because they involve math-sensitive operations in Solidity, the use of elapsedTimePercentage (division followed by multiplication). In the case of tranche, we only perform a sum (i.e. addition), which should be safe.

[PaulRBerg]

lil feedback below