From f6426ca1832d61e3b752124673eddf61553b7b56 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Tue, 9 Sep 2025 22:23:07 +0100 Subject: [PATCH 1/4] Switch CI pipeline bookworm:gcc-13 from arm64 to amd64 (#5779) --- .github/scripts/strategy-matrix/generate.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/scripts/strategy-matrix/generate.py b/.github/scripts/strategy-matrix/generate.py index b6f6601291..ac39803fff 100755 --- a/.github/scripts/strategy-matrix/generate.py +++ b/.github/scripts/strategy-matrix/generate.py @@ -45,7 +45,7 @@ def generate_strategy_matrix(all: bool, config: Config) -> list: # Only generate a subset of configurations in PRs. if not all: # Debian: - # - Bookworm using GCC 13: Release and Unity on linux/arm64, set + # - Bookworm using GCC 13: Release and Unity on linux/amd64, set # the reference fee to 500. # - Bookworm using GCC 15: Debug and no Unity on linux/amd64, enable # code coverage (which will be done below). @@ -57,7 +57,7 @@ def generate_strategy_matrix(all: bool, config: Config) -> list: if os['distro_name'] == 'debian': skip = True if os['distro_version'] == 'bookworm': - if f'{os['compiler_name']}-{os['compiler_version']}' == 'gcc-13' and build_type == 'Release' and '-Dunity=ON' in cmake_args and architecture['platform'] == 'linux/arm64': + if f'{os['compiler_name']}-{os['compiler_version']}' == 'gcc-13' and build_type == 'Release' and '-Dunity=ON' in cmake_args and architecture['platform'] == 'linux/amd64': cmake_args = f'-DUNIT_TEST_REFERENCE_FEE=500 {cmake_args}' skip = False if f'{os['compiler_name']}-{os['compiler_version']}' == 'gcc-15' and build_type == 'Debug' and '-Dunity=OFF' in cmake_args and architecture['platform'] == 'linux/amd64': From cdbe70b2a7d54e85cf2e5eddd55e93bc206a5ef6 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Wed, 10 Sep 2025 08:35:58 +0100 Subject: [PATCH 2/4] ci: Use default conan install format (#5784) --- .github/actions/build-deps/action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/build-deps/action.yml b/.github/actions/build-deps/action.yml index c3b405e70f..351d8a6361 100644 --- a/.github/actions/build-deps/action.yml +++ b/.github/actions/build-deps/action.yml @@ -30,4 +30,4 @@ runs: --options:host '&:tests=True' \ --options:host '&:xrpld=True' \ --settings:all build_type=${{ inputs.build_type }} \ - --format=json .. + .. From 3d92375d127acb61ca7b931d561e5cca7762cabf Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Wed, 10 Sep 2025 09:20:45 +0100 Subject: [PATCH 3/4] ci: Add missing dependencies to workflows (#5783) --- .github/workflows/on-pr.yml | 2 ++ .github/workflows/on-trigger.yml | 2 ++ .github/workflows/upload-conan-deps.yml | 1 + 3 files changed, 5 insertions(+) diff --git a/.github/workflows/on-pr.yml b/.github/workflows/on-pr.yml index f194bd1e37..c480cc5476 100644 --- a/.github/workflows/on-pr.yml +++ b/.github/workflows/on-pr.yml @@ -60,8 +60,10 @@ jobs: # Keep the paths below in sync with those in `on-trigger.yml`. .github/actions/build-deps/** .github/actions/build-test/** + .github/actions/setup-conan/** .github/scripts/strategy-matrix/** .github/workflows/build-test.yml + .github/workflows/reusable-strategy-matrix.yml .codecov.yml cmake/** conan/** diff --git a/.github/workflows/on-trigger.yml b/.github/workflows/on-trigger.yml index 14884391ef..7c17621d67 100644 --- a/.github/workflows/on-trigger.yml +++ b/.github/workflows/on-trigger.yml @@ -21,8 +21,10 @@ on: # Keep the paths below in sync with those in `on-pr.yml`. - ".github/actions/build-deps/**" - ".github/actions/build-test/**" + - ".github/actions/setup-conan/**" - ".github/scripts/strategy-matrix/**" - ".github/workflows/build-test.yml" + - ".github/workflows/reusable-strategy-matrix.yml" - ".codecov.yml" - "cmake/**" - "conan/**" diff --git a/.github/workflows/upload-conan-deps.yml b/.github/workflows/upload-conan-deps.yml index 07e9a60dbd..5af72a9e41 100644 --- a/.github/workflows/upload-conan-deps.yml +++ b/.github/workflows/upload-conan-deps.yml @@ -28,6 +28,7 @@ on: - .github/workflows/reusable-strategy-matrix.yml - .github/actions/build-deps/action.yml + - .github/actions/setup-conan/action.yml - ".github/scripts/strategy-matrix/**" - conanfile.py From 61d628d654fd03135a47bf0dea28f56b03a8bd22 Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Wed, 10 Sep 2025 13:47:33 -0400 Subject: [PATCH 4/4] fix: Add restrictions to Permission Delegation: fixDelegateV1_1 (#5650) - Amendment: fixDelegateV1_1 - In DelegateSet, disallow invalid PermissionValues like 0, and transaction values when the transaction's amendment is not enabled. Acts as if the transaction doesn't exist, which is the same thing older versions without the amendment will do. - Payment burn/mint should disallow DEX currency exchange. - Support MPT for Payment burn/mint. --- include/xrpl/protocol/Permissions.h | 7 +- include/xrpl/protocol/TxFormats.h | 2 +- include/xrpl/protocol/detail/features.macro | 1 + .../xrpl/protocol/detail/transactions.macro | 332 ++++++++++++--- include/xrpl/protocol/jss.h | 2 +- src/libxrpl/protocol/Permissions.cpp | 42 +- src/libxrpl/protocol/TxFormats.cpp | 2 +- src/test/app/AMMClawback_test.cpp | 3 +- src/test/app/Delegate_test.cpp | 390 +++++++++++++++++- src/xrpld/app/tx/detail/DelegateSet.cpp | 22 +- src/xrpld/app/tx/detail/Payment.cpp | 27 +- src/xrpld/app/tx/detail/applySteps.cpp | 4 +- 12 files changed, 732 insertions(+), 102 deletions(-) diff --git a/include/xrpl/protocol/Permissions.h b/include/xrpl/protocol/Permissions.h index 67f3eea8d7..cf49ff7382 100644 --- a/include/xrpl/protocol/Permissions.h +++ b/include/xrpl/protocol/Permissions.h @@ -20,6 +20,8 @@ #ifndef RIPPLE_PROTOCOL_PERMISSION_H_INCLUDED #define RIPPLE_PROTOCOL_PERMISSION_H_INCLUDED +#include +#include #include #include @@ -53,6 +55,8 @@ class Permission private: Permission(); + std::unordered_map txFeatureMap_; + std::unordered_map delegatableTx_; std::unordered_map @@ -80,7 +84,8 @@ public: getGranularTxType(GranularPermissionType const& gpType) const; bool - isDelegatable(std::uint32_t const& permissionValue) const; + isDelegatable(std::uint32_t const& permissionValue, Rules const& rules) + const; // for tx level permission, permission value is equal to tx type plus one uint32_t diff --git a/include/xrpl/protocol/TxFormats.h b/include/xrpl/protocol/TxFormats.h index 70b721a3d7..d17eea7644 100644 --- a/include/xrpl/protocol/TxFormats.h +++ b/include/xrpl/protocol/TxFormats.h @@ -59,7 +59,7 @@ enum TxType : std::uint16_t #pragma push_macro("TRANSACTION") #undef TRANSACTION -#define TRANSACTION(tag, value, name, delegatable, fields) tag = value, +#define TRANSACTION(tag, value, ...) tag = value, #include diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 264fad7fc2..9aacbbe3d9 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -32,6 +32,7 @@ // If you add an amendment here, then do not forget to increment `numFeatures` // in include/xrpl/protocol/Feature.h. +XRPL_FIX (DelegateV1_1, Supported::no, VoteBehavior::DefaultNo) XRPL_FIX (PriceOracleOrder, Supported::no, VoteBehavior::DefaultNo) XRPL_FIX (MPTDeliveredAmount, Supported::no, VoteBehavior::DefaultNo) XRPL_FIX (AMMClawbackRounding, Supported::yes, VoteBehavior::DefaultNo) diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index 1131e24f61..bfbc18aa1b 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -22,14 +22,17 @@ #endif /** - * TRANSACTION(tag, value, name, delegatable, fields) + * TRANSACTION(tag, value, name, delegatable, amendments, fields) * * You must define a transactor class in the `ripple` namespace named `name`, * and include its header in `src/xrpld/app/tx/detail/applySteps.cpp`. */ /** This transaction type executes a payment. */ -TRANSACTION(ttPAYMENT, 0, Payment, Delegation::delegatable, ({ +TRANSACTION(ttPAYMENT, 0, Payment, + Delegation::delegatable, + uint256{}, + ({ {sfDestination, soeREQUIRED}, {sfAmount, soeREQUIRED, soeMPTSupported}, {sfSendMax, soeOPTIONAL, soeMPTSupported}, @@ -42,7 +45,10 @@ TRANSACTION(ttPAYMENT, 0, Payment, Delegation::delegatable, ({ })) /** This transaction type creates an escrow object. */ -TRANSACTION(ttESCROW_CREATE, 1, EscrowCreate, Delegation::delegatable, ({ +TRANSACTION(ttESCROW_CREATE, 1, EscrowCreate, + Delegation::delegatable, + uint256{}, + ({ {sfDestination, soeREQUIRED}, {sfAmount, soeREQUIRED, soeMPTSupported}, {sfCondition, soeOPTIONAL}, @@ -52,7 +58,10 @@ TRANSACTION(ttESCROW_CREATE, 1, EscrowCreate, Delegation::delegatable, ({ })) /** This transaction type completes an existing escrow. */ -TRANSACTION(ttESCROW_FINISH, 2, EscrowFinish, Delegation::delegatable, ({ +TRANSACTION(ttESCROW_FINISH, 2, EscrowFinish, + Delegation::delegatable, + uint256{}, + ({ {sfOwner, soeREQUIRED}, {sfOfferSequence, soeREQUIRED}, {sfFulfillment, soeOPTIONAL}, @@ -62,7 +71,10 @@ TRANSACTION(ttESCROW_FINISH, 2, EscrowFinish, Delegation::delegatable, ({ /** This transaction type adjusts various account settings. */ -TRANSACTION(ttACCOUNT_SET, 3, AccountSet, Delegation::notDelegatable, ({ +TRANSACTION(ttACCOUNT_SET, 3, AccountSet, + Delegation::notDelegatable, + uint256{}, + ({ {sfEmailHash, soeOPTIONAL}, {sfWalletLocator, soeOPTIONAL}, {sfWalletSize, soeOPTIONAL}, @@ -76,20 +88,29 @@ TRANSACTION(ttACCOUNT_SET, 3, AccountSet, Delegation::notDelegatable, ({ })) /** This transaction type cancels an existing escrow. */ -TRANSACTION(ttESCROW_CANCEL, 4, EscrowCancel, Delegation::delegatable, ({ +TRANSACTION(ttESCROW_CANCEL, 4, EscrowCancel, + Delegation::delegatable, + uint256{}, + ({ {sfOwner, soeREQUIRED}, {sfOfferSequence, soeREQUIRED}, })) /** This transaction type sets or clears an account's "regular key". */ -TRANSACTION(ttREGULAR_KEY_SET, 5, SetRegularKey, Delegation::notDelegatable, ({ +TRANSACTION(ttREGULAR_KEY_SET, 5, SetRegularKey, + Delegation::notDelegatable, + uint256{}, + ({ {sfRegularKey, soeOPTIONAL}, })) // 6 deprecated /** This transaction type creates an offer to trade one asset for another. */ -TRANSACTION(ttOFFER_CREATE, 7, OfferCreate, Delegation::delegatable, ({ +TRANSACTION(ttOFFER_CREATE, 7, OfferCreate, + Delegation::delegatable, + uint256{}, + ({ {sfTakerPays, soeREQUIRED}, {sfTakerGets, soeREQUIRED}, {sfExpiration, soeOPTIONAL}, @@ -98,14 +119,20 @@ TRANSACTION(ttOFFER_CREATE, 7, OfferCreate, Delegation::delegatable, ({ })) /** This transaction type cancels existing offers to trade one asset for another. */ -TRANSACTION(ttOFFER_CANCEL, 8, OfferCancel, Delegation::delegatable, ({ +TRANSACTION(ttOFFER_CANCEL, 8, OfferCancel, + Delegation::delegatable, + uint256{}, + ({ {sfOfferSequence, soeREQUIRED}, })) // 9 deprecated /** This transaction type creates a new set of tickets. */ -TRANSACTION(ttTICKET_CREATE, 10, TicketCreate, Delegation::delegatable, ({ +TRANSACTION(ttTICKET_CREATE, 10, TicketCreate, + Delegation::delegatable, + featureTicketBatch, + ({ {sfTicketCount, soeREQUIRED}, })) @@ -114,13 +141,19 @@ TRANSACTION(ttTICKET_CREATE, 10, TicketCreate, Delegation::delegatable, ({ /** This transaction type modifies the signer list associated with an account. */ // The SignerEntries are optional because a SignerList is deleted by // setting the SignerQuorum to zero and omitting SignerEntries. -TRANSACTION(ttSIGNER_LIST_SET, 12, SignerListSet, Delegation::notDelegatable, ({ +TRANSACTION(ttSIGNER_LIST_SET, 12, SignerListSet, + Delegation::notDelegatable, + uint256{}, + ({ {sfSignerQuorum, soeREQUIRED}, {sfSignerEntries, soeOPTIONAL}, })) /** This transaction type creates a new unidirectional XRP payment channel. */ -TRANSACTION(ttPAYCHAN_CREATE, 13, PaymentChannelCreate, Delegation::delegatable, ({ +TRANSACTION(ttPAYCHAN_CREATE, 13, PaymentChannelCreate, + Delegation::delegatable, + uint256{}, + ({ {sfDestination, soeREQUIRED}, {sfAmount, soeREQUIRED}, {sfSettleDelay, soeREQUIRED}, @@ -130,14 +163,20 @@ TRANSACTION(ttPAYCHAN_CREATE, 13, PaymentChannelCreate, Delegation::delegatable, })) /** This transaction type funds an existing unidirectional XRP payment channel. */ -TRANSACTION(ttPAYCHAN_FUND, 14, PaymentChannelFund, Delegation::delegatable, ({ +TRANSACTION(ttPAYCHAN_FUND, 14, PaymentChannelFund, + Delegation::delegatable, + uint256{}, + ({ {sfChannel, soeREQUIRED}, {sfAmount, soeREQUIRED}, {sfExpiration, soeOPTIONAL}, })) /** This transaction type submits a claim against an existing unidirectional payment channel. */ -TRANSACTION(ttPAYCHAN_CLAIM, 15, PaymentChannelClaim, Delegation::delegatable, ({ +TRANSACTION(ttPAYCHAN_CLAIM, 15, PaymentChannelClaim, + Delegation::delegatable, + uint256{}, + ({ {sfChannel, soeREQUIRED}, {sfAmount, soeOPTIONAL}, {sfBalance, soeOPTIONAL}, @@ -147,7 +186,10 @@ TRANSACTION(ttPAYCHAN_CLAIM, 15, PaymentChannelClaim, Delegation::delegatable, ( })) /** This transaction type creates a new check. */ -TRANSACTION(ttCHECK_CREATE, 16, CheckCreate, Delegation::delegatable, ({ +TRANSACTION(ttCHECK_CREATE, 16, CheckCreate, + Delegation::delegatable, + featureChecks, + ({ {sfDestination, soeREQUIRED}, {sfSendMax, soeREQUIRED}, {sfExpiration, soeOPTIONAL}, @@ -156,19 +198,28 @@ TRANSACTION(ttCHECK_CREATE, 16, CheckCreate, Delegation::delegatable, ({ })) /** This transaction type cashes an existing check. */ -TRANSACTION(ttCHECK_CASH, 17, CheckCash, Delegation::delegatable, ({ +TRANSACTION(ttCHECK_CASH, 17, CheckCash, + Delegation::delegatable, + featureChecks, + ({ {sfCheckID, soeREQUIRED}, {sfAmount, soeOPTIONAL}, {sfDeliverMin, soeOPTIONAL}, })) /** This transaction type cancels an existing check. */ -TRANSACTION(ttCHECK_CANCEL, 18, CheckCancel, Delegation::delegatable, ({ +TRANSACTION(ttCHECK_CANCEL, 18, CheckCancel, + Delegation::delegatable, + featureChecks, + ({ {sfCheckID, soeREQUIRED}, })) /** This transaction type grants or revokes authorization to transfer funds. */ -TRANSACTION(ttDEPOSIT_PREAUTH, 19, DepositPreauth, Delegation::delegatable, ({ +TRANSACTION(ttDEPOSIT_PREAUTH, 19, DepositPreauth, + Delegation::delegatable, + featureDepositPreauth, + ({ {sfAuthorize, soeOPTIONAL}, {sfUnauthorize, soeOPTIONAL}, {sfAuthorizeCredentials, soeOPTIONAL}, @@ -176,14 +227,20 @@ TRANSACTION(ttDEPOSIT_PREAUTH, 19, DepositPreauth, Delegation::delegatable, ({ })) /** This transaction type modifies a trustline between two accounts. */ -TRANSACTION(ttTRUST_SET, 20, TrustSet, Delegation::delegatable, ({ +TRANSACTION(ttTRUST_SET, 20, TrustSet, + Delegation::delegatable, + uint256{}, + ({ {sfLimitAmount, soeOPTIONAL}, {sfQualityIn, soeOPTIONAL}, {sfQualityOut, soeOPTIONAL}, })) /** This transaction type deletes an existing account. */ -TRANSACTION(ttACCOUNT_DELETE, 21, AccountDelete, Delegation::notDelegatable, ({ +TRANSACTION(ttACCOUNT_DELETE, 21, AccountDelete, + Delegation::notDelegatable, + uint256{}, + ({ {sfDestination, soeREQUIRED}, {sfDestinationTag, soeOPTIONAL}, {sfCredentialIDs, soeOPTIONAL}, @@ -192,7 +249,10 @@ TRANSACTION(ttACCOUNT_DELETE, 21, AccountDelete, Delegation::notDelegatable, ({ // 22 reserved /** This transaction mints a new NFT. */ -TRANSACTION(ttNFTOKEN_MINT, 25, NFTokenMint, Delegation::delegatable, ({ +TRANSACTION(ttNFTOKEN_MINT, 25, NFTokenMint, + Delegation::delegatable, + featureNonFungibleTokensV1, + ({ {sfNFTokenTaxon, soeREQUIRED}, {sfTransferFee, soeOPTIONAL}, {sfIssuer, soeOPTIONAL}, @@ -203,13 +263,19 @@ TRANSACTION(ttNFTOKEN_MINT, 25, NFTokenMint, Delegation::delegatable, ({ })) /** This transaction burns (i.e. destroys) an existing NFT. */ -TRANSACTION(ttNFTOKEN_BURN, 26, NFTokenBurn, Delegation::delegatable, ({ +TRANSACTION(ttNFTOKEN_BURN, 26, NFTokenBurn, + Delegation::delegatable, + featureNonFungibleTokensV1, + ({ {sfNFTokenID, soeREQUIRED}, {sfOwner, soeOPTIONAL}, })) /** This transaction creates a new offer to buy or sell an NFT. */ -TRANSACTION(ttNFTOKEN_CREATE_OFFER, 27, NFTokenCreateOffer, Delegation::delegatable, ({ +TRANSACTION(ttNFTOKEN_CREATE_OFFER, 27, NFTokenCreateOffer, + Delegation::delegatable, + featureNonFungibleTokensV1, + ({ {sfNFTokenID, soeREQUIRED}, {sfAmount, soeREQUIRED}, {sfDestination, soeOPTIONAL}, @@ -218,25 +284,37 @@ TRANSACTION(ttNFTOKEN_CREATE_OFFER, 27, NFTokenCreateOffer, Delegation::delegata })) /** This transaction cancels an existing offer to buy or sell an existing NFT. */ -TRANSACTION(ttNFTOKEN_CANCEL_OFFER, 28, NFTokenCancelOffer, Delegation::delegatable, ({ +TRANSACTION(ttNFTOKEN_CANCEL_OFFER, 28, NFTokenCancelOffer, + Delegation::delegatable, + featureNonFungibleTokensV1, + ({ {sfNFTokenOffers, soeREQUIRED}, })) /** This transaction accepts an existing offer to buy or sell an existing NFT. */ -TRANSACTION(ttNFTOKEN_ACCEPT_OFFER, 29, NFTokenAcceptOffer, Delegation::delegatable, ({ +TRANSACTION(ttNFTOKEN_ACCEPT_OFFER, 29, NFTokenAcceptOffer, + Delegation::delegatable, + featureNonFungibleTokensV1, + ({ {sfNFTokenBuyOffer, soeOPTIONAL}, {sfNFTokenSellOffer, soeOPTIONAL}, {sfNFTokenBrokerFee, soeOPTIONAL}, })) /** This transaction claws back issued tokens. */ -TRANSACTION(ttCLAWBACK, 30, Clawback, Delegation::delegatable, ({ +TRANSACTION(ttCLAWBACK, 30, Clawback, + Delegation::delegatable, + featureClawback, + ({ {sfAmount, soeREQUIRED, soeMPTSupported}, {sfHolder, soeOPTIONAL}, })) /** This transaction claws back tokens from an AMM pool. */ -TRANSACTION(ttAMM_CLAWBACK, 31, AMMClawback, Delegation::delegatable, ({ +TRANSACTION(ttAMM_CLAWBACK, 31, AMMClawback, + Delegation::delegatable, + featureAMMClawback, + ({ {sfHolder, soeREQUIRED}, {sfAsset, soeREQUIRED}, {sfAsset2, soeREQUIRED}, @@ -244,14 +322,20 @@ TRANSACTION(ttAMM_CLAWBACK, 31, AMMClawback, Delegation::delegatable, ({ })) /** This transaction type creates an AMM instance */ -TRANSACTION(ttAMM_CREATE, 35, AMMCreate, Delegation::delegatable, ({ +TRANSACTION(ttAMM_CREATE, 35, AMMCreate, + Delegation::delegatable, + featureAMM, + ({ {sfAmount, soeREQUIRED}, {sfAmount2, soeREQUIRED}, {sfTradingFee, soeREQUIRED}, })) /** This transaction type deposits into an AMM instance */ -TRANSACTION(ttAMM_DEPOSIT, 36, AMMDeposit, Delegation::delegatable, ({ +TRANSACTION(ttAMM_DEPOSIT, 36, AMMDeposit, + Delegation::delegatable, + featureAMM, + ({ {sfAsset, soeREQUIRED}, {sfAsset2, soeREQUIRED}, {sfAmount, soeOPTIONAL}, @@ -262,7 +346,10 @@ TRANSACTION(ttAMM_DEPOSIT, 36, AMMDeposit, Delegation::delegatable, ({ })) /** This transaction type withdraws from an AMM instance */ -TRANSACTION(ttAMM_WITHDRAW, 37, AMMWithdraw, Delegation::delegatable, ({ +TRANSACTION(ttAMM_WITHDRAW, 37, AMMWithdraw, + Delegation::delegatable, + featureAMM, + ({ {sfAsset, soeREQUIRED}, {sfAsset2, soeREQUIRED}, {sfAmount, soeOPTIONAL}, @@ -272,14 +359,20 @@ TRANSACTION(ttAMM_WITHDRAW, 37, AMMWithdraw, Delegation::delegatable, ({ })) /** This transaction type votes for the trading fee */ -TRANSACTION(ttAMM_VOTE, 38, AMMVote, Delegation::delegatable, ({ +TRANSACTION(ttAMM_VOTE, 38, AMMVote, + Delegation::delegatable, + featureAMM, + ({ {sfAsset, soeREQUIRED}, {sfAsset2, soeREQUIRED}, {sfTradingFee, soeREQUIRED}, })) /** This transaction type bids for the auction slot */ -TRANSACTION(ttAMM_BID, 39, AMMBid, Delegation::delegatable, ({ +TRANSACTION(ttAMM_BID, 39, AMMBid, + Delegation::delegatable, + featureAMM, + ({ {sfAsset, soeREQUIRED}, {sfAsset2, soeREQUIRED}, {sfBidMin, soeOPTIONAL}, @@ -288,20 +381,29 @@ TRANSACTION(ttAMM_BID, 39, AMMBid, Delegation::delegatable, ({ })) /** This transaction type deletes AMM in the empty state */ -TRANSACTION(ttAMM_DELETE, 40, AMMDelete, Delegation::delegatable, ({ +TRANSACTION(ttAMM_DELETE, 40, AMMDelete, + Delegation::delegatable, + featureAMM, + ({ {sfAsset, soeREQUIRED}, {sfAsset2, soeREQUIRED}, })) /** This transactions creates a crosschain sequence number */ -TRANSACTION(ttXCHAIN_CREATE_CLAIM_ID, 41, XChainCreateClaimID, Delegation::delegatable, ({ +TRANSACTION(ttXCHAIN_CREATE_CLAIM_ID, 41, XChainCreateClaimID, + Delegation::delegatable, + featureXChainBridge, + ({ {sfXChainBridge, soeREQUIRED}, {sfSignatureReward, soeREQUIRED}, {sfOtherChainSource, soeREQUIRED}, })) /** This transactions initiates a crosschain transaction */ -TRANSACTION(ttXCHAIN_COMMIT, 42, XChainCommit, Delegation::delegatable, ({ +TRANSACTION(ttXCHAIN_COMMIT, 42, XChainCommit, + Delegation::delegatable, + featureXChainBridge, + ({ {sfXChainBridge, soeREQUIRED}, {sfXChainClaimID, soeREQUIRED}, {sfAmount, soeREQUIRED}, @@ -309,7 +411,10 @@ TRANSACTION(ttXCHAIN_COMMIT, 42, XChainCommit, Delegation::delegatable, ({ })) /** This transaction completes a crosschain transaction */ -TRANSACTION(ttXCHAIN_CLAIM, 43, XChainClaim, Delegation::delegatable, ({ +TRANSACTION(ttXCHAIN_CLAIM, 43, XChainClaim, + Delegation::delegatable, + featureXChainBridge, + ({ {sfXChainBridge, soeREQUIRED}, {sfXChainClaimID, soeREQUIRED}, {sfDestination, soeREQUIRED}, @@ -318,7 +423,10 @@ TRANSACTION(ttXCHAIN_CLAIM, 43, XChainClaim, Delegation::delegatable, ({ })) /** This transaction initiates a crosschain account create transaction */ -TRANSACTION(ttXCHAIN_ACCOUNT_CREATE_COMMIT, 44, XChainAccountCreateCommit, Delegation::delegatable, ({ +TRANSACTION(ttXCHAIN_ACCOUNT_CREATE_COMMIT, 44, XChainAccountCreateCommit, + Delegation::delegatable, + featureXChainBridge, + ({ {sfXChainBridge, soeREQUIRED}, {sfDestination, soeREQUIRED}, {sfAmount, soeREQUIRED}, @@ -326,7 +434,10 @@ TRANSACTION(ttXCHAIN_ACCOUNT_CREATE_COMMIT, 44, XChainAccountCreateCommit, Deleg })) /** This transaction adds an attestation to a claim */ -TRANSACTION(ttXCHAIN_ADD_CLAIM_ATTESTATION, 45, XChainAddClaimAttestation, Delegation::delegatable, ({ +TRANSACTION(ttXCHAIN_ADD_CLAIM_ATTESTATION, 45, XChainAddClaimAttestation, + Delegation::delegatable, + featureXChainBridge, + ({ {sfXChainBridge, soeREQUIRED}, {sfAttestationSignerAccount, soeREQUIRED}, @@ -342,7 +453,10 @@ TRANSACTION(ttXCHAIN_ADD_CLAIM_ATTESTATION, 45, XChainAddClaimAttestation, Deleg })) /** This transaction adds an attestation to an account */ -TRANSACTION(ttXCHAIN_ADD_ACCOUNT_CREATE_ATTESTATION, 46, XChainAddAccountCreateAttestation, Delegation::delegatable, ({ +TRANSACTION(ttXCHAIN_ADD_ACCOUNT_CREATE_ATTESTATION, 46, XChainAddAccountCreateAttestation, + Delegation::delegatable, + featureXChainBridge, + ({ {sfXChainBridge, soeREQUIRED}, {sfAttestationSignerAccount, soeREQUIRED}, @@ -359,31 +473,46 @@ TRANSACTION(ttXCHAIN_ADD_ACCOUNT_CREATE_ATTESTATION, 46, XChainAddAccountCreateA })) /** This transaction modifies a sidechain */ -TRANSACTION(ttXCHAIN_MODIFY_BRIDGE, 47, XChainModifyBridge, Delegation::delegatable, ({ +TRANSACTION(ttXCHAIN_MODIFY_BRIDGE, 47, XChainModifyBridge, + Delegation::delegatable, + featureXChainBridge, + ({ {sfXChainBridge, soeREQUIRED}, {sfSignatureReward, soeOPTIONAL}, {sfMinAccountCreateAmount, soeOPTIONAL}, })) /** This transactions creates a sidechain */ -TRANSACTION(ttXCHAIN_CREATE_BRIDGE, 48, XChainCreateBridge, Delegation::delegatable, ({ +TRANSACTION(ttXCHAIN_CREATE_BRIDGE, 48, XChainCreateBridge, + Delegation::delegatable, + featureXChainBridge, + ({ {sfXChainBridge, soeREQUIRED}, {sfSignatureReward, soeREQUIRED}, {sfMinAccountCreateAmount, soeOPTIONAL}, })) /** This transaction type creates or updates a DID */ -TRANSACTION(ttDID_SET, 49, DIDSet, Delegation::delegatable, ({ +TRANSACTION(ttDID_SET, 49, DIDSet, + Delegation::delegatable, + featureDID, + ({ {sfDIDDocument, soeOPTIONAL}, {sfURI, soeOPTIONAL}, {sfData, soeOPTIONAL}, })) /** This transaction type deletes a DID */ -TRANSACTION(ttDID_DELETE, 50, DIDDelete, Delegation::delegatable, ({})) +TRANSACTION(ttDID_DELETE, 50, DIDDelete, + Delegation::delegatable, + featureDID, + ({})) /** This transaction type creates an Oracle instance */ -TRANSACTION(ttORACLE_SET, 51, OracleSet, Delegation::delegatable, ({ +TRANSACTION(ttORACLE_SET, 51, OracleSet, + Delegation::delegatable, + featurePriceOracle, + ({ {sfOracleDocumentID, soeREQUIRED}, {sfProvider, soeOPTIONAL}, {sfURI, soeOPTIONAL}, @@ -393,18 +522,27 @@ TRANSACTION(ttORACLE_SET, 51, OracleSet, Delegation::delegatable, ({ })) /** This transaction type deletes an Oracle instance */ -TRANSACTION(ttORACLE_DELETE, 52, OracleDelete, Delegation::delegatable, ({ +TRANSACTION(ttORACLE_DELETE, 52, OracleDelete, + Delegation::delegatable, + featurePriceOracle, + ({ {sfOracleDocumentID, soeREQUIRED}, })) /** This transaction type fixes a problem in the ledger state */ -TRANSACTION(ttLEDGER_STATE_FIX, 53, LedgerStateFix, Delegation::delegatable, ({ +TRANSACTION(ttLEDGER_STATE_FIX, 53, LedgerStateFix, + Delegation::delegatable, + fixNFTokenPageLinks, + ({ {sfLedgerFixType, soeREQUIRED}, {sfOwner, soeOPTIONAL}, })) /** This transaction type creates a MPTokensIssuance instance */ -TRANSACTION(ttMPTOKEN_ISSUANCE_CREATE, 54, MPTokenIssuanceCreate, Delegation::delegatable, ({ +TRANSACTION(ttMPTOKEN_ISSUANCE_CREATE, 54, MPTokenIssuanceCreate, + Delegation::delegatable, + featureMPTokensV1, + ({ {sfAssetScale, soeOPTIONAL}, {sfTransferFee, soeOPTIONAL}, {sfMaximumAmount, soeOPTIONAL}, @@ -413,25 +551,37 @@ TRANSACTION(ttMPTOKEN_ISSUANCE_CREATE, 54, MPTokenIssuanceCreate, Delegation::de })) /** This transaction type destroys a MPTokensIssuance instance */ -TRANSACTION(ttMPTOKEN_ISSUANCE_DESTROY, 55, MPTokenIssuanceDestroy, Delegation::delegatable, ({ +TRANSACTION(ttMPTOKEN_ISSUANCE_DESTROY, 55, MPTokenIssuanceDestroy, + Delegation::delegatable, + featureMPTokensV1, + ({ {sfMPTokenIssuanceID, soeREQUIRED}, })) /** This transaction type sets flags on a MPTokensIssuance or MPToken instance */ -TRANSACTION(ttMPTOKEN_ISSUANCE_SET, 56, MPTokenIssuanceSet, Delegation::delegatable, ({ +TRANSACTION(ttMPTOKEN_ISSUANCE_SET, 56, MPTokenIssuanceSet, + Delegation::delegatable, + featureMPTokensV1, + ({ {sfMPTokenIssuanceID, soeREQUIRED}, {sfHolder, soeOPTIONAL}, {sfDomainID, soeOPTIONAL}, })) /** This transaction type authorizes a MPToken instance */ -TRANSACTION(ttMPTOKEN_AUTHORIZE, 57, MPTokenAuthorize, Delegation::delegatable, ({ +TRANSACTION(ttMPTOKEN_AUTHORIZE, 57, MPTokenAuthorize, + Delegation::delegatable, + featureMPTokensV1, + ({ {sfMPTokenIssuanceID, soeREQUIRED}, {sfHolder, soeOPTIONAL}, })) /** This transaction type create an Credential instance */ -TRANSACTION(ttCREDENTIAL_CREATE, 58, CredentialCreate, Delegation::delegatable, ({ +TRANSACTION(ttCREDENTIAL_CREATE, 58, CredentialCreate, + Delegation::delegatable, + featureCredentials, + ({ {sfSubject, soeREQUIRED}, {sfCredentialType, soeREQUIRED}, {sfExpiration, soeOPTIONAL}, @@ -439,44 +589,65 @@ TRANSACTION(ttCREDENTIAL_CREATE, 58, CredentialCreate, Delegation::delegatable, })) /** This transaction type accept an Credential object */ -TRANSACTION(ttCREDENTIAL_ACCEPT, 59, CredentialAccept, Delegation::delegatable, ({ +TRANSACTION(ttCREDENTIAL_ACCEPT, 59, CredentialAccept, + Delegation::delegatable, + featureCredentials, + ({ {sfIssuer, soeREQUIRED}, {sfCredentialType, soeREQUIRED}, })) /** This transaction type delete an Credential object */ -TRANSACTION(ttCREDENTIAL_DELETE, 60, CredentialDelete, Delegation::delegatable, ({ +TRANSACTION(ttCREDENTIAL_DELETE, 60, CredentialDelete, + Delegation::delegatable, + featureCredentials, + ({ {sfSubject, soeOPTIONAL}, {sfIssuer, soeOPTIONAL}, {sfCredentialType, soeREQUIRED}, })) /** This transaction type modify a NFToken */ -TRANSACTION(ttNFTOKEN_MODIFY, 61, NFTokenModify, Delegation::delegatable, ({ +TRANSACTION(ttNFTOKEN_MODIFY, 61, NFTokenModify, + Delegation::delegatable, + featureDynamicNFT, + ({ {sfNFTokenID, soeREQUIRED}, {sfOwner, soeOPTIONAL}, {sfURI, soeOPTIONAL}, })) /** This transaction type creates or modifies a Permissioned Domain */ -TRANSACTION(ttPERMISSIONED_DOMAIN_SET, 62, PermissionedDomainSet, Delegation::delegatable, ({ +TRANSACTION(ttPERMISSIONED_DOMAIN_SET, 62, PermissionedDomainSet, + Delegation::delegatable, + featurePermissionedDomains, + ({ {sfDomainID, soeOPTIONAL}, {sfAcceptedCredentials, soeREQUIRED}, })) /** This transaction type deletes a Permissioned Domain */ -TRANSACTION(ttPERMISSIONED_DOMAIN_DELETE, 63, PermissionedDomainDelete, Delegation::delegatable, ({ +TRANSACTION(ttPERMISSIONED_DOMAIN_DELETE, 63, PermissionedDomainDelete, + Delegation::delegatable, + featurePermissionedDomains, + ({ {sfDomainID, soeREQUIRED}, })) /** This transaction type delegates authorized account specified permissions */ -TRANSACTION(ttDELEGATE_SET, 64, DelegateSet, Delegation::notDelegatable, ({ +TRANSACTION(ttDELEGATE_SET, 64, DelegateSet, + Delegation::notDelegatable, + featurePermissionDelegation, + ({ {sfAuthorize, soeREQUIRED}, {sfPermissions, soeREQUIRED}, })) /** This transaction creates a single asset vault. */ -TRANSACTION(ttVAULT_CREATE, 65, VaultCreate, Delegation::delegatable, ({ +TRANSACTION(ttVAULT_CREATE, 65, VaultCreate, + Delegation::delegatable, + featureSingleAssetVault, + ({ {sfAsset, soeREQUIRED, soeMPTSupported}, {sfAssetsMaximum, soeOPTIONAL}, {sfMPTokenMetadata, soeOPTIONAL}, @@ -487,7 +658,10 @@ TRANSACTION(ttVAULT_CREATE, 65, VaultCreate, Delegation::delegatable, ({ })) /** This transaction updates a single asset vault. */ -TRANSACTION(ttVAULT_SET, 66, VaultSet, Delegation::delegatable, ({ +TRANSACTION(ttVAULT_SET, 66, VaultSet, + Delegation::delegatable, + featureSingleAssetVault, + ({ {sfVaultID, soeREQUIRED}, {sfAssetsMaximum, soeOPTIONAL}, {sfDomainID, soeOPTIONAL}, @@ -495,18 +669,27 @@ TRANSACTION(ttVAULT_SET, 66, VaultSet, Delegation::delegatable, ({ })) /** This transaction deletes a single asset vault. */ -TRANSACTION(ttVAULT_DELETE, 67, VaultDelete, Delegation::delegatable, ({ +TRANSACTION(ttVAULT_DELETE, 67, VaultDelete, + Delegation::delegatable, + featureSingleAssetVault, + ({ {sfVaultID, soeREQUIRED}, })) /** This transaction trades assets for shares with a vault. */ -TRANSACTION(ttVAULT_DEPOSIT, 68, VaultDeposit, Delegation::delegatable, ({ +TRANSACTION(ttVAULT_DEPOSIT, 68, VaultDeposit, + Delegation::delegatable, + featureSingleAssetVault, + ({ {sfVaultID, soeREQUIRED}, {sfAmount, soeREQUIRED, soeMPTSupported}, })) /** This transaction trades shares for assets with a vault. */ -TRANSACTION(ttVAULT_WITHDRAW, 69, VaultWithdraw, Delegation::delegatable, ({ +TRANSACTION(ttVAULT_WITHDRAW, 69, VaultWithdraw, + Delegation::delegatable, + featureSingleAssetVault, + ({ {sfVaultID, soeREQUIRED}, {sfAmount, soeREQUIRED, soeMPTSupported}, {sfDestination, soeOPTIONAL}, @@ -514,14 +697,20 @@ TRANSACTION(ttVAULT_WITHDRAW, 69, VaultWithdraw, Delegation::delegatable, ({ })) /** This transaction claws back tokens from a vault. */ -TRANSACTION(ttVAULT_CLAWBACK, 70, VaultClawback, Delegation::delegatable, ({ +TRANSACTION(ttVAULT_CLAWBACK, 70, VaultClawback, + Delegation::delegatable, + featureSingleAssetVault, + ({ {sfVaultID, soeREQUIRED}, {sfHolder, soeREQUIRED}, {sfAmount, soeOPTIONAL, soeMPTSupported}, })) /** This transaction type batches together transactions. */ -TRANSACTION(ttBATCH, 71, Batch, Delegation::notDelegatable, ({ +TRANSACTION(ttBATCH, 71, Batch, + Delegation::notDelegatable, + featureBatch, + ({ {sfRawTransactions, soeREQUIRED}, {sfBatchSigners, soeOPTIONAL}, })) @@ -530,7 +719,10 @@ TRANSACTION(ttBATCH, 71, Batch, Delegation::notDelegatable, ({ For details, see: https://xrpl.org/amendments.html */ -TRANSACTION(ttAMENDMENT, 100, EnableAmendment, Delegation::notDelegatable, ({ +TRANSACTION(ttAMENDMENT, 100, EnableAmendment, + Delegation::notDelegatable, + uint256{}, + ({ {sfLedgerSequence, soeREQUIRED}, {sfAmendment, soeREQUIRED}, })) @@ -538,7 +730,10 @@ TRANSACTION(ttAMENDMENT, 100, EnableAmendment, Delegation::notDelegatable, ({ /** This system-generated transaction type is used to update the network's fee settings. For details, see: https://xrpl.org/fee-voting.html */ -TRANSACTION(ttFEE, 101, SetFee, Delegation::notDelegatable, ({ +TRANSACTION(ttFEE, 101, SetFee, + Delegation::notDelegatable, + uint256{}, + ({ {sfLedgerSequence, soeOPTIONAL}, // Old version uses raw numbers {sfBaseFee, soeOPTIONAL}, @@ -555,7 +750,10 @@ TRANSACTION(ttFEE, 101, SetFee, Delegation::notDelegatable, ({ For details, see: https://xrpl.org/negative-unl.html */ -TRANSACTION(ttUNL_MODIFY, 102, UNLModify, Delegation::notDelegatable, ({ +TRANSACTION(ttUNL_MODIFY, 102, UNLModify, + Delegation::notDelegatable, + uint256{}, + ({ {sfUNLModifyDisabling, soeREQUIRED}, {sfLedgerSequence, soeREQUIRED}, {sfUNLModifyValidator, soeREQUIRED}, diff --git a/include/xrpl/protocol/jss.h b/include/xrpl/protocol/jss.h index 68d2497aca..d847cf6012 100644 --- a/include/xrpl/protocol/jss.h +++ b/include/xrpl/protocol/jss.h @@ -710,7 +710,7 @@ JSS(write_load); // out: GetCounts #pragma push_macro("TRANSACTION") #undef TRANSACTION -#define TRANSACTION(tag, value, name, delegatable, fields) JSS(name); +#define TRANSACTION(tag, value, name, ...) JSS(name); #include diff --git a/src/libxrpl/protocol/Permissions.cpp b/src/libxrpl/protocol/Permissions.cpp index ca8cb26f36..781799f128 100644 --- a/src/libxrpl/protocol/Permissions.cpp +++ b/src/libxrpl/protocol/Permissions.cpp @@ -18,6 +18,7 @@ //============================================================================== #include +#include #include #include @@ -25,11 +26,24 @@ namespace ripple { Permission::Permission() { + txFeatureMap_ = { +#pragma push_macro("TRANSACTION") +#undef TRANSACTION + +#define TRANSACTION(tag, value, name, delegatable, amendment, ...) \ + {value, amendment}, + +#include + +#undef TRANSACTION +#pragma pop_macro("TRANSACTION") + }; + delegatableTx_ = { #pragma push_macro("TRANSACTION") #undef TRANSACTION -#define TRANSACTION(tag, value, name, delegatable, fields) {value, delegatable}, +#define TRANSACTION(tag, value, name, delegatable, ...) {value, delegatable}, #include @@ -118,7 +132,9 @@ Permission::getGranularTxType(GranularPermissionType const& gpType) const } bool -Permission::isDelegatable(std::uint32_t const& permissionValue) const +Permission::isDelegatable( + std::uint32_t const& permissionValue, + Rules const& rules) const { auto const granularPermission = getGranularName(static_cast(permissionValue)); @@ -126,7 +142,27 @@ Permission::isDelegatable(std::uint32_t const& permissionValue) const // granular permissions are always allowed to be delegated return true; - auto const it = delegatableTx_.find(permissionValue - 1); + auto const txType = permissionToTxType(permissionValue); + auto const it = delegatableTx_.find(txType); + + if (rules.enabled(fixDelegateV1_1)) + { + if (it == delegatableTx_.end()) + return false; + + auto const txFeaturesIt = txFeatureMap_.find(txType); + XRPL_ASSERT( + txFeaturesIt != txFeatureMap_.end(), + "ripple::Permissions::isDelegatable : tx exists in txFeatureMap_"); + + // fixDelegateV1_1: Delegation is only allowed if the required amendment + // for the transaction is enabled. For transactions that do not require + // an amendment, delegation is always allowed. + if (txFeaturesIt->second != uint256{} && + !rules.enabled(txFeaturesIt->second)) + return false; + } + if (it != delegatableTx_.end() && it->second == Delegation::notDelegatable) return false; diff --git a/src/libxrpl/protocol/TxFormats.cpp b/src/libxrpl/protocol/TxFormats.cpp index 5edffeb666..c10c023ee9 100644 --- a/src/libxrpl/protocol/TxFormats.cpp +++ b/src/libxrpl/protocol/TxFormats.cpp @@ -55,7 +55,7 @@ TxFormats::TxFormats() #undef TRANSACTION #define UNWRAP(...) __VA_ARGS__ -#define TRANSACTION(tag, value, name, delegatable, fields) \ +#define TRANSACTION(tag, value, name, delegatable, amendment, fields) \ add(jss::name, tag, UNWRAP fields, commonFields); #include diff --git a/src/test/app/AMMClawback_test.cpp b/src/test/app/AMMClawback_test.cpp index 9564911664..707113fe32 100644 --- a/src/test/app/AMMClawback_test.cpp +++ b/src/test/app/AMMClawback_test.cpp @@ -2442,8 +2442,7 @@ class AMMClawback_test : public beast::unit_test::suite void run() override { - FeatureBitset const all{ - jtx::testable_amendments() | fixAMMClawbackRounding}; + FeatureBitset const all = jtx::testable_amendments(); testInvalidRequest(); testFeatureDisabled(all - featureAMMClawback); diff --git a/src/test/app/Delegate_test.cpp b/src/test/app/Delegate_test.cpp index 44cb6a54b6..ea5e073a55 100644 --- a/src/test/app/Delegate_test.cpp +++ b/src/test/app/Delegate_test.cpp @@ -16,6 +16,7 @@ //============================================================================== #include +#include #include #include @@ -139,12 +140,12 @@ class Delegate_test : public beast::unit_test::suite } void - testInvalidRequest() + testInvalidRequest(FeatureBitset features) { testcase("test invalid DelegateSet"); using namespace jtx; - Env env(*this); + Env env(*this, features); Account gw{"gateway"}; Account alice{"alice"}; Account bob{"bob"}; @@ -216,22 +217,17 @@ class Delegate_test : public beast::unit_test::suite } // non-delegatable transaction + auto const res = features[fixDelegateV1_1] ? ter(temMALFORMED) + : ter(tecNO_PERMISSION); { - env(delegate::set(gw, alice, {"SetRegularKey"}), - ter(tecNO_PERMISSION)); - env(delegate::set(gw, alice, {"AccountSet"}), - ter(tecNO_PERMISSION)); - env(delegate::set(gw, alice, {"SignerListSet"}), - ter(tecNO_PERMISSION)); - env(delegate::set(gw, alice, {"DelegateSet"}), - ter(tecNO_PERMISSION)); - env(delegate::set(gw, alice, {"SetRegularKey"}), - ter(tecNO_PERMISSION)); - env(delegate::set(gw, alice, {"EnableAmendment"}), - ter(tecNO_PERMISSION)); - env(delegate::set(gw, alice, {"UNLModify"}), ter(tecNO_PERMISSION)); - env(delegate::set(gw, alice, {"SetFee"}), ter(tecNO_PERMISSION)); - env(delegate::set(gw, alice, {"Batch"}), ter(tecNO_PERMISSION)); + env(delegate::set(gw, alice, {"SetRegularKey"}), res); + env(delegate::set(gw, alice, {"AccountSet"}), res); + env(delegate::set(gw, alice, {"SignerListSet"}), res); + env(delegate::set(gw, alice, {"DelegateSet"}), res); + env(delegate::set(gw, alice, {"EnableAmendment"}), res); + env(delegate::set(gw, alice, {"UNLModify"}), res); + env(delegate::set(gw, alice, {"SetFee"}), res); + env(delegate::set(gw, alice, {"Batch"}), res); } } @@ -536,7 +532,7 @@ class Delegate_test : public beast::unit_test::suite } void - testPaymentGranular() + testPaymentGranular(FeatureBitset features) { testcase("test payment granular"); using namespace jtx; @@ -706,6 +702,158 @@ class Delegate_test : public beast::unit_test::suite env.require(balance(alice, USD(50))); BEAST_EXPECT(env.balance(bob, USD) == USD(0)); } + + // disallow cross currency payment with only PaymentBurn/PaymentMint + // permission + { + Env env(*this, features); + Account const alice{"alice"}; + Account const bob{"bob"}; + Account const gw{"gateway"}; + Account const carol{"carol"}; + auto const USD = gw["USD"]; + + env.fund(XRP(10000), alice, bob, carol, gw); + env.close(); + env.trust(USD(50000), alice); + env.trust(USD(50000), bob); + env.trust(USD(50000), carol); + env(pay(gw, alice, USD(10000))); + env(pay(gw, bob, USD(10000))); + env(pay(gw, carol, USD(10000))); + env.close(); + + auto const result = features[fixDelegateV1_1] + ? static_cast(tecNO_DELEGATE_PERMISSION) + : static_cast(tesSUCCESS); + auto const offerCount = features[fixDelegateV1_1] ? 1 : 0; + + // PaymentMint + { + env(offer(carol, XRP(100), USD(501))); + BEAST_EXPECT(expectOffers(env, carol, 1)); + env(delegate::set(gw, bob, {"PaymentMint"})); + env.close(); + + // post-amendment: fixDelegateV1_1 + // bob can not send cross currency payment on behalf of the gw, + // even with PaymentMint permission and gw being the issuer. + env(pay(gw, alice, USD(5000)), + path(~USD), + sendmax(XRP(1001)), + txflags(tfPartialPayment), + delegate::as(bob), + ter(result)); + BEAST_EXPECT(expectOffers(env, carol, offerCount)); + + // succeed with direct payment + env(pay(gw, alice, USD(100)), delegate::as(bob)); + env.close(); + } + + // PaymentBurn + { + env(offer(bob, XRP(100), USD(501))); + BEAST_EXPECT(expectOffers(env, bob, 1)); + env(delegate::set(alice, bob, {"PaymentBurn"})); + env.close(); + + // post-amendment: fixDelegateV1_1 + // bob can not send cross currency payment on behalf of alice, + // even with PaymentBurn permission and gw being the issuer. + env(pay(alice, gw, USD(5000)), + path(~USD), + sendmax(XRP(1001)), + txflags(tfPartialPayment), + delegate::as(bob), + ter(result)); + BEAST_EXPECT(expectOffers(env, bob, offerCount)); + + // succeed with direct payment + env(pay(alice, gw, USD(100)), delegate::as(bob)); + env.close(); + } + } + + // PaymentMint and PaymentBurn for MPT + { + std::string logs; + Env env(*this, features, std::make_unique(&logs)); + Account const alice{"alice"}; + Account const bob{"bob"}; + Account const gw{"gateway"}; + + MPTTester mpt(env, gw, {.holders = {alice, bob}}); + mpt.create( + {.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanTransfer}); + + mpt.authorize({.account = alice}); + mpt.authorize({.account = bob}); + + auto const MPT = mpt["MPT"]; + env(pay(gw, alice, MPT(500))); + env(pay(gw, bob, MPT(500))); + env.close(); + auto aliceMPT = env.balance(alice, MPT); + auto bobMPT = env.balance(bob, MPT); + + // PaymentMint + { + env(delegate::set(gw, bob, {"PaymentMint"})); + env.close(); + + if (!features[fixDelegateV1_1]) + { + // pre-amendment: PaymentMint is not supported for MPT + env(pay(gw, alice, MPT(50)), + delegate::as(bob), + ter(tefEXCEPTION)); + } + else + { + env(pay(gw, alice, MPT(50)), delegate::as(bob)); + BEAST_EXPECT(env.balance(alice, MPT) == aliceMPT + MPT(50)); + BEAST_EXPECT(env.balance(bob, MPT) == bobMPT); + aliceMPT = env.balance(alice, MPT); + } + } + + // PaymentBurn + { + env(delegate::set(alice, bob, {"PaymentBurn"})); + env.close(); + + if (!features[fixDelegateV1_1]) + { + // pre-amendment: PaymentBurn is not supported for MPT + env(pay(alice, gw, MPT(50)), + delegate::as(bob), + ter(tefEXCEPTION)); + } + else + { + env(pay(alice, gw, MPT(50)), delegate::as(bob)); + BEAST_EXPECT(env.balance(alice, MPT) == aliceMPT - MPT(50)); + BEAST_EXPECT(env.balance(bob, MPT) == bobMPT); + aliceMPT = env.balance(alice, MPT); + } + } + + // Payment transaction for MPT is allowed for both pre and post + // amendment + { + env(delegate::set( + alice, bob, {"PaymentBurn", "PaymentMint", "Payment"})); + env.close(); + env(pay(alice, gw, MPT(50)), delegate::as(bob)); + BEAST_EXPECT(env.balance(alice, MPT) == aliceMPT - MPT(50)); + BEAST_EXPECT(env.balance(bob, MPT) == bobMPT); + aliceMPT = env.balance(alice, MPT); + env(pay(alice, bob, MPT(100)), delegate::as(bob)); + BEAST_EXPECT(env.balance(alice, MPT) == aliceMPT - MPT(100)); + BEAST_EXPECT(env.balance(bob, MPT) == bobMPT + MPT(100)); + } + } } void @@ -1476,18 +1624,216 @@ class Delegate_test : public beast::unit_test::suite BEAST_EXPECT(env.balance(edward) == edwardBalance); } + void + testPermissionValue(FeatureBitset features) + { + testcase("test permission value"); + using namespace jtx; + + Env env(*this, features); + + Account alice{"alice"}; + Account bob{"bob"}; + env.fund(XRP(100000), alice, bob); + env.close(); + + auto buildRequest = [&](auto value) -> Json::Value { + Json::Value jv; + jv[jss::TransactionType] = jss::DelegateSet; + jv[jss::Account] = alice.human(); + jv[sfAuthorize.jsonName] = bob.human(); + + Json::Value permissionsJson(Json::arrayValue); + Json::Value permissionValue; + permissionValue[sfPermissionValue.jsonName] = value; + Json::Value permissionObj; + permissionObj[sfPermission.jsonName] = permissionValue; + permissionsJson.append(permissionObj); + jv[sfPermissions.jsonName] = permissionsJson; + + return jv; + }; + + // invalid permission value. + // neither granular permission nor transaction level permission + for (auto value : {0, 100000, 54321}) + { + auto jv = buildRequest(value); + if (!features[fixDelegateV1_1]) + env(jv); + else + env(jv, ter(temMALFORMED)); + } + } + + void + testTxReqireFeatures(FeatureBitset features) + { + testcase("test delegate disabled tx"); + using namespace jtx; + + // map of tx and required feature. + // non-delegatable tx are not included. + // NFTokenMint, NFTokenBurn, NFTokenCreateOffer, NFTokenCancelOffer, + // NFTokenAcceptOffer are not included, they are tested separately. + std::unordered_map txRequiredFeatures{ + {"TicketCreate", featureTicketBatch}, + {"CheckCreate", featureChecks}, + {"CheckCash", featureChecks}, + {"CheckCancel", featureChecks}, + {"DepositPreauth", featureDepositPreauth}, + {"Clawback", featureClawback}, + {"AMMClawback", featureAMMClawback}, + {"AMMCreate", featureAMM}, + {"AMMDeposit", featureAMM}, + {"AMMWithdraw", featureAMM}, + {"AMMVote", featureAMM}, + {"AMMBid", featureAMM}, + {"AMMDelete", featureAMM}, + {"XChainCreateClaimID", featureXChainBridge}, + {"XChainCommit", featureXChainBridge}, + {"XChainClaim", featureXChainBridge}, + {"XChainAccountCreateCommit", featureXChainBridge}, + {"XChainAddClaimAttestation", featureXChainBridge}, + {"XChainAddAccountCreateAttestation", featureXChainBridge}, + {"XChainModifyBridge", featureXChainBridge}, + {"XChainCreateBridge", featureXChainBridge}, + {"DIDSet", featureDID}, + {"DIDDelete", featureDID}, + {"OracleSet", featurePriceOracle}, + {"OracleDelete", featurePriceOracle}, + {"LedgerStateFix", fixNFTokenPageLinks}, + {"MPTokenIssuanceCreate", featureMPTokensV1}, + {"MPTokenIssuanceDestroy", featureMPTokensV1}, + {"MPTokenIssuanceSet", featureMPTokensV1}, + {"MPTokenAuthorize", featureMPTokensV1}, + {"CredentialCreate", featureCredentials}, + {"CredentialAccept", featureCredentials}, + {"CredentialDelete", featureCredentials}, + {"NFTokenModify", featureDynamicNFT}, + {"PermissionedDomainSet", featurePermissionedDomains}, + {"PermissionedDomainDelete", featurePermissionedDomains}, + {"VaultCreate", featureSingleAssetVault}, + {"VaultSet", featureSingleAssetVault}, + {"VaultDelete", featureSingleAssetVault}, + {"VaultDeposit", featureSingleAssetVault}, + {"VaultWithdraw", featureSingleAssetVault}, + {"VaultClawback", featureSingleAssetVault}}; + + // fixDelegateV1_1 post-amendment: can not delegate tx if any + // required feature disabled. + { + auto txAmendmentDisabled = [&](FeatureBitset features, + std::string const& tx) { + BEAST_EXPECT(txRequiredFeatures.contains(tx)); + + Env env(*this, features - txRequiredFeatures[tx]); + + Account const alice{"alice"}; + Account const bob{"bob"}; + env.fund(XRP(100000), alice, bob); + env.close(); + + if (!features[fixDelegateV1_1]) + env(delegate::set(alice, bob, {tx})); + else + env(delegate::set(alice, bob, {tx}), ter(temMALFORMED)); + }; + + for (auto const& tx : txRequiredFeatures) + txAmendmentDisabled(features, tx.first); + } + + // if all the required features in txRequiredFeatures are enabled, will + // succeed + { + auto txAmendmentEnabled = [&](std::string const& tx) { + Env env(*this, features); + + Account const alice{"alice"}; + Account const bob{"bob"}; + env.fund(XRP(100000), alice, bob); + env.close(); + + env(delegate::set(alice, bob, {tx})); + }; + + for (auto const& tx : txRequiredFeatures) + txAmendmentEnabled(tx.first); + } + + // NFTokenMint, NFTokenBurn, NFTokenCreateOffer, NFTokenCancelOffer, and + // NFTokenAcceptOffer are tested separately. Since + // featureNonFungibleTokensV1_1 includes the functionality of + // featureNonFungibleTokensV1, fixNFTokenNegOffer, and fixNFTokenDirV1, + // both featureNonFungibleTokensV1_1 and featureNonFungibleTokensV1 need + // to be disabled to block these transactions from being delegated. + { + Env env( + *this, + features - featureNonFungibleTokensV1 - + featureNonFungibleTokensV1_1); + + Account const alice{"alice"}; + Account const bob{"bob"}; + env.fund(XRP(100000), alice, bob); + env.close(); + + for (auto const tx : + {"NFTokenMint", + "NFTokenBurn", + "NFTokenCreateOffer", + "NFTokenCancelOffer", + "NFTokenAcceptOffer"}) + { + if (!features[fixDelegateV1_1]) + env(delegate::set(alice, bob, {tx})); + else + env(delegate::set(alice, bob, {tx}), ter(temMALFORMED)); + } + } + + // NFTokenMint, NFTokenBurn, NFTokenCreateOffer, NFTokenCancelOffer, and + // NFTokenAcceptOffer are allowed to be delegated if either + // featureNonFungibleTokensV1 or featureNonFungibleTokensV1_1 is + // enabled. + { + for (auto const feature : + {featureNonFungibleTokensV1, featureNonFungibleTokensV1_1}) + { + Env env(*this, features - feature); + Account const alice{"alice"}; + Account const bob{"bob"}; + env.fund(XRP(100000), alice, bob); + env.close(); + + for (auto const tx : + {"NFTokenMint", + "NFTokenBurn", + "NFTokenCreateOffer", + "NFTokenCancelOffer", + "NFTokenAcceptOffer"}) + env(delegate::set(alice, bob, {tx})); + } + } + } + void run() override { + FeatureBitset const all = jtx::testable_amendments(); + testFeatureDisabled(); testDelegateSet(); - testInvalidRequest(); + testInvalidRequest(all); + testInvalidRequest(all - fixDelegateV1_1); testReserve(); testFee(); testSequence(); testAccountDelete(); testDelegateTransaction(); - testPaymentGranular(); + testPaymentGranular(all); + testPaymentGranular(all - fixDelegateV1_1); testTrustSetGranular(); testAccountSetGranular(); testMPTokenIssuanceSetGranular(); @@ -1495,6 +1841,10 @@ class Delegate_test : public beast::unit_test::suite testSingleSignBadSecret(); testMultiSign(); testMultiSignQuorumNotMet(); + testPermissionValue(all); + testPermissionValue(all - fixDelegateV1_1); + testTxReqireFeatures(all); + testTxReqireFeatures(all - fixDelegateV1_1); } }; BEAST_DEFINE_TESTSUITE(Delegate, app, ripple); diff --git a/src/xrpld/app/tx/detail/DelegateSet.cpp b/src/xrpld/app/tx/detail/DelegateSet.cpp index 708cdf0dc2..ddeb01b399 100644 --- a/src/xrpld/app/tx/detail/DelegateSet.cpp +++ b/src/xrpld/app/tx/detail/DelegateSet.cpp @@ -23,7 +23,6 @@ #include #include #include -#include #include namespace ripple { @@ -51,6 +50,11 @@ DelegateSet::preflight(PreflightContext const& ctx) { if (!permissionSet.insert(permission[sfPermissionValue]).second) return temMALFORMED; + + if (ctx.rules.enabled(fixDelegateV1_1) && + !Permission::getInstance().isDelegatable( + permission[sfPermissionValue], ctx.rules)) + return temMALFORMED; } return preflight2(ctx); @@ -68,9 +72,21 @@ DelegateSet::preclaim(PreclaimContext const& ctx) auto const& permissions = ctx.tx.getFieldArray(sfPermissions); for (auto const& permission : permissions) { - auto const permissionValue = permission[sfPermissionValue]; - if (!Permission::getInstance().isDelegatable(permissionValue)) + if (!ctx.view.rules().enabled(fixDelegateV1_1) && + !Permission::getInstance().isDelegatable( + permission[sfPermissionValue], ctx.view.rules())) + { + // Before fixDelegateV1_1: + // - The check was performed during preclaim. + // - Transactions from amendments not yet enabled could still be + // delegated. + // + // After fixDelegateV1_1: + // - The check is performed during preflight. + // - Transactions from amendments not yet enabled can no longer be + // delegated. return tecNO_PERMISSION; + } } return tesSUCCESS; diff --git a/src/xrpld/app/tx/detail/Payment.cpp b/src/xrpld/app/tx/detail/Payment.cpp index 386b170ed1..784330b203 100644 --- a/src/xrpld/app/tx/detail/Payment.cpp +++ b/src/xrpld/app/tx/detail/Payment.cpp @@ -265,8 +265,33 @@ Payment::checkPermission(ReadView const& view, STTx const& tx) loadGranularPermission(sle, ttPAYMENT, granularPermissions); auto const& dstAmount = tx.getFieldAmount(sfAmount); - auto const& amountIssue = dstAmount.issue(); + // post-amendment: disallow cross currency payments for PaymentMint and + // PaymentBurn + if (view.rules().enabled(fixDelegateV1_1)) + { + auto const& amountAsset = dstAmount.asset(); + if (tx.isFieldPresent(sfSendMax) && + tx[sfSendMax].asset() != amountAsset) + return tecNO_DELEGATE_PERMISSION; + if (granularPermissions.contains(PaymentMint) && !isXRP(amountAsset) && + amountAsset.getIssuer() == tx[sfAccount]) + return tesSUCCESS; + + if (granularPermissions.contains(PaymentBurn) && !isXRP(amountAsset) && + amountAsset.getIssuer() == tx[sfDestination]) + return tesSUCCESS; + + return tecNO_DELEGATE_PERMISSION; + } + + // Calling dstAmount.issue() in the next line would throw if it holds MPT. + // That exception would be caught in preclaim and returned as tefEXCEPTION. + // This check is just a cleaner, more explicit way to get the same result. + if (dstAmount.holds()) + return tefEXCEPTION; + + auto const& amountIssue = dstAmount.issue(); if (granularPermissions.contains(PaymentMint) && !isXRP(amountIssue) && amountIssue.account == tx[sfAccount]) return tesSUCCESS; diff --git a/src/xrpld/app/tx/detail/applySteps.cpp b/src/xrpld/app/tx/detail/applySteps.cpp index 34259ebef0..03ef7244f8 100644 --- a/src/xrpld/app/tx/detail/applySteps.cpp +++ b/src/xrpld/app/tx/detail/applySteps.cpp @@ -97,8 +97,8 @@ with_txn_type(TxType txnType, F&& f) #pragma push_macro("TRANSACTION") #undef TRANSACTION -#define TRANSACTION(tag, value, name, delegatable, fields) \ - case tag: \ +#define TRANSACTION(tag, value, name, ...) \ + case tag: \ return f.template operator()(); #include