use support list instead of prohibit list

include/xrpl/protocol/Permissions.h

@@ -69,6 +69,8 @@ class Permission
 
     std::unordered_map<GranularPermissionType, TxType> granularTxTypeMap;
 
+    std::unordered_set<std::string> supportedTransactions;
+
 public:
     static Permission const&
     getInstance();
@@ -84,7 +86,7 @@ class Permission
     getGranularTxType(GranularPermissionType const& gpType) const;
 
     bool
-    isProhibited(std::string const& name) const;
+    isSupported(std::string const& name) const;
 };
 
 }  // namespace ripple

include/xrpl/protocol/detail/ledger_entries.macro

@@ -460,10 +460,6 @@ LEDGER_ENTRY(ltPERMISSIONED_DOMAIN, 0x0082, PermissionedDomain, permissioned_dom
     {sfPreviousTxnLgrSeq,   soeREQUIRED},
 }))
 
-#undef EXPAND
-#undef LEDGER_ENTRY_DUPLICATE
-
-
 /** A ledger object representing permissions an account has delegated to another account.
     \sa keylet::accountPermission
  */
@@ -475,3 +471,6 @@ LEDGER_ENTRY(ltACCOUNT_PERMISSION, 0x0083, AccountPermission, account_permission
     {sfPreviousTxnID,        soeREQUIRED},
     {sfPreviousTxnLgrSeq,    soeREQUIRED},
 }))
+
+#undef EXPAND
+#undef LEDGER_ENTRY_DUPLICATE

src/libxrpl/protocol/BuildInfo.cpp

@@ -33,7 +33,7 @@ namespace BuildInfo {
 //  and follow the format described at http://semver.org/
 //------------------------------------------------------------------------------
 // clang-format off
-char const* const versionString = "2.4.0-rc1"
+char const* const versionString = "2.4.0-b3"
 // clang-format on
 
 #if defined(DEBUG) || defined(SANITIZER)

src/libxrpl/protocol/Permissions.cpp

@@ -51,6 +51,61 @@ Permission::Permission()
         {PaymentBurn, ttPAYMENT},
         {MPTokenIssuanceLock, ttMPTOKEN_ISSUANCE_SET},
         {MPTokenIssuanceUnlock, ttMPTOKEN_ISSUANCE_SET}};
+
+    // We do not allow delegating AccountSet, SetRegularKey, SignerListSet,
+    // AccountPermissionSet and AccountDelete for security reason. To support
+    // more newly added transactions, we need to add it here and have it fully
+    // tested. Newly addded transactions are not supported for delegation
+    // automatically for safety.
+    supportedTransactions = {
+        "Payment",
+        "EscrowCreate",
+        "EscrowFinish",
+        "EscrowCancel",
+        "OfferCreate",
+        "OfferCancel",
+        "TicketCreate",
+        "PaymentChannelCreate",
+        "PaymentChannelFund",
+        "PaymentChannelClaim",
+        "CheckCreate",
+        "CheckCash",
+        "CheckCancel",
+        "DepositPreauth",
+        "TrustSet",
+        "NFTokenMint",
+        "NFTokenBurn",
+        "NFTokenCreateOffer",
+        "NFTokenCancelOffer",
+        "NFTokenAcceptOffer",
+        "NFTokenModify",
+        "Clawback",
+        "AMMClawback",
+        "AMMCreate",
+        "AMMDeposit",
+        "AMMWithdraw",
+        "AMMVote",
+        "AMMBid",
+        "AMMDelete",
+        "XChainCreateClaimID",
+        "XChainCommit",
+        "XChainClaim",
+        "XChainAccountCreateCommit",
+        "XChainAddClaimAttestation",
+        "XChainAddAccountCreateAttestation",
+        "XChainModifyBridge",
+        "XChainCreateBridge",
+        "DIDSet",
+        "DIDDelete",
+        "OracleSet",
+        "OracleDelete",
+        "MPTokenIssuanceCreate",
+        "MPTokenIssuanceDestroy",
+        "MPTokenIssuanceSet",
+        "MPTokenAuthorize",
+        "CredentialCreate",
+        "CredentialAccept",
+        "CredentialDelete"};
 }
 
 Permission const&
@@ -81,13 +136,9 @@ Permission::getGranularTxType(GranularPermissionType const& gpType) const
 }
 
 bool
