Undelegatebw resets the undelegation timer each time it has been reset

[Fletch153]

If the undelegatebw action is called when funds are currently unstaking, then the unstaking period for both the new action and the original action are set to 72 hours.

Expected behaviour would be that the first action's unstaking period should not be reset by re-calling undelegatebw.

Whilst keeping track of each undelegate-period would use up more resources, it would be useful for dApps and users who need to undelegate on a regular basis. A solution can be implemented where those who want to keep track of the undelegation-period can choose to spend extra resources doing this.

[j30231]

I propose the following new unstaking methods as an option.

In order to prevent the increase of ram resources, unstaking is performed only to reflect the behavior at the time of final undelegation. This means that from the time of final unstaking, all unstaking is carried out over 72 hours.

I would like to make this the default method, and I would like to propose a different unstaking method through options.

The unstaking method through the option is as follows. If additional unstaking is performed after the initial unstaking, the existing unstaking is maintained and additional unstaking is performed. In this case, additional unstaking ram resources will also be consumed.

However, the part about resource consumption is not a big problem because only the owner with the right to request additional unstaking. If onwer don't want to, onwer don't need to request it, and onwer don't need to unstak it with the traditional default method, because ram resources are not consumed.

[arhag]

@Fletch153, @j30231:

I would rather avoid the cognitive complexity of multiple alternative methods of unstaking. But I do agree that it is a frustrating problem to have the timer reset on each undelegatebw action.

In my opinion, ideally there would be one method that gives you a guarantee of accessing your unstaked funds with a reasonable bound on both the time it takes from unstaking to liquid access and on the RAM usage required regardless of the frequency or pattern of undelegatebw calls.

So here is my alternative proposal and I'd like to hear what you think about it.

The high-level summary points of the implications of this alternative proposal are:

• A stakeholder continues to have the guarantee that if they unstake some amount of tokens via an undelegatebw they will be able to retrieve those tokens (perhaps with the manual refund action) exactly three days after the undelegatebw execution as long as they do not do any further undelegatebw actions.
• A stakeholder would now have the additional guarantee that they could retrieve the tokens they unstake (via a manual refund action if necessary) within 3 to 4 days after the undelegatebw execution. This is guarantee is provided regardless of both how many other undelegatebw actions they execute afterwards and when they execute them.
• The amount of RAM usage per user needed to provide these guarantees is fixed and in fact no greater than 64 additional bytes compared to the RAM usage of the current mechanism. However, it is possible to be smarter about how the tables are organized to actually use less RAM with the new method than our current implementation.

If you are interested in the details of how it might be implemented continue reading.

Rough design of the alternative unstaking mechanism

A table record for each staked account would need to reserve 104 bytes to hold int64_t liquid_balance_amount and vector<stake_withdrawal> withdrawal_queue, where withdrawal_queue always consists of four elements and a stake_withdrawal struct is defined as:

struct stake_withdrawal {
   int64_t net_amount = 0;
   int64_t cpu_amount = 0;
   eosio::block_timestamp boundary_time;
   uint32_t ms_since_boundary_time_of_last_update = 0;

   bool empty()const {
      return net_amount == 0 && cpu_amount == 0 && boundary_time.slot == 0 && ms_since_boundary_time_of_last_update == 0;
   }

   eosio::time_point last_update()const { 
      return  boundary_time.to_time_point() + eosio::milliseconds( ms_since_boundary_time_of_last_update );
   }

   void add_amounts( int64_t net, int64_t cpu ) {
      eosio::block_timestamp now{ current_time_point() };
      ms_since_boundary_time_of_last_update = (now.slot - boundary_time.slot) * 500;
      net_amount += net;
      cpu_amount += cpu;
   }
};

A default constructed stake_withdrawal element in the withdrawal_queue vector is considered to be an "empty" slot in the withdrawal queue.

Any time an event occurs that either needs to modify the withdrawal_queue or liquid_balance_amount (such as a delegatebw or undelegatebw action or a refund action which would pull as much of the liquid balance of the user that is available out of the eosio.stake account via an eosio.token::transfer inline action and send it to their eosio.token balance), the withdrawal queue is first processed. This means that the values in liquid_balance_amount and withdrawal_queue are lazy; they only update when they may possibly need to be up-to-date.

Processing the withdrawal queue consists of pulling out as many non-empty stake_withdrawal elements from the front of the queue which have a last_update() at least 3 days older than the current time (and sliding the slots of the queue to the front as each element is pulled), and adding the value of net_amount + cpu_amount for each pulled stake_withdrawal element to liquid_balance_amount. Sliding the slots of the queue consists of moving slot 1 to slot 0, slot 2 to slot 1, etc. and then replacing the last slot with a default constructed stake_withdrawal (i.e. an empty slot).

Adding a new unstaked event to the queue (which would be done on a undelegatebw action that unstakes net amount of the tokens from network bandwidth and cpu amount of the tokens from CPU bandwidth) involves the following procedure:

1. If the withdrawal queue only consists of empty slots, first set withdrawal_queue.front().boundary_time to current_time_point() and then do withdrawal_queue.front().add_amounts(net, cpu).
2. Otherwise, if the first empty slot in the withdrawal queue is slot i which is not the first slot (i.e. i > 0): i. let auto threshold = withdrawal_queue[i-1].boundary_time.to_time_point() + eosio::days(1); and ii. if current_time_point() < threshold, do withdrawal_queue[i-1].add_amounts(net, cpu), iii. otherwise, first set withdrawal_queue[i].boundary_time to threshold and then do withdrawal_queue[i].add_amounts(net, cpu).
3. Otherwise, if the withdrawal queue only consists of non-empty slots, simply do withdrawal_queue.back().add_amounts(net, cpu).

When sourcing new funds for staking (which would be done on a delegatebw action), if one is delegating to someone else, the funds would first come from liquid_balance_amount before pulling the remainder (if any) from the user's eosio.token balance.

It is also possible (without breaking the intentions behind the delays to avoid double spending bandwidth resources) to track the amounts being withdrawn at a per-delegation level so that delegating funds back to an account could more efficiently come from the withdrawal queue before resorting to liquid_balance_amount or the eosio.token balance. That would come at a cost of more RAM usage and higher complexity, so I will avoid getting into that in this comment.

Also note that if it was not desirable to maintain the current behavior of sourcing the funds (while keeping network and CPU assets separate) for a delegatebw to oneself from the withdrawal queue first, then it could be possible to replace the net_amount and cpu_amount fields of stake_withdrawal with just a single int64_t amount field. This would save 32 bytes of memory per account.

[gleehokie]

@Fletch153 we are looking for some feedback on this issue before we implement it

[Fletch153]

@gleehookie

It would be useful to keep it how it is right now, but have the option to keep track of each additional undelegation and it's time.

This would allow me to both undelegate as normal minimising ram costs, or pay for multiple undelegations at the cost of more ram

[deckb]

Moving to the new eosio.system repo.

This issue needs to be addressed as part of the deprecation of deferred transactions.