diff --git a/src/ripple/app/tx/impl/CFTokenAuthorize.cpp b/src/ripple/app/tx/impl/CFTokenAuthorize.cpp index a76c2ae08fb..0623893ed33 100644 --- a/src/ripple/app/tx/impl/CFTokenAuthorize.cpp +++ b/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 const& sle) { diff --git a/src/ripple/protocol/TER.h b/src/ripple/protocol/TER.h index 075497240e5..04825711730 100644 --- a/src/ripple/protocol/TER.h +++ b/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 + }; //------------------------------------------------------------------------------ diff --git a/src/ripple/protocol/impl/TER.cpp b/src/ripple/protocol/impl/TER.cpp index 1c2db3feb3b..e1fdf9099d3 100644 --- a/src/ripple/protocol/impl/TER.cpp +++ b/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."), diff --git a/src/test/app/CFToken_test.cpp b/src/test/app/CFToken_test.cpp index 3295860188b..898fb039835 100644 --- a/src/test/app/CFToken_test.cpp +++ b/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,7 +247,7 @@ 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)); @@ -249,6 +255,58 @@ class CFToken_test : public beast::unit_test::suite 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 @@ -256,6 +314,14 @@ class CFToken_test : public beast::unit_test::suite { 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();