guhu95 - Loss of rewards for all users due to mismatching reward multipliers for re-lock-ups

[sherlock-admin4]

guhu95

high

Loss of rewards for all users due to mismatching reward multipliers for re-lock-ups

Summary

After the first lock period expires, users who were initially locked for a long duration continue to earn rewards at the highest multiplier while being effectively locked only for the default (short) duration, causing a mismatch between lock-up duration and reward multiplier.

This excessively high multipler causes other users to earn a much lower share of rewards due to their much lower (but correct) multiplier. This results in loss of reward for the users, disincentivises them from participating, and overpays rewards at high rates for short lock-up durations.

Vulnerability Detail

The remaining unlock period is calculated in calcRemainUnlockPeriod: once the original lock period has passed, the function starts using the default relock time to calculate the remaining unlock period:

        } else {
            // If the current time exceeds the adjusted lock period, return the remaining time based on the default relock time.
            return defaultRelockTime - (block.timestamp - lockTime) % defaultRelockTime;
        }

However, while the user is now locked for a much shorter duration, their rewards multiplier is not updated, and they continue earning new rewards at much higher share than other users that are locked for similar durations.

Consider this example, using simple numbers for clarity:

• The default duration and the shortest duration are both 1 day. The longest lock duration is 10 days.
• The multiplier for the 1 day lock-up is 1x and for 10 days is 10x.

1. User A completes one full duration of 10 days. Beyond that point, they are implicitly locked continuously for 1 day (the default duration) while still earning rewards at the 10x multiplier, until they call exitLateById (which removes their balance).
2. User B who starts locking at the default 1 day duration is locked in the same way - can only withdraw tokens after 1 day, but earns rewards at a much lower 1x multiplier - one tenth of User A's multiplier.

If A and B are the only users, with same balances, from that point onward A will earn 90.9% of rewards, and B only 9%, despite having the exact same lock-up and balance.

While the duration calculation is intended design choice, and matches the documentation and README, the lack of adjustment of the reward multipliers to the duration logic is not documented and results in losses for new users.

Impact

This results in certain loss of rewards for all other users due to lower share of rewards relative to their lock-up.

In addition to the direct loss of rewards, it can significantly disincentivize new users from participating in the lock-staking contract. This is because the contract overpays the users with expired long locks, without receiving the benefit of a long lock-up period in return.

Additionally, since the contract's main purpose is staking at different reward rates for different lock-up durations, this breaks core functionality, due to the mismatch between durations and reward multipliers.

Code Snippet

https://github.com/sherlock-audit/2024-05-gamma-staking/blob/main/StakingV2/src/Lock.sol#L604-L605

Tool used

Manual Review

Recommendation

-        if (lockPeriod <= defaultRelockTime || (block.timestamp - lockTime) < lockPeriod) {
            return lockPeriod - (block.timestamp - lockTime) % lockPeriod;
-        } else {
-            return defaultRelockTime - (block.timestamp - lockTime) % defaultRelockTime;
-        }

Avoid implicitly updating to the default lock period in the calculations. Instead, assume the user continues relocking at their original lock period and multiplier. This maintains fairness for all users based on their original lock choices and matches user expectations since if the user has chosen a long lock period, they similarly have a long period in which to call exitLateById before the auto-relock rollover without any negative consequences.

[bjp333]

For this recommendation, we didn't mention in our design, but it is intended to keep those users at the higher multiplier even when they auto-relock at the shorter defaultRelockTime.

It still fulfills its function in that the user is incentivized to stay locked to maintain his higher multiplier. Yes, he does have the option to leave within the shorter timeframe while still earning at a higher multiplier, but he is still incentivized to stay locked because if he exits, he will lose his multiplier for good and will have to wait a whole year to obtain that status again.

What we don't want to happen is that a user who staked for a year gets relocked for another year or our investors/team who are locked for multiple years to get relocked automatically at multiple years.

[bjp333]

Please see here for how it would look in practice.

