Remove usage of ForEachAccount

src/masternodes/accounts.cpp

@@ -63,11 +63,6 @@ Res CAccountsView::SubBalances(const CScript &owner, const CBalances &balances)
     return Res::Ok();
 }
 
-void CAccountsView::ForEachAccount(std::function<bool(const CScript &)> callback, const CScript &start) {
-    ForEach<ByHeightKey, CScript, uint32_t>(
-        [&callback](const CScript &owner, CLazySerialize<uint32_t>) { return callback(owner); }, start);
-}
-
 Res CAccountsView::UpdateBalancesHeight(const CScript &owner, uint32_t height) {
     WriteBy<ByHeightKey>(owner, height);
     return Res::Ok();

src/masternodes/accounts.h

@@ -55,7 +55,6 @@ struct CFuturesUserValue {
 
 class CAccountsView : public virtual CStorageView {
 public:
-    void ForEachAccount(std::function<bool(const CScript &)> callback, const CScript &start = {});
     void ForEachBalance(std::function<bool(const CScript &, const CTokenAmount &)> callback,
                         const BalanceKey &start = {});
     CTokenAmount GetBalance(const CScript &owner, DCT_ID tokenID) const;

src/masternodes/mn_rpc.cpp

@@ -19,13 +19,20 @@ CAccounts GetAllMineAccounts(CWallet * const pwallet) {
     CCustomCSView mnview(*pcustomcsview);
     auto targetHeight = ::ChainActive().Height() + 1;
 
-    mnview.ForEachAccount([&](CScript const & account) {
-        if (IsMineCached(*pwallet, account) == ISMINE_SPENDABLE) {
-            mnview.CalculateOwnerRewards(account, targetHeight);
-            mnview.ForEachBalance([&](CScript const & owner, CTokenAmount balance) {
-                return account == owner && walletAccounts[owner].Add(balance);
-            }, {account, DCT_ID{}});
+    // ForEachBalance is in account order, so we only need to check if the
+    // last record is the same as the current one to know whether we can skip
+    // CalculateOwnerRewards or if it needs to be called.
+    CScript lastCalculatedOwner;
+
+    mnview.ForEachBalance([&](const CScript &owner, const CTokenAmount &balance) {
+        if (IsMineCached(*pwallet, owner) == ISMINE_SPENDABLE) {
+            if (lastCalculatedOwner != owner) {
+                mnview.CalculateOwnerRewards(owner, targetHeight);
+                lastCalculatedOwner = owner;
+            }
+            walletAccounts[owner].Add(balance);
         }
+
         return true;
     });
 

src/masternodes/rpc_accounts.cpp

@@ -358,26 +358,27 @@ UniValue listaccounts(const JSONRPCRequest& request) {
     CCustomCSView mnview(*pcustomcsview);
     auto targetHeight = ::ChainActive().Height() + 1;
 
-    mnview.ForEachAccount([&](CScript const & account) {
+    // ForEachBalance is in account order, so we only need to check if the
+    // last record is the same as the current one to know whether we can skip
+    // CalculateOwnerRewards or if it needs to be called.
+    CScript lastCalculatedOwner;
 
-        if (isMineOnly && IsMineCached(*pwallet, account) != ISMINE_SPENDABLE) {
+    mnview.ForEachBalance([&](const CScript &owner, const CTokenAmount &balance) {
+        if (isMineOnly && IsMineCached(*pwallet, owner) != ISMINE_SPENDABLE) {
             return true;
         }
 
-        mnview.CalculateOwnerRewards(account, targetHeight);
+        if (lastCalculatedOwner != owner) {
+            mnview.CalculateOwnerRewards(owner, targetHeight);
+            lastCalculatedOwner = owner;
+        }
 
-        // output the relavant balances only for account
-        mnview.ForEachBalance([&](CScript const & owner, CTokenAmount balance) {
-            if (account != owner) {
-                return false;
-            }
-            ret.push_back(accountToJSON(owner, balance, verbose, indexed_amounts));
-            return --limit != 0;
-        }, {account, start.tokenID});
+        ret.push_back(accountToJSON(owner, balance, verbose, indexed_amounts));
 
         start.tokenID = DCT_ID{}; // reset to start id
-        return limit != 0;
-    }, start.owner);
+
+        return --limit != 0;
+    }, {start.owner, start.tokenID});
 
     return GetRPCResultCache().Set(request, ret);
 }
@@ -549,15 +550,23 @@ UniValue gettokenbalances(const JSONRPCRequest& request) {
     CCustomCSView mnview(*pcustomcsview);
     auto targetHeight = ::ChainActive().Height() + 1;
 
-    mnview.ForEachAccount([&](CScript const & account) {
-        if (IsMineCached(*pwallet, account) == ISMINE_SPENDABLE) {
-            mnview.CalculateOwnerRewards(account, targetHeight);
-            mnview.ForEachBalance([&](CScript const & owner, CTokenAmount balance) {
-                return account == owner && totalBalances.Add(balance);
-            }, {account, DCT_ID{}});
+    // ForEachBalance is in account order, so we only need to check if the
+    // last record is the same as the current one to know whether we can skip
+    // CalculateOwnerRewards or if it needs to be called.
+    CScript lastCalculatedOwner;
+
+    mnview.ForEachBalance([&](CScript const & owner, CTokenAmount balance) {
+        if (IsMineCached(*pwallet, owner)) {
+            if (lastCalculatedOwner != owner) {
+                mnview.CalculateOwnerRewards(owner, targetHeight);
+                lastCalculatedOwner = owner;
+            }
+            totalBalances.Add(balance);
         }
+
         return true;
     });
+
     auto it = totalBalances.balances.lower_bound(start);
     for (size_t i = 0; it != totalBalances.balances.end() && i < limit; it++, i++) {
         auto bal = CTokenAmount{(*it).first, (*it).second};

test/functional/feature_on_chain_government_fee_distribution.py

@@ -91,14 +91,16 @@ def test_cfp_fee_distribution(self, amount, expectedFee, burnPct, vote, cycles=2
             mn0 = self.nodes[0].getmasternode(self.mn0)[self.mn0]
             account0 = self.nodes[0].getaccount(mn0['ownerAuthAddress'])
             assert_equal(account0[0], '{}@DFI'.format(expectedAmount * votingCycles))
+            assert_equal(self.nodes[0].gettokenbalances(), [f"{expectedAmount * votingCycles}@0"])
             history = self.nodes[0].listaccounthistory(mn0['ownerAuthAddress'], {"txtype": "ProposalFeeRedistribution"})
             assert_equal(len(history), votingCycles)
             for i in range(votingCycles):
                 assert_equal(history[i]['amounts'][0], '{}@DFI'.format(expectedAmount))
 
             mn1 = self.nodes[0].getmasternode(self.mn1)[self.mn1]
             account1 = self.nodes[0].getaccount(mn1['ownerAuthAddress'])
-            assert_equal(account1[0], '{}@DFI'.format(expectedAmount* votingCycles))
+            assert_equal(account1[0], '{}@DFI'.format(expectedAmount * votingCycles))
+            assert_equal(self.nodes[1].gettokenbalances(), [f"{expectedAmount * votingCycles}@0"])
             history = self.nodes[0].listaccounthistory(mn1['ownerAuthAddress'], {"txtype": "ProposalFeeRedistribution"})
             assert_equal(len(history), votingCycles)
             for i in range(votingCycles):
@@ -107,7 +109,8 @@ def test_cfp_fee_distribution(self, amount, expectedFee, burnPct, vote, cycles=2
             # Fee should be redistributed to reward address
             mn2 = self.nodes[0].getmasternode(self.mn2)[self.mn2]
             account2 = self.nodes[0].getaccount(mn2['ownerAuthAddress'])
-            assert_equal(account2[0], '{}@DFI'.format(expectedAmount* votingCycles))
+            assert_equal(account2[0], '{}@DFI'.format(expectedAmount * votingCycles))
+            assert_equal(self.nodes[2].gettokenbalances(), [f"{expectedAmount * votingCycles}@0"])
             history = self.nodes[0].listaccounthistory(mn2['ownerAuthAddress'], {"txtype": "ProposalFeeRedistribution"})
             assert_equal(len(history), votingCycles)
             for i in range(votingCycles):
@@ -158,11 +161,11 @@ def setup(self):
         assert_equal(self.nodes[0].getblockcount(), 101)
 
         # activate on-chain governance
-        self.nodes[0].setgov({"ATTRIBUTES":{'v0/params/feature/gov':'true'}})
+        self.nodes[0].setgov({"ATTRIBUTES": {'v0/params/feature/gov': 'true'}})
         self.nodes[0].generate(1)
 
         # activate fee redistribution
-        self.nodes[0].setgov({"ATTRIBUTES":{'v0/gov/proposals/fee_redistribution':'true'}})
+        self.nodes[0].setgov({"ATTRIBUTES": {'v0/gov/proposals/fee_redistribution': 'true'}})
         self.nodes[0].generate(1)
 
         self.sync_blocks()
@@ -187,7 +190,8 @@ def run_test(self):
         self.sync_blocks()
 
         self.test_cfp_fee_distribution(amount=1000, expectedFee=20, burnPct=30, vote="yes", cycles=1)
-        self.test_cfp_fee_distribution(amount=1000, expectedFee=20, burnPct=30, vote="yes", cycles=3, changeFeeAndBurnPCT=True)
+        self.test_cfp_fee_distribution(amount=1000, expectedFee=20, burnPct=30, vote="yes", cycles=3,
+                                       changeFeeAndBurnPCT=True)
 
 if __name__ == '__main__':
     CFPFeeDistributionTest().main ()