tests and improvements

src/ripple/app/tx/impl/CFTokenAuthorize.cpp

@@ -58,7 +58,7 @@ CFTokenAuthorize::preclaim(PreclaimContext const& ctx)
     auto const holderID = ctx.tx[~sfCFTokenHolder]; 
 
     if (holderID && !(ctx.view.exists(keylet::account(*holderID))))
-        return tecNO_DST; // TODO: Change code
+        return tecNO_DST;
 
     std::uint32_t const cftIssuanceFlags = sleCftIssuance->getFieldU32(sfFlags);
 
@@ -86,14 +86,14 @@ CFTokenAuthorize::preclaim(PreclaimContext const& ctx)
         // issuer wants to unauthorize the holder
         if (txFlags & tfCFTUnathorize){
             if (!(sleCftFlags & lsfCFTAuthorized))
-                return temINVALID_FLAG; // TODO chanage code
+                return temINVALID_FLAG;
 
         }
         // authorize a holder
         else {
             //make sure the holder is not already authorized
             if (sleCftFlags & lsfCFTAuthorized)
-                return temMALFORMED; // TODO: change code
+                return tecCFTOKEN_ALREADY_AUTHORIZED;
         }
     }
     // if non-issuer account submits this tx, then they are trying either:
@@ -116,7 +116,7 @@ CFTokenAuthorize::preclaim(PreclaimContext const& ctx)
         // if holder wants to use and create a cft
         else{
             if (sleCft)
-                return tecDUPLICATE;
+                return tecCFTOKEN_EXISTS;
         }
     }
     return tesSUCCESS;
@@ -144,6 +144,8 @@ CFTokenAuthorize::doApply()
             return tecINTERNAL;
 
         auto const sleCft = view().peek(keylet::cftoken(cftIssuanceID, *holderID));
+        if (!sleCft)
+            return tecINTERNAL;
 
         std::uint32_t const flagsIn = sleCft->getFieldU32(sfFlags);
         std::uint32_t flagsOut = flagsIn;
