fixEmptyDID: fix amendment to handle empty DID edge cases

XRPLF · Mar 14, 2024 · 71ffbbe · 71ffbbe
1 parent af9cabe
commit 71ffbbe
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 10 deletions.
diff --git a/src/ripple/app/tx/impl/DID.cpp b/src/ripple/app/tx/impl/DID.cpp
@@ -161,6 +161,13 @@ DIDSet::doApply()
     set(sfURI);
     set(sfDIDDocument);
     set(sfData);
+    if (ctx_.view().rules().enabled(fixEmptyDID) &&
+        !sleDID->isFieldPresent(sfURI) &&
+        !sleDID->isFieldPresent(sfDIDDocument) &&
+        !sleDID->isFieldPresent(sfData))
+    {
+        return tecEMPTY_DID;
+    }
 
     return addSLE(ctx_, sleDID, account_);
 }

diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h
@@ -74,7 +74,7 @@ namespace detail {
 // Feature.cpp. Because it's only used to reserve storage, and determine how
 // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
 // the actual number of amendments. A LogicError on startup will verify this.
-static constexpr std::size_t numFeatures = 68;
+static constexpr std::size_t numFeatures = 69;
 
 /** Amendments that this server supports and the default voting behavior.
    Whether they are enabled depends on the Rules defined in the validated
@@ -355,6 +355,7 @@ extern uint256 const fixFillOrKill;
 extern uint256 const fixNFTokenReserve;
 extern uint256 const fixInnerObjTemplate;
 extern uint256 const featurePriceOracle;
+extern uint256 const fixEmptyDID;
 
 }  // namespace ripple
 

diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp
@@ -458,10 +458,11 @@ REGISTER_FEATURE(AMM,                           Supported::yes, VoteBehavior::De
 REGISTER_FEATURE(XChainBridge,                  Supported::yes, VoteBehavior::DefaultNo);
 REGISTER_FIX    (fixDisallowIncomingV1,         Supported::yes, VoteBehavior::DefaultNo);
 REGISTER_FEATURE(DID,                           Supported::yes, VoteBehavior::DefaultNo);
-REGISTER_FIX(fixFillOrKill,                     Supported::yes, VoteBehavior::DefaultNo);
+REGISTER_FIX    (fixFillOrKill,                 Supported::yes, VoteBehavior::DefaultNo);
 REGISTER_FIX    (fixNFTokenReserve,             Supported::yes, VoteBehavior::DefaultNo);
-REGISTER_FIX(fixInnerObjTemplate,               Supported::yes, VoteBehavior::DefaultNo);
+REGISTER_FIX    (fixInnerObjTemplate,           Supported::yes, VoteBehavior::DefaultNo);
 REGISTER_FEATURE(PriceOracle,                   Supported::yes, VoteBehavior::DefaultNo);
+REGISTER_FIX    (fixEmptyDID,                   Supported::yes, VoteBehavior::DefaultNo);
 
 // The following amendments are obsolete, but must remain supported
 // because they could potentially get enabled.

diff --git a/src/test/app/DID_test.cpp b/src/test/app/DID_test.cpp
@@ -31,7 +31,7 @@ namespace ripple {
 namespace test {
 
 // Helper function that returns the owner count of an account root.
-std::uint32_t
+static std::uint32_t
 ownerCount(test::jtx::Env const& env, test::jtx::Account const& acct)
 {
     std::uint32_t ret{0};
@@ -130,7 +130,7 @@ struct DID_test : public beast::unit_test::suite
         using namespace jtx;
         using namespace std::chrono;
 
-        Env env(*this);
+        Env env{*this, features};
         Account const alice{"alice"};
         env.fund(XRP(5000), alice);
         env.close();
@@ -177,6 +177,16 @@ struct DID_test : public beast::unit_test::suite
         env.close();
         BEAST_EXPECT(ownerCount(env, alice) == 0);
 
+        // some empty fields, some optional fields
+        // pre-fix amendment
+        auto const fixEnabled = env.current()->rules().enabled(fixEmptyDID);
+        env(did::set(alice),
+            did::uri(""),
+            fixEnabled ? ter(tecEMPTY_DID) : ter(tesSUCCESS));
+        env.close();
+        auto const expectedOwnerReserve = fixEnabled ? 0 : 1;
+        BEAST_EXPECT(ownerCount(env, alice) == expectedOwnerReserve);
+
         // Modifying a DID to become empty is checked in testSetModify
     }
 
@@ -188,7 +198,7 @@ struct DID_test : public beast::unit_test::suite
         using namespace jtx;
         using namespace std::chrono;
 
-        Env env(*this);
+        Env env{*this, features};
         Account const alice{"alice"};
         env.fund(XRP(5000), alice);
         env.close();
@@ -219,7 +229,7 @@ struct DID_test : public beast::unit_test::suite
         using namespace jtx;
         using namespace std::chrono;
 
-        Env env(*this);
+        Env env{*this, features};
         Account const alice{"alice"};
         Account const bob{"bob"};
         Account const charlie{"charlie"};
@@ -273,7 +283,7 @@ struct DID_test : public beast::unit_test::suite
         using namespace jtx;
         using namespace std::chrono;
 
-        Env env(*this);
+        Env env{*this, features};
         Account const alice{"alice"};
         env.fund(XRP(5000), alice);
         env.close();
@@ -390,13 +400,19 @@ struct DID_test : public beast::unit_test::suite
     run() override
     {
         using namespace test::jtx;
-        FeatureBitset const all{
-            supported_amendments() | FeatureBitset{featureDID}};
+        FeatureBitset const all{supported_amendments()};
+        FeatureBitset const emptyDID{fixEmptyDID};
         testEnabled(all);
         testAccountReserve(all);
         testSetInvalid(all);
         testDeleteInvalid(all);
         testSetModify(all);
+
+        testEnabled(all - emptyDID);
+        testAccountReserve(all - emptyDID);
+        testSetInvalid(all - emptyDID);
+        testDeleteInvalid(all - emptyDID);
+        testSetModify(all - emptyDID);
     }
 };