[santipu03]

Considering that this "intended design" wasn't mentioned during the time of the audit, I believe this issue may remain open because it broke a core principle of the Lock contract, which is that users should have the right multipliers according to their lock periods.

[samuraii77]

Escalate

This issue should not be valid for a few reasons.

Firstly, this is clearly a design decision as stated by the protocol. The sponsors have clearly stated how the automatic relock is supposed to function, both in the Discord chat as well as in the code comments. A user locks his funds with a certain lockup period and multiplier and after the lockup period, his funds get automatically locked for the lower time period. It functions exactly the way they explained it, thus this is a design decision. The issue reported is a long opinion of how they believe the protocol should function but that is nothing more than an informational finding.

Furthermore, the only way for someone to get the benefits mentioned in the report, is for the user to actually go through the long lock-up period. Thus, the reason the watson believes the issue is valid: In addition to the direct loss of rewards, it can significantly disincentivize new users from participating in the lock-staking contract., is not actually valid because if users were disincentivized to stake, then this issue would not even exist as no one would stake and the issue reported would not arise.

Overall, this is clearly a design decision and it is clear that the longer lock-up periods have both advantages and disadvantages. You can't withdraw your funds for a longer time but you also get benefits for that. The report above just explained something everyone already knows and is clearly a design decision.

[sherlock-admin3]

Escalate

This issue should not be valid for a few reasons.

Firstly, this is clearly a design decision as stated by the protocol. The sponsors have clearly stated how the automatic relock is supposed to function, both in the Discord chat as well as in the code comments. A user locks his funds with a certain lockup period and multiplier and after the lockup period, his funds get automatically locked for the lower time period. It functions exactly the way they explained it, thus this is a design decision. The issue reported is a long opinion of how they believe the protocol should function but that is nothing more than an informational finding.

Furthermore, the only way for someone to get the benefits mentioned in the report, is for the user to actually go through the long lock-up period. Thus, the reason the watson believes the issue is valid: In addition to the direct loss of rewards, it can significantly disincentivize new users from participating in the lock-staking contract., is not actually valid because if users were disincentivized to stake, then this issue would not even exist as no one would stake and the issue reported would not arise.

Overall, this is clearly a design decision and it is clear that the longer lock-up periods have both advantages and disadvantages. You can't withdraw your funds for a longer time but you also get benefits for that. The report above just explained something everyone already knows and is clearly a design decision.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[pkqs90]

Fully agree with @samuraii77. Also adding my thoughts here:

The lock and relock mechanisms are designed to incentivize users to commit to longer lock periods. The longer the initial lock, the higher the multiplier. Whether the multiplier should remain the same during the relock period is definitely a design decision and does not cause any loss to users.

Using the sponsor's example, say there are two lock options, and the relock period is 30 days:

1. Lock for 30 days with a 1x multiplier.
2. Lock for 360 days with a 2x multiplier.

If a user chooses option 2 and locks for 360 days, they will have a 2x multiplier for those 360 days. For subsequent relocks, whether they should retain the 2x multiplier is debatable. Some might argue it's unfair to users who chose option 1, while others could argue that maintaining the 2x multiplier further incentivizes user to longer lock periods initially.

The protocol has chosen option 2 and decided to keep the original multiplier to encourage longer locks. Not choosing the other option is not a valid issue.

Also, according to Sherlock's judging rules, this should not be considered a valid issue, as the design decision does not imply any loss of funds. Rewarding longer staking periods is definitely not a loss of funds.

Design decisions are not valid issues. Even if the design is suboptimal, but doesn't imply any loss of funds, these issues are considered informational.

[IWildSniperI]

i don't know where is this mentioned as intended design during the audit? i really think this was a flow in relocks

[adamidarrha]

@IWildSniperI it wasn't mentioned as intended design anywhere. the first comment of the sponsor confirms that: we didn't mention in our design, but it is intended to keep those users at the higher multiplier even when they auto-relock at the shorter defaultRelockTime.

