[ICD] Add counter invalidation events to the ICD Test Event trigger handler

src/BUILD.gn

@@ -99,6 +99,7 @@ if (chip_build_tests) {
         "${chip_root}/src/lib/support/tests",
         "${chip_root}/src/lib/support/tests:tests_nltest",
         "${chip_root}/src/protocols/secure_channel/tests",
+        "${chip_root}/src/protocols/secure_channel/tests:tests_nltest",
         "${chip_root}/src/system/tests",
         "${chip_root}/src/transport/tests",
       ]

src/app/icd/server/ICDManager.cpp

@@ -30,8 +30,10 @@
 namespace {
 enum class ICDTestEventTriggerEvent : uint64_t
 {
-    kAddActiveModeReq    = 0x0046'0000'00000001,
-    kRemoveActiveModeReq = 0x0046'0000'00000002,
+    kAddActiveModeReq            = 0x0046'0000'00000001,
+    kRemoveActiveModeReq         = 0x0046'0000'00000002,
+    kInvalidateHalfCounterValues = 0x0046'0000'00000003,
+    kInvalidateAllCounterValues  = 0x0046'0000'00000004,
 };
 } // namespace
 
@@ -666,6 +668,14 @@ CHIP_ERROR ICDManager::HandleEventTrigger(uint64_t eventTrigger)
     case ICDTestEventTriggerEvent::kRemoveActiveModeReq:
         SetKeepActiveModeRequirements(KeepActiveFlag::kTestEventTriggerActiveMode, false);
         break;
+#if CHIP_CONFIG_ENABLE_ICD_CIP
+    case ICDTestEventTriggerEvent::kInvalidateHalfCounterValues:
+        err = ICDConfigurationData::GetInstance().GetICDCounter().InvalidateHalfCheckInCouterValues();
+        break;
+    case ICDTestEventTriggerEvent::kInvalidateAllCounterValues:
+        err = ICDConfigurationData::GetInstance().GetICDCounter().InvalidateAllCheckInCounterValues();
+        break;
+#endif // CHIP_CONFIG_ENABLE_ICD_CIP
     default:
         err = CHIP_ERROR_INVALID_ARGUMENT;
         break;

src/app/tests/TestICDManager.cpp

@@ -70,8 +70,10 @@ constexpr uint8_t kKeyBuffer2b[] = {
 // Taken from the ICDManager Implementation
 enum class ICDTestEventTriggerEvent : uint64_t
 {
-    kAddActiveModeReq    = 0x0046'0000'00000001,
-    kRemoveActiveModeReq = 0x0046'0000'00000002,
+    kAddActiveModeReq            = 0x0046'0000'00000001,
+    kRemoveActiveModeReq         = 0x0046'0000'00000002,
+    kInvalidateHalfCounterValues = 0x0046'0000'00000003,
+    kInvalidateAllCounterValues  = 0x0046'0000'00000004,
 };
 
 class TestICDStateObserver : public app::ICDStateObserver
@@ -742,6 +744,48 @@ class TestICDManager
         NL_TEST_ASSERT(aSuite, ctx->mICDManager.mOperationalState == ICDManager::OperationalState::IdleMode);
     }
 
