Apply #1245 from v3 so that vars from setgovheight gets Apply() called

src/masternodes/govvariables/attributes.cpp

@@ -1020,6 +1020,11 @@ Res ATTRIBUTES::Apply(CCustomCSView & mnview, const uint32_t height)
                 if (auto it{value->find(split)}; it == value->end()) {
                     continue;
                 }
+
+                if (attrV0->key <= height) {
+                    return Res::Err("Cannot be set at or below current height");
+                }
+
                 CDataStructureV0 lockKey{AttributeTypes::Locks, ParamIDs::TokenID, split};
                 if (GetValue(lockKey, false)) {
                     continue;
@@ -1029,10 +1034,6 @@ Res ATTRIBUTES::Apply(CCustomCSView & mnview, const uint32_t height)
                     return Res::Err("Auto lock. No loan token with id (%d)", split);
                 }
 
-                if (attrV0->key <= height) {
-                    return Res::Err("Cannot be set at or below current height");
-                }
-
                 CGovernanceHeightMessage lock;
                 auto var = GovVariable::Create("ATTRIBUTES");
                 if (!var) {

src/masternodes/gv.cpp

@@ -42,7 +42,7 @@ Res CGovView::SetStoredVariables(const std::set<std::shared_ptr<GovVariable>>& g
 
 std::set<std::shared_ptr<GovVariable>> CGovView::GetStoredVariables(const uint32_t height)
 {
-    // Popualte a set of Gov vars for specified height
+    // Populate a set of Gov vars for specified height
     std::set<std::shared_ptr<GovVariable>> govVars;
     auto it = LowerBound<ByHeightVars>(GovVarKey{height, {}});
     for (; it.Valid() && it.Key().height == height; it.Next()) {
@@ -55,6 +55,21 @@ std::set<std::shared_ptr<GovVariable>> CGovView::GetStoredVariables(const uint32
     return govVars;
 }
 
+std::vector<std::pair<uint32_t, std::shared_ptr<GovVariable>>> CGovView::GetStoredVariablesRange(const uint32_t startHeight, const uint32_t endHeight)
+{
+    // Populate a set of Gov vars for specified height
+    std::vector<std::pair<uint32_t, std::shared_ptr<GovVariable>>> govVars;
+    auto it = LowerBound<ByHeightVars>(GovVarKey{startHeight, {}});
+    for (; it.Valid() && it.Key().height >= startHeight && it.Key().height <= endHeight; it.Next()) {
+        auto var = GovVariable::Create(it.Key().name);
+        if (var) {
+            it.Value(*var);
+            govVars.emplace_back(it.Key().height, var);
+        }
+    }
+    return govVars;
+}
+
 std::map<std::string, std::map<uint64_t, std::shared_ptr<GovVariable>>> CGovView::GetAllStoredVariables()
 {
     // Populate map by var name as key and map of height and Gov vars as elements.

src/masternodes/gv.h

@@ -44,6 +44,7 @@ class CGovView : public virtual CStorageView
 
     Res SetStoredVariables(const std::set<std::shared_ptr<GovVariable>>& govVars, const uint32_t height);
     std::set<std::shared_ptr<GovVariable>> GetStoredVariables(const uint32_t height);
+    std::vector<std::pair<uint32_t, std::shared_ptr<GovVariable>>> GetStoredVariablesRange(const uint32_t startHeight, const uint32_t endHeight);
     std::map<std::string, std::map<uint64_t, std::shared_ptr<GovVariable>>> GetAllStoredVariables();
     void EraseStoredVariables(const uint32_t height);
 

src/masternodes/mn_checks.cpp

@@ -1700,9 +1700,32 @@ class CCustomTxApplyVisitor : public CCustomTxVisitor
         }
 
         // Validate GovVariables before storing
-        auto result = obj.govVar->Validate(mnview);
-        if (!result) {
-            return Res::Err("%s: %s", obj.govVar->GetName(), result.msg);
+        // TODO remove GW check after fork height. No conflict expected as attrs should not been set by height before.
+        if (height >= uint32_t(consensus.FortCanningCrunchHeight) && obj.govVar->GetName() == "ATTRIBUTES") {
+
+            auto govVar = mnview.GetAttributes();
+            if (!govVar) {
+                return Res::Err("%s: %s", obj.govVar->GetName(), "Failed to get existing ATTRIBUTES");
+            }
+
+            auto storedGovVars = mnview.GetStoredVariablesRange(height, obj.startHeight);
+            storedGovVars.emplace_back(obj.startHeight, obj.govVar);
+
+            Res res{};
+            CCustomCSView govCache(mnview);
+            for (const auto& [varHeight, var] : storedGovVars) {
+                if (var->GetName() == "ATTRIBUTES") {
+                    if (!(res = govVar->Import(var->Export())) ||
+                        !(res = govVar->Validate(govCache)) ||
+                        !(res = govVar->Apply(govCache, varHeight))) {
+                        return Res::Err("%s: Cumulative application of Gov vars failed: %s", obj.govVar->GetName(), res.msg);
+                    }
+                }
+            }
+        } else {
+            auto result = obj.govVar->Validate(mnview);
+            if (!result)
+                return Res::Err("%s: %s", obj.govVar->GetName(), result.msg);
         }
 
         // Store pending Gov var change

src/validation.cpp

@@ -2835,7 +2835,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
     CreationTxs creationTxs;
     auto counter_n = 1;
     for (const auto& [id, multiplier] : splits) {
-        LogPrintf("Preparing for token split (id=%d, mul=%d, n=%d/%d, height: %d)\n", 
+        LogPrintf("Preparing for token split (id=%d, mul=%d, n=%d/%d, height: %d)\n",
         id, multiplier, counter_n++, splits.size(), pindex->nHeight);
         uint256 tokenCreationTx{};
         std::vector<uint256> poolCreationTx;
@@ -2872,7 +2872,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
 
         std::vector<std::pair<DCT_ID, uint256>> poolPairs;
         poolPairs.reserve(poolsToMigrate.size());
-        std::transform(poolsToMigrate.begin(), poolsToMigrate.end(), 
+        std::transform(poolsToMigrate.begin(), poolsToMigrate.end(),
             poolCreationTx.begin(), std::back_inserter(poolPairs),
             [](DCT_ID a, uint256 b) { return std::make_pair(a, b); });
 
@@ -3564,7 +3564,7 @@ void CChainState::ProcessFutures(const CBlockIndex* pindex, CCustomCSView& cache
         if (source->symbol == "DUSD") {
             const DCT_ID destId{futuresValues.destination};
             const auto destToken = view.GetLoanTokenByID(destId);
-            assert(destToken);            
+            assert(destToken);
             try {
                 const auto& premiumPrice = futuresPrices.at(destId).premium;
                 if (premiumPrice > 0) {
@@ -3857,7 +3857,7 @@ static Res PoolSplits(CCustomCSView& view, CAmount& totalBalance, ATTRIBUTES& at
     auto time = GetTimeMillis();
     LogPrintf("Pool migration in progress.. (token %d -> %d, height: %d)\n",
             oldTokenId.v, newTokenId.v, pindex->nHeight);
-    
+
     try {
         assert(creationTxs.count(oldTokenId.v));
         for (const auto& [oldPoolId, creationTx] : creationTxs.at(oldTokenId.v).second) {
@@ -3911,7 +3911,7 @@ static Res PoolSplits(CCustomCSView& view, CAmount& totalBalance, ATTRIBUTES& at
             }
 
             LogPrintf("Pool migration: Old pair (id: %d, token a: %d, b: %d, reserve a: %d, b: %d, liquidity: %d)\n",
-                oldPoolId.v, oldPoolPair->idTokenA.v, oldPoolPair->idTokenB.v, 
+                oldPoolId.v, oldPoolPair->idTokenA.v, oldPoolPair->idTokenB.v,
                 oldPoolPair->reserveA, oldPoolPair->reserveB, oldPoolPair->totalLiquidity);
 
             CPoolPair newPoolPair{*oldPoolPair};
@@ -4059,7 +4059,7 @@ static Res PoolSplits(CCustomCSView& view, CAmount& totalBalance, ATTRIBUTES& at
 
             LogPrintf("Pool migration: New pair (id: %d, token a: %d, b: %d, reserve a: %d, b: %d, liquidity: %d)\n",
                 newPoolId.v,
-                newPoolPair.idTokenA.v, newPoolPair.idTokenB.v, 
+                newPoolPair.idTokenA.v, newPoolPair.idTokenB.v,
                 newPoolPair.reserveA, newPoolPair.reserveB, newPoolPair.totalLiquidity);
 
             res = view.SetPoolPair(newPoolId, pindex->nHeight, newPoolPair);
@@ -4222,7 +4222,7 @@ void CChainState::ProcessTokenSplits(const CBlock& block, const CBlockIndex* pin
     for (const auto& [id, multiplier] : splits) {
         auto time = GetTimeMillis();
         LogPrintf("Token split in progress.. (id: %d, mul: %d, height: %d)\n", id, multiplier, pindex->nHeight);
-        
+
         if (!cache.AreTokensLocked({id})) {
             LogPrintf("Token split failed. No locks.\n");
             continue;
@@ -4337,7 +4337,7 @@ void CChainState::ProcessTokenSplits(const CBlock& block, const CBlockIndex* pin
         });
 
         LogPrintf("Token split info: Rebalance "  /* Continued */
-        "(id: %d, symbol: %s, add: %d, sub: %d, total: %d)\n", 
+        "(id: %d, symbol: %s, add: %d, sub: %d, total: %d)\n",
         id, newToken.symbol, addAccounts.size(), subAccounts.size(), totalBalance);
 
         res = view.AddMintedTokens(newTokenId, totalBalance);

test/functional/feature_setgov.py

@@ -657,6 +657,7 @@ def run_test(self):
         assert_raises_rpc_error(-32600, "ATTRIBUTES: Pool tokens cannot be split", self.nodes[0].setgov, {"ATTRIBUTES":{'v0/oracles/splits/1201': '1/50'}})
         assert_raises_rpc_error(-32600, "ATTRIBUTES: Cannot be set at or below current height", self.nodes[0].setgov, {"ATTRIBUTES":{f'v0/oracles/splits/{self.nodes[0].getblockcount()}': '5/50'}})
         assert_raises_rpc_error(-32600, "ATTRIBUTES: Cannot be set at or below current height", self.nodes[0].setgov, {"ATTRIBUTES":{f'v0/oracles/splits/{self.nodes[0].getblockcount() + 1}': '5/50'}})
+        assert_raises_rpc_error(-32600, "ATTRIBUTES: Cumulative application of Gov vars failed: Cannot be set at or below current height", self.nodes[0].setgovheight, {"ATTRIBUTES":{f'v0/oracles/splits/2000': '5/50'}}, 2000)
         assert_raises_rpc_error(-32600, "No loan token with id (4)", self.nodes[0].setgov, {"ATTRIBUTES":{'v0/oracles/splits/4000':'4/2'}})
         assert_raises_rpc_error(-32600, "ATTRIBUTES: Price feed DUFF/USD does not belong to any oracle", self.nodes[0].setgov, {"ATTRIBUTES":{'v0/token/5/fixed_interval_price_id':'DUFF/USD'}})
         assert_raises_rpc_error(-5, "Empty token / currency", self.nodes[0].setgov, {"ATTRIBUTES":{'v0/token/5/fixed_interval_price_id':' /USD'}})
@@ -710,9 +711,15 @@ def run_test(self):
         self.nodes[0].generate(1)
         assert_equal(self.nodes[0].listgovs()[8][1]['3928'], {'v0/locks/token/4': 'true'})
 
-        self.nodes[0].setgov({"ATTRIBUTES":{'v0/oracles/splits/4001':'5/10'}})
+        self.nodes[0].setgovheight({"ATTRIBUTES":{'v0/oracles/splits/4001':'5/10'}}, 1210)
         self.nodes[0].generate(1)
 
+        attriutes = self.nodes[0].getgov('ATTRIBUTES')['ATTRIBUTES']
+        assert_equal(attriutes['v0/oracles/splits/4000'], '4/50')
+        assert_equal(attriutes['v0/oracles/splits/4001'], '5/5')
+
+        self.nodes[0].generate(4)
+
         attriutes = self.nodes[0].getgov('ATTRIBUTES')['ATTRIBUTES']
         assert_equal(attriutes['v0/oracles/splits/4000'], '4/50')
         assert_equal(attriutes['v0/oracles/splits/4001'], '5/10')