[dataset] enforce successful Dataset saves in non-volatile (#10237)

This commit updates error handling when saving Active/Pending
Operational Datasets in non-volatile settings. Previously,
`kErrorNotImplemented` was permitted as a return value due to legacy
behavior. Since non-volatile storage is now a mandatory requirement
for Thread operation, we enforce successful saves by asserting on any
error encountered during this process. Methods returning error types
are updated, and some now return `void`.

The public APIs `otDatasetSetActive()` and `otDatasetSetPending()` are
affected by this change. They now always return `OT_ERROR_NONE` and
can essentially be treated as having a void return type. The `otError`
return type is maintained solely for backward compatibility.

include/openthread/dataset.h

@@ -380,12 +380,15 @@ otError otDatasetGetActiveTlvs(otInstance *aInstance, otOperationalDatasetTlvs *
  * its Parent. Note that a router-capable device will not transition to the Router or Leader roles until it has a
  * complete Active Dataset.
  *
+ * This function consistently returns `OT_ERROR_NONE` and can effectively be treated as having a `void` return type.
+ * Previously, other errors (e.g., `OT_ERROR_NOT_IMPLEMENTED`) were allowed for legacy reasons. However, as
+ * non-volatile storage is now mandatory for Thread operation, any failure to save the dataset will trigger an
+ * assertion. The `otError` return type is retained for backward compatibility.
+ *
  * @param[in]  aInstance A pointer to an OpenThread instance.
  * @param[in]  aDataset  A pointer to the Active Operational Dataset.
  *
- * @retval OT_ERROR_NONE             Successfully set the Active Operational Dataset.
- * @retval OT_ERROR_NO_BUFS          Insufficient buffer space to set the Active Operational Dataset.
- * @retval OT_ERROR_NOT_IMPLEMENTED  The platform does not implement settings functionality.
+ * @retval OT_ERROR_NONE    Successfully set the Active Operational Dataset.
  *
  */
 otError otDatasetSetActive(otInstance *aInstance, const otOperationalDataset *aDataset);
@@ -409,9 +412,8 @@ otError otDatasetSetActive(otInstance *aInstance, const otOperationalDataset *aD
  * @param[in]  aInstance A pointer to an OpenThread instance.
  * @param[in]  aDataset  A pointer to the Active Operational Dataset.
  *
- * @retval OT_ERROR_NONE             Successfully set the Active Operational Dataset.
- * @retval OT_ERROR_NO_BUFS          Insufficient buffer space to set the Active Operational Dataset.
- * @retval OT_ERROR_NOT_IMPLEMENTED  The platform does not implement settings functionality.
+ * @retval OT_ERROR_NONE          Successfully set the Active Operational Dataset.
+ * @retval OT_ERROR_INVALID_ARGS  The @p aDataset is invalid. It is too long or contains incorrect TLV formatting.
  *
  */
 otError otDatasetSetActiveTlvs(otInstance *aInstance, const otOperationalDatasetTlvs *aDataset);
@@ -443,12 +445,15 @@ otError otDatasetGetPendingTlvs(otInstance *aInstance, otOperationalDatasetTlvs
 /**
  * Sets the Pending Operational Dataset.
  *
+ * This function consistently returns `OT_ERROR_NONE` and can effectively be treated as having a `void` return type.
+ * Previously, other errors (e.g., `OT_ERROR_NOT_IMPLEMENTED`) were allowed for legacy reasons. However, as
+ * non-volatile storage is now mandatory for Thread operation, any failure to save the dataset will trigger an
+ * assertion. The `otError` return type is retained for backward compatibility.
+ *
  * @param[in]  aInstance A pointer to an OpenThread instance.
  * @param[in]  aDataset  A pointer to the Pending Operational Dataset.
  *
- * @retval OT_ERROR_NONE             Successfully set the Pending Operational Dataset.
- * @retval OT_ERROR_NO_BUFS          Insufficient buffer space to set the Pending Operational Dataset.
- * @retval OT_ERROR_NOT_IMPLEMENTED  The platform does not implement settings functionality.
+ * @retval OT_ERROR_NONE    Successfully set the Pending Operational Dataset.
  *
  */
 otError otDatasetSetPending(otInstance *aInstance, const otOperationalDataset *aDataset);
@@ -459,9 +464,8 @@ otError otDatasetSetPending(otInstance *aInstance, const otOperationalDataset *a
  * @param[in]  aInstance A pointer to an OpenThread instance.
  * @param[in]  aDataset  A pointer to the Pending Operational Dataset.
  *
- * @retval OT_ERROR_NONE             Successfully set the Pending Operational Dataset.
- * @retval OT_ERROR_NO_BUFS          Insufficient buffer space to set the Pending Operational Dataset.
- * @retval OT_ERROR_NOT_IMPLEMENTED  The platform does not implement settings functionality.
+ * @retval OT_ERROR_NONE          Successfully set the Pending Operational Dataset.
+ * @retval OT_ERROR_INVALID_ARGS  The @p aDataset is invalid. It is too long or contains incorrect TLV formatting.
  *
  */
 otError otDatasetSetPendingTlvs(otInstance *aInstance, const otOperationalDatasetTlvs *aDataset);

include/openthread/instance.h

@@ -53,7 +53,7 @@ extern "C" {
  * @note This number versions both OpenThread platform and user APIs.
  *
  */
-#define OPENTHREAD_API_VERSION (412)
+#define OPENTHREAD_API_VERSION (413)
 
 /**
  * @addtogroup api-instance

src/core/api/dataset_api.cpp

@@ -61,7 +61,9 @@ otError otDatasetGetActiveTlvs(otInstance *aInstance, otOperationalDatasetTlvs *
 
 otError otDatasetSetActive(otInstance *aInstance, const otOperationalDataset *aDataset)
 {
-    return AsCoreType(aInstance).Get<MeshCoP::ActiveDatasetManager>().SaveLocal(AsCoreType(aDataset));
+    AsCoreType(aInstance).Get<MeshCoP::ActiveDatasetManager>().SaveLocal(AsCoreType(aDataset));
+
+    return OT_ERROR_NONE;
 }
 
 otError otDatasetSetActiveTlvs(otInstance *aInstance, const otOperationalDatasetTlvs *aDataset)
@@ -85,7 +87,9 @@ otError otDatasetGetPendingTlvs(otInstance *aInstance, otOperationalDatasetTlvs
 
 otError otDatasetSetPending(otInstance *aInstance, const otOperationalDataset *aDataset)
 {
-    return AsCoreType(aInstance).Get<MeshCoP::PendingDatasetManager>().SaveLocal(AsCoreType(aDataset));
+    AsCoreType(aInstance).Get<MeshCoP::PendingDatasetManager>().SaveLocal(AsCoreType(aDataset));
+
+    return OT_ERROR_NONE;
 }
 
 otError otDatasetSetPendingTlvs(otInstance *aInstance, const otOperationalDatasetTlvs *aDataset)

src/core/common/settings.cpp

@@ -35,6 +35,7 @@
 
 #include "common/array.hpp"
 #include "common/code_utils.hpp"
+#include "common/debug.hpp"
 #include "common/locator_getters.hpp"
 #include "common/num_utils.hpp"
 #include "instance/instance.hpp"
@@ -221,14 +222,14 @@ Settings::Key Settings::KeyForDatasetType(MeshCoP::Dataset::Type aType)
     return (aType == MeshCoP::Dataset::kActive) ? kKeyActiveDataset : kKeyPendingDataset;
 }
 
-Error Settings::SaveOperationalDataset(MeshCoP::Dataset::Type aType, const MeshCoP::Dataset &aDataset)
+void Settings::SaveOperationalDataset(MeshCoP::Dataset::Type aType, const MeshCoP::Dataset &aDataset)
 {
     Key   key   = KeyForDatasetType(aType);
     Error error = Get<SettingsDriver>().Set(key, aDataset.GetBytes(), aDataset.GetLength());
 
     Log(kActionSave, error, key);
 
-    return error;
+    SuccessOrAssert(error);
 }
 
 Error Settings::ReadOperationalDataset(MeshCoP::Dataset::Type aType, MeshCoP::Dataset &aDataset) const
@@ -242,17 +243,17 @@ Error Settings::ReadOperationalDataset(MeshCoP::Dataset::Type aType, MeshCoP::Da
     aDataset.SetLength(static_cast<uint8_t>(length));
 
 exit:
+    OT_ASSERT(error != kErrorNotImplemented);
     return error;
 }
 
-Error Settings::DeleteOperationalDataset(MeshCoP::Dataset::Type aType)
+void Settings::DeleteOperationalDataset(MeshCoP::Dataset::Type aType)
 {
     Key   key   = KeyForDatasetType(aType);
     Error error = Get<SettingsDriver>().Delete(key);
 
     Log(kActionDelete, error, key);
-
-    return error;
+    OT_ASSERT(error != kErrorNotImplemented);
 }
 
 #if OPENTHREAD_FTD

src/core/common/settings.hpp

@@ -881,11 +881,8 @@ class Settings : public SettingsBase, private NonCopyable
      * @param[in]   aType       The Dataset type (active or pending) to save.
      * @param[in]   aDataset    A reference to a `Dataset` object to be saved.
      *
-     * @retval kErrorNone             Successfully saved the Dataset.
-     * @retval kErrorNotImplemented   The platform does not implement settings functionality.
-     *
      */
-    Error SaveOperationalDataset(MeshCoP::Dataset::Type aType, const MeshCoP::Dataset &aDataset);
+    void SaveOperationalDataset(MeshCoP::Dataset::Type aType, const MeshCoP::Dataset &aDataset);
 
     /**
      * Reads the Operational Dataset (active or pending).
@@ -895,7 +892,6 @@ class Settings : public SettingsBase, private NonCopyable
      *
      * @retval kErrorNone             Successfully read the Dataset.
      * @retval kErrorNotFound         No corresponding value in the setting store.
-     * @retval kErrorNotImplemented   The platform does not implement settings functionality.
      *
      */
     Error ReadOperationalDataset(MeshCoP::Dataset::Type aType, MeshCoP::Dataset &aDataset) const;
@@ -905,11 +901,8 @@ class Settings : public SettingsBase, private NonCopyable
      *
      * @param[in]   aType            The Dataset type (active or pending) to delete.
      *
-     * @retval kErrorNone            Successfully deleted the Dataset.
-     * @retval kErrorNotImplemented  The platform does not implement settings functionality.
-     *
      */
-    Error DeleteOperationalDataset(MeshCoP::Dataset::Type aType);
+    void DeleteOperationalDataset(MeshCoP::Dataset::Type aType);
 
     /**
      * Reads a specified settings entry.

src/core/meshcop/dataset_local.cpp

@@ -66,7 +66,7 @@ void DatasetLocal::Clear(void)
 #if OPENTHREAD_CONFIG_PLATFORM_KEY_REFERENCES_ENABLE
     DestroySecurelyStoredKeys();
 #endif
-    IgnoreError(Get<Settings>().DeleteOperationalDataset(mType));
+    Get<Settings>().DeleteOperationalDataset(mType);
     mTimestamp.Clear();
     mTimestampPresent = false;
     mSaved            = false;
@@ -146,18 +146,15 @@ Error DatasetLocal::Read(Dataset::Tlvs &aDatasetTlvs) const
     return error;
 }
 
-Error DatasetLocal::Save(const Dataset &aDataset)
+void DatasetLocal::Save(const Dataset &aDataset)
 {
-    Error error = kErrorNone;
-
 #if OPENTHREAD_CONFIG_PLATFORM_KEY_REFERENCES_ENABLE
     DestroySecurelyStoredKeys();
 #endif
 
     if (aDataset.GetLength() == 0)
     {
-        // do not propagate error back
-        IgnoreError(Get<Settings>().DeleteOperationalDataset(mType));
+        Get<Settings>().DeleteOperationalDataset(mType);
         mSaved = false;
         LogInfo("%s dataset deleted", Dataset::TypeToString(mType));
     }
@@ -169,9 +166,9 @@ Error DatasetLocal::Save(const Dataset &aDataset)
 
         dataset.SetFrom(aDataset);
         MoveKeysToSecureStorage(dataset);
-        SuccessOrExit(error = Get<Settings>().SaveOperationalDataset(mType, dataset));
+        Get<Settings>().SaveOperationalDataset(mType, dataset);
 #else
-        SuccessOrExit(error = Get<Settings>().SaveOperationalDataset(mType, aDataset));
+        Get<Settings>().SaveOperationalDataset(mType, aDataset);
 #endif
 
         mSaved = true;
@@ -180,9 +177,6 @@ Error DatasetLocal::Save(const Dataset &aDataset)
 
     mTimestampPresent = (aDataset.ReadTimestamp(mType, mTimestamp) == kErrorNone);
     mUpdateTime       = TimerMilli::GetNow();
-
-exit:
-    return error;
 }
 
 #if OPENTHREAD_CONFIG_PLATFORM_KEY_REFERENCES_ENABLE
@@ -239,7 +233,7 @@ void DatasetLocal::EmplaceSecurelyStoredKeys(Dataset &aDataset) const
 
         dataset.SetFrom(aDataset);
         MoveKeysToSecureStorage(dataset);
-        SuccessOrAssert(Get<Settings>().SaveOperationalDataset(mType, dataset));
+        Get<Settings>().SaveOperationalDataset(mType, dataset);
     }
 }
 

src/core/meshcop/dataset_local.hpp

@@ -155,11 +155,8 @@ class DatasetLocal : public InstanceLocator
      *
      * @param[in]  aDataset  The Dataset to save.
      *
-     * @retval kErrorNone             Successfully saved the dataset.
-     * @retval kErrorNotImplemented   The platform does not implement settings functionality.
-     *
      */
-    Error Save(const Dataset &aDataset);
+    void Save(const Dataset &aDataset);
 
 private:
 #if OPENTHREAD_CONFIG_PLATFORM_KEY_REFERENCES_ENABLE

src/core/meshcop/dataset_manager.cpp

@@ -163,7 +163,7 @@ Error DatasetManager::Save(const Dataset &aDataset)
 
     if (isNetworkKeyUpdated || compare > 0)
     {
-        SuccessOrExit(error = mLocal.Save(aDataset));
+        mLocal.Save(aDataset);
 
 #if OPENTHREAD_FTD
         Get<NetworkData::Leader>().IncrementVersionAndStableVersion();
@@ -180,32 +180,31 @@ Error DatasetManager::Save(const Dataset &aDataset)
     return error;
 }
 
-Error DatasetManager::SaveLocal(const Dataset::Info &aDatasetInfo)
+void DatasetManager::SaveLocal(const Dataset::Info &aDatasetInfo)
 {
     Dataset dataset;
 
     dataset.SetFrom(aDatasetInfo);
-
-    return SaveLocal(dataset);
+    SaveLocal(dataset);
 }
 
 Error DatasetManager::SaveLocal(const Dataset::Tlvs &aDatasetTlvs)
 {
-    Error   error;
+    Error   error = kErrorInvalidArgs;
     Dataset dataset;
 
-    SuccessOrExit(error = dataset.SetFrom(aDatasetTlvs));
-    error = SaveLocal(dataset);
+    SuccessOrExit(dataset.SetFrom(aDatasetTlvs));
+    SuccessOrExit(dataset.ValidateTlvs());
+    SaveLocal(dataset);
+    error = kErrorNone;
 
 exit:
     return error;
 }
 
-Error DatasetManager::SaveLocal(const Dataset &aDataset)
+void DatasetManager::SaveLocal(const Dataset &aDataset)
 {
-    Error error;
-
-    SuccessOrExit(error = mLocal.Save(aDataset));
+    mLocal.Save(aDataset);
 
     if (IsPendingDataset())
     {
@@ -237,9 +236,6 @@ Error DatasetManager::SaveLocal(const Dataset &aDataset)
     }
 
     SignalDatasetChange();
-
-exit:
-    return error;
 }
 
 void DatasetManager::SignalDatasetChange(void) const

src/core/meshcop/dataset_manager.hpp

@@ -115,30 +115,24 @@ class DatasetManager : public InstanceLocator
      *
      * @param[in]  aDataset  The Operational Dataset.
      *
-     * @retval kErrorNone   Successfully applied configuration.
-     * @retval kErrorParse  The dataset has at least one TLV with invalid format.
-     *
      */
-    Error SaveLocal(const Dataset &aDataset);
+    void SaveLocal(const Dataset &aDataset);
 
     /**
      * Saves the Operational Dataset in non-volatile memory.
      *
      * @param[in]  aDatasetInfo  The Operational Dataset as `Dataset::Info`.
      *
-     * @retval kErrorNone             Successfully saved the dataset.
-     * @retval kErrorNotImplemented   The platform does not implement settings functionality.
-     *
      */
-    Error SaveLocal(const Dataset::Info &aDatasetInfo);
+    void SaveLocal(const Dataset::Info &aDatasetInfo);
 
     /**
      * Saves the Operational Dataset in non-volatile memory.
      *
      * @param[in]  aDatasetTlvs  The Operational Dataset as `Dataset::Tlvs`.
      *
-     * @retval kErrorNone             Successfully saved the dataset.
-     * @retval kErrorNotImplemented   The platform does not implement settings functionality.
+     * @retval kErrorNone         Successfully saved the dataset.
+     * @retval kErrorInvalidArgs  The @p aDatasetTlvs is invalid. It is too long or contains incorrect TLV formatting.
      *
      */
     Error SaveLocal(const Dataset::Tlvs &aDatasetTlvs);

src/core/meshcop/dataset_manager_ftd.cpp

@@ -376,7 +376,7 @@ Error ActiveDatasetManager::GenerateLocal(void)
         IgnoreError(dataset.WriteTlv(tlv));
     }
 
-    SuccessOrExit(error = mLocal.Save(dataset));
+    mLocal.Save(dataset);
     IgnoreError(Restore());
 
     LogInfo("Generated local dataset");

src/core/meshcop/dataset_updater.cpp

@@ -149,7 +149,7 @@ void DatasetUpdater::PreparePendingDataset(void)
         IgnoreError(dataset.Write<ActiveTimestampTlv>(timestamp));
     }
 
-    SuccessOrExit(error = Get<PendingDatasetManager>().SaveLocal(dataset));
+    Get<PendingDatasetManager>().SaveLocal(dataset);
 
 exit:
     if (error != kErrorNone)

src/core/meshcop/joiner.cpp

@@ -533,7 +533,7 @@ template <> void Joiner::HandleTmf<kUriJoinerEntrust>(Coap::Message &aMessage, c
     datasetInfo.Set<Dataset::kChannel>(Get<Mac::Mac>().GetPanChannel());
     datasetInfo.Set<Dataset::kPanId>(Get<Mac::Mac>().GetPanId());
 
-    IgnoreError(Get<ActiveDatasetManager>().SaveLocal(datasetInfo));
+    Get<ActiveDatasetManager>().SaveLocal(datasetInfo);
 
     LogInfo("Joiner successful!");