+    static void TestHandleTestEventTriggerInvalidateHalfCounterValues(nlTestSuite * aSuite, void * aContext)
+    {
+        TestContext * ctx = static_cast<TestContext *>(aContext);
+
+        constexpr uint32_t startValue    = 1;
+        constexpr uint32_t expectedValue = 2147483648;
+
+        // Set starting value
+        uint32_t currentValue = ICDConfigurationData::GetInstance().GetICDCounter().GetValue();
+        uint32_t delta        = startValue - currentValue;
+
+        NL_TEST_ASSERT(aSuite, ICDConfigurationData::GetInstance().GetICDCounter().AdvanceBy(delta) == CHIP_NO_ERROR);
+        NL_TEST_ASSERT(aSuite, ICDConfigurationData::GetInstance().GetICDCounter().GetValue() == startValue);
+
+        // Trigger ICD kInvalidateHalfCounterValues event
+        ctx->mICDManager.HandleEventTrigger(static_cast<uint64_t>(ICDTestEventTriggerEvent::kInvalidateHalfCounterValues));
+
+        // Validate counter has the expected value
+        NL_TEST_ASSERT(aSuite, ICDConfigurationData::GetInstance().GetICDCounter().GetValue() == expectedValue);
+    }
+
+    static void TestHandleTestEventTriggerInvalidateAllCounterValues(nlTestSuite * aSuite, void * aContext)
+    {
+        TestContext * ctx = static_cast<TestContext *>(aContext);
+
+        constexpr uint32_t startValue    = 105;
+        constexpr uint32_t expectedValue = 104;
+
+        // Set starting value
+        uint32_t currentValue = ICDConfigurationData::GetInstance().GetICDCounter().GetValue();
+        uint32_t delta        = startValue - currentValue;
+
+        NL_TEST_ASSERT(aSuite, ICDConfigurationData::GetInstance().GetICDCounter().AdvanceBy(delta) == CHIP_NO_ERROR);
+        NL_TEST_ASSERT(aSuite, ICDConfigurationData::GetInstance().GetICDCounter().GetValue() == startValue);
+
+        // Trigger ICD kInvalidateAllCounterValues event
+        ctx->mICDManager.HandleEventTrigger(static_cast<uint64_t>(ICDTestEventTriggerEvent::kInvalidateAllCounterValues));
+
+        // Validate counter has the expected value
+        NL_TEST_ASSERT(aSuite, ICDConfigurationData::GetInstance().GetICDCounter().GetValue() == expectedValue);
+    }
+
     /**
      * @brief Test verifies when OnEnterIdleMode is called during normal operations.
      *        Without the ActiveMode timer being extended
@@ -1083,6 +1127,10 @@ static const nlTest sTests[] = {
     NL_TEST_DEF("TestICDStayActive", TestICDManager::TestICDMStayActive),
     NL_TEST_DEF("TestShouldCheckInMsgsBeSentAtActiveModeFunction", TestICDManager::TestShouldCheckInMsgsBeSentAtActiveModeFunction),
     NL_TEST_DEF("TestHandleTestEventTriggerActiveModeReq", TestICDManager::TestHandleTestEventTriggerActiveModeReq),
+    NL_TEST_DEF("TestHandleTestEventTriggerInvalidateHalfCounterValues",
+                TestICDManager::TestHandleTestEventTriggerInvalidateHalfCounterValues),
+    NL_TEST_DEF("TestHandleTestEventTriggerInvalidateAllCounterValues",
+                TestICDManager::TestHandleTestEventTriggerInvalidateAllCounterValues),
     NL_TEST_DEF("TestICDStateObserverOnEnterIdleModeActiveModeDuration",
                 TestICDManager::TestICDStateObserverOnEnterIdleModeActiveModeDuration),
     NL_TEST_DEF("TestICDStateObserverOnEnterIdleModeActiveModeThreshold",

src/lib/support/CHIPCounter.h

@@ -45,16 +45,23 @@ class Counter
     virtual ~Counter() {}
 
     /**
-     *  @brief
-     *  Advance the value of the counter.
+     *  @brief Advance the value of the counter.
      *
      *  @return A CHIP error code if anything failed, CHIP_NO_ERROR otherwise.
      */
     virtual CHIP_ERROR Advance() = 0;
 
     /**
-     *  @brief
-     *  Get the current value of the counter.
+     * @brief Advances the current counter value by N
+     *
+     * @param value value of N
+     *
+     * @return A CHIP error code if anything failed, CHIP_NO_ERROR otherwise.
+     */
+    virtual CHIP_ERROR AdvanceBy(T value) = 0;
+
+    /**
+     *  @brief Get the current value of the counter.
      *
      *  @return The current value of the counter.
      */
