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 .. + .. 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': 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 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 60066536ce..d17eea7644 100644 --- a/include/xrpl/protocol/TxFormats.h +++ b/include/xrpl/protocol/TxFormats.h @@ -59,8 +59,7 @@ enum TxType : std::uint16_t #pragma push_macro("TRANSACTION") #undef TRANSACTION -#define TRANSACTION(tag, value, ...) \ - 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 09e1c183b2..df2f9a3761 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -22,7 +22,7 @@ #endif /** - * TRANSACTION(tag, value, name, delegatable, privileges, fields) + * TRANSACTION(tag, value, name, delegatable, amendments, privileges, fields) * * To ease maintenance, you may replace any unneeded values with "..." * e.g. #define TRANSACTION(tag, value, name, ...) @@ -45,6 +45,7 @@ #endif TRANSACTION(ttPAYMENT, 0, Payment, Delegation::delegatable, + uint256{}, createAcct, ({ {sfDestination, soeREQUIRED}, @@ -64,6 +65,7 @@ TRANSACTION(ttPAYMENT, 0, Payment, #endif TRANSACTION(ttESCROW_CREATE, 1, EscrowCreate, Delegation::delegatable, + uint256{}, noPriv, ({ {sfDestination, soeREQUIRED}, @@ -77,6 +79,7 @@ TRANSACTION(ttESCROW_CREATE, 1, EscrowCreate, /** This transaction type completes an existing escrow. */ TRANSACTION(ttESCROW_FINISH, 2, EscrowFinish, Delegation::delegatable, + uint256{}, noPriv, ({ {sfOwner, soeREQUIRED}, @@ -93,6 +96,7 @@ TRANSACTION(ttESCROW_FINISH, 2, EscrowFinish, #endif TRANSACTION(ttACCOUNT_SET, 3, AccountSet, Delegation::notDelegatable, + uint256{}, noPriv, ({ {sfEmailHash, soeOPTIONAL}, @@ -113,6 +117,7 @@ TRANSACTION(ttACCOUNT_SET, 3, AccountSet, #endif TRANSACTION(ttESCROW_CANCEL, 4, EscrowCancel, Delegation::delegatable, + uint256{}, noPriv, ({ {sfOwner, soeREQUIRED}, @@ -125,6 +130,7 @@ TRANSACTION(ttESCROW_CANCEL, 4, EscrowCancel, #endif TRANSACTION(ttREGULAR_KEY_SET, 5, SetRegularKey, Delegation::notDelegatable, + uint256{}, noPriv, ({ {sfRegularKey, soeOPTIONAL}, @@ -138,6 +144,7 @@ TRANSACTION(ttREGULAR_KEY_SET, 5, SetRegularKey, #endif TRANSACTION(ttOFFER_CREATE, 7, OfferCreate, Delegation::delegatable, + uint256{}, noPriv, ({ {sfTakerPays, soeREQUIRED}, @@ -153,6 +160,7 @@ TRANSACTION(ttOFFER_CREATE, 7, OfferCreate, #endif TRANSACTION(ttOFFER_CANCEL, 8, OfferCancel, Delegation::delegatable, + uint256{}, noPriv, ({ {sfOfferSequence, soeREQUIRED}, @@ -166,6 +174,7 @@ TRANSACTION(ttOFFER_CANCEL, 8, OfferCancel, #endif TRANSACTION(ttTICKET_CREATE, 10, TicketCreate, Delegation::delegatable, + featureTicketBatch, noPriv, ({ {sfTicketCount, soeREQUIRED}, @@ -181,6 +190,7 @@ TRANSACTION(ttTICKET_CREATE, 10, TicketCreate, #endif TRANSACTION(ttSIGNER_LIST_SET, 12, SignerListSet, Delegation::notDelegatable, + uint256{}, noPriv, ({ {sfSignerQuorum, soeREQUIRED}, @@ -193,6 +203,7 @@ TRANSACTION(ttSIGNER_LIST_SET, 12, SignerListSet, #endif TRANSACTION(ttPAYCHAN_CREATE, 13, PaymentChannelCreate, Delegation::delegatable, + uint256{}, noPriv, ({ {sfDestination, soeREQUIRED}, @@ -206,6 +217,7 @@ TRANSACTION(ttPAYCHAN_CREATE, 13, PaymentChannelCreate, /** This transaction type funds an existing unidirectional XRP payment channel. */ TRANSACTION(ttPAYCHAN_FUND, 14, PaymentChannelFund, Delegation::delegatable, + uint256{}, noPriv, ({ {sfChannel, soeREQUIRED}, @@ -216,6 +228,7 @@ TRANSACTION(ttPAYCHAN_FUND, 14, PaymentChannelFund, /** This transaction type submits a claim against an existing unidirectional payment channel. */ TRANSACTION(ttPAYCHAN_CLAIM, 15, PaymentChannelClaim, Delegation::delegatable, + uint256{}, noPriv, ({ {sfChannel, soeREQUIRED}, @@ -232,6 +245,7 @@ TRANSACTION(ttPAYCHAN_CLAIM, 15, PaymentChannelClaim, #endif TRANSACTION(ttCHECK_CREATE, 16, CheckCreate, Delegation::delegatable, + featureChecks, noPriv, ({ {sfDestination, soeREQUIRED}, @@ -247,6 +261,7 @@ TRANSACTION(ttCHECK_CREATE, 16, CheckCreate, #endif TRANSACTION(ttCHECK_CASH, 17, CheckCash, Delegation::delegatable, + featureChecks, noPriv, ({ {sfCheckID, soeREQUIRED}, @@ -260,6 +275,7 @@ TRANSACTION(ttCHECK_CASH, 17, CheckCash, #endif TRANSACTION(ttCHECK_CANCEL, 18, CheckCancel, Delegation::delegatable, + featureChecks, noPriv, ({ {sfCheckID, soeREQUIRED}, @@ -271,6 +287,7 @@ TRANSACTION(ttCHECK_CANCEL, 18, CheckCancel, #endif TRANSACTION(ttDEPOSIT_PREAUTH, 19, DepositPreauth, Delegation::delegatable, + featureDepositPreauth, noPriv, ({ {sfAuthorize, soeOPTIONAL}, @@ -285,6 +302,7 @@ TRANSACTION(ttDEPOSIT_PREAUTH, 19, DepositPreauth, #endif TRANSACTION(ttTRUST_SET, 20, TrustSet, Delegation::delegatable, + uint256{}, noPriv, ({ {sfLimitAmount, soeOPTIONAL}, @@ -298,6 +316,7 @@ TRANSACTION(ttTRUST_SET, 20, TrustSet, #endif TRANSACTION(ttACCOUNT_DELETE, 21, AccountDelete, Delegation::notDelegatable, + uint256{}, mustDeleteAcct, ({ {sfDestination, soeREQUIRED}, @@ -313,6 +332,7 @@ TRANSACTION(ttACCOUNT_DELETE, 21, AccountDelete, #endif TRANSACTION(ttNFTOKEN_MINT, 25, NFTokenMint, Delegation::delegatable, + featureNonFungibleTokensV1, changeNFTCounts, ({ {sfNFTokenTaxon, soeREQUIRED}, @@ -330,6 +350,7 @@ TRANSACTION(ttNFTOKEN_MINT, 25, NFTokenMint, #endif TRANSACTION(ttNFTOKEN_BURN, 26, NFTokenBurn, Delegation::delegatable, + featureNonFungibleTokensV1, changeNFTCounts, ({ {sfNFTokenID, soeREQUIRED}, @@ -342,6 +363,7 @@ TRANSACTION(ttNFTOKEN_BURN, 26, NFTokenBurn, #endif TRANSACTION(ttNFTOKEN_CREATE_OFFER, 27, NFTokenCreateOffer, Delegation::delegatable, + featureNonFungibleTokensV1, noPriv, ({ {sfNFTokenID, soeREQUIRED}, @@ -357,6 +379,7 @@ TRANSACTION(ttNFTOKEN_CREATE_OFFER, 27, NFTokenCreateOffer, #endif TRANSACTION(ttNFTOKEN_CANCEL_OFFER, 28, NFTokenCancelOffer, Delegation::delegatable, + featureNonFungibleTokensV1, noPriv, ({ {sfNFTokenOffers, soeREQUIRED}, @@ -368,6 +391,7 @@ TRANSACTION(ttNFTOKEN_CANCEL_OFFER, 28, NFTokenCancelOffer, #endif TRANSACTION(ttNFTOKEN_ACCEPT_OFFER, 29, NFTokenAcceptOffer, Delegation::delegatable, + featureNonFungibleTokensV1, noPriv, ({ {sfNFTokenBuyOffer, soeOPTIONAL}, @@ -381,6 +405,7 @@ TRANSACTION(ttNFTOKEN_ACCEPT_OFFER, 29, NFTokenAcceptOffer, #endif TRANSACTION(ttCLAWBACK, 30, Clawback, Delegation::delegatable, + featureClawback, noPriv, ({ {sfAmount, soeREQUIRED, soeMPTSupported}, @@ -393,6 +418,7 @@ TRANSACTION(ttCLAWBACK, 30, Clawback, #endif TRANSACTION(ttAMM_CLAWBACK, 31, AMMClawback, Delegation::delegatable, + featureAMMClawback, mayDeleteAcct | overrideFreeze, ({ {sfHolder, soeREQUIRED}, @@ -407,6 +433,7 @@ TRANSACTION(ttAMM_CLAWBACK, 31, AMMClawback, #endif TRANSACTION(ttAMM_CREATE, 35, AMMCreate, Delegation::delegatable, + featureAMM, createPseudoAcct, ({ {sfAmount, soeREQUIRED}, @@ -420,6 +447,7 @@ TRANSACTION(ttAMM_CREATE, 35, AMMCreate, #endif TRANSACTION(ttAMM_DEPOSIT, 36, AMMDeposit, Delegation::delegatable, + featureAMM, noPriv, ({ {sfAsset, soeREQUIRED}, @@ -437,6 +465,7 @@ TRANSACTION(ttAMM_DEPOSIT, 36, AMMDeposit, #endif TRANSACTION(ttAMM_WITHDRAW, 37, AMMWithdraw, Delegation::delegatable, + featureAMM, mayDeleteAcct, ({ {sfAsset, soeREQUIRED}, @@ -453,6 +482,7 @@ TRANSACTION(ttAMM_WITHDRAW, 37, AMMWithdraw, #endif TRANSACTION(ttAMM_VOTE, 38, AMMVote, Delegation::delegatable, + featureAMM, noPriv, ({ {sfAsset, soeREQUIRED}, @@ -466,6 +496,7 @@ TRANSACTION(ttAMM_VOTE, 38, AMMVote, #endif TRANSACTION(ttAMM_BID, 39, AMMBid, Delegation::delegatable, + featureAMM, noPriv, ({ {sfAsset, soeREQUIRED}, @@ -481,6 +512,7 @@ TRANSACTION(ttAMM_BID, 39, AMMBid, #endif TRANSACTION(ttAMM_DELETE, 40, AMMDelete, Delegation::delegatable, + featureAMM, mustDeleteAcct, ({ {sfAsset, soeREQUIRED}, @@ -493,6 +525,7 @@ TRANSACTION(ttAMM_DELETE, 40, AMMDelete, #endif TRANSACTION(ttXCHAIN_CREATE_CLAIM_ID, 41, XChainCreateClaimID, Delegation::delegatable, + featureXChainBridge, noPriv, ({ {sfXChainBridge, soeREQUIRED}, @@ -503,6 +536,7 @@ TRANSACTION(ttXCHAIN_CREATE_CLAIM_ID, 41, XChainCreateClaimID, /** This transactions initiates a crosschain transaction */ TRANSACTION(ttXCHAIN_COMMIT, 42, XChainCommit, Delegation::delegatable, + featureXChainBridge, noPriv, ({ {sfXChainBridge, soeREQUIRED}, @@ -514,6 +548,7 @@ TRANSACTION(ttXCHAIN_COMMIT, 42, XChainCommit, /** This transaction completes a crosschain transaction */ TRANSACTION(ttXCHAIN_CLAIM, 43, XChainClaim, Delegation::delegatable, + featureXChainBridge, noPriv, ({ {sfXChainBridge, soeREQUIRED}, @@ -526,6 +561,7 @@ TRANSACTION(ttXCHAIN_CLAIM, 43, XChainClaim, /** This transaction initiates a crosschain account create transaction */ TRANSACTION(ttXCHAIN_ACCOUNT_CREATE_COMMIT, 44, XChainAccountCreateCommit, Delegation::delegatable, + featureXChainBridge, noPriv, ({ {sfXChainBridge, soeREQUIRED}, @@ -537,6 +573,7 @@ TRANSACTION(ttXCHAIN_ACCOUNT_CREATE_COMMIT, 44, XChainAccountCreateCommit, /** This transaction adds an attestation to a claim */ TRANSACTION(ttXCHAIN_ADD_CLAIM_ATTESTATION, 45, XChainAddClaimAttestation, Delegation::delegatable, + featureXChainBridge, createAcct, ({ {sfXChainBridge, soeREQUIRED}, @@ -557,6 +594,7 @@ TRANSACTION(ttXCHAIN_ADD_CLAIM_ATTESTATION, 45, XChainAddClaimAttestation, TRANSACTION(ttXCHAIN_ADD_ACCOUNT_CREATE_ATTESTATION, 46, XChainAddAccountCreateAttestation, Delegation::delegatable, + featureXChainBridge, createAcct, ({ {sfXChainBridge, soeREQUIRED}, @@ -577,6 +615,7 @@ TRANSACTION(ttXCHAIN_ADD_ACCOUNT_CREATE_ATTESTATION, 46, /** This transaction modifies a sidechain */ TRANSACTION(ttXCHAIN_MODIFY_BRIDGE, 47, XChainModifyBridge, Delegation::delegatable, + featureXChainBridge, noPriv, ({ {sfXChainBridge, soeREQUIRED}, @@ -587,6 +626,7 @@ TRANSACTION(ttXCHAIN_MODIFY_BRIDGE, 47, XChainModifyBridge, /** This transactions creates a sidechain */ TRANSACTION(ttXCHAIN_CREATE_BRIDGE, 48, XChainCreateBridge, Delegation::delegatable, + featureXChainBridge, noPriv, ({ {sfXChainBridge, soeREQUIRED}, @@ -600,6 +640,7 @@ TRANSACTION(ttXCHAIN_CREATE_BRIDGE, 48, XChainCreateBridge, #endif TRANSACTION(ttDID_SET, 49, DIDSet, Delegation::delegatable, + featureDID, noPriv, ({ {sfDIDDocument, soeOPTIONAL}, @@ -610,6 +651,7 @@ TRANSACTION(ttDID_SET, 49, DIDSet, /** This transaction type deletes a DID */ TRANSACTION(ttDID_DELETE, 50, DIDDelete, Delegation::delegatable, + featureDID, noPriv, ({})) @@ -619,6 +661,7 @@ TRANSACTION(ttDID_DELETE, 50, DIDDelete, #endif TRANSACTION(ttORACLE_SET, 51, OracleSet, Delegation::delegatable, + featurePriceOracle, noPriv, ({ {sfOracleDocumentID, soeREQUIRED}, @@ -635,6 +678,7 @@ TRANSACTION(ttORACLE_SET, 51, OracleSet, #endif TRANSACTION(ttORACLE_DELETE, 52, OracleDelete, Delegation::delegatable, + featurePriceOracle, noPriv, ({ {sfOracleDocumentID, soeREQUIRED}, @@ -646,6 +690,7 @@ TRANSACTION(ttORACLE_DELETE, 52, OracleDelete, #endif TRANSACTION(ttLEDGER_STATE_FIX, 53, LedgerStateFix, Delegation::delegatable, + fixNFTokenPageLinks, noPriv, ({ {sfLedgerFixType, soeREQUIRED}, @@ -658,6 +703,7 @@ TRANSACTION(ttLEDGER_STATE_FIX, 53, LedgerStateFix, #endif TRANSACTION(ttMPTOKEN_ISSUANCE_CREATE, 54, MPTokenIssuanceCreate, Delegation::delegatable, + featureMPTokensV1, createMPTIssuance, ({ {sfAssetScale, soeOPTIONAL}, @@ -673,6 +719,7 @@ TRANSACTION(ttMPTOKEN_ISSUANCE_CREATE, 54, MPTokenIssuanceCreate, #endif TRANSACTION(ttMPTOKEN_ISSUANCE_DESTROY, 55, MPTokenIssuanceDestroy, Delegation::delegatable, + featureMPTokensV1, destroyMPTIssuance, ({ {sfMPTokenIssuanceID, soeREQUIRED}, @@ -684,6 +731,7 @@ TRANSACTION(ttMPTOKEN_ISSUANCE_DESTROY, 55, MPTokenIssuanceDestroy, #endif TRANSACTION(ttMPTOKEN_ISSUANCE_SET, 56, MPTokenIssuanceSet, Delegation::delegatable, + featureMPTokensV1, noPriv, ({ {sfMPTokenIssuanceID, soeREQUIRED}, @@ -697,6 +745,7 @@ TRANSACTION(ttMPTOKEN_ISSUANCE_SET, 56, MPTokenIssuanceSet, #endif TRANSACTION(ttMPTOKEN_AUTHORIZE, 57, MPTokenAuthorize, Delegation::delegatable, + featureMPTokensV1, mustAuthorizeMPT, ({ {sfMPTokenIssuanceID, soeREQUIRED}, @@ -709,6 +758,7 @@ TRANSACTION(ttMPTOKEN_AUTHORIZE, 57, MPTokenAuthorize, #endif TRANSACTION(ttCREDENTIAL_CREATE, 58, CredentialCreate, Delegation::delegatable, + featureCredentials, noPriv, ({ {sfSubject, soeREQUIRED}, @@ -720,6 +770,7 @@ TRANSACTION(ttCREDENTIAL_CREATE, 58, CredentialCreate, /** This transaction type accept an Credential object */ TRANSACTION(ttCREDENTIAL_ACCEPT, 59, CredentialAccept, Delegation::delegatable, + featureCredentials, noPriv, ({ {sfIssuer, soeREQUIRED}, @@ -729,6 +780,7 @@ TRANSACTION(ttCREDENTIAL_ACCEPT, 59, CredentialAccept, /** This transaction type delete an Credential object */ TRANSACTION(ttCREDENTIAL_DELETE, 60, CredentialDelete, Delegation::delegatable, + featureCredentials, noPriv, ({ {sfSubject, soeOPTIONAL}, @@ -742,6 +794,7 @@ TRANSACTION(ttCREDENTIAL_DELETE, 60, CredentialDelete, #endif TRANSACTION(ttNFTOKEN_MODIFY, 61, NFTokenModify, Delegation::delegatable, + featureDynamicNFT, noPriv, ({ {sfNFTokenID, soeREQUIRED}, @@ -755,6 +808,7 @@ TRANSACTION(ttNFTOKEN_MODIFY, 61, NFTokenModify, #endif TRANSACTION(ttPERMISSIONED_DOMAIN_SET, 62, PermissionedDomainSet, Delegation::delegatable, + featurePermissionedDomains, noPriv, ({ {sfDomainID, soeOPTIONAL}, @@ -767,6 +821,7 @@ TRANSACTION(ttPERMISSIONED_DOMAIN_SET, 62, PermissionedDomainSet, #endif TRANSACTION(ttPERMISSIONED_DOMAIN_DELETE, 63, PermissionedDomainDelete, Delegation::delegatable, + featurePermissionedDomains, noPriv, ({ {sfDomainID, soeREQUIRED}, @@ -778,6 +833,7 @@ TRANSACTION(ttPERMISSIONED_DOMAIN_DELETE, 63, PermissionedDomainDelete, #endif TRANSACTION(ttDELEGATE_SET, 64, DelegateSet, Delegation::notDelegatable, + featurePermissionDelegation, noPriv, ({ {sfAuthorize, soeREQUIRED}, @@ -790,6 +846,7 @@ TRANSACTION(ttDELEGATE_SET, 64, DelegateSet, #endif TRANSACTION(ttVAULT_CREATE, 65, VaultCreate, Delegation::delegatable, + featureSingleAssetVault, createPseudoAcct | createMPTIssuance, ({ {sfAsset, soeREQUIRED, soeMPTSupported}, @@ -807,6 +864,7 @@ TRANSACTION(ttVAULT_CREATE, 65, VaultCreate, #endif TRANSACTION(ttVAULT_SET, 66, VaultSet, Delegation::delegatable, + featureSingleAssetVault, noPriv, ({ {sfVaultID, soeREQUIRED}, @@ -821,6 +879,7 @@ TRANSACTION(ttVAULT_SET, 66, VaultSet, #endif TRANSACTION(ttVAULT_DELETE, 67, VaultDelete, Delegation::delegatable, + featureSingleAssetVault, mustDeleteAcct | destroyMPTIssuance, ({ {sfVaultID, soeREQUIRED}, @@ -832,6 +891,7 @@ TRANSACTION(ttVAULT_DELETE, 67, VaultDelete, #endif TRANSACTION(ttVAULT_DEPOSIT, 68, VaultDeposit, Delegation::delegatable, + featureSingleAssetVault, mayAuthorizeMPT, ({ {sfVaultID, soeREQUIRED}, @@ -844,6 +904,7 @@ TRANSACTION(ttVAULT_DEPOSIT, 68, VaultDeposit, #endif TRANSACTION(ttVAULT_WITHDRAW, 69, VaultWithdraw, Delegation::delegatable, + featureSingleAssetVault, mayDeleteMPT, ({ {sfVaultID, soeREQUIRED}, @@ -858,6 +919,7 @@ TRANSACTION(ttVAULT_WITHDRAW, 69, VaultWithdraw, #endif TRANSACTION(ttVAULT_CLAWBACK, 70, VaultClawback, Delegation::delegatable, + featureSingleAssetVault, mayDeleteMPT, ({ {sfVaultID, soeREQUIRED}, @@ -871,10 +933,11 @@ TRANSACTION(ttVAULT_CLAWBACK, 70, VaultClawback, #endif TRANSACTION(ttBATCH, 71, Batch, Delegation::notDelegatable, + featureBatch, noPriv, ({ - {sfRawTransactions, soeREQUIRED}, - {sfBatchSigners, soeOPTIONAL}, + {sfRawTransactions, soeREQUIRED}, + {sfBatchSigners, soeOPTIONAL}, })) /** This system-generated transaction type is used to update the status of the various amendments. @@ -886,6 +949,7 @@ TRANSACTION(ttBATCH, 71, Batch, #endif TRANSACTION(ttAMENDMENT, 100, EnableAmendment, Delegation::notDelegatable, + uint256{}, noPriv, ({ {sfLedgerSequence, soeREQUIRED}, @@ -897,6 +961,7 @@ TRANSACTION(ttAMENDMENT, 100, EnableAmendment, */ TRANSACTION(ttFEE, 101, SetFee, Delegation::notDelegatable, + uint256{}, noPriv, ({ {sfLedgerSequence, soeOPTIONAL}, @@ -917,6 +982,7 @@ TRANSACTION(ttFEE, 101, SetFee, */ TRANSACTION(ttUNL_MODIFY, 102, UNLModify, Delegation::notDelegatable, + uint256{}, noPriv, ({ {sfUNLModifyDisabling, soeREQUIRED}, diff --git a/src/libxrpl/protocol/Permissions.cpp b/src/libxrpl/protocol/Permissions.cpp index 35f788e9eb..781799f128 100644 --- a/src/libxrpl/protocol/Permissions.cpp +++ b/src/libxrpl/protocol/Permissions.cpp @@ -18,6 +18,7 @@ //============================================================================== #include +#include #include #include @@ -25,6 +26,19 @@ 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 @@ -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 f5e2125b3c..1966c29b51 100644 --- a/src/libxrpl/protocol/TxFormats.cpp +++ b/src/libxrpl/protocol/TxFormats.cpp @@ -55,7 +55,8 @@ TxFormats::TxFormats() #undef TRANSACTION #define UNWRAP(...) __VA_ARGS__ -#define TRANSACTION(tag, value, name, delegatable, privileges, fields) \ +#define TRANSACTION( \ + tag, value, name, delegatable, amendment, privileges, 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/test/app/EscrowToken_test.cpp b/src/test/app/EscrowToken_test.cpp index e81064c825..223b86a5d1 100644 --- a/src/test/app/EscrowToken_test.cpp +++ b/src/test/app/EscrowToken_test.cpp @@ -3426,7 +3426,7 @@ struct EscrowToken_test : public beast::unit_test::suite auto const preAliceMPT = env.balance(alice, MPT); auto const preOutstanding = env.balance(gw, MPT); auto const preEscrowed = issuerMPTEscrowed(env, MPT); - BEAST_EXPECT(preOutstanding == MPT(10'000)); + BEAST_EXPECT(preOutstanding == MPT(-10'000)); BEAST_EXPECT(preEscrowed == 0); env(escrow::create(alice, gw, MPT(1'000)), @@ -3449,7 +3449,7 @@ struct EscrowToken_test : public beast::unit_test::suite BEAST_EXPECT(env.balance(alice, MPT) == preAliceMPT - MPT(1'000)); BEAST_EXPECT(mptEscrowed(env, alice, MPT) == 0); - BEAST_EXPECT(env.balance(gw, MPT) == preOutstanding - MPT(1'000)); + BEAST_EXPECT(env.balance(gw, MPT) == preOutstanding + MPT(1'000)); BEAST_EXPECT(issuerMPTEscrowed(env, MPT) == preEscrowed); } } @@ -3759,7 +3759,7 @@ struct EscrowToken_test : public beast::unit_test::suite BEAST_EXPECT(mptEscrowed(env, alice, MPT) == 10); BEAST_EXPECT(env.balance(bob, MPT) == MPT(0)); BEAST_EXPECT(mptEscrowed(env, bob, MPT) == 0); - BEAST_EXPECT(env.balance(gw, MPT) == MPT(10)); + BEAST_EXPECT(env.balance(gw, MPT) == MPT(-10)); mptGw.authorize({.account = bob, .flags = tfMPTUnauthorize}); mptGw.destroy( {.id = mptGw.issuanceID(), diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index 9e14d9e094..60ac5f44bd 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -3503,7 +3503,7 @@ class Vault_test : public beast::unit_test::suite STAmount(d.asset, Number(100, 0))); BEAST_EXPECT( env.balance(d.vaultAccount, d.shares) == - STAmount(d.share, Number(1000, 0))); + STAmount(d.share, Number(-1000, 0))); { testcase("Scale redeem exact"); @@ -3528,7 +3528,7 @@ class Vault_test : public beast::unit_test::suite STAmount(d.asset, Number(90, 0))); BEAST_EXPECT( env.balance(d.vaultAccount, d.shares) == - STAmount(d.share, Number(900, 0))); + STAmount(d.share, Number(-900, 0))); } { @@ -3563,7 +3563,7 @@ class Vault_test : public beast::unit_test::suite STAmount(d.asset, Number(900 - 25, -1))); BEAST_EXPECT( env.balance(d.vaultAccount, d.shares) == - STAmount(d.share, Number(900 - 25, 0))); + STAmount(d.share, -Number(900 - 25, 0))); } { @@ -3590,7 +3590,7 @@ class Vault_test : public beast::unit_test::suite STAmount(d.asset, Number(875 - 21, -1))); BEAST_EXPECT( env.balance(d.vaultAccount, d.shares) == - STAmount(d.share, Number(875 - 21, 0))); + STAmount(d.share, -Number(875 - 21, 0))); } { @@ -3651,7 +3651,7 @@ class Vault_test : public beast::unit_test::suite STAmount(d.asset, Number(100, 0))); BEAST_EXPECT( env.balance(d.vaultAccount, d.shares) == - STAmount(d.share, Number(1000, 0))); + STAmount(d.share, Number(-1000, 0))); { testcase("Scale withdraw exact"); @@ -3679,7 +3679,7 @@ class Vault_test : public beast::unit_test::suite STAmount(d.asset, Number(90, 0))); BEAST_EXPECT( env.balance(d.vaultAccount, d.shares) == - STAmount(d.share, Number(900, 0))); + STAmount(d.share, Number(-900, 0))); } { @@ -3726,7 +3726,7 @@ class Vault_test : public beast::unit_test::suite STAmount(d.asset, Number(900 - 25, -1))); BEAST_EXPECT( env.balance(d.vaultAccount, d.shares) == - STAmount(d.share, Number(900 - 25, 0))); + STAmount(d.share, -Number(900 - 25, 0))); } { @@ -3755,7 +3755,7 @@ class Vault_test : public beast::unit_test::suite STAmount(d.asset, Number(875 - 38, -1))); BEAST_EXPECT( env.balance(d.vaultAccount, d.shares) == - STAmount(d.share, Number(875 - 38, 0))); + STAmount(d.share, -Number(875 - 38, 0))); } { @@ -3784,7 +3784,7 @@ class Vault_test : public beast::unit_test::suite STAmount(d.asset, Number(837 - 37, -1))); BEAST_EXPECT( env.balance(d.vaultAccount, d.shares) == - STAmount(d.share, Number(837 - 37, 0))); + STAmount(d.share, -Number(837 - 37, 0))); } { @@ -3807,7 +3807,7 @@ class Vault_test : public beast::unit_test::suite STAmount(d.asset, Number(800 - 1, -1))); BEAST_EXPECT( env.balance(d.vaultAccount, d.shares) == - STAmount(d.share, Number(800 - 1, 0))); + STAmount(d.share, -Number(800 - 1, 0))); } { @@ -3870,7 +3870,7 @@ class Vault_test : public beast::unit_test::suite STAmount(d.asset, Number(100, 0))); BEAST_EXPECT( env.balance(d.vaultAccount, d.shares) == - STAmount(d.share, Number(1000, 0))); + STAmount(d.share, -Number(1000, 0))); { testcase("Scale clawback exact"); // assetsToSharesWithdraw: @@ -3898,7 +3898,7 @@ class Vault_test : public beast::unit_test::suite STAmount(d.asset, Number(90, 0))); BEAST_EXPECT( env.balance(d.vaultAccount, d.shares) == - STAmount(d.share, Number(900, 0))); + STAmount(d.share, -Number(900, 0))); } { @@ -3938,7 +3938,7 @@ class Vault_test : public beast::unit_test::suite STAmount(d.asset, Number(900 - 25, -1))); BEAST_EXPECT( env.balance(d.vaultAccount, d.shares) == - STAmount(d.share, Number(900 - 25, 0))); + STAmount(d.share, -Number(900 - 25, 0))); } { @@ -3968,7 +3968,7 @@ class Vault_test : public beast::unit_test::suite STAmount(d.asset, Number(875 - 38, -1))); BEAST_EXPECT( env.balance(d.vaultAccount, d.shares) == - STAmount(d.share, Number(875 - 38, 0))); + STAmount(d.share, -Number(875 - 38, 0))); } { @@ -3998,7 +3998,7 @@ class Vault_test : public beast::unit_test::suite STAmount(d.asset, Number(837 - 37, -1))); BEAST_EXPECT( env.balance(d.vaultAccount, d.shares) == - STAmount(d.share, Number(837 - 37, 0))); + STAmount(d.share, -Number(837 - 37, 0))); } { @@ -4022,7 +4022,7 @@ class Vault_test : public beast::unit_test::suite STAmount(d.asset, Number(800 - 1, -1))); BEAST_EXPECT( env.balance(d.vaultAccount, d.shares) == - STAmount(d.share, Number(800 - 1, 0))); + STAmount(d.share, -Number(800 - 1, 0))); } { diff --git a/src/test/jtx/impl/Env.cpp b/src/test/jtx/impl/Env.cpp index 9ba80aec42..ae99e1b5d6 100644 --- a/src/test/jtx/impl/Env.cpp +++ b/src/test/jtx/impl/Env.cpp @@ -218,7 +218,9 @@ Env::balance(Account const& account, MPTIssue const& mptIssue) const if (!sle) return {STAmount(mptIssue, 0), account.name()}; - STAmount const amount{mptIssue, sle->getFieldU64(sfOutstandingAmount)}; + // Make it negative + STAmount const amount{ + mptIssue, sle->getFieldU64(sfOutstandingAmount), 0, true}; return {amount, lookup(issuer).name()}; } else diff --git a/src/xrpld/app/tx/detail/DelegateSet.cpp b/src/xrpld/app/tx/detail/DelegateSet.cpp index 5e6b17f187..7119c9ac27 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 tesSUCCESS; @@ -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/Escrow.cpp b/src/xrpld/app/tx/detail/Escrow.cpp index 0cf07c8c07..fea1db99c3 100644 --- a/src/xrpld/app/tx/detail/Escrow.cpp +++ b/src/xrpld/app/tx/detail/Escrow.cpp @@ -81,8 +81,8 @@ constexpr HashRouterFlags SF_CF_VALID = HashRouterFlags::PRIVATE6; TxConsequences EscrowCreate::makeTxConsequences(PreflightContext const& ctx) { - return TxConsequences{ - ctx.tx, isXRP(ctx.tx[sfAmount]) ? ctx.tx[sfAmount].xrp() : beast::zero}; + auto const amount = ctx.tx[sfAmount]; + return TxConsequences{ctx.tx, isXRP(amount) ? amount.xrp() : beast::zero}; } template diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index 1a2b69c579..7af93a5e3e 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -90,9 +90,9 @@ operator|(Privilege lhs, Privilege rhs) #pragma push_macro("TRANSACTION") #undef TRANSACTION -#define TRANSACTION(tag, value, name, delegatable, privileges, ...) \ - case tag: { \ - return (privileges) & priv; \ +#define TRANSACTION(tag, value, name, delegatable, amendment, privileges, ...) \ + case tag: { \ + return (privileges) & priv; \ } bool diff --git a/src/xrpld/app/tx/detail/Payment.cpp b/src/xrpld/app/tx/detail/Payment.cpp index 40561741be..1a3ef361b8 100644 --- a/src/xrpld/app/tx/detail/Payment.cpp +++ b/src/xrpld/app/tx/detail/Payment.cpp @@ -270,8 +270,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;