From 33e666bfff33378d1f7b3f4498fdbe1a4ef2027c Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Thu, 3 Apr 2025 19:58:44 -0400 Subject: [PATCH] Enumerate transaction privileges for invariants - Allows them to be defined in transactions.macro instead of needing to scrutinize every existing Invariant class. - List is not necessarily comprehensive, but does cover every check where more than one transaction type is involved. --- include/xrpl/protocol/TxFlags.h | 2 +- include/xrpl/protocol/TxFormats.h | 2 +- .../xrpl/protocol/detail/transactions.macro | 175 ++++++++++-------- include/xrpl/protocol/jss.h | 2 +- src/libxrpl/protocol/TxFormats.cpp | 2 +- src/test/app/LoanBroker_test.cpp | 13 +- src/xrpld/app/tx/detail/InvariantCheck.cpp | 124 +++++++------ src/xrpld/app/tx/detail/applySteps.cpp | 4 +- 8 files changed, 182 insertions(+), 142 deletions(-) diff --git a/include/xrpl/protocol/TxFlags.h b/include/xrpl/protocol/TxFlags.h index 7e6a6a152a..d6aa7eb26e 100644 --- a/include/xrpl/protocol/TxFlags.h +++ b/include/xrpl/protocol/TxFlags.h @@ -139,7 +139,7 @@ constexpr std::uint32_t const tfTransferable = 0x00000008; constexpr std::uint32_t const tfMutable = 0x00000010; // MPTokenIssuanceCreate flags: -// NOTE - there is intentionally no flag here for lsfMPTLocked, which this transaction cannot mutate. +// NOTE - there is intentionally no flag here for lsfMPTLocked, which this transaction cannot mutate. constexpr std::uint32_t const tfMPTCanLock = lsfMPTCanLock; constexpr std::uint32_t const tfMPTRequireAuth = lsfMPTRequireAuth; constexpr std::uint32_t const tfMPTCanEscrow = lsfMPTCanEscrow; diff --git a/include/xrpl/protocol/TxFormats.h b/include/xrpl/protocol/TxFormats.h index 728bafde4f..e57c4624ea 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, acctCreate, fields) tag = value, +#define TRANSACTION(tag, value, name, privileges, fields) tag = value, #include diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index 80b76f897a..72b7857037 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -27,14 +27,30 @@ * You must define a transactor class in the `ripple` namespace named `name`, * and include its header in `src/xrpld/app/tx/detail/applySteps.cpp`. * - * The fourth parameter of the TRANSACTION macro is a keyword - * indicating whether the transaction can create an ltACCOUNT_ROOT. - * Valid values are "noCreate", "createFull", and "createPseudo". + * The fourth parameter of the TRANSACTION macro is a bitfield + * defining which operations the transaction can perform. + * * The values are only used in InvariantCheck.cpp + * Valid values are + noPriv - The transaction can not do any of the enumerated operations + createAcct - The transaction can create a new ACCOUNT_ROOT object. + createPseudoAcct - The transaction can create a pseudo account, + which implies createAcct + mustDeleteAcct - The transaction must delete an ACCOUNT_ROOT object + mayDeleteAcct - The transaction may delete an ACCOUNT_ROOT object, + but does not have to + overrideFreeze - The transaction can override some freeze rules + changeNFTCounts - The transaction can mint or burn an NFT + createMPTIssuance - The transaction can create a new MPT issuance + destroyMPTIssuance - The transaction can destroy an MPT issuance + mustAuthorizeMPT - The transaction MUST create or delete an MPT + object (except by issuer) + mayAuthorizeMPT - The transaction MAY create or delete an MPT + object (except by issuer) */ /** This transaction type executes a payment. */ -TRANSACTION(ttPAYMENT, 0, Payment, createFull, ({ +TRANSACTION(ttPAYMENT, 0, Payment, createAcct, ({ {sfDestination, soeREQUIRED}, {sfAmount, soeREQUIRED, soeMPTSupported}, {sfSendMax, soeOPTIONAL, soeMPTSupported}, @@ -46,7 +62,7 @@ TRANSACTION(ttPAYMENT, 0, Payment, createFull, ({ })) /** This transaction type creates an escrow object. */ -TRANSACTION(ttESCROW_CREATE, 1, EscrowCreate, noCreate, ({ +TRANSACTION(ttESCROW_CREATE, 1, EscrowCreate, noPriv, ({ {sfDestination, soeREQUIRED}, {sfAmount, soeREQUIRED}, {sfCondition, soeOPTIONAL}, @@ -56,7 +72,7 @@ TRANSACTION(ttESCROW_CREATE, 1, EscrowCreate, noCreate, ({ })) /** This transaction type completes an existing escrow. */ -TRANSACTION(ttESCROW_FINISH, 2, EscrowFinish, noCreate, ({ +TRANSACTION(ttESCROW_FINISH, 2, EscrowFinish, noPriv, ({ {sfOwner, soeREQUIRED}, {sfOfferSequence, soeREQUIRED}, {sfFulfillment, soeOPTIONAL}, @@ -66,7 +82,7 @@ TRANSACTION(ttESCROW_FINISH, 2, EscrowFinish, noCreate, ({ /** This transaction type adjusts various account settings. */ -TRANSACTION(ttACCOUNT_SET, 3, AccountSet, noCreate, ({ +TRANSACTION(ttACCOUNT_SET, 3, AccountSet, noPriv, ({ {sfEmailHash, soeOPTIONAL}, {sfWalletLocator, soeOPTIONAL}, {sfWalletSize, soeOPTIONAL}, @@ -80,20 +96,20 @@ TRANSACTION(ttACCOUNT_SET, 3, AccountSet, noCreate, ({ })) /** This transaction type cancels an existing escrow. */ -TRANSACTION(ttESCROW_CANCEL, 4, EscrowCancel, noCreate, ({ +TRANSACTION(ttESCROW_CANCEL, 4, EscrowCancel, noPriv, ({ {sfOwner, soeREQUIRED}, {sfOfferSequence, soeREQUIRED}, })) /** This transaction type sets or clears an account's "regular key". */ -TRANSACTION(ttREGULAR_KEY_SET, 5, SetRegularKey, noCreate, ({ +TRANSACTION(ttREGULAR_KEY_SET, 5, SetRegularKey, noPriv, ({ {sfRegularKey, soeOPTIONAL}, })) // 6 deprecated /** This transaction type creates an offer to trade one asset for another. */ -TRANSACTION(ttOFFER_CREATE, 7, OfferCreate, noCreate, ({ +TRANSACTION(ttOFFER_CREATE, 7, OfferCreate, noPriv, ({ {sfTakerPays, soeREQUIRED}, {sfTakerGets, soeREQUIRED}, {sfExpiration, soeOPTIONAL}, @@ -101,14 +117,14 @@ TRANSACTION(ttOFFER_CREATE, 7, OfferCreate, noCreate, ({ })) /** This transaction type cancels existing offers to trade one asset for another. */ -TRANSACTION(ttOFFER_CANCEL, 8, OfferCancel, noCreate, ({ +TRANSACTION(ttOFFER_CANCEL, 8, OfferCancel, noPriv, ({ {sfOfferSequence, soeREQUIRED}, })) // 9 deprecated /** This transaction type creates a new set of tickets. */ -TRANSACTION(ttTICKET_CREATE, 10, TicketCreate, noCreate, ({ +TRANSACTION(ttTICKET_CREATE, 10, TicketCreate, noPriv, ({ {sfTicketCount, soeREQUIRED}, })) @@ -117,13 +133,13 @@ TRANSACTION(ttTICKET_CREATE, 10, TicketCreate, noCreate, ({ /** 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, noCreate, ({ +TRANSACTION(ttSIGNER_LIST_SET, 12, SignerListSet, noPriv, ({ {sfSignerQuorum, soeREQUIRED}, {sfSignerEntries, soeOPTIONAL}, })) /** This transaction type creates a new unidirectional XRP payment channel. */ -TRANSACTION(ttPAYCHAN_CREATE, 13, PaymentChannelCreate, noCreate, ({ +TRANSACTION(ttPAYCHAN_CREATE, 13, PaymentChannelCreate, noPriv, ({ {sfDestination, soeREQUIRED}, {sfAmount, soeREQUIRED}, {sfSettleDelay, soeREQUIRED}, @@ -133,14 +149,14 @@ TRANSACTION(ttPAYCHAN_CREATE, 13, PaymentChannelCreate, noCreate, ({ })) /** This transaction type funds an existing unidirectional XRP payment channel. */ -TRANSACTION(ttPAYCHAN_FUND, 14, PaymentChannelFund, noCreate, ({ +TRANSACTION(ttPAYCHAN_FUND, 14, PaymentChannelFund, noPriv, ({ {sfChannel, soeREQUIRED}, {sfAmount, soeREQUIRED}, {sfExpiration, soeOPTIONAL}, })) /** This transaction type submits a claim against an existing unidirectional payment channel. */ -TRANSACTION(ttPAYCHAN_CLAIM, 15, PaymentChannelClaim, noCreate, ({ +TRANSACTION(ttPAYCHAN_CLAIM, 15, PaymentChannelClaim, noPriv, ({ {sfChannel, soeREQUIRED}, {sfAmount, soeOPTIONAL}, {sfBalance, soeOPTIONAL}, @@ -150,7 +166,7 @@ TRANSACTION(ttPAYCHAN_CLAIM, 15, PaymentChannelClaim, noCreate, ({ })) /** This transaction type creates a new check. */ -TRANSACTION(ttCHECK_CREATE, 16, CheckCreate, noCreate, ({ +TRANSACTION(ttCHECK_CREATE, 16, CheckCreate, noPriv, ({ {sfDestination, soeREQUIRED}, {sfSendMax, soeREQUIRED}, {sfExpiration, soeOPTIONAL}, @@ -159,19 +175,19 @@ TRANSACTION(ttCHECK_CREATE, 16, CheckCreate, noCreate, ({ })) /** This transaction type cashes an existing check. */ -TRANSACTION(ttCHECK_CASH, 17, CheckCash, noCreate, ({ +TRANSACTION(ttCHECK_CASH, 17, CheckCash, noPriv, ({ {sfCheckID, soeREQUIRED}, {sfAmount, soeOPTIONAL}, {sfDeliverMin, soeOPTIONAL}, })) /** This transaction type cancels an existing check. */ -TRANSACTION(ttCHECK_CANCEL, 18, CheckCancel, noCreate, ({ +TRANSACTION(ttCHECK_CANCEL, 18, CheckCancel, noPriv, ({ {sfCheckID, soeREQUIRED}, })) /** This transaction type grants or revokes authorization to transfer funds. */ -TRANSACTION(ttDEPOSIT_PREAUTH, 19, DepositPreauth, noCreate, ({ +TRANSACTION(ttDEPOSIT_PREAUTH, 19, DepositPreauth, noPriv, ({ {sfAuthorize, soeOPTIONAL}, {sfUnauthorize, soeOPTIONAL}, {sfAuthorizeCredentials, soeOPTIONAL}, @@ -179,14 +195,14 @@ TRANSACTION(ttDEPOSIT_PREAUTH, 19, DepositPreauth, noCreate, ({ })) /** This transaction type modifies a trustline between two accounts. */ -TRANSACTION(ttTRUST_SET, 20, TrustSet, noCreate, ({ +TRANSACTION(ttTRUST_SET, 20, TrustSet, noPriv, ({ {sfLimitAmount, soeOPTIONAL}, {sfQualityIn, soeOPTIONAL}, {sfQualityOut, soeOPTIONAL}, })) /** This transaction type deletes an existing account. */ -TRANSACTION(ttACCOUNT_DELETE, 21, AccountDelete, noCreate, ({ +TRANSACTION(ttACCOUNT_DELETE, 21, AccountDelete, mustDeleteAcct, ({ {sfDestination, soeREQUIRED}, {sfDestinationTag, soeOPTIONAL}, {sfCredentialIDs, soeOPTIONAL}, @@ -195,7 +211,7 @@ TRANSACTION(ttACCOUNT_DELETE, 21, AccountDelete, noCreate, ({ // 22 reserved /** This transaction mints a new NFT. */ -TRANSACTION(ttNFTOKEN_MINT, 25, NFTokenMint, noCreate, ({ +TRANSACTION(ttNFTOKEN_MINT, 25, NFTokenMint, changeNFTCounts, ({ {sfNFTokenTaxon, soeREQUIRED}, {sfTransferFee, soeOPTIONAL}, {sfIssuer, soeOPTIONAL}, @@ -206,13 +222,13 @@ TRANSACTION(ttNFTOKEN_MINT, 25, NFTokenMint, noCreate, ({ })) /** This transaction burns (i.e. destroys) an existing NFT. */ -TRANSACTION(ttNFTOKEN_BURN, 26, NFTokenBurn, noCreate, ({ +TRANSACTION(ttNFTOKEN_BURN, 26, NFTokenBurn, changeNFTCounts, ({ {sfNFTokenID, soeREQUIRED}, {sfOwner, soeOPTIONAL}, })) /** This transaction creates a new offer to buy or sell an NFT. */ -TRANSACTION(ttNFTOKEN_CREATE_OFFER, 27, NFTokenCreateOffer, noCreate, ({ +TRANSACTION(ttNFTOKEN_CREATE_OFFER, 27, NFTokenCreateOffer, noPriv, ({ {sfNFTokenID, soeREQUIRED}, {sfAmount, soeREQUIRED}, {sfDestination, soeOPTIONAL}, @@ -221,25 +237,26 @@ TRANSACTION(ttNFTOKEN_CREATE_OFFER, 27, NFTokenCreateOffer, noCreate, ({ })) /** This transaction cancels an existing offer to buy or sell an existing NFT. */ -TRANSACTION(ttNFTOKEN_CANCEL_OFFER, 28, NFTokenCancelOffer, noCreate, ({ +TRANSACTION(ttNFTOKEN_CANCEL_OFFER, 28, NFTokenCancelOffer, noPriv, ({ {sfNFTokenOffers, soeREQUIRED}, })) /** This transaction accepts an existing offer to buy or sell an existing NFT. */ -TRANSACTION(ttNFTOKEN_ACCEPT_OFFER, 29, NFTokenAcceptOffer, noCreate, ({ +TRANSACTION(ttNFTOKEN_ACCEPT_OFFER, 29, NFTokenAcceptOffer, noPriv, ({ {sfNFTokenBuyOffer, soeOPTIONAL}, {sfNFTokenSellOffer, soeOPTIONAL}, {sfNFTokenBrokerFee, soeOPTIONAL}, })) /** This transaction claws back issued tokens. */ -TRANSACTION(ttCLAWBACK, 30, Clawback, noCreate, ({ +TRANSACTION(ttCLAWBACK, 30, Clawback, noPriv, ({ {sfAmount, soeREQUIRED, soeMPTSupported}, {sfHolder, soeOPTIONAL}, })) /** This transaction claws back tokens from an AMM pool. */ -TRANSACTION(ttAMM_CLAWBACK, 31, AMMClawback, noCreate, ({ +TRANSACTION(ttAMM_CLAWBACK, 31, AMMClawback, + mayDeleteAcct | overrideFreeze, ({ {sfHolder, soeREQUIRED}, {sfAsset, soeREQUIRED}, {sfAsset2, soeREQUIRED}, @@ -247,14 +264,14 @@ TRANSACTION(ttAMM_CLAWBACK, 31, AMMClawback, noCreate, ({ })) /** This transaction type creates an AMM instance */ -TRANSACTION(ttAMM_CREATE, 35, AMMCreate, createPseudo, ({ +TRANSACTION(ttAMM_CREATE, 35, AMMCreate, createPseudoAcct, ({ {sfAmount, soeREQUIRED}, {sfAmount2, soeREQUIRED}, {sfTradingFee, soeREQUIRED}, })) /** This transaction type deposits into an AMM instance */ -TRANSACTION(ttAMM_DEPOSIT, 36, AMMDeposit, noCreate, ({ +TRANSACTION(ttAMM_DEPOSIT, 36, AMMDeposit, noPriv, ({ {sfAsset, soeREQUIRED}, {sfAsset2, soeREQUIRED}, {sfAmount, soeOPTIONAL}, @@ -265,7 +282,7 @@ TRANSACTION(ttAMM_DEPOSIT, 36, AMMDeposit, noCreate, ({ })) /** This transaction type withdraws from an AMM instance */ -TRANSACTION(ttAMM_WITHDRAW, 37, AMMWithdraw, noCreate, ({ +TRANSACTION(ttAMM_WITHDRAW, 37, AMMWithdraw, mayDeleteAcct, ({ {sfAsset, soeREQUIRED}, {sfAsset2, soeREQUIRED}, {sfAmount, soeOPTIONAL}, @@ -275,14 +292,14 @@ TRANSACTION(ttAMM_WITHDRAW, 37, AMMWithdraw, noCreate, ({ })) /** This transaction type votes for the trading fee */ -TRANSACTION(ttAMM_VOTE, 38, AMMVote, noCreate, ({ +TRANSACTION(ttAMM_VOTE, 38, AMMVote, noPriv, ({ {sfAsset, soeREQUIRED}, {sfAsset2, soeREQUIRED}, {sfTradingFee, soeREQUIRED}, })) /** This transaction type bids for the auction slot */ -TRANSACTION(ttAMM_BID, 39, AMMBid, noCreate, ({ +TRANSACTION(ttAMM_BID, 39, AMMBid, noPriv, ({ {sfAsset, soeREQUIRED}, {sfAsset2, soeREQUIRED}, {sfBidMin, soeOPTIONAL}, @@ -291,20 +308,20 @@ TRANSACTION(ttAMM_BID, 39, AMMBid, noCreate, ({ })) /** This transaction type deletes AMM in the empty state */ -TRANSACTION(ttAMM_DELETE, 40, AMMDelete, noCreate, ({ +TRANSACTION(ttAMM_DELETE, 40, AMMDelete, mustDeleteAcct, ({ {sfAsset, soeREQUIRED}, {sfAsset2, soeREQUIRED}, })) /** This transactions creates a crosschain sequence number */ -TRANSACTION(ttXCHAIN_CREATE_CLAIM_ID, 41, XChainCreateClaimID, noCreate, ({ +TRANSACTION(ttXCHAIN_CREATE_CLAIM_ID, 41, XChainCreateClaimID, noPriv, ({ {sfXChainBridge, soeREQUIRED}, {sfSignatureReward, soeREQUIRED}, {sfOtherChainSource, soeREQUIRED}, })) /** This transactions initiates a crosschain transaction */ -TRANSACTION(ttXCHAIN_COMMIT, 42, XChainCommit, noCreate, ({ +TRANSACTION(ttXCHAIN_COMMIT, 42, XChainCommit, noPriv, ({ {sfXChainBridge, soeREQUIRED}, {sfXChainClaimID, soeREQUIRED}, {sfAmount, soeREQUIRED}, @@ -312,7 +329,7 @@ TRANSACTION(ttXCHAIN_COMMIT, 42, XChainCommit, noCreate, ({ })) /** This transaction completes a crosschain transaction */ -TRANSACTION(ttXCHAIN_CLAIM, 43, XChainClaim, noCreate, ({ +TRANSACTION(ttXCHAIN_CLAIM, 43, XChainClaim, noPriv, ({ {sfXChainBridge, soeREQUIRED}, {sfXChainClaimID, soeREQUIRED}, {sfDestination, soeREQUIRED}, @@ -321,7 +338,7 @@ TRANSACTION(ttXCHAIN_CLAIM, 43, XChainClaim, noCreate, ({ })) /** This transaction initiates a crosschain account create transaction */ -TRANSACTION(ttXCHAIN_ACCOUNT_CREATE_COMMIT, 44, XChainAccountCreateCommit, noCreate, ({ +TRANSACTION(ttXCHAIN_ACCOUNT_CREATE_COMMIT, 44, XChainAccountCreateCommit, noPriv, ({ {sfXChainBridge, soeREQUIRED}, {sfDestination, soeREQUIRED}, {sfAmount, soeREQUIRED}, @@ -329,7 +346,7 @@ TRANSACTION(ttXCHAIN_ACCOUNT_CREATE_COMMIT, 44, XChainAccountCreateCommit, noCre })) /** This transaction adds an attestation to a claim */ -TRANSACTION(ttXCHAIN_ADD_CLAIM_ATTESTATION, 45, XChainAddClaimAttestation, createFull, ({ +TRANSACTION(ttXCHAIN_ADD_CLAIM_ATTESTATION, 45, XChainAddClaimAttestation, createAcct, ({ {sfXChainBridge, soeREQUIRED}, {sfAttestationSignerAccount, soeREQUIRED}, @@ -345,7 +362,7 @@ TRANSACTION(ttXCHAIN_ADD_CLAIM_ATTESTATION, 45, XChainAddClaimAttestation, creat })) /** This transaction adds an attestation to an account */ -TRANSACTION(ttXCHAIN_ADD_ACCOUNT_CREATE_ATTESTATION, 46, XChainAddAccountCreateAttestation, createFull, ({ +TRANSACTION(ttXCHAIN_ADD_ACCOUNT_CREATE_ATTESTATION, 46, XChainAddAccountCreateAttestation, createAcct, ({ {sfXChainBridge, soeREQUIRED}, {sfAttestationSignerAccount, soeREQUIRED}, @@ -362,31 +379,31 @@ TRANSACTION(ttXCHAIN_ADD_ACCOUNT_CREATE_ATTESTATION, 46, XChainAddAccountCreateA })) /** This transaction modifies a sidechain */ -TRANSACTION(ttXCHAIN_MODIFY_BRIDGE, 47, XChainModifyBridge, noCreate, ({ +TRANSACTION(ttXCHAIN_MODIFY_BRIDGE, 47, XChainModifyBridge, noPriv, ({ {sfXChainBridge, soeREQUIRED}, {sfSignatureReward, soeOPTIONAL}, {sfMinAccountCreateAmount, soeOPTIONAL}, })) /** This transactions creates a sidechain */ -TRANSACTION(ttXCHAIN_CREATE_BRIDGE, 48, XChainCreateBridge, noCreate, ({ +TRANSACTION(ttXCHAIN_CREATE_BRIDGE, 48, XChainCreateBridge, noPriv, ({ {sfXChainBridge, soeREQUIRED}, {sfSignatureReward, soeREQUIRED}, {sfMinAccountCreateAmount, soeOPTIONAL}, })) /** This transaction type creates or updates a DID */ -TRANSACTION(ttDID_SET, 49, DIDSet, noCreate, ({ +TRANSACTION(ttDID_SET, 49, DIDSet, noPriv, ({ {sfDIDDocument, soeOPTIONAL}, {sfURI, soeOPTIONAL}, {sfData, soeOPTIONAL}, })) /** This transaction type deletes a DID */ -TRANSACTION(ttDID_DELETE, 50, DIDDelete, noCreate, ({})) +TRANSACTION(ttDID_DELETE, 50, DIDDelete, noPriv, ({})) /** This transaction type creates an Oracle instance */ -TRANSACTION(ttORACLE_SET, 51, OracleSet, noCreate, ({ +TRANSACTION(ttORACLE_SET, 51, OracleSet, noPriv, ({ {sfOracleDocumentID, soeREQUIRED}, {sfProvider, soeOPTIONAL}, {sfURI, soeOPTIONAL}, @@ -396,18 +413,19 @@ TRANSACTION(ttORACLE_SET, 51, OracleSet, noCreate, ({ })) /** This transaction type deletes an Oracle instance */ -TRANSACTION(ttORACLE_DELETE, 52, OracleDelete, noCreate, ({ +TRANSACTION(ttORACLE_DELETE, 52, OracleDelete, noPriv, ({ {sfOracleDocumentID, soeREQUIRED}, })) /** This transaction type fixes a problem in the ledger state */ -TRANSACTION(ttLEDGER_STATE_FIX, 53, LedgerStateFix, noCreate, ({ +TRANSACTION(ttLEDGER_STATE_FIX, 53, LedgerStateFix, noPriv, ({ {sfLedgerFixType, soeREQUIRED}, {sfOwner, soeOPTIONAL}, })) /** This transaction type creates a MPTokensIssuance instance */ -TRANSACTION(ttMPTOKEN_ISSUANCE_CREATE, 54, MPTokenIssuanceCreate, noCreate, ({ +TRANSACTION(ttMPTOKEN_ISSUANCE_CREATE, 54, MPTokenIssuanceCreate, + createMPTIssuance, ({ {sfAssetScale, soeOPTIONAL}, {sfTransferFee, soeOPTIONAL}, {sfMaximumAmount, soeOPTIONAL}, @@ -415,24 +433,25 @@ TRANSACTION(ttMPTOKEN_ISSUANCE_CREATE, 54, MPTokenIssuanceCreate, noCreate, ({ })) /** This transaction type destroys a MPTokensIssuance instance */ -TRANSACTION(ttMPTOKEN_ISSUANCE_DESTROY, 55, MPTokenIssuanceDestroy, noCreate, ({ +TRANSACTION(ttMPTOKEN_ISSUANCE_DESTROY, 55, MPTokenIssuanceDestroy, + destroyMPTIssuance, ({ {sfMPTokenIssuanceID, soeREQUIRED}, })) /** This transaction type sets flags on a MPTokensIssuance or MPToken instance */ -TRANSACTION(ttMPTOKEN_ISSUANCE_SET, 56, MPTokenIssuanceSet, noCreate, ({ +TRANSACTION(ttMPTOKEN_ISSUANCE_SET, 56, MPTokenIssuanceSet, noPriv, ({ {sfMPTokenIssuanceID, soeREQUIRED}, {sfHolder, soeOPTIONAL}, })) /** This transaction type authorizes a MPToken instance */ -TRANSACTION(ttMPTOKEN_AUTHORIZE, 57, MPTokenAuthorize, noCreate, ({ +TRANSACTION(ttMPTOKEN_AUTHORIZE, 57, MPTokenAuthorize, mustAuthorizeMPT, ({ {sfMPTokenIssuanceID, soeREQUIRED}, {sfHolder, soeOPTIONAL}, })) /** This transaction type create an Credential instance */ -TRANSACTION(ttCREDENTIAL_CREATE, 58, CredentialCreate, noCreate, ({ +TRANSACTION(ttCREDENTIAL_CREATE, 58, CredentialCreate, noPriv, ({ {sfSubject, soeREQUIRED}, {sfCredentialType, soeREQUIRED}, {sfExpiration, soeOPTIONAL}, @@ -440,38 +459,39 @@ TRANSACTION(ttCREDENTIAL_CREATE, 58, CredentialCreate, noCreate, ({ })) /** This transaction type accept an Credential object */ -TRANSACTION(ttCREDENTIAL_ACCEPT, 59, CredentialAccept, noCreate, ({ +TRANSACTION(ttCREDENTIAL_ACCEPT, 59, CredentialAccept, noPriv, ({ {sfIssuer, soeREQUIRED}, {sfCredentialType, soeREQUIRED}, })) /** This transaction type delete an Credential object */ -TRANSACTION(ttCREDENTIAL_DELETE, 60, CredentialDelete, noCreate, ({ +TRANSACTION(ttCREDENTIAL_DELETE, 60, CredentialDelete, noPriv, ({ {sfSubject, soeOPTIONAL}, {sfIssuer, soeOPTIONAL}, {sfCredentialType, soeREQUIRED}, })) /** This transaction type modify a NFToken */ -TRANSACTION(ttNFTOKEN_MODIFY, 61, NFTokenModify, noCreate, ({ +TRANSACTION(ttNFTOKEN_MODIFY, 61, NFTokenModify, noPriv, ({ {sfNFTokenID, soeREQUIRED}, {sfOwner, soeOPTIONAL}, {sfURI, soeOPTIONAL}, })) /** This transaction type creates or modifies a Permissioned Domain */ -TRANSACTION(ttPERMISSIONED_DOMAIN_SET, 62, PermissionedDomainSet, noCreate, ({ +TRANSACTION(ttPERMISSIONED_DOMAIN_SET, 62, PermissionedDomainSet, noPriv, ({ {sfDomainID, soeOPTIONAL}, {sfAcceptedCredentials, soeREQUIRED}, })) /** This transaction type deletes a Permissioned Domain */ -TRANSACTION(ttPERMISSIONED_DOMAIN_DELETE, 63, PermissionedDomainDelete, noCreate, ({ +TRANSACTION(ttPERMISSIONED_DOMAIN_DELETE, 63, PermissionedDomainDelete, noPriv, ({ {sfDomainID, soeREQUIRED}, })) /** This transaction creates a single asset vault. */ -TRANSACTION(ttVAULT_CREATE, 64, VaultCreate, createPseudo, ({ +TRANSACTION(ttVAULT_CREATE, 64, VaultCreate, + createPseudoAcct | createMPTIssuance, ({ {sfAsset, soeREQUIRED, soeMPTSupported}, {sfAssetsMaximum, soeOPTIONAL}, {sfMPTokenMetadata, soeOPTIONAL}, @@ -481,7 +501,7 @@ TRANSACTION(ttVAULT_CREATE, 64, VaultCreate, createPseudo, ({ })) /** This transaction updates a single asset vault. */ -TRANSACTION(ttVAULT_SET, 65, VaultSet, noCreate, ({ +TRANSACTION(ttVAULT_SET, 65, VaultSet, noPriv, ({ {sfVaultID, soeREQUIRED}, {sfAssetsMaximum, soeOPTIONAL}, {sfDomainID, soeOPTIONAL}, // PermissionedDomainID @@ -489,32 +509,34 @@ TRANSACTION(ttVAULT_SET, 65, VaultSet, noCreate, ({ })) /** This transaction deletes a single asset vault. */ -TRANSACTION(ttVAULT_DELETE, 66, VaultDelete, noCreate, ({ +TRANSACTION(ttVAULT_DELETE, 66, VaultDelete, + mustDeleteAcct | destroyMPTIssuance, ({ {sfVaultID, soeREQUIRED}, })) /** This transaction trades assets for shares with a vault. */ -TRANSACTION(ttVAULT_DEPOSIT, 67, VaultDeposit, noCreate, ({ +TRANSACTION(ttVAULT_DEPOSIT, 67, VaultDeposit, mayAuthorizeMPT, ({ {sfVaultID, soeREQUIRED}, {sfAmount, soeREQUIRED, soeMPTSupported}, })) /** This transaction trades shares for assets with a vault. */ -TRANSACTION(ttVAULT_WITHDRAW, 68, VaultWithdraw, noCreate, ({ +TRANSACTION(ttVAULT_WITHDRAW, 68, VaultWithdraw, noPriv, ({ {sfVaultID, soeREQUIRED}, {sfAmount, soeREQUIRED, soeMPTSupported}, {sfDestination, soeOPTIONAL}, })) /** This transaction claws back tokens from a vault. */ -TRANSACTION(ttVAULT_CLAWBACK, 69, VaultClawback, noCreate, ({ +TRANSACTION(ttVAULT_CLAWBACK, 69, VaultClawback, noPriv, ({ {sfVaultID, soeREQUIRED}, {sfHolder, soeREQUIRED}, {sfAmount, soeOPTIONAL, soeMPTSupported}, })) /** This transaction creates and updates a Loan Broker */ -TRANSACTION(ttLOAN_BROKER_SET, 70, LoanBrokerSet, createPseudo, ({ +TRANSACTION(ttLOAN_BROKER_SET, 70, LoanBrokerSet, + createPseudoAcct | mayAuthorizeMPT, ({ {sfVaultID, soeREQUIRED}, {sfLoanBrokerID, soeOPTIONAL}, {sfData, soeOPTIONAL}, @@ -526,24 +548,25 @@ TRANSACTION(ttLOAN_BROKER_SET, 70, LoanBrokerSet, createPseudo, ({ #if 0 /** This transaction deletes a Loan Broker */ -TRANSACTION(ttLOAN_BROKER_DELETE, 71, LoanBrokerDelete, noCreate, ({ +TRANSACTION(ttLOAN_BROKER_DELETE, 71, LoanBrokerDelete, + acctDelete | mayAuthorizeMPT, ({ {sfLoanBrokerID, soeREQUIRED}, })) /** This transaction deposits First Loss Capital into a Loan Broker */ -TRANSACTION(ttLOAN_BROKER_COVER_DEPOSIT, 72, LoanBrokerCoverDeposit, noCreate, ({ +TRANSACTION(ttLOAN_BROKER_COVER_DEPOSIT, 72, LoanBrokerCoverDeposit, noPriv, ({ {sfLoanBrokerID, soeREQUIRED}, {sfNumber, soeREQUIRED}, })) /** This transaction withdraws First Loss Capital from a Loan Broker */ -TRANSACTION(ttLOAN_BROKER_COVER_WITHDRAW, 73, LoanBrokerCoverWithdraw, noCreate, ({ +TRANSACTION(ttLOAN_BROKER_COVER_WITHDRAW, 73, LoanBrokerCoverWithdraw, noPriv, ({ {sfLoanBrokerID, soeREQUIRED}, {sfNumber, soeREQUIRED}, })) /** This transaction creates a Loan */ -TRANSACTION(ttLOAN_SET, 74, LoanSet, noCreate, ({ +TRANSACTION(ttLOAN_SET, 74, LoanSet, noPriv, ({ {sfLoanBrokerID, soeREQUIRED}, {sfData, soeOPTIONAL}, {sfCounterparty, soeOPTIONAL}, @@ -563,23 +586,23 @@ TRANSACTION(ttLOAN_SET, 74, LoanSet, noCreate, ({ })) /** This transaction deletes an existing Loan */ -TRANSACTION(ttLOAN_DELETE, 75, LoanDelete, noCreate, ({ +TRANSACTION(ttLOAN_DELETE, 75, LoanDelete, noPriv, ({ {sfLoanID, soeREQUIRED}, })) /** This transaction is used to change the delinquency status of an existing Loan */ -TRANSACTION(ttLOAN_MANAGE, 76, LoanManage, noCreate, ({ +TRANSACTION(ttLOAN_MANAGE, 76, LoanManage, noPriv, ({ {sfLoanID, soeREQUIRED}, })) /** The Borrower uses this transaction to draws funds from the Loan. */ -TRANSACTION(ttLOAN_DRAW, 77, LoanDraw, noCreate, ({ +TRANSACTION(ttLOAN_DRAW, 77, LoanDraw, noPriv, ({ {sfLoanID, soeREQUIRED}, {sfAmount, soeREQUIRED}, })) /** The Borrower uses this transaction to make a Payment on the Loan. */ -TRANSACTION(ttLOAN_PAY, 77, LoanPay, noCreate, ({ +TRANSACTION(ttLOAN_PAY, 77, LoanPay, noPriv, ({ {sfLoanID, soeREQUIRED}, {sfAmount, soeREQUIRED}, })) @@ -589,7 +612,7 @@ TRANSACTION(ttLOAN_PAY, 77, LoanPay, noCreate, ({ For details, see: https://xrpl.org/amendments.html */ -TRANSACTION(ttAMENDMENT, 100, EnableAmendment, noCreate, ({ +TRANSACTION(ttAMENDMENT, 100, EnableAmendment, noPriv, ({ {sfLedgerSequence, soeREQUIRED}, {sfAmendment, soeREQUIRED}, })) @@ -597,7 +620,7 @@ TRANSACTION(ttAMENDMENT, 100, EnableAmendment, noCreate, ({ /** 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, noCreate, ({ +TRANSACTION(ttFEE, 101, SetFee, noPriv, ({ {sfLedgerSequence, soeOPTIONAL}, // Old version uses raw numbers {sfBaseFee, soeOPTIONAL}, @@ -614,7 +637,7 @@ TRANSACTION(ttFEE, 101, SetFee, noCreate, ({ For details, see: https://xrpl.org/negative-unl.html */ -TRANSACTION(ttUNL_MODIFY, 102, UNLModify, noCreate, ({ +TRANSACTION(ttUNL_MODIFY, 102, UNLModify, noPriv, ({ {sfUNLModifyDisabling, soeREQUIRED}, {sfLedgerSequence, soeREQUIRED}, {sfUNLModifyValidator, soeREQUIRED}, diff --git a/include/xrpl/protocol/jss.h b/include/xrpl/protocol/jss.h index 26b673dcd2..bae053c70b 100644 --- a/include/xrpl/protocol/jss.h +++ b/include/xrpl/protocol/jss.h @@ -702,7 +702,7 @@ JSS(write_load); // out: GetCounts #pragma push_macro("TRANSACTION") #undef TRANSACTION -#define TRANSACTION(tag, value, name, acctCreate, fields) JSS(name); +#define TRANSACTION(tag, value, name, privileges, fields) JSS(name); #include diff --git a/src/libxrpl/protocol/TxFormats.cpp b/src/libxrpl/protocol/TxFormats.cpp index b230eb03e5..b8187ff1e8 100644 --- a/src/libxrpl/protocol/TxFormats.cpp +++ b/src/libxrpl/protocol/TxFormats.cpp @@ -54,7 +54,7 @@ TxFormats::TxFormats() #undef TRANSACTION #define UNWRAP(...) __VA_ARGS__ -#define TRANSACTION(tag, value, name, acctCreate, fields) \ +#define TRANSACTION(tag, value, name, privileges, fields) \ add(jss::name, tag, UNWRAP fields, commonFields); #include diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index 71edf3453f..5acf3f1f84 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/src/test/app/LoanBroker_test.cpp @@ -40,15 +40,21 @@ namespace test { class LoanBroker_test : public beast::unit_test::suite { + // Ensure that all the features needed for Lending Protocol are included, + // even if they are set to unsupported. + FeatureBitset const all{ + jtx::supported_amendments() | featureMPTokensV1 | + featureSingleAssetVault | featureLendingProtocol}; + void testDisabled() { + testcase("Disabled"); // Lending Protocol depends on Single Asset Vault (SAV). Test // combinations of the two amendments. // Single Asset Vault depends on MPTokensV1, but don't test every combo // of that. using namespace jtx; - FeatureBitset const all{jtx::supported_amendments()}; auto failAll = [this](FeatureBitset features, bool goodVault = false) { Env env(*this, features); @@ -68,7 +74,7 @@ class LoanBroker_test : public beast::unit_test::suite // Can't create a loan broker regardless of whether the vault exists env(set(alice, keylet.key), ter(temDISABLED)); }; - // failAll(all - featureMPTokensV1); + failAll(all - featureMPTokensV1); failAll(all - featureSingleAssetVault - featureLendingProtocol); failAll(all - featureSingleAssetVault); failAll(all - featureLendingProtocol, true); @@ -77,11 +83,12 @@ class LoanBroker_test : public beast::unit_test::suite void testCreateAndUpdate() { + testcase("Create and update"); using namespace jtx; // Create 3 loan brokers: one for XRP, one for an IOU, and one for an // MPT. That'll require three corresponding SAVs. - Env env(*this); + Env env(*this, all); Account issuer{"issuer"}; // For simplicity, alice will be the sole actor for the vault & brokers. diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index f3357bee64..e9b60c1791 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -35,6 +35,56 @@ namespace ripple { +enum Privilege { + noPriv = + 0x0000, // The transaction can not do any of the enumerated operations + createAcct = + 0x0001, // The transaction can create a new ACCOUNT_ROOT object. + createPseudoAcct = 0x0003, // The transaction can create a pseudo account, + // which implies createAcct + mustDeleteAcct = + 0x0004, // The transaction must delete an ACCOUNT_ROOT object + mayDeleteAcct = 0x0008, // The transaction may delete an ACCOUNT_ROOT + // object, but does not have to + overrideFreeze = 0x0010, // The transaction can override some freeze rules + changeNFTCounts = 0x0020, // The transaction can mint or burn an NFT + createMPTIssuance = + 0x0040, // The transaction can create a new MPT issuance + destroyMPTIssuance = 0x0080, // The transaction can destroy an MPT issuance + mustAuthorizeMPT = 0x0100, // The transaction MUST create or delete an MPT + // object (except by issuer) + mayAuthorizeMPT = 0x0200, // The transaction MAY create or delete an MPT + // object (except by issuer) +}; +constexpr Privilege +operator|(Privilege const& lhs, Privilege const& rhs) +{ + return safe_cast( + safe_cast>(lhs) | + safe_cast>(rhs)); +} + +#pragma push_macro("TRANSACTION") +#undef TRANSACTION + +#define TRANSACTION(tag, value, name, privileges, fields) \ + case tag: { \ + return (privileges) & priv; \ + } + +bool +checkMyPrivilege(STTx const& tx, Privilege priv) +{ + switch (tx.getTxnType()) + { +#include + } + return false; +}; + +#undef TRANSACTION +#pragma pop_macro("TRANSACTION") + void TransactionFeeCheck::visitEntry( bool, @@ -329,10 +379,7 @@ AccountRootsNotDeleted::finalize( // transaction when the total AMM LP Tokens balance goes to 0. // A successful AccountDelete or AMMDelete MUST delete exactly // one account root. - if ((tx.getTxnType() == ttACCOUNT_DELETE || - tx.getTxnType() == ttAMM_DELETE || - tx.getTxnType() == ttVAULT_DELETE) && - result == tesSUCCESS) + if (checkMyPrivilege(tx, mustDeleteAcct) && result == tesSUCCESS) { if (accountsDeleted_ == 1) return true; @@ -349,9 +396,8 @@ AccountRootsNotDeleted::finalize( // A successful AMMWithdraw/AMMClawback MAY delete one account root // when the total AMM LP Tokens balance goes to 0. Not every AMM withdraw // deletes the AMM account, accountsDeleted_ is set if it is deleted. - if ((tx.getTxnType() == ttAMM_WITHDRAW || - tx.getTxnType() == ttAMM_CLAWBACK) && - result == tesSUCCESS && accountsDeleted_ == 1) + if (checkMyPrivilege(tx, mayDeleteAcct) && result == tesSUCCESS && + accountsDeleted_ == 1) return true; if (accountsDeleted_ == 0) @@ -832,7 +878,7 @@ TransfersNotFrozen::validateFrozenState( } // AMMClawbacks are allowed to override some freeze rules - if ((!isAMMLine || globalFreeze) && tx.getTxnType() == ttAMM_CLAWBACK) + if ((!isAMMLine || globalFreeze) && checkMyPrivilege(tx, overrideFreeze)) { JLOG(j.debug()) << "Invariant check allowing funds to be moved " << (change.balanceChangeSign > 0 ? "to" : "from") @@ -891,54 +937,15 @@ ValidNewAccountRoot::finalize( return false; } - enum Creation { - noCreate, // - createFull, // - createPseudo - }; -#pragma push_macro("TRANSACTION") -#undef TRANSACTION - -#define TRANSACTION(tag, value, name, acctCreate, fields) \ - case tag: { \ - return acctCreate != noCreate; \ - } - - auto creator = [](STTx const& tx) { - switch (tx.getTxnType()) - { -#include - } - return false; - }; - -#undef TRANSACTION - -#define TRANSACTION(tag, value, name, acctCreate, fields) \ - case tag: { \ - return acctCreate == createPseudo; \ - } - - auto pseudoCreator = [](STTx const& tx) { - switch (tx.getTxnType()) - { -#include - } - return false; - }; - -#undef TRANSACTION -#pragma pop_macro("TRANSACTION") - // From this point on we know exactly one account was created. - if (creator(tx) && result == tesSUCCESS) + if (checkMyPrivilege(tx, createAcct) && result == tesSUCCESS) { bool const pseudoAccount = (pseudoAccount_ && (view.rules().enabled(featureSingleAssetVault) || view.rules().enabled(featureLendingProtocol))); - if (pseudoAccount && !pseudoCreator(tx)) + if (pseudoAccount && !checkMyPrivilege(tx, createPseudoAcct)) { JLOG(j.fatal()) << "Invariant failed: pseudo-account created by a " "wrong transaction type"; @@ -1173,7 +1180,7 @@ NFTokenCountTracking::finalize( beast::Journal const& j) { if (TxType const txType = tx.getTxnType(); - txType != ttNFTOKEN_MINT && txType != ttNFTOKEN_BURN) + !checkMyPrivilege(tx, changeNFTCounts)) { if (beforeMintedTotal != afterMintedTotal) { @@ -1363,8 +1370,7 @@ ValidMPTIssuance::finalize( { if (result == tesSUCCESS) { - if (tx.getTxnType() == ttMPTOKEN_ISSUANCE_CREATE || - tx.getTxnType() == ttVAULT_CREATE) + if (checkMyPrivilege(tx, createMPTIssuance)) { if (mptIssuancesCreated_ == 0) { @@ -1385,8 +1391,7 @@ ValidMPTIssuance::finalize( return mptIssuancesCreated_ == 1 && mptIssuancesDeleted_ == 0; } - if (tx.getTxnType() == ttMPTOKEN_ISSUANCE_DESTROY || - tx.getTxnType() == ttVAULT_DELETE) + if (checkMyPrivilege(tx, destroyMPTIssuance)) { if (mptIssuancesDeleted_ == 0) { @@ -1407,8 +1412,7 @@ ValidMPTIssuance::finalize( return mptIssuancesCreated_ == 0 && mptIssuancesDeleted_ == 1; } - if (tx.getTxnType() == ttMPTOKEN_AUTHORIZE || - tx.getTxnType() == ttVAULT_DEPOSIT) + if (checkMyPrivilege(tx, mustAuthorizeMPT | mayAuthorizeMPT)) { bool const submittedByIssuer = tx.isFieldPresent(sfHolder); @@ -1424,6 +1428,12 @@ ValidMPTIssuance::finalize( "succeeded but deleted issuances"; return false; } + else if (mptokensCreated_ + mptokensDeleted_ > 1) + { + JLOG(j.fatal()) << "Invariant failed: MPT authorize succeeded " + "but created/deleted bad number mptokens"; + return false; + } else if ( submittedByIssuer && (mptokensCreated_ > 0 || mptokensDeleted_ > 0)) @@ -1434,7 +1444,7 @@ ValidMPTIssuance::finalize( return false; } else if ( - !submittedByIssuer && (tx.getTxnType() != ttVAULT_DEPOSIT) && + !submittedByIssuer && !checkMyPrivilege(tx, mayAuthorizeMPT) && (mptokensCreated_ + mptokensDeleted_ != 1)) { // if the holder submitted this tx, then a mptoken must be diff --git a/src/xrpld/app/tx/detail/applySteps.cpp b/src/xrpld/app/tx/detail/applySteps.cpp index 11f4ba39d6..cc0e6ef6c1 100644 --- a/src/xrpld/app/tx/detail/applySteps.cpp +++ b/src/xrpld/app/tx/detail/applySteps.cpp @@ -96,8 +96,8 @@ with_txn_type(TxType txnType, F&& f) #pragma push_macro("TRANSACTION") #undef TRANSACTION -#define TRANSACTION(tag, value, name, acctCreate, fields) \ - case tag: \ +#define TRANSACTION(tag, value, name, privileges, fields) \ + case tag: \ return f.template operator()(); #include