From 154da5a95e795eac16a6072fb11ab9cdbcde2817 Mon Sep 17 00:00:00 2001
From: Peter John Bushnell <bushsolo@gmail.com>
Date: Tue, 9 Apr 2024 11:00:08 +0100
Subject: [PATCH] Check future swap limitation block period more than sample
 period (#2886)

---
 src/dfi/errors.h                              |  4 +++
 src/dfi/govvariables/attributes.cpp           | 23 +++++++++++-
 .../feature_future_swap_limitation.py         | 36 +++++++++++++++++++
 3 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/src/dfi/errors.h b/src/dfi/errors.h
index baf1faf2c7..1a4689d07d 100644
--- a/src/dfi/errors.h
+++ b/src/dfi/errors.h
@@ -156,6 +156,8 @@ class DeFiErrors {
 
     static Res GovVarVerifyPositiveNumber() { return Res::Err("Value must be a positive integer"); }
 
+    static Res GovVarVerifyMoreThanZero() { return Res::Err("Value must more than zero"); }
+
     static Res GovVarInvalidNumber() { return Res::Err("Amount must be a valid number"); }
 
     static Res GovVarVerifySplitValues() { return Res::Err("Two int values expected for split in id/mutliplier"); }
@@ -288,6 +290,8 @@ class DeFiErrors {
 
     static Res GovVarUnsupportedValue() { return Res::Err("Unsupported value"); }
 
+    static Res GovVarValidateBlockPeriod() { return Res::Err("Block period must be more than sampling period"); }
+
     static Res GovVarValidateUnsupportedKey() { return Res::Err("Unsupported key"); }
 
     static Res GovVarValidateSplitDFI() { return Res::Err("Tokenised DFI cannot be split"); }
diff --git a/src/dfi/govvariables/attributes.cpp b/src/dfi/govvariables/attributes.cpp
index 4a06bd7d95..05db304162 100644
--- a/src/dfi/govvariables/attributes.cpp
+++ b/src/dfi/govvariables/attributes.cpp
@@ -10,6 +10,7 @@
 #include <dfi/historywriter.h>    /// CHiistoryWriter
 #include <dfi/masternodes.h>      /// CCustomCSView
 #include <dfi/mn_checks.h>        /// GetAggregatePrice / CustomTxType
+#include <dfi/validation.h>       /// DEFAULT_LIQUIDITY_CALC_SAMPLING_PERIOD
 #include <validation.h>           /// GetNextAccPosition
 
 #include <amount.h>   /// GetDecimaleString
@@ -508,6 +509,14 @@ static ResVal<CAttributeValue> VerifyInt64(const std::string &str) {
     return {int64, Res::Ok()};
 }
 
+static ResVal<CAttributeValue> VerifyMoreThenZeroInt64(const std::string &str) {
+    CAmount int64;
+    if (!ParseInt64(str, &int64) || int64 < 1) {
+        return DeFiErrors::GovVarVerifyMoreThanZero();
+    }
+    return {int64, Res::Ok()};
+}
+
 static ResVal<CAttributeValue> VerifyFloat(const std::string &str) {
     CAmount amount = 0;
     if (!ParseFixedPoint(str, 8, &amount)) {
@@ -804,7 +813,7 @@ const std::map<uint8_t, std::map<uint8_t, std::function<ResVal<CAttributeValue>(
                  {DFIPKeys::EmissionUnusedFund, VerifyBool},
                  {DFIPKeys::MintTokens, VerifyBool},
                  {DFIPKeys::TransferDomain, VerifyBool},
-                 {DFIPKeys::LiquidityCalcSamplingPeriod, VerifyInt64},
+                 {DFIPKeys::LiquidityCalcSamplingPeriod, VerifyMoreThenZeroInt64},
                  {DFIPKeys::AverageLiquidityPercentage, VerifyPctInt64},
              }},
             {AttributeTypes::Locks,
@@ -2054,6 +2063,18 @@ Res ATTRIBUTES::Validate(const CCustomCSView &view) const {
                     if (view.GetLastHeight() < Params().GetConsensus().DF23Height) {
                         return DeFiErrors::GovVarValidateDF23Height();
                     }
+                    if (attrV0->key == DFIPKeys::BlockPeriod) {
+                        CDataStructureV0 samplingKey{
+                            AttributeTypes::Param, ParamIDs::DFIP2211F, DFIPKeys::LiquidityCalcSamplingPeriod};
+                        const auto samplingPeriod = GetValue(samplingKey, DEFAULT_LIQUIDITY_CALC_SAMPLING_PERIOD);
+                        const auto blockPeriod = std::get_if<CAmount>(&value);
+                        if (!blockPeriod) {
+                            return DeFiErrors::GovVarUnsupportedValue();
+                        }
+                        if (*blockPeriod < samplingPeriod) {
+                            return DeFiErrors::GovVarValidateBlockPeriod();
+                        }
+                    }
                 } else if (attrV0->typeId != ParamIDs::DFIP2201) {
                     return Res::Err("Unrecognised param id");
                 }
diff --git a/test/functional/feature_future_swap_limitation.py b/test/functional/feature_future_swap_limitation.py
index 54380b80f1..2a69e3e6d4 100755
--- a/test/functional/feature_future_swap_limitation.py
+++ b/test/functional/feature_future_swap_limitation.py
@@ -221,6 +221,30 @@ def test_future_swap_limitation(self):
         # Move to fork height
         self.nodes[0].generate(150 - self.nodes[0].getblockcount())
 
+        # Try and set block period below default sample size
+        assert_raises_rpc_error(
+            -32600,
+            "Block period must be more than sampling period",
+            self.nodes[0].setgov,
+            {
+                "ATTRIBUTES": {
+                    "v0/params/dfip2211f/block_period": "20",
+                }
+            },
+        )
+
+        # Try and set sample size of zero
+        assert_raises_rpc_error(
+            -5,
+            "Value must more than zero",
+            self.nodes[0].setgov,
+            {
+                "ATTRIBUTES": {
+                    "v0/params/dfip2211f/liquidity_calc_sampling_period": "0",
+                }
+            },
+        )
+
         # Set future swap limitaiton
         self.nodes[0].setgov(
             {
@@ -234,6 +258,18 @@ def test_future_swap_limitation(self):
         )
         self.nodes[0].generate(1)
 
+        # Try and set block period below defined sample size
+        assert_raises_rpc_error(
+            -32600,
+            "Block period must be more than sampling period",
+            self.nodes[0].setgov,
+            {
+                "ATTRIBUTES": {
+                    "v0/params/dfip2211f/block_period": "1",
+                }
+            },
+        )
+
         # Verify Gov vars
         result = self.nodes[0].getgov("ATTRIBUTES")["ATTRIBUTES"]
         assert_equal(result["v0/params/dfip2211f/active"], "true")