1. This issue is not about arguing a design decision but about breaking a fundamental invariant for staking protocols:

   • The core principle of staking protocols is that the longer you lock your tokens, the bigger your lock multiplier. The current relocking mechanism breaks this invariant in a silent way because it is handled automatically by modulo logic. This allows a user with relocks to keep their high lock multiplier with a shorter duration, which undermines the intended reward structure and creates an imbalance in the protocol.

2. Regarding the argument that this is a design decision mentioned in the documentation, it is important to note that the lock multiplier is never discussed in the docs , readme nor in the comments. only the relock durations are discussed , as stated by the sponsor in the first comment in this issue:

For this recommendation, we didn't mention in our design ...

3. this issue is not about a loss of funds, but about breaking of protocol functionality , failing to have lock multipliers in accordance with the lock duration. see issue #192 for more explanation.

[pkqs90]

@IWildSniperI it wasn't mentioned as intended design anywhere. the first comment of the sponsor confirms that: we didn't mention in our design, but it is intended to keep those users at the higher multiplier even when they auto-relock at the shorter defaultRelockTime.

1. This issue is not about arguing a design decision but about breaking a fundamental invariant for staking protocols:

• The core principle of staking protocols is that the longer you lock your tokens, the bigger your lock multiplier. The current relocking mechanism breaks this invariant in a silent way because it is handled automatically by modulo logic. This allows a user with relocks to keep their high lock multiplier with a shorter duration, which undermines the intended reward structure and creates an imbalance in the protocol.

2. Regarding the argument that this is a design decision mentioned in the documentation, it is important to note that the lock multiplier is never discussed in the docs , readme nor in the comments. only the relock durations are discussed , as stated by the sponsor in the first comment in this issue:

For this recommendation, we didn't mention in our design ...

3. this issue is not about a loss of funds, but about breaking of protocol functionality , failing to have lock multipliers in accordance with the lock duration. see issue #192 for more explanation.

I'd like to point out that 1) nowhere in the docs or the code states "the longer you lock your tokens, the bigger your lock multiplier" is a "core principal", and 2) extending that to relocking mechanism and claiming it breaks a "core principal" does not make any sense.

[samuraii77]

@IWildSniperI it wasn't mentioned as intended design anywhere. the first comment of the sponsor confirms that: we didn't mention in our design, but it is intended to keep those users at the higher multiplier even when they auto-relock at the shorter defaultRelockTime.

1. This issue is not about arguing a design decision but about breaking a fundamental invariant for staking protocols:

• The core principle of staking protocols is that the longer you lock your tokens, the bigger your lock multiplier. The current relocking mechanism breaks this invariant in a silent way because it is handled automatically by modulo logic. This allows a user with relocks to keep their high lock multiplier with a shorter duration, which undermines the intended reward structure and creates an imbalance in the protocol.

2. Regarding the argument that this is a design decision mentioned in the documentation, it is important to note that the lock multiplier is never discussed in the docs , readme nor in the comments. only the relock durations are discussed , as stated by the sponsor in the first comment in this issue:

For this recommendation, we didn't mention in our design ...

3. this issue is not about a loss of funds, but about breaking of protocol functionality , failing to have lock multipliers in accordance with the lock duration. see issue #192 for more explanation.

That's right, it was never mentioned that a user should keep the higher multiplier, nor was it mentioned that a user should not keep the higher multiplier, thus we should base our decision based on how the protocol explained the auto relock should function. As stated in my escalation comment, it functions the exact way it is supposed to. Since you yourself said that the relock multipliers were never discussed, then your issue is clearly a report of how you believe the protocol should function, not how it actually functions. You can't base your whole argument and report based on something you believe and was never mentioned anywhere.

Also, as mentioned by @pkqs90 , these core principals and invariants you are talking about are never mentioned. As I mentioned already, you are basing your report on what you believe should be the case but not how it actually is.

[wildflowerzx]

