delegate bandwidth refacroting, allow to return pending refund to stake #3180

contracts/eosio.system/delegate_bandwidth.cpp

@@ -213,27 +213,32 @@ namespace eosiosystem {
       }
    }
 
-   void system_contract::delegatebw( account_name from, account_name receiver,
-                                     asset stake_net_quantity, 
-                                     asset stake_cpu_quantity, bool transfer )
+   void validate_b1_vesting( int64_t stake ) {
+      const int64_t seconds_per_year = 60*60*24*365;
+      const int64_t base_time = 1527811200; /// 2018-06-01
+      const int64_t max_claimable = 100'000'000'0000ll;
+      const int64_t claimable = int64_t(max_claimable * double(now()-base_time) / (10*seconds_per_year) );
+
+      eosio_assert( max_claimable - claimable <= stake, "b1 can only claim their tokens over 10 years" );
+   }
 
+   void system_contract::changebw( account_name from, account_name receiver,
+                                   asset stake_net_quantity, asset stake_cpu_quantity, bool transfer )
    {
       require_auth( from );
-
-      eosio_assert( stake_cpu_quantity >= asset(0), "must stake a positive amount" );
-      eosio_assert( stake_net_quantity >= asset(0), "must stake a positive amount" );
-
-      auto total_stake = stake_cpu_quantity.amount + stake_net_quantity.amount;
-      eosio_assert( total_stake > 0, "must stake a positive amount" );
+      eosio_assert( stake_net_quantity != asset(0) || stake_cpu_quantity != asset(0), "should stake non-zero amount" );
 
       account_name source_stake_from = from;
+      if ( transfer ) {
+         from = receiver;
+      }
 
-      if( transfer ) from = receiver;
-
+      // update stake delegated from "from" to "receiver"
+      {
          del_bandwidth_table     del_tbl( _self, from);
          auto itr = del_tbl.find( receiver );
          if( itr == del_tbl.end() ) {
-         del_tbl.emplace( from, [&]( auto& dbo ){
+            itr = del_tbl.emplace( from, [&]( auto& dbo ){
                   dbo.from          = from;
                   dbo.to            = receiver;
                   dbo.net_weight    = stake_net_quantity;
@@ -246,7 +251,15 @@ namespace eosiosystem {
                   dbo.cpu_weight    += stake_cpu_quantity;
                });
          }
+         eosio_assert( asset(0) <= itr->net_weight, "insufficient staked net bandwidth" );
+         eosio_assert( asset(0) <= itr->cpu_weight, "insufficient staked cpu bandwidth" );
+         if ( itr->net_weight == asset(0) && itr->cpu_weight == asset(0) ) {
+            del_tbl.erase( itr );
+         }
+      } // itr can be invalid, should go out of scope
 
+      // update totals of "receiver"
+      {
          user_resources_table   totals_tbl( _self, receiver );
          auto tot_itr = totals_tbl.find( receiver );
          if( tot_itr ==  totals_tbl.end() ) {
@@ -261,114 +274,110 @@ namespace eosiosystem {
                   tot.cpu_weight    += stake_cpu_quantity;
                });
          }
+         eosio_assert( asset(0) <= tot_itr->net_weight, "insufficient staked total net bandwidth" );
+         eosio_assert( asset(0) <= tot_itr->cpu_weight, "insufficient staked total cpu bandwidth" );
 
-      set_resource_limits( tot_itr->owner, tot_itr->ram_bytes, tot_itr->net_weight.amount, tot_itr->cpu_weight.amount );
-
-      if( N(eosio) != source_stake_from ) {
-         INLINE_ACTION_SENDER(eosio::token, transfer)( N(eosio.token), {from,N(active)},
-                                                       { source_stake_from, N(eosio), asset(total_stake), std::string("stake bandwidth") } );
-      }
-
-      auto from_voter = _voters.find(from);
-      if( from_voter == _voters.end() ) {
-         from_voter = _voters.emplace( from, [&]( auto& v ) {
-            v.owner  = from;
-            v.staked = total_stake;
-         });
-      } else {
-         _voters.modify( from_voter, 0, [&]( auto& v ) {
-            v.staked += total_stake;
-         });
-      }
-
-      if( from_voter->producers.size() || from_voter->proxy ) {
-         voteproducer( from, from_voter->proxy, from_voter->producers );
-      }
-   } // delegatebw
-
-
-   void validate_b1_vesting( int64_t stake ) {
-      const int64_t seconds_per_year = 60*60*24*365;
-      const int64_t base_time = 1527811200; /// 2018-06-01
-      const int64_t max_claimable = 100'000'000'0000ll;
-      const int64_t claimable = int64_t(max_claimable * double(now()-base_time) / (10*seconds_per_year) );
-
-      eosio_assert( max_claimable - claimable <= stake, "b1 can only claim their tokens over 10 years" );
-   }
-
-   void system_contract::undelegatebw( account_name from, account_name receiver,
-                                       asset unstake_net_quantity, asset unstake_cpu_quantity )
-   {
-      eosio_assert( unstake_cpu_quantity >= asset(), "must unstake a positive amount" );
-      eosio_assert( unstake_net_quantity >= asset(), "must unstake a positive amount" );
-
-      require_auth( from );
-
-      del_bandwidth_table     del_tbl( _self, from );
-      const auto& dbw = del_tbl.get( receiver );
-      eosio_assert( dbw.net_weight.amount >= unstake_net_quantity.amount, "insufficient staked net bandwidth" );
-      eosio_assert( dbw.cpu_weight.amount >= unstake_cpu_quantity.amount, "insufficient staked cpu bandwidth" );
-
-      auto total_refund = unstake_cpu_quantity.amount + unstake_net_quantity.amount;
-
-      _voters.modify( _voters.get(from), 0, [&]( auto& v ) {
-         v.staked -= total_refund;
-         if( from == N(b1) ) {
-            validate_b1_vesting( v.staked );
-         }
-      });
-
+         set_resource_limits( receiver, tot_itr->ram_bytes, tot_itr->net_weight.amount, tot_itr->cpu_weight.amount );
 
-      eosio_assert( total_refund > 0, "must unstake a positive amount" );
-
-      if( dbw.net_weight == unstake_net_quantity && dbw.cpu_weight == unstake_cpu_quantity ) {
-         del_tbl.erase( dbw );
-      } else { 
-         del_tbl.modify( dbw, from, [&]( auto& dbo ){
-               dbo.net_weight -= unstake_net_quantity;
-               dbo.cpu_weight -= unstake_cpu_quantity;
-         });
+         if ( tot_itr->net_weight == asset(0) && tot_itr->cpu_weight == asset(0) ) {
+            totals_tbl.erase( tot_itr );
          }
+      } // tot_itr can be invalid, should go out of scope
 
-      user_resources_table totals_tbl( _self, receiver );
-
-      const auto& totals = totals_tbl.get( receiver );
-      totals_tbl.modify( totals, 0, [&]( auto& tot ) {
-            tot.net_weight -= unstake_net_quantity;
-            tot.cpu_weight -= unstake_cpu_quantity;
-      });
-
-      set_resource_limits( receiver, totals.ram_bytes, totals.net_weight.amount, totals.cpu_weight.amount );
+      // transfer money or schedule a refund
+      asset total_update = stake_net_quantity + stake_cpu_quantity;
 
+      if ( N(eosio) != source_stake_from ) { //for eosio transfer/refund doesn't make sense
          refunds_table refunds_tbl( _self, from );
-      //create refund request
          auto req = refunds_tbl.find( from );
+
+         //create/update/delete refund
+         asset update_balance = total_update;
+         bool need_deferred_trx = false;
          if ( req != refunds_tbl.end() ) {
             refunds_tbl.modify( req, 0, [&]( refund_request& r ) {
-               r.amount += unstake_net_quantity + unstake_cpu_quantity;
+                  r.amount -= update_balance;
+                  if ( r.amount < asset(0) ){
+                     update_balance = -r.amount;
+                     r.amount = asset(0);
+                  }
                   r.request_time = now();
                });
+            eosio_assert( asset(0) <= req->amount, "negative refund amount" ); //should never happen
+
+            if ( req->amount == asset(0) ) {
+               refunds_tbl.erase( req );
+               need_deferred_trx = false;
             } else {
+               need_deferred_trx = true;
+            }
+         } else if ( update_balance < asset(0) ) {
             refunds_tbl.emplace( from, [&]( refund_request& r ) {
                   r.owner = from;
-               r.amount = unstake_net_quantity + unstake_cpu_quantity;
+                  r.amount = -total_update;
                   r.request_time = now();
                });
+            need_deferred_trx = true;
+         } // else stake increases with no existing row in refunds_tbl -> nothing to do with refunds_tbl
+
+         if ( asset(0) < update_balance ) {
+            INLINE_ACTION_SENDER(eosio::token, transfer)( N(eosio.token), {from,N(active)},
+               { source_stake_from, N(eosio), asset(update_balance), std::string("stake bandwidth") } );
          }
 
-      //create or replace deferred transaction
-      //refund act;
-      //act.owner = from;
+         if ( need_deferred_trx ) {
             eosio::transaction out;
             out.actions.emplace_back( permission_level{ from, N(active) }, _self, N(refund), from );
             out.delay_sec = refund_delay;
             out.send( from, receiver, true );
+         } else {
+            //cancel_deferred( from );
+         }
+      }
 
-      const auto& fromv = _voters.get( from );
+      // update voting power
+      {
+         auto from_voter = _voters.find(from);
+         if( from_voter == _voters.end() ) {
+            from_voter = _voters.emplace( from, [&]( auto& v ) {
+                  v.owner  = from;
+                  v.staked = total_update.amount;
+               });
+         } else {
+            _voters.modify( from_voter, 0, [&]( auto& v ) {
+                  v.staked += total_update.amount;
+               });
+         }
+         eosio_assert( 0 <= from_voter->staked, "stake for voting cannot be negative");
+         if( from == N(b1) ) {
+            validate_b1_vesting( from_voter->staked );
+         }
 
-      if( fromv.producers.size() || fromv.proxy ) {
-         voteproducer( from, fromv.proxy, fromv.producers );
+         if( from_voter->producers.size() || from_voter->proxy ) {
+            voteproducer( from, from_voter->proxy, from_voter->producers );
          }
+      }
+   }
+
+   void system_contract::delegatebw( account_name from, account_name receiver,
+                                     asset stake_net_quantity,
+                                     asset stake_cpu_quantity, bool transfer )
+   {
+      eosio_assert( stake_cpu_quantity >= asset(0), "must stake a positive amount" );
+      eosio_assert( stake_net_quantity >= asset(0), "must stake a positive amount" );
+      eosio_assert( stake_net_quantity + stake_cpu_quantity > asset(0), "must stake a positive amount" );
+
+      changebw( from, receiver, stake_net_quantity, stake_cpu_quantity, transfer);
+   } // delegatebw
+
+   void system_contract::undelegatebw( account_name from, account_name receiver,
+                                       asset unstake_net_quantity, asset unstake_cpu_quantity )
+   {
+      eosio_assert( asset() <= unstake_cpu_quantity, "must unstake a positive amount" );
+      eosio_assert( asset() <= unstake_net_quantity, "must unstake a positive amount" );
+      eosio_assert( asset() < unstake_cpu_quantity + unstake_net_quantity, "must unstake a positive amount" );
+
+      changebw( from, receiver, -unstake_net_quantity, -unstake_cpu_quantity, false);
    } // undelegatebw
 
 

contracts/eosio.system/eosio.system.hpp

@@ -134,6 +134,9 @@ namespace eosiosystem {
 
          // functions defined in delegate_bandwidth.cpp
 
+         void changebw( account_name from, account_name receiver,
+                        asset stake_net_quantity, asset stake_cpu_quantity, bool transfer );
+
          /**
           *  Stakes SYS from the balance of 'from' for the benfit of 'receiver'. 
           *  If transfer == true, then 'receiver' can unstake to their account

unittests/eosio.system_tests.cpp

@@ -754,9 +754,12 @@ BOOST_FIXTURE_TEST_CASE( delegate_to_another_user, eosio_system_tester ) try {
    BOOST_REQUIRE_EQUAL( true, get_voter_info( "bob111111111" ).is_null() );
 
    //bob111111111 should not be able to unstake what was staked by alice1111111
-   BOOST_REQUIRE_EQUAL( error("condition: assertion failed: unable to find key"),
+   BOOST_REQUIRE_EQUAL( error("condition: assertion failed: insufficient staked cpu bandwidth"),
                         unstake( "bob111111111", "0.0000 EOS", "10.0000 EOS" )
    );
+   BOOST_REQUIRE_EQUAL( error("condition: assertion failed: insufficient staked net bandwidth"),
+                        unstake( "bob111111111", "10.0000 EOS", "0.0000 EOS" )
+   );
 
    issue( "carol1111111", "1000.0000 EOS",  config::system_account_name );
    BOOST_REQUIRE_EQUAL( success(), stake( "carol1111111", "bob111111111", "20.0000 EOS", "10.0000 EOS" ) );