[FIP10] Add new delegation lockin #98

Andrew-Pohl · 2021-03-12T01:24:04Z

current lockin = 5days

leonprou · 2021-03-12T14:44:23Z

contracts/ConsensusUtils.sol

@@ -94,6 +95,9 @@ contract ConsensusUtils is EternalStorage, ValidatorSet {

    _delegatedAmountAdd(_staker, _validator, _amount);
    _stakeAmountAdd(_validator, _amount);
+    if (_staker != _validator) {
+      _setUnboundingPeriod(_staker);


what about the case where staker = validator
It means that the validator stakes himself, right? I think it also need an unbounding period.

Correct, I will change this actually should really lock everyone in. Sorry for the massive delay in this comment only just seen it!

sirpy · 2021-09-26T08:05:57Z

@leonprou @Andrew-Pohl
We are using fuse staking as a "defi" investment for our users. A user can stake through our contract to Fuse.
Now that means that every user that will stake will reset the bounding period for everyone (because our contract is the single delegator)
Also in general, what if I delegated 100K fuse and today i've added 10K fuse to my delegation, why is my whole 110K now bounded?
A better approach would be some sort of an average like (10k/100k * bounding period) + left bounding period
so If I already have 100K at stake and I delegate 10K more i will only be bounded for 0.5days plus any bounding period I have left if any

Andrew-Pohl · 2021-09-26T08:12:09Z

@leonprou @Andrew-Pohl
We are using fuse staking as a "defi" investment for our users. A user can stake through our contract to Fuse.
Now that means that every user that will stake will reset the bounding period for everyone (because our contract is the single delegator)
Also in general, what if I delegated 100K fuse and today i've added 10K fuse to my delegation, why is my whole 110K now bounded?
A better approach would be some sort of an average like (10k/100k * bounding period) + left bounding period
so If I already have 100K at stake and I delegate 10K more i will only be bounded for 0.5days plus any bounding period I have left if any

Hey, the logic of this has changed slightly (will push the changes today/tomorrow). The unbounding block has moved from stake to withdraw.

Now when a user request a withdraw it will move their tokens out of the staking pool (so they no longer receive interest) and then start the block countdown. Once the countdown is over they can then claim their fuse from the "unstaking pool". This frees up the problems you mentioned.

sirpy · 2021-09-26T10:18:10Z

@Andrew-Pohl
ok will wait to see the code.
i'm not sure it will solve the issue, unless you implemented some complicated logic.
for example what happens for 2 consecutive withdraw requests? ie withard 10k then 2 days later withdraw another 10k?

On first withdraw request, tokens are taken off the pool (they no longer receive interest) the subsequent withdraw request will check that unbounding has passed before release the tokens.

sirpy · 2021-09-30T20:15:50Z

contracts/ConsensusUtils.sol

  function _withdraw(address _staker, uint256 _amount, address _validator) internal {
    require(_validator != address(0));
    require(_amount > 0);
-    require(_amount <= stakeAmount(_validator));
-    require(_amount <= delegatedAmount(_staker, _validator));
+    if (unBoundingAmount(_staker) != 0)


@Andrew-Pohl @leonprou
this logic prevents multiple withdraw requests, and still breaks wrapping fuse staking for DEFI.

user A requests to withdraw from the defi contract

defi contract calls withdraw -> now unbounding amount of contract > 0

user B requests to withdraw BEFORE unbounding period is over

defi contract calls withdraw, this IF resolves to true and _unboundStake is called and reverted because unbounding block has not reached yet

User B needs to wait for unbounding period before he can request a withdraw

sirpy

I still suggest to implement an average when staking. so your staking needs to be locked for an average of X days before you can withdraw.

If I staked for a month,a year why do I deserve this unbounding period?

sirpy

w

sirpy · 2021-10-21T05:38:17Z

contracts/ConsensusUtils.sol

@@ -21,6 +21,7 @@ contract ConsensusUtils is EternalStorage, ValidatorSet {
  uint256 public constant SNAPSHOTS_PER_CYCLE = 0; // snapshot each 288 minutes [34560/10/60*5]
  uint256 public constant DEFAULT_VALIDATOR_FEE = 15e16; // 15%
  uint256 public constant UNBOUNDING_PERIOD = CYCLE_DURATION_BLOCKS;
+  uint256 public constant MAX_WITHDRAW_QUEUE_LENGTH = 500;


we have at the moment 40K daily users. if they start using this, it could easily reach that.
i dont think this should be limited. how can this be used to attack anything?

i guess you would need to add a way to limit the amount of records we iterate over when withdrawing, cause it might get stuck forever with out of gas

perhaps a withdraw helper that stops if gasLeft<100K and also returns a bool indicating if reached gaslimit

Yes you might be right here, maybe put the limit on the amount claimed in one transaction rather than the queue itself...

We will need some limit on the queue because it could be an attack vector by withdrawing 1X10-18 repeatedly causing a massive array! (but this could be bigger than 500)

well most of the times you would only attack yourself as the array is per address.

in our case, the contract itself can limit user withdrawals to min X% of their staked tokens, so a user wouldnt be able to DDOS other withdrawals.

as mentioned, in any case we require a solution that would let you withdraw in chunks or until a specific amount of gas is left, in case of a large array

• Increased withdraw queue size to 20k • Add limit on unbounding chunk size to reduce gas over spending • Add view function to track available unbounding amount

sirpy · 2021-11-02T13:18:55Z

contracts/ConsensusUtils.sol

@@ -571,4 +576,22 @@ contract ConsensusUtils is EternalStorage, ValidatorSet {
  function _setValidatorFee(address _validator, uint256 _amount) internal {
    uintStorage[keccak256(abi.encodePacked("validatorFee", _validator))] = _amount;
  }
+
+  function unboundingAmount() public view returns(uint256) {


if queue is long this will fail if used inside a smart contract.
we can keep this, but add another view helper canWithdraw() which simply returns true if first in queue can be withdrawn.
so some contract can easily call unbound() then check if still canWithdraw() and call unbound again

I still dont think it is necessary to limit the queue length, as it is per account, that account owner can only attack itself.
for example in our use case, we can simply limit users to withdraw at least 10% of their funds and at least 1 fuse, so they cant create a huge queue withdrawing small amounts

alonweinstein · 2024-07-08T11:15:02Z

contracts/ConsensusUtils.sol

+    //loop through queue and check if we have anything to withdraw
+    for (uint256 i = 0; i < oldArrayLength; i ++)
+    {
+      if (i > MAX_WITHDRAW_CHUNK_SIZE)


why test this inside the loop?

alonweinstein · 2024-07-08T11:16:02Z

contracts/ConsensusUtils.sol

+
+    //shift queue and pop back
+    for (i = 0; i < oldArrayLength - index; i++) {
+      uintArrayStorage[keccak256(abi.encodePacked("unboundingQueueBlock", _staker))][i] = uintArrayStorage[keccak256(abi.encodePacked("unboundingQueueBlock", _staker))][i + index];


have you considered the gas costs for all these keccak256 calls? is there a less gas-intensive way to handle this?

alonweinstein · 2024-07-08T11:17:46Z

contracts/ConsensusUtils.sol

+
+    //check if we have anything
+    require(toReturn != 0);
+    index = i - 1;


isn't i scoped to the for loop?

Add new delegation lockin

c2b5727

current lockin = 5days

Andrew-Pohl requested a review from leonprou March 12, 2021 01:24

fix test

64eb628

leonprou reviewed Mar 29, 2021

View reviewed changes

Change logic of unbounding period

daa1fd5

On first withdraw request, tokens are taken off the pool (they no longer receive interest) the subsequent withdraw request will check that unbounding has passed before release the tokens.

sirpy reviewed Sep 30, 2021

View reviewed changes

sirpy suggested changes Sep 30, 2021

View reviewed changes

leonprou changed the title ~~Add new delegation lockin~~ [FIP10] Add new delegation lockin Oct 14, 2021

Add withdraw queuing

3f1f91e

sirpy reviewed Oct 21, 2021

View reviewed changes

Changes to FIP10 flow

ef93e79

• Increased withdraw queue size to 20k • Add limit on unbounding chunk size to reduce gas over spending • Add view function to track available unbounding amount

sirpy reviewed Nov 2, 2021

View reviewed changes

alonweinstein reviewed Jul 8, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FIP10] Add new delegation lockin #98

[FIP10] Add new delegation lockin #98

Andrew-Pohl commented Mar 12, 2021

leonprou Mar 12, 2021

Andrew-Pohl Jul 28, 2021

sirpy commented Sep 26, 2021 •

edited by Andrew-Pohl

Loading

Andrew-Pohl commented Sep 26, 2021 •

edited

Loading

sirpy commented Sep 26, 2021

sirpy Sep 30, 2021 •

edited

Loading

sirpy left a comment

sirpy left a comment

sirpy Oct 21, 2021

sirpy Oct 21, 2021

sirpy Oct 21, 2021

Andrew-Pohl Oct 22, 2021 •

edited

Loading

sirpy Oct 24, 2021

sirpy Nov 2, 2021

alonweinstein Jul 8, 2024

alonweinstein Jul 8, 2024

alonweinstein Jul 8, 2024

[FIP10] Add new delegation lockin #98

Are you sure you want to change the base?

[FIP10] Add new delegation lockin #98

Conversation

Andrew-Pohl commented Mar 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sirpy commented Sep 26, 2021 • edited by Andrew-Pohl Loading

Andrew-Pohl commented Sep 26, 2021 • edited Loading

sirpy commented Sep 26, 2021

sirpy Sep 30, 2021 • edited Loading

Choose a reason for hiding this comment

sirpy left a comment

Choose a reason for hiding this comment

sirpy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Andrew-Pohl Oct 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sirpy commented Sep 26, 2021 •

edited by Andrew-Pohl

Loading

Andrew-Pohl commented Sep 26, 2021 •

edited

Loading

sirpy Sep 30, 2021 •

edited

Loading

Andrew-Pohl Oct 22, 2021 •

edited

Loading