I believe it is fair for watsons to assume this is an issue since it was not explicitly stated in the contest details and public channels that this is a design decision until after the contest. It allows stakers to purposely set up expired locks to gain rewards after notification of rewards and exit early.

[IWildSniperI]

I believe it is fair for watsons to assume this is an issue since it was not explicitly stated in the contest details and public channels that this is a design decision until after the contest. It allows stakers to purposely set up expired locks to gain rewards after notification of rewards and exit early.

Exactly

[adamidarrha]

last comment and i will leave it to the lead judge to decide.

That's right, it was never mentioned that a user should keep the higher multiplier, nor was it mentioned that a user should not keep the higher multiplier, thus we should base our decision based on how the protocol explained the auto relock should function. As stated in my escalation comment, it functions the exact way it is supposed to. Since you yourself said that the relock multipliers were never discussed, then your issue is clearly a report of how you believe the protocol should function, not how it actually functions. You can't base your whole argument and report based on something you believe and was never mentioned anywhere.

1. Invariants don't need to be explicitly stated by the developers to exist. We draw upon our experiences with similar protocols to establish invariants. It's the auditor's responsibility to identify and validate these invariants.

2. A functionality doesn't need to be written in the docs for you to find an issue with it. In this case, the lock multipliers functionality should reflect the lock duration. However, it doesn't in certain cases, which is what this issue discusses.

Unless there are new points to raise, let's avoid excessive comments and allow the lead judge to decide.

[guhu95]

Design decisions

The locked multipliers is not a new design space, it is a known pattern. If the decision to break it is deliberate, it's expected to be explicit. In contrast, The custom duration logic approach was clearly documented. The outdated multipliers - and the loss of funds that results from it was not.

Loss of funds

There is a clear loss of funds here. The rewards are finite, and their over-allocation to some stakers - due to the short-lock-up-but-highest-multiplier is at the expense of all other stakers.

High severity

@santipu03 doesn't the certain loss of funds qualify as "Definite loss of funds without (extensive) limitations of external conditions." ? The loss is certain on the first expiry of the longer locks - all subsequent rewards are under-paid to all other stakers.

[0xreadyplayer1]

i somehow agree with @samuraii77 & @pkqs90 . But the thing i want to understand by the reporter is

how it's dis-incentivization , the person who is re-locking has staked for more time than others and might need incentive for longer duration than the people who staked for less time. additionally it would be unfair ( the issue might be valid )if the other people who has re-locked too would get less rewards

if Alice staked for 30 days , with 1x multiplier , now relocked for 360 days, they indeed deserve a good incentive hence a greater multiplier than others - protocol intends to do it and i see no harm

if person stakes his capital for only smaller lock period like 30 days vs someone who staked 30+ days , surely the other person had greater risk and commitment to the protocol