@@ -76,8 +83,7 @@ class MonotonicallyIncreasingCounter : public Counter<T>
     ~MonotonicallyIncreasingCounter() override{};
 
     /**
-     *  @brief
-     *    Initialize a MonotonicallyIncreasingCounter object.
+     *  @brief Initialize a MonotonicallyIncreasingCounter object.
      *
      *  @param[in] aStartValue  The starting value of the counter.
      *
@@ -93,8 +99,7 @@ class MonotonicallyIncreasingCounter : public Counter<T>
     }
 
     /**
-     *  @brief
-     *  Advance the value of the counter.
+     *  @brief Advance the value of the counter.
      *
      *  @return A CHIP error code if something fails, CHIP_NO_ERROR otherwise
      */
@@ -108,8 +113,20 @@ class MonotonicallyIncreasingCounter : public Counter<T>
     }
 
     /**
-     *  @brief
-     *  Get the current value of the counter.
+     * @brief Advances the current counter value by N
+     *
+     * @param value value of N
+     *
+     * @return A CHIP error code if something fails, CHIP_NO_ERROR otherwise
+     */
+    CHIP_ERROR AdvanceBy(T value) override
+    {
+        mCounterValue = static_cast<T>(mCounterValue + value);
+        return CHIP_NO_ERROR;
+    }
+
+    /**
+     *  @brief Get the current value of the counter.
      *
      *  @return The current value of the counter.
      */

src/lib/support/PersistedCounter.h

@@ -32,6 +32,7 @@
 #include <lib/support/CHIPCounter.h>
 #include <lib/support/CodeUtils.h>
 #include <lib/support/DefaultStorageKeyAllocator.h>
+#include <limits>
 
 namespace chip {
 
@@ -104,16 +105,50 @@ class PersistedCounter : public MonotonicallyIncreasingCounter<T>
         }
 #endif
 
-        ReturnErrorOnFailure(PersistNextEpochStart(startValue + aEpoch));
+        ReturnErrorOnFailure(PersistNextEpochStart(static_cast<T>(startValue + aEpoch)));
 
         // This will set the starting value, after which we're ready.
         return MonotonicallyIncreasingCounter<T>::Init(startValue);
     }
 
     /**
-     *  @brief
-     *  Increment the counter and write to persisted storage if we've completed
-     *  the current epoch.
+     *  @brief Increment the counter by N and write to persisted storage if we've completed the current epoch.
+     *
+     *  @param value value of N
+     *
+     *  @return Any error returned by a write to persisted storage.
+     */
+    CHIP_ERROR AdvanceBy(T value) override
+    {
+        VerifyOrReturnError(mStorage != nullptr, CHIP_ERROR_INCORRECT_STATE);
+        VerifyOrReturnError(mKey.IsInitialized(), CHIP_ERROR_INCORRECT_STATE);
+
+        // If value is 0, we do not need to do anything
+        VerifyOrReturnError(value > 0, CHIP_NO_ERROR);
+
+        // We should update the persisted epoch value if :
+        // 1- Sum of the current counter and value is greater or equal to the mNextEpoch.
+        //    This is the standard operating case.
+        // 2- Increasing the current counter by value would cause a roll over. This would cause the current value to be < to the
+        //    mNextEpoch so we force an update.
+        bool shouldDoEpochUpdate = ((MonotonicallyIncreasingCounter<T>::GetValue() + value) >= mNextEpoch) ||
+            (MonotonicallyIncreasingCounter<T>::GetValue() > std::numeric_limits<T>::max() - value);
+
+        ReturnErrorOnFailure(MonotonicallyIncreasingCounter<T>::AdvanceBy(value));
+
+        if (shouldDoEpochUpdate)
+        {
+            // Since AdvanceBy allows the counter to be increased by an arbitrary value, it is possible that the new counter value
+            // is greater than mNextEpoch + mEpoch. As such, we want the next Epoch value to be calculated from the new current
+            // value.
+            PersistAndVerifyNextEpochStart(MonotonicallyIncreasingCounter<T>::GetValue());
+        }
+
+        return CHIP_NO_ERROR;
+    }
+
+    /**
+     *  @brief Increment the counter and write to persisted storage if we've completed the current epoch.
      *
      *  @return Any error returned by a write to persisted storage.
      */