-Permission::isProhibited(std::string const& name) const
+Permission::isSupported(std::string const& name) const
 {
-    // We do not allow delegating the following transaction permissions to other
-    // accounts for security reason.
-    if (name == "AccountSet" || name == "SetRegularKey" ||
-        name == "SignerListSet" || name == "AccountPermissionSet" ||
-        name == "AccountDelete")
+    if (supportedTransactions.contains(name))
         return true;
 
     return false;

src/libxrpl/protocol/STParsedJSON.cpp

@@ -371,12 +371,13 @@ parseLeaf(
                         {
                             // if it's not granular permission, parse as
                             // transaction type permission.
-                            if (Permission::getInstance().isProhibited(
+                            if (!Permission::getInstance().isSupported(
                                     strValue))
                             {
                                 // we do not allow delegating some transaction
                                 // type permissions to other accounts for
-                                // security reason.
+                                // security reason. And also newly added
+                                // transactions are not enabled automatically.
                                 error = invalid_data(json_name, fieldName);
                                 return ret;
                             }

src/test/app/AccountPermission_test.cpp

@@ -52,8 +52,13 @@ class AccountPermission_test : public beast::unit_test::suite
             ter(temDISABLED));
 
         // can not send transaction on behalf of other account when feature
-        // disabled
+        // disabled, onBehalfOf, delegateSequence, and delegateTicketSequence
+        // should not appear in the request.
         env(pay(bob, alice, XRP(50)), onBehalfOf(gw), ter(temDISABLED));
+        env(pay(bob, alice, XRP(50)), delegateSequence(1), ter(temDISABLED));
+        env(pay(bob, alice, XRP(50)),
+            delegateTicketSequence(1),
+            ter(temDISABLED));
     }
 
     void
@@ -2778,6 +2783,32 @@ class AccountPermission_test : public beast::unit_test::suite
             }
         }
 
+        // test dynamic nft, modify onbehalf of other account
+        {
+            Env env(*this, features);
+            Account alice{"alice"};
+            Account bob{"bob"};
+            env.fund(XRP(1000000), alice, bob);
+            env.close();
+
+            uint256 const nftId{token::getNextID(env, alice, 0u, tfMutable)};
+            env(token::mint(alice, 0u), txflags(tfMutable));
+            env.close();
+
+            // bob does not have permission to modify the nft
+            env(token::modify(bob, nftId),
+                token::owner(alice),
+                ter(tecNO_PERMISSION));
+            env.close();
+
+            // now alice gives bob permission to modify the nft
+            env(account_permission::accountPermissionSet(
+                alice, bob, {"NFTokenModify"}));
+            env.close();
+            env(token::modify(bob, nftId), onBehalfOf(alice));
+            env.close();
+        }
+
         // mint with flagTransferable
         {
             Env env(*this, features);

src/test/jtx/impl/mpt.cpp

@@ -66,7 +66,6 @@ MPTTester::MPTTester(Env& env, Account const& account, MPTInit const& arg)
     if (arg.fund)
     {
         env_.fund(arg.xrp, issuer_);
-        // env_.fund(arg.xrp, sender_);
         for (auto it : holders_)
             env_.fund(arg.xrpHolders, it.second);
     }

src/test/jtx/mpt.h

@@ -242,7 +242,6 @@ class MPTTester
         auto const err = env_.ter();
         if (close_)
             env_.close();
-        // auto const account = arg.onBehalfOf ? *arg.onBehalfOf : issuer_;
         if (arg.ownerCount)
             env_.require(owners(issuer_, *arg.ownerCount));
         if (arg.holderCount)

src/xrpld/app/tx/detail/Transactor.cpp

@@ -81,9 +81,13 @@ preflight0(PreflightContext const& ctx)
 NotTEC
 preflight1(PreflightContext const& ctx)
 {
-    if (!ctx.rules.enabled(featureAccountPermission) &&
-        ctx.tx.isFieldPresent(sfOnBehalfOf))
-        return temDISABLED;
+    if (!ctx.rules.enabled(featureAccountPermission))
+    {
+        if (ctx.tx.isFieldPresent(sfOnBehalfOf) ||
+            ctx.tx.isFieldPresent(sfDelegateSequence) ||
+            ctx.tx.isFieldPresent(sfDelegateTicketSequence))
+            return temDISABLED;
+    }
 
     // This is inappropriate in preflight0, because only Change transactions
     // skip this function, and those do not allow an sfTicketSequence field.
@@ -432,10 +436,15 @@ Transactor::checkDelegateSeqProxy(
 {
     if (!tx.isFieldPresent(sfDelegateSequence))
     {
-        JLOG(j.trace()) << "applyTransaction: has no DelegateTicketSequence";
+        JLOG(j.trace()) << "applyTransaction: has no DelegateSequence";
         return temBAD_SEQUENCE;
     }
 
+    // we only call this function when it is delegated, so this should not
+    // happen
+    if (!tx.isFieldPresent(sfOnBehalfOf))
+        return temMALFORMED;  // LCOV_EXEL_LINE
+
     auto const id = tx.getAccountID(sfOnBehalfOf);
     auto const sle = view.read(keylet::account(id));
     if (!sle)

src/xrpld/app/tx/detail/applySteps.cpp

@@ -190,8 +190,18 @@ invoke_preclaim(PreclaimContext const& ctx)
 
                 if (ctx.tx.isDelegated())
                 {
+                    // if this is a delegated transaction, check if the account
+                    // has authorization.
+                    result = T::checkPermissions(
+                        ctx.view, ctx.tx.getSTTx(), permissions);
+
+                    if (result != tesSUCCESS)
+                        return std::make_pair(result, permissions);
+
+                    // check delegate sequence
                     result = T::checkDelegateSeqProxy(
                         ctx.view, ctx.tx.getSTTx(), ctx.j);
+
                     if (result != tesSUCCESS)
                         return std::make_pair(result, permissions);
                 }
@@ -211,17 +221,6 @@ invoke_preclaim(PreclaimContext const& ctx)
 
                 if (result != tesSUCCESS)
                     return std::make_pair(result, permissions);
-
-                if (ctx.tx.isDelegated())
-                {
-                    // if this is a delegated transaction, check if the account
-                    // has authorization.
-                    result = T::checkPermissions(
-                        ctx.view, ctx.tx.getSTTx(), permissions);
-
-                    if (result != tesSUCCESS)
-                        return std::make_pair(result, permissions);
-                }
             }
 
             return std::make_pair(T::preclaim(ctx), permissions);