if Bob and Charlie also do it like Alice did , they will get the same ( it will be a problem if they don't )

is not it ?

maybe i'm missing something ....

[santipu03]

After reading all the new arguments brought to the table, I think this comment best represents this issue. I believe this report falls into a design decision issue:

Design decisions are not valid issues. Even if the design is suboptimal, but doesn't imply any loss of funds, these issues are considered informational.

[IWildSniperI]

I agree on your last decision, but this seems opening up a bad door to watsons

They never know if its a design decision till they submit and get invalidated

[zrax-x]

From a security perspective, I think we need to follow the principle of “presumption of guilt”. Otherwise, many issues may be missed.

During the audit contest, the information we can obtain about the decision is:

Upon expiry of any lock time, the positions are automatically relocked for the duration of the lesser of the original lock time or the default lock time.

It does not mention that the multiplier will be maintained. Therefore, we should not speculate whether this is a design decision, but treat it as an issue.

[0xreadyplayer1]

agree with @santipu03

[IWildSniperI]

@zrax-x well said sir

[santipu03]

Please, avoid adding new comments unless there are new points to raise.

[guhu95]

... I believe this report falls into a design decision issue:

Design decisions are not valid issues. Even if the design is suboptimal, but doesn't imply any loss of funds, these issues are considered informational.

@santipu03 the key point is whether the rewards loss to other users is "loss of funds" or not. If it is, than the issue doesn't fall under "design decisions" (since it says "..but doesn't imply any loss of funds").

My reasoning above is that because the rewards are finite, their under-payment (relative to lock-up time) is loss of funds for all other users.

What is your reasoning for not considering it loss of funds?

[santipu03]

The term 'under-payment' implies a deviation from expected amounts, but if the protocol is designed to distribute rewards in this specific way, then it aligns with its intended function. As such, these payments are not considered under-payments but the result of the protocol’s design choices. Since there is no unintended financial discrepancy, this scenario does not constitute a 'loss of funds'.

[bjp333]

Just adding my two cents here regarding the concept of having people who are locking for longer periods keep their elevated multiplier at the defaultRelockTime. This wasnt explicitly mentioned, but equally, it should not be assumed that they should go back to earning at a lower multiplier because these stakers who already went thru a year lock period are fundamentally different from the protocol's perspective than a user who just staked recently at the shorter 30-day lock. They have already earned their multiplier by waiting the whole year are thus disincentivized more than shorter term users to unlock to avoid losing this benefit.

GMX has a bit different design but similar incentive mechanism in spirit where the longer you stake, the more multiplier you get which would prevent them from wanting to unstake. So they have a carrot vs stick approach to locking. Ours is more of a hybrid. Where you do have to lock up initially but in order to keep your benefit after a year lock, you have to remain locked otherwise you would have to lock for another year.

This is evidenced by our design decision to allow for relocking at the defaultRelockTime vs their original lock time because we felt that the sting of being locked for another year if you forgot to unlock would be more detrimental to attracting longer term stakers than the benefit of keeping their multiplier after having already gone thru a year.

[nevillehuang]

As far as I can tell, this issue highlights a possible way to arbitrarily create expired locks to bypass lock durations while retaining multipliers and claiming rewards. From my understanding rewards can be claimed at any given point of time and multiple times as long as position is not withdrawn/late exited (since locked balance with multiplier is retained).

Design decisions should be explicitly made known in detail during the contest details/discussed publicly, and since it was not, I believe this issue should remain valid. Additonally, for every notification of rewards, these expired locks could be engineered to constitute most of the rewards and can force unecessary relocks for new stakers.

Planning to reject escalation and leave issue as it is since it suggested invalidation.

[Henrychang26]

@nevillehuang,

Locks are not necessarily "expired"; they just haven't been exited yet. The protocol is designed to auto-relock unless users choose to exit. For instance, if a user's initial lock is 360 days and it auto-relocks for another 30 days, without the user unstaking/exiting late, this effectively becomes one continuous lock of 390 days. Comparing the multiplier of a 390-day lock to a new 30-day lock doesn't make sense.

The only argument here is that both locks are able to exit after the same 30-day period while having different multipliers. However, the staker initially committed to 360 days without exiting.

Based on the issue highlighted in this report: Assume the multiplier for a 360-day lock is 5 and for a 30-day lock is 1.

A 390-day lock, without exiting, should have a multiplier of 1. A new 360-day lock should have a multiplier of 5. This contradicts the protocol's intention.

[nevillehuang]

@Henrychang26 I get your point but I need more clarification from sponsor. @bjp333 Could you maybe elaborate more on why gamma believe this is a valid design decision instead of adopting something similar to GMX?

As far as I can tell, just by engineering this expired locks, the user retains the locked balance forever, and since cumulatedRewards never decreases, this essentially means every reward notification shared across all stakers will lean towards this staked positions with the opportunity to always exit early. Wouldn't this hybrid design decision essentially discourage new stakers from locking (be it short or long duration) and encourage all users to just stake towards expired locks (with 30 day automatic relock and allowing essentially early exits with higher multipliers)

[guhu95]

I understand the intention (retroactively communicated) for the design, but it still contradicts the invariant of lock-up time vs. multiplier. The multipliers must be higher for higher opportunity costs, otherwise there's no "lock-up" - which is the whole point of the contract.

Here's an example: imagine a third-party vault contract that breaks deposit into locks of max durations (e.g., 10 GAMMA denomination locks). At some point, after that wrapper accumulated enough "expired" locks, new stakers using that vault will be able to deposit and withdraw as they please with short lock ups, while receiving rewards at max multiplier. At that point, there is no reason to stake for long duration, and actual staking is only short term (via the wrapper), but effective multiplier is at max. This is not a new attack vector, but an example to show how core functionality is broken if the invariant is broken. The "vault" here is getting the "loyalty" bonus, but this just opens up a way for everyone to get the same "bonus".

It's OK for the project at accept that risk, but it needs to be known during the contest to count as "acceptable / known risk".

[Henrychang26]

@guhu95 No such invariant was stated on contest page

I understand the intention (retroactively communicated) for the design, but it still contradicts the invariant of lock-up time vs. multiplier. The multipliers must be higher for higher opportunity costs, otherwise there's no "lock-up" - which is the whole point of the contract.

@nevillehuang The trade-off here is that a newly created 360-day lock will have the same multiplier as a lock position that has been active for 390 days(or potentially longer).

As far as I can tell, just by engineering this expired locks, the user retains the locked balance forever, and since cumulatedRewards never decreases, this essentially means every reward notification shared across all stakers will lean towards this staked positions with the opportunity to always exit early. Wouldn't this hybrid design decision essentially discourage new stakers and encourage all users to just stake towards expired locks (with 30 day automatic relock and allowing essentially early exits with higher multipliers)

[pkqs90]

@Henrychang26 I get your point but I need more clarification from sponsor. @bjp333 Could you maybe elaborate more on why gamma believe this is a valid design decision instead of adopting something similar to GMX?

As far as I can tell, just by engineering this expired locks, the user retains the locked balance forever, and since cumulatedRewards never decreases, this essentially means every reward notification shared across all stakers will lean towards this staked positions with the opportunity to always exit early. Wouldn't this hybrid design decision essentially discourage new stakers from locking (be it short or long duration) and encourage all users to just stake towards expired locks (with 30 day automatic relock and allowing essentially early exits with higher multipliers)

Hey @nevillehuang, I think there may be some misunderstanding to how the relock period works. In this https://github.com/sherlock-audit/2024-05-gamma-staking-judging/issues/10#issuecomment-2137341919 I provided an example of how the relocks and multiplier works.

""" Using the sponsor's example, say there are two lock options, and the relock period is 30 days:

1. Lock for 30 days with a 1x multiplier.
2. Lock for 360 days with a 2x multiplier.

If a user chooses option 2 and locks for 360 days, they will have a 2x multiplier for those 360 days. For subsequent relocks, whether they should retain the 2x multiplier is debatable.

1. Retain the 2x multiplier: This would further incentivize users to longer lock periods initially.
2. Not retain the 2x multiplier: This would not incentivize longer lock periods as much, but would bring more rewards to shorter lock periods.

The protocol has chosen the first option and decided to keep the original multiplier to encourage longer locks. Not choosing the other option is not a valid issue. """

What I'm trying to say is, the protocol chose this design because they want to incentivize the longer locks more. I don't think this means it would be a loss of funds to the shorter locks. The shorter lock users can simply lock a longer period if they think its worth it.

Going back to Sherlock rules, in this issue, the protocol decided to incentivize usergroup A more than usergroup B, and it doens't make sense to claim it is a loss of funds for usergroup B. To my understanding, loss of funds should be something that users can directly steal, and not something the protocol had the intention to give.

Design decisions are not valid issues. Even if the design is suboptimal, but doesn't imply any loss of funds, these issues are considered informational.

[guhu95]

@Henrychang26 if you expect all invariants in the README you're expecting a full formal specification with hundreds of invariants. Only the non-obvious ones can be listed. This invariant is both common and obvious.

[nevillehuang]

Lets say a large number of stakers engineer such positions, there are finite amount of rewards to notify, wouldn’t that discourage new short/longer term staking since all rewards (that are periodically sent) would be wiped.

My understanding is expired locked positions essentially seem to mean user staking have the ability to unlock as early as one second after getting notified rewards considering how the relock time is computed for expired locks (1 locked position get same rewards with multipliers multiple times, given the fact that reward rates always increase). I don’t believe this is fair for new long term stakers.

[Henrychang26]

How is this unfair to new long-term stakers?

Consider the following scenario:

Alice initially locks her stake for 360 days and then re-locks for another 30 days. She receives all the rewards during this period if she is the only staker. On day 390, Bob creates a new lock position for 360 days with the same amount of staked tokens. Bob's multiplier is the same as Alice's. From this point forward, Alice and Bob will each receive 50% of the rewards notified.

Alice's position has been locked for 390 days, while Bob's will be locked for 360 days. Doesn't this setup encourage more new long-term stakers, since they can achieve the same multiplier with a shorter commitment?

[0xRajkumar]

Agree with you @Henrychang26,

The whole point of this is essentially to reward users who stake for a longer period. This is the model on which the staking contract is built, and that's why the project has considered this as invalid.

[IWildSniperI]

Relocks means what? Re: means initiating the action again In such a simple language meaning i see that explicitly stating during audit that this was intended design has to be made.

For example: in the readme protocol stated that people exitLate don't ear reward from the moment they exit If this wasn't stated, then as auditors we would have supposed that there is a flow in exitLate logic(as stakers stay staked to the end and yet receive no reward)

This is me just supporting what nevi said as a judging simple rule

[samuraii77]

I agree with @Henrychang26.

Also, even if it was somehow unfair to new stakers, this still remains a design decision and quoting Sherlock rules here:

Design decisions are not valid issues. Even if the design is suboptimal, but doesn't imply any loss of funds, these issues are considered informational.

Also, the protocol team clearly mentioned how the automatic relock is supposed to function and in fact, that is exactly how the automatic relock functions. I don't understand why there is even a debate going on right now as the protocol functions exactly as explained by the sponsor in this regard, a user locks his funds and upon relock, the lock period he gets is the lower of the default lock period and the one he chose. It was never mentioned that his multiplier should change as well. It is extremely logical that if the sponsors didn't mention the multiplier should change, then it shouldn't change.

The whole argument here for the watsons supporting the validity of the issue is that since the sponsors didn't mention the multiplier shouldn't change, then it should change however that is completely irrational. Let's imagine a scenario to show how exactly irrational this argument is:

I am standing on my desk and a person brings me food. I ask him why he brought it since I never told him to and his argument is that he brought it because I didn't tell him not to.

If that sounds like a rational argument, then I believe this issue deserves a medium severity tag. However, for me, it is clear that this issue nothing more than an informational one.

[zrax-x]

Because this design is unfair, and we were not aware of this being a design decision during the audit.

[nevillehuang]

@guhu95 @zrax-x @IWildSniperI Can you guys provide a numerical example/PoC of why this is unfair to help me visualize the issue and its impact?

I am now leaning toward invalidating the issue because the arguments by sponsors and watsons of retaining multipliers make sense to me if you see the staked positions as for example,

• 365 days - 3x multiplier
• 395 days - 3x multiplier

It seems fair for users who have staked a total of 395 days (365 days + 30 day relock) to retain the multiplier.

[adamidarrha]

@nevillehuang look at the poc in issue #192 .

[IWildSniperI]

My concern of judging this in the contest as valid was 'breake core functionality' of accounting. But if this is the intended design of incentiving long term people, i really wished a hint in the readme or discord channel

For more details of impact will leave it to watsons tagged

[guhu95]

@nevillehuang

This is the numerical example from the issue:

Consider this example, using simple numbers for clarity:

• The default duration and the shortest duration are both 1 day. The longest lock duration is 10 days.
• The multiplier for the 1 day lock-up is 1x and for 10 days is 10x.

1. User A completes one full duration of 10 days. Beyond that point, they are implicitly locked continuously for 1 day (the default duration) while still earning rewards at the 10x multiplier, until they call exitLateById (which removes their balance).
2. User B who starts locking at the default 1 day duration is locked in the same way - can only withdraw tokens after 1 day, but earns rewards at a much lower 1x multiplier - one tenth of User A's multiplier.

If A and B are the only users, with same balances, from that point onward A will earn 90.9% of rewards, and B only 9%, despite having the exact same lock-up and balance.

Consider also this example from the above comments explaining why the "retroactive loyalty bonus" doesn't make sense because accounts aren't users:

Here's an example: imagine a third-party vault contract that breaks deposit into locks of max durations (e.g., 10 GAMMA denomination locks). At some point, after that wrapper accumulated enough "expired" locks, new stakers using that vault will be able to deposit and withdraw as they please with short lock ups, while receiving rewards at max multiplier. At that point, there is no reason to stake for long duration, and actual staking is only short term (via the wrapper), but effective multiplier is at max. This is not a new attack vector, but an example to show how core functionality is broken if the invariant is broken. The "vault" here is getting the "loyalty" bonus, but this just opens up a way for everyone to get the same "bonus".

If there is a "retroactive" loyalty bonus, because tokens are fungible, and addresses are not people - the max bonus becomes available for everyone, showing a specific way how the whole point of having multipliers becomes broken.

Finally, separately from the fairness is the question of not making it a known decision. Without making it known the breakage of the core functionality (lock-ups matching multipliers) - it appears as a vulnerability, so should be judged according to the information available during the contest and not according to retroactive explanations.

[nevillehuang]

@guhu95 @zrax-x @adamidarrha

I don't see how the multipliers are broken, and there seems to me that there is no loss of funds here since for all users not unstaked, the protocol would be responsible for transferring and notifying rewards for users to collect before their rewards mapping are set to zero, if not it will revert.

I also don't see a good reasoning as to why having a 365 day and 366+ day locked position retaining the same multiplier is unfair (same applies to all multipliers). Both positions are still technically staked and are only withdrawn from the 365th day onwards. I believe this issue is invalid so planning to accept escalation and leave issue as it is.

[zrax-x]

@nevillehuang I believe there is a cognitive bias here for users. First of all, this design is secretive, and when users don’t know how the system works(you can't expect every user to be familiar with the code), they will take the initiative to restake to obtain a higher multiplier. However, the fact is that for these active users, they will lose flexibility compared to lazy users. For example, both user A and user B staked for 360 days. After 360 days, the active user A chose to stake for another 360 days to get a higher multiplier. As for user B, he did nothing, but enjoyed the same multiplier and could unlock it faster.

[nevillehuang]

@zrax-x Not quite sure what you meant there. User B did not unlock faster. He unlocked after the first full 365 + more is staked i.e. only after 365th day, meaning he would have staked for over 365 days

[zrax-x]

@nevillehuang I mean that after 365 days, User B's lock time becomes 30 days. However, for user A who actively re-stake for 365 days, his/her lock time is still 365 days (They do not know the inner workings of the system, so always follow the time-multiplier rule.). The final result is that user A is harmed by this secretive design.

[adamidarrha]

@nevillehuang he will have a stake time of 30 days after the 365 days passed, but he keeps his multiplier, somebody who restakes after 365 days passed, will need to restake again for 365 to keep the same multiplier.

[nevillehuang]

I consider the scenario mentioned here simply users not understanding the protocol's relock mechanism and would amount to a documentation improvement.

The final decision will be to accept the escalation and invalidate the issue, since this is a design decision with no implied loss of funds.

[WangSecurity]

Result: Invalid Has duplicates

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• samuraii77: accepted