@@ -126,18 +161,32 @@ class PersistedCounter : public MonotonicallyIncreasingCounter<T>
 
         if (MonotonicallyIncreasingCounter<T>::GetValue() >= mNextEpoch)
         {
-            // Value advanced past the previously persisted "start point".
-            // Ensure that a new starting point is persisted.
-            ReturnErrorOnFailure(PersistNextEpochStart(mNextEpoch + mEpoch));
-
-            // Advancing the epoch should have ensured that the current value
-            // is valid
-            VerifyOrReturnError(MonotonicallyIncreasingCounter<T>::GetValue() < mNextEpoch, CHIP_ERROR_INTERNAL);
+            ReturnErrorOnFailure(PersistAndVerifyNextEpochStart(mNextEpoch));
         }
+
         return CHIP_NO_ERROR;
     }
 
 private:
+    CHIP_ERROR PersistAndVerifyNextEpochStart(T refEpoch)
+    {
+        // Value advanced past the previously persisted "start point".
+        // Ensure that a new starting point is persisted.
+        ReturnErrorOnFailure(PersistNextEpochStart(static_cast<T>(refEpoch + mEpoch)));
+
+        // Advancing the epoch should have ensured that the current value is valid
+        VerifyOrReturnError(static_cast<T>(MonotonicallyIncreasingCounter<T>::GetValue() + mEpoch) == mNextEpoch,
+                            CHIP_ERROR_INTERNAL);
+
+        // Previous check did not take into consideration that the counter value can be equal to the max counter value or
+        // rollover.
+        // TODO(#33175): PersistedCounter allows rollover so this check is incorrect. We need a Counter class that adequatly
+        // manages rollover behavior for counters that cannot rollover.
+        // VerifyOrReturnError(MonotonicallyIncreasingCounter<T>::GetValue() < mNextEpoch, CHIP_ERROR_INTERNAL);
+
+        return CHIP_NO_ERROR;
+    }
+
     /**
      *  @brief
      *    Write out the counter value to persistent storage.
@@ -146,7 +195,8 @@ class PersistedCounter : public MonotonicallyIncreasingCounter<T>
      *
      *  @return Any error returned by a write to persistent storage.
      */
-    CHIP_ERROR PersistNextEpochStart(T aStartValue)
+    CHIP_ERROR
+    PersistNextEpochStart(T aStartValue)
     {
         mNextEpoch = aStartValue;
 #if CHIP_CONFIG_PERSISTED_COUNTER_DEBUG_LOGGING

src/lib/support/tests/BUILD.gn

@@ -28,11 +28,13 @@ chip_test_suite("tests") {
     "TestBufferWriter.cpp",
     "TestBytesCircularBuffer.cpp",
     "TestBytesToHex.cpp",
+    "TestCHIPCounter.cpp",
     "TestDefer.cpp",
     "TestErrorStr.cpp",
     "TestFixedBufferAllocator.cpp",
     "TestFold.cpp",
     "TestIniEscaping.cpp",
+    "TestPersistedCounter.cpp",
     "TestPrivateHeap.cpp",
     "TestSafeInt.cpp",
     "TestSafeString.cpp",
@@ -74,13 +76,11 @@ chip_test_suite_using_nltest("tests_nltest") {
   output_name = "libSupportTestsNL"
 
   test_sources = [
-    "TestCHIPCounter.cpp",
     "TestCHIPMem.cpp",
     "TestCHIPMemString.cpp",
     "TestIntrusiveList.cpp",
     "TestJsonToTlv.cpp",
     "TestJsonToTlvToJson.cpp",
-    "TestPersistedCounter.cpp",
     "TestPool.cpp",
     "TestStateMachine.cpp",
     "TestThreadOperationalDataset.cpp",