@@ -197,18 +199,21 @@ CFTokenAuthorize::doApply()
         //      - add the new cftokenKey to both the owner and cft directries
         //      - create the CFToken object for the holder
         else{
-            if (mPriorBalance < view().fees().accountReserve((*sleAcct)[sfOwnerCount] + 1))
+            std::uint32_t const uOwnerCount = sleAcct->getFieldU32(sfOwnerCount);
+            XRPAmount const reserveCreate(
+                (uOwnerCount < 2) ? XRPAmount(beast::zero)
+                                : view().fees().accountReserve(uOwnerCount + 1));
+
+            if (mPriorBalance < reserveCreate)
                 return tecINSUFFICIENT_RESERVE;      
 
             auto const cftokenKey = keylet::cftoken(cftIssuanceID, account_);
-
 
             auto const ownerNode = view().dirInsert(
                 keylet::ownerDir(account_), cftokenKey, describeOwnerDir(account_));
 
             if (!ownerNode)
                 return tecDIR_FULL;
-
 
             auto const cftNode = view().dirInsert(keylet::cft_dir(cftIssuanceID), cftokenKey,     
                 [&cftIssuanceID](std::shared_ptr<SLE> const& sle) {

src/ripple/protocol/TER.h

@@ -332,7 +332,10 @@ enum TECcodes : TERUnderlyingType {
     tecXCHAIN_SELF_COMMIT = 184,
     tecXCHAIN_BAD_PUBLIC_KEY_ACCOUNT_PAIR = 185,
     tecXCHAIN_CREATE_ACCOUNT_DISABLED = 186,
-    tecEMPTY_DID = 187
+    tecEMPTY_DID = 187,
+    tecCFTOKEN_EXISTS = 188,
+    tecCFTOKEN_ALREADY_AUTHORIZED = 189
+
 };
 
 //------------------------------------------------------------------------------

src/ripple/protocol/impl/TER.cpp

@@ -111,6 +111,8 @@ transResults()
         MAKE_ERROR(tecXCHAIN_BAD_PUBLIC_KEY_ACCOUNT_PAIR, "Bad public key account pair in an xchain transaction."),
         MAKE_ERROR(tecXCHAIN_CREATE_ACCOUNT_DISABLED, "This bridge does not support account creation."),
         MAKE_ERROR(tecEMPTY_DID,                     "The DID object did not have a URI or DIDDocument field."),
+        MAKE_ERROR(tecCFTOKEN_EXISTS,                "The account already owns the CFToken object."),
+        MAKE_ERROR(tecCFTOKEN_ALREADY_AUTHORIZED,    "The CFToken that the issuer tries to authorize is already authorized."),
 
         MAKE_ERROR(tefALREADY,                     "The exact transaction was already in this ledger."),
         MAKE_ERROR(tefBAD_ADD_AUTH,                "Not authorized to add account."),

src/test/app/CFToken_test.cpp

@@ -99,7 +99,7 @@ class CFToken_test : public beast::unit_test::suite
         using namespace test::jtx;
         // Validate fields in CFTokenAuthorize (preflight)
         {
-            Env env{*this, features};
+            Env env{*this, features - featureCFTokensV1};
             Account const alice("alice"); //issuer
             Account const bob("bob"); //holder
 
@@ -109,6 +109,12 @@ class CFToken_test : public beast::unit_test::suite
             BEAST_EXPECT(env.ownerCount(alice) == 0);
 
             auto const id = keylet::cftIssuance(alice.id(), env.seq(alice));
+
+            env(cft::authorize(bob, id.key, std::nullopt), ter(temDISABLED));
+            env.close();
+
+            env.enableFeature(featureCFTokensV1);
+
             env(cft::create(alice));
             env.close();
 
@@ -182,7 +188,7 @@ class CFToken_test : public beast::unit_test::suite
             BEAST_EXPECT(env.ownerCount(bob) == 1);
 
             // bob cannot create the cftoken the second time
-            env(cft::authorize(bob, id.key, std::nullopt), ter(tecDUPLICATE));
+            env(cft::authorize(bob, id.key, std::nullopt), ter(tecCFTOKEN_EXISTS));
             env.close();
 
             // TODO: check where cftoken balance is nonzero
@@ -241,21 +247,81 @@ class CFToken_test : public beast::unit_test::suite
             env.close();  
 
             // if alice tries to set again, it will fail
-            env(cft::authorize(alice, id.key, bob), ter(temMALFORMED));
+            env(cft::authorize(alice, id.key, bob), ter(tecCFTOKEN_ALREADY_AUTHORIZED));
             env.close();        
 
             env(cft::authorize(bob, id.key, std::nullopt), txflags(tfCFTUnathorize));
             env.close();
 
             BEAST_EXPECT(env.ownerCount(bob) == 0);
         }
+
+        // Test cftoken reserve requirement - first two cfts free (doApply)
+        {
+            Env env{*this, features};
+            auto const acctReserve = env.current()->fees().accountReserve(0);
+            auto const incReserve = env.current()->fees().increment;
+
+            Account const alice("alice");
+            Account const bob("bob");
+
+            env.fund(XRP(10000), alice);
+            env.fund(acctReserve + XRP(1), bob);
+            env.close();
+
+            BEAST_EXPECT(env.ownerCount(alice) == 0);
+
+            auto const id1 = keylet::cftIssuance(alice.id(), env.seq(alice));
+            env(cft::create(alice));
+            env.close();
+
+            auto const id2 = keylet::cftIssuance(alice.id(), env.seq(alice));
+            env(cft::create(alice));
+            env.close();
+
+            auto const id3 = keylet::cftIssuance(alice.id(), env.seq(alice));
+            env(cft::create(alice));
+            env.close();
+
+            BEAST_EXPECT(env.ownerCount(alice) == 3);
+
+            // first cft for free
+            env(cft::authorize(bob, id1.key, std::nullopt));
+            env.close();
+
+            BEAST_EXPECT(env.ownerCount(bob) == 1);
+
+            // second cft free
+            env(cft::authorize(bob, id2.key, std::nullopt));
+            env.close();
+            BEAST_EXPECT(env.ownerCount(bob) == 2);
+
+            env(cft::authorize(bob, id3.key, std::nullopt), ter(tecINSUFFICIENT_RESERVE));
+            env.close();
+
+            env(pay(env.master, bob, drops(incReserve + incReserve + incReserve)));
+            env.close();
+
+            env(cft::authorize(bob, id3.key, std::nullopt));
+            env.close();
+
+            BEAST_EXPECT(env.ownerCount(bob) == 3);
+        }
     }
 
     void
     testBasicAuthorize(FeatureBitset features)
     {
         testcase("Basic authorize");
 
+        auto const cftIsAuthorized =[](test::jtx::Env const& env,
+                                        ripple::uint256 const cftIssuanceID,
+                                         test::jtx::Account const& holder) -> bool {
+            auto const sleCft = env.le(keylet::cftoken(cftIssuanceID, holder));
+            uint32_t const cftFlags = sleCft->getFlags();
+            return cftFlags & lsfCFTAuthorized;
+        };
+
         using namespace test::jtx;
         // Basic authorization without allowlisting
         {
@@ -279,6 +345,8 @@ class CFToken_test : public beast::unit_test::suite
 
             BEAST_EXPECT(env.ownerCount(bob) == 1);
 
+            BEAST_EXPECT(!cftIsAuthorized(env, id.key, bob));
+
             env(cft::authorize(bob, id.key, std::nullopt), txflags(tfCFTUnathorize));
             env.close();
 
@@ -307,9 +375,15 @@ class CFToken_test : public beast::unit_test::suite
 
             BEAST_EXPECT(env.ownerCount(bob) == 1);
 
+            BEAST_EXPECT(!cftIsAuthorized(env, id.key, bob));
+
             env(cft::authorize(alice, id.key, bob));
             env.close();
 
+            BEAST_EXPECT(cftIsAuthorized(env, id.key, bob));
+
+            BEAST_EXPECT(env.ownerCount(bob) == 1);
+
             env(cft::authorize(bob, id.key, std::nullopt), txflags(tfCFTUnathorize));
             env.close();