From 9ee58766ff8ba43dae698258b5057833b3abb13e Mon Sep 17 00:00:00 2001 From: Peter John Bushnell Date: Wed, 23 Oct 2024 11:51:43 +0100 Subject: [PATCH] Only consolidate per-block rewards after moving to static rewards (#3099) * Block consolidating rewards after static rewards * Skip static reward consolidation * Skip setting height for skipStatic if not per-block update happened * Make args and other vars const * Add consolidate reward test --- src/dfi/masternodes.cpp | 29 ++- src/dfi/masternodes.h | 2 +- src/dfi/rpc_accounts.cpp | 3 + src/dfi/validation.cpp | 5 +- src/init.cpp | 8 +- src/validation.h | 3 +- .../functional/feature_consolidate_rewards.py | 233 ++++++++++++++++++ test/functional/test_runner.py | 1 + 8 files changed, 269 insertions(+), 15 deletions(-) create mode 100755 test/functional/feature_consolidate_rewards.py diff --git a/src/dfi/masternodes.cpp b/src/dfi/masternodes.cpp index 7265365a35d..cf697c7a1ee 100644 --- a/src/dfi/masternodes.cpp +++ b/src/dfi/masternodes.cpp @@ -970,13 +970,19 @@ bool CCustomCSView::CanSpend(const uint256 &txId, int height) const { return !pair || pair->second.destructionTx != uint256{} || pair->second.IsPoolShare(); } -bool CCustomCSView::CalculateOwnerRewards(const CScript &owner, uint32_t targetHeight) { - auto balanceHeight = GetBalancesHeight(owner); +bool CCustomCSView::CalculateOwnerRewards(const CScript &owner, const uint32_t targetHeight, const bool skipStatic) { + const auto balanceHeight = GetBalancesHeight(owner); if (balanceHeight >= targetHeight) { return false; } + + // Calculate per-block rewards up to the fork height + const auto targetPerBlockHeight = + targetHeight >= Params().GetConsensus().DF24Height ? Params().GetConsensus().DF24Height : targetHeight; + bool perBlockUpdated{}; + ForEachPoolId([&](DCT_ID const &poolId) { - auto height = GetShare(poolId, owner); + const auto height = GetShare(poolId, owner); if (!height || *height >= targetHeight) { return true; // no share or target height is before a pool share' one } @@ -990,13 +996,11 @@ bool CCustomCSView::CalculateOwnerRewards(const CScript &owner, uint32_t targetH }; if (beginHeight < Params().GetConsensus().DF24Height) { - // Calculate just up to the fork height - const auto targetNewHeight = - targetHeight >= Params().GetConsensus().DF24Height ? Params().GetConsensus().DF24Height : targetHeight; - CalculatePoolRewards(poolId, onLiquidity, beginHeight, targetNewHeight, onReward); + perBlockUpdated = true; + CalculatePoolRewards(poolId, onLiquidity, beginHeight, targetPerBlockHeight, onReward); } - if (targetHeight >= Params().GetConsensus().DF24Height) { + if (!skipStatic && targetHeight >= Params().GetConsensus().DF24Height) { // Calculate from the fork height const auto beginNewHeight = beginHeight < Params().GetConsensus().DF24Height ? Params().GetConsensus().DF24Height - 1 @@ -1007,7 +1011,14 @@ bool CCustomCSView::CalculateOwnerRewards(const CScript &owner, uint32_t targetH return true; }); - return UpdateBalancesHeight(owner, targetHeight); + // If no per-block update to occured then do not update the height. + if (skipStatic && !perBlockUpdated) { + return true; + } + + const auto updateHeight = skipStatic ? targetPerBlockHeight : targetHeight; + + return UpdateBalancesHeight(owner, updateHeight); } double CVaultAssets::calcRatio(uint64_t maxRatio) const { diff --git a/src/dfi/masternodes.h b/src/dfi/masternodes.h index 788cd50287e..2abe4f804c4 100644 --- a/src/dfi/masternodes.h +++ b/src/dfi/masternodes.h @@ -581,7 +581,7 @@ class CCustomCSView : public CMasternodesView, bool CanSpend(const uint256 &txId, int height) const; - bool CalculateOwnerRewards(const CScript &owner, uint32_t height); + bool CalculateOwnerRewards(const CScript &owner, const uint32_t height, const bool skipStatic = false); ResVal GetAmountInCurrency(CAmount amount, CTokenCurrencyPair priceFeedId, diff --git a/src/dfi/rpc_accounts.cpp b/src/dfi/rpc_accounts.cpp index b58ce5d7469..54363401add 100644 --- a/src/dfi/rpc_accounts.cpp +++ b/src/dfi/rpc_accounts.cpp @@ -3720,6 +3720,9 @@ UniValue logdbhashes(const JSONRPCRequest &request) { pcursor->Next(); } + // Delete iterator + delete pcursor; + // Finalize the hash unsigned char hash[CSHA256::OUTPUT_SIZE]; hasher.Finalize(hash); diff --git a/src/dfi/validation.cpp b/src/dfi/validation.cpp index 3ec3fd0a2e4..2c4b31e0443 100644 --- a/src/dfi/validation.cpp +++ b/src/dfi/validation.cpp @@ -1563,7 +1563,8 @@ void ConsolidateRewards(CCustomCSView &view, int height, const std::unordered_set &owners, bool interruptOnShutdown, - int numWorkers) { + int numWorkers, + bool skipStatic) { int nWorkers = numWorkers < 1 ? RewardConsolidationWorkersCount() : numWorkers; auto rewardsTime = GetTimeMicros(); boost::asio::thread_pool workerPool(nWorkers); @@ -1581,7 +1582,7 @@ void ConsolidateRewards(CCustomCSView &view, return; } auto tempView = std::make_unique(view); - tempView->CalculateOwnerRewards(account, height); + tempView->CalculateOwnerRewards(account, height, skipStatic); boost::asio::post(mergeWorker, [&, tempView = std::move(tempView)]() { if (interruptOnShutdown && ShutdownRequested()) { diff --git a/src/init.cpp b/src/init.cpp index b530a395026..f98e7812c1d 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2253,6 +2253,10 @@ bool AppInitMain(InitInterfaces& interfaces) } if (gArgs.IsArgSet("-consolidaterewards")) { + // Due to higher precision reward consolidation after the DF24 fork consolidate rewards + // cannot be used. The following skipStatic flag is used to skip the static consolidation + // and only run the per-block consolidation. + const bool skipStatic = ::ChainActive().Height() >= chainparams.GetConsensus().DF24Height; const std::vector tokenSymbolArgs = gArgs.GetArgs("-consolidaterewards"); auto fullRewardConsolidation = false; for (const auto &tokenSymbolInput : tokenSymbolArgs) { @@ -2273,7 +2277,7 @@ bool AppInitMain(InitInterfaces& interfaces) } return true; }); - ConsolidateRewards(*pcustomcsview, ::ChainActive().Height(), ownersToConsolidate, true); + ConsolidateRewards(*pcustomcsview, ::ChainActive().Height(), ownersToConsolidate, true, skipStatic); } else { //one set for all tokens, ConsolidateRewards runs on the address, so no need to run multiple times for multiple token inputs std::unordered_set ownersToConsolidate; @@ -2293,7 +2297,7 @@ bool AppInitMain(InitInterfaces& interfaces) return true; }); } - ConsolidateRewards(*pcustomcsview, ::ChainActive().Height(), ownersToConsolidate, true); + ConsolidateRewards(*pcustomcsview, ::ChainActive().Height(), ownersToConsolidate, true, skipStatic); } pcustomcsview->Flush(); } diff --git a/src/validation.h b/src/validation.h index a2d9a064bf9..1cd83f8e81e 100644 --- a/src/validation.h +++ b/src/validation.h @@ -936,7 +936,8 @@ void ConsolidateRewards(CCustomCSView &view, int height, const std::unordered_set &owners, bool interruptOnShutdown, - int numWorkers = 0); + int numWorkers = 0, + bool skipStatic = false); extern std::map mapBurnAmounts; diff --git a/test/functional/feature_consolidate_rewards.py b/test/functional/feature_consolidate_rewards.py new file mode 100755 index 00000000000..c3a5f5c27fa --- /dev/null +++ b/test/functional/feature_consolidate_rewards.py @@ -0,0 +1,233 @@ +#!/usr/bin/env python3 +# Copyright (c) 2014-2019 The Bitcoin Core developers +# Copyright (c) DeFi Blockchain Developers +# Distributed under the MIT software license, see the accompanying +# file LICENSE or http://www.opensource.org/licenses/mit-license.php. +"""Test consolidate rewards""" + +from test_framework.test_framework import DefiTestFramework + +from test_framework.util import assert_equal, connect_nodes_bi + +import time + + +class ConsolidateRewardsTest(DefiTestFramework): + def set_test_params(self): + self.num_nodes = 2 + self.setup_clean_chain = True + self.df24height = 200 + self.args = [ + "-txnotokens=0", + "-subsidytest=1", + "-regtest-minttoken-simulate-mainnet=1", + "-amkheight=1", + "-bayfrontheight=1", + "-bayfrontmarinaheight=1", + "-bayfrontgardensheight=1", + "-clarkequayheight=1", + "-dakotaheight=1", + "-dakotacrescentheight=1", + "-eunosheight=1", + "-eunospayaheight=1", + "-fortcanningheight=1", + "-fortcanningmuseumheight=1", + "-fortcanningparkheight=1", + "-fortcanninghillheight=1", + "-fortcanningroadheight=1", + "-fortcanningcrunchheight=1", + "-fortcanningspringheight=1", + "-fortcanninggreatworldheight=1", + "-grandcentralheight=1", + "-grandcentralepilogueheight=1", + "-metachainheight=105", + "-df23height=110", + f"-df24height={self.df24height}", + ] + self.extra_args = [self.args, self.args] + + def run_test(self): + + # Set up + self.setup() + + # Consolidate before fork + self.pre_fork24_consolidate() + + # Consolidate after fork + self.post_fork24_consolidate() + + def setup(self): + + # Generate chain + self.nodes[0].generate(110) + + # Symbols + self.symbolDFI = "DFI" + self.symbolGOOGL = "GOOGL" + self.symbolDUSD = "DUSD" + self.symbolGD = "GOOGL-DUSD" + + # Price feeds + price_feed = [ + {"currency": "USD", "token": self.symbolDFI}, + {"currency": "USD", "token": self.symbolGOOGL}, + ] + + # Appoint oracle + oracle_address = self.nodes[0].getnewaddress("", "legacy") + self.oracle = self.nodes[0].appointoracle(oracle_address, price_feed, 10) + self.nodes[0].generate(1) + + # Set Oracle prices + oracle_prices = [ + {"currency": "USD", "tokenAmount": f"1@{self.symbolDFI}"}, + {"currency": "USD", "tokenAmount": f"1@{self.symbolGOOGL}"}, + ] + self.nodes[0].setoracledata(self.oracle, int(time.time()), oracle_prices) + self.nodes[0].generate(10) + + # Set loan tokens + self.nodes[0].setloantoken( + { + "symbol": self.symbolGOOGL, + "name": self.symbolGOOGL, + "fixedIntervalPriceId": f"{self.symbolGOOGL}/USD", + "isDAT": True, + "interest": 0, + } + ) + self.nodes[0].setloantoken( + { + "symbol": self.symbolDUSD, + "name": self.symbolDUSD, + "fixedIntervalPriceId": f"{self.symbolDUSD}/USD", + "mintable": True, + "interest": 0, + } + ) + self.nodes[0].generate(1) + + # Create pool pair + self.nodes[0].createpoolpair( + { + "tokenA": self.symbolGOOGL, + "tokenB": self.symbolDUSD, + "commission": 0.001, + "status": True, + "ownerAddress": self.nodes[0].get_genesis_keys().ownerAuthAddress, + "symbol": self.symbolGD, + } + ) + self.nodes[0].generate(1) + + # Store token IDs + self.idDUSD = list(self.nodes[0].gettoken(self.symbolDUSD).keys())[0] + self.idGOOGL = list(self.nodes[0].gettoken(self.symbolGOOGL).keys())[0] + self.idGD = list(self.nodes[0].gettoken(self.symbolGD).keys())[0] + + # Create new address + self.address = self.nodes[0].getnewaddress("", "legacy") + + # Mint tokens to address + self.nodes[0].minttokens([f"100000@{self.idDUSD}", f"100000@{self.idGOOGL}"]) + self.nodes[0].generate(1) + + # Set loan token split + self.nodes[0].setgov({"LP_LOAN_TOKEN_SPLITS": {self.idGD: 1}}) + self.nodes[0].generate(1) + + # Fund pools + self.nodes[0].addpoolliquidity( + { + self.nodes[0] + .get_genesis_keys() + .ownerAuthAddress: [ + f"100000@{self.symbolDUSD}", + f"100000@{self.symbolGOOGL}", + ] + }, + self.address, + ) + self.nodes[0].generate(1) + self.sync_blocks() + + def pre_fork24_consolidate(self): + + # Compare hash before consolidation + hash_0 = self.nodes[0].logdbhashes()["dvmhash"] + hash_1 = self.nodes[1].logdbhashes()["dvmhash"] + assert_equal(hash_0, hash_1) + + # Generate rewards + self.nodes[0].generate(10) + self.sync_blocks() + + # Stop node + self.stop_node(1) + + # Start node with consolidation + self.args.append(f"-consolidaterewards={self.symbolGD}") + self.start_node(1, self.args) + connect_nodes_bi(self.nodes, 0, 1) + + # Split Google + self.nodes[0].setgov( + { + "ATTRIBUTES": { + f"v0/oracles/splits/{self.nodes[0].getblockcount() + 2}": f"{self.idGOOGL}/2" + } + } + ) + self.nodes[0].generate(2) + self.sync_blocks() + + # Update ID + self.idGOOGL = list(self.nodes[0].gettoken(self.symbolGOOGL).keys())[0] + + # Compare hash before consolidation + hash_0 = self.nodes[0].logdbhashes()["dvmhash"] + hash_1 = self.nodes[1].logdbhashes()["dvmhash"] + assert_equal(hash_0, hash_1) + + def post_fork24_consolidate(self): + + # Move to fork + self.nodes[0].generate(self.df24height - self.nodes[0].getblockcount()) + + # Generate post fork rewards + self.nodes[0].generate(10) + self.sync_blocks() + + # Compare hash before consolidation + hash_0 = self.nodes[0].logdbhashes()["dvmhash"] + hash_1 = self.nodes[1].logdbhashes()["dvmhash"] + assert_equal(hash_0, hash_1) + + # Stop node + self.stop_node(1) + + # Start node with consolidation + self.args.append(f"-consolidaterewards={self.symbolGD}") + self.start_node(1, self.args) + connect_nodes_bi(self.nodes, 0, 1) + + # Split Google + self.nodes[0].setgov( + { + "ATTRIBUTES": { + f"v0/oracles/splits/{self.nodes[0].getblockcount() + 2}": f"{self.idGOOGL}/2" + } + } + ) + self.nodes[0].generate(2) + self.sync_blocks() + + # Compare hash before consolidation + hash_0 = self.nodes[0].logdbhashes()["dvmhash"] + hash_1 = self.nodes[1].logdbhashes()["dvmhash"] + assert_equal(hash_0, hash_1) + + +if __name__ == "__main__": + ConsolidateRewardsTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index c7aba03ce86..5d79a0f596f 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -181,6 +181,7 @@ "wallet_watchonly.py", "wallet_watchonly.py --usecli", "feature_poolpair.py", + "feature_consolidate_rewards.py", "feature_poolpair_liquidity.py", "feature_icx_orderbook.py", "feature_icx_orderbook_errors.py",