From 3eebdae3f02cf1a4cfc48067796374057ac254bc Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Mon, 27 Jan 2025 16:34:41 +0000 Subject: [PATCH] Implement MPT domain checks --- include/xrpl/protocol/LedgerFormats.h | 1 + .../xrpl/protocol/detail/ledger_entries.macro | 3 +- src/xrpld/app/misc/CredentialHelpers.cpp | 26 ++-- src/xrpld/app/misc/CredentialHelpers.h | 8 +- src/xrpld/app/tx/detail/CashCheck.cpp | 2 +- src/xrpld/app/tx/detail/MPTokenAuthorize.h | 2 +- .../app/tx/detail/MPTokenIssuanceCreate.cpp | 3 + .../app/tx/detail/MPTokenIssuanceCreate.h | 1 + src/xrpld/app/tx/detail/VaultCreate.cpp | 3 +- src/xrpld/app/tx/detail/VaultDeposit.cpp | 67 +++------ src/xrpld/ledger/View.h | 28 +++- src/xrpld/ledger/detail/View.cpp | 133 ++++++++++++++++-- 12 files changed, 185 insertions(+), 92 deletions(-) diff --git a/include/xrpl/protocol/LedgerFormats.h b/include/xrpl/protocol/LedgerFormats.h index 0590dab0fb..90b487780f 100644 --- a/include/xrpl/protocol/LedgerFormats.h +++ b/include/xrpl/protocol/LedgerFormats.h @@ -186,6 +186,7 @@ enum LedgerSpecificFlags { // ltMPTOKEN lsfMPTAuthorized = 0x00000002, + lsfMPTDomainCheck = 0x00000004, // ltCREDENTIAL lsfAccepted = 0x00010000, diff --git a/include/xrpl/protocol/detail/ledger_entries.macro b/include/xrpl/protocol/detail/ledger_entries.macro index c7a8bf668b..e68b0de474 100644 --- a/include/xrpl/protocol/detail/ledger_entries.macro +++ b/include/xrpl/protocol/detail/ledger_entries.macro @@ -405,6 +405,7 @@ LEDGER_ENTRY(ltMPTOKEN_ISSUANCE, 0x007e, MPTokenIssuance, mpt_issuance, ({ {sfMPTokenMetadata, soeOPTIONAL}, {sfPreviousTxnID, soeREQUIRED}, {sfPreviousTxnLgrSeq, soeREQUIRED}, + {sfDomainID, soeOPTIONAL}, })) /** A ledger object which tracks MPToken @@ -479,8 +480,8 @@ LEDGER_ENTRY(ltVAULT, 0x0083, Vault, vault, ({ {sfAssetMaximum, soeDEFAULT}, {sfLossUnrealized, soeDEFAULT}, {sfMPTokenIssuanceID, soeREQUIRED}, // sfShare - {sfDomainID, soeOPTIONAL}, // PermissionedDomainID // no ShareTotal ever (use MPTIssuance.sfOutstandingAmount) + // no PermissionedDomainID (use MPTIssuance.sfDomainID) // no WithdrawalPolicy yet })) diff --git a/src/xrpld/app/misc/CredentialHelpers.cpp b/src/xrpld/app/misc/CredentialHelpers.cpp index 89822522b5..543c2254d4 100644 --- a/src/xrpld/app/misc/CredentialHelpers.cpp +++ b/src/xrpld/app/misc/CredentialHelpers.cpp @@ -204,7 +204,9 @@ valid(ReadView const& view, uint256 domainID, AccountID const& subject) auto const sleCredential = view.read(keylet::credential(subject, issuer, type)); - if (sleCredential && sleCredential->getFlags() & lsfAccepted) + // Do not check for expired credentials here, we need ApplyView& for + // that, to allow us to delete them (see verifyDomain below) + if (sleCredential && (sleCredential->getFlags() & lsfAccepted)) return tesSUCCESS; } @@ -300,16 +302,11 @@ checkArray(STArray const& credentials, unsigned maxSize, beast::Journal j) TER verifyDomain( - ApplyContext& ctx, - AccountID const& src, - AccountID const& dst, - std::shared_ptr const& object) + ApplyView& view, + AccountID const& account, + uint256 domainID, + beast::Journal j) { - if (!object->isFieldPresent(sfDomainID)) - return tesSUCCESS; - auto const domainID = object->getFieldH256(sfDomainID); - - auto& view = ctx.view(); auto const slePD = view.read(keylet::permissionedDomain(domainID)); if (!slePD || !slePD->isFieldPresent(sfAcceptedCredentials)) return tefINTERNAL; @@ -324,18 +321,13 @@ verifyDomain( auto const issuer = h.getAccountID(sfIssuer); auto const type = makeSlice(h.getFieldVL(sfCredentialType)); - auto const keyletCredential = keylet::credential(dst, issuer, type); + auto const keyletCredential = keylet::credential(account, issuer, type); if (view.exists(keyletCredential)) credentials.push_back(keyletCredential.key); } // Result intentionally ignored. - [[maybe_unused]] bool _ = - credentials::removeExpired(view, credentials, ctx.journal); - - // Only do this check after we have removed expired credentials. - if (src == dst) - return tesSUCCESS; + [[maybe_unused]] bool _ = credentials::removeExpired(view, credentials, j); for (auto const& h : credentials) { diff --git a/src/xrpld/app/misc/CredentialHelpers.h b/src/xrpld/app/misc/CredentialHelpers.h index 476be32414..604c9e3014 100644 --- a/src/xrpld/app/misc/CredentialHelpers.h +++ b/src/xrpld/app/misc/CredentialHelpers.h @@ -84,10 +84,10 @@ checkArray(STArray const& credentials, unsigned maxSize, beast::Journal j); // object TER verifyDomain( - ApplyContext& ctx, - AccountID const& src, - AccountID const& dst, - std::shared_ptr const& object); + ApplyView& view, + AccountID const& account, + uint256 domainID, + beast::Journal j); // Check expired credentials and for existing DepositPreauth ledger object TER diff --git a/src/xrpld/app/tx/detail/CashCheck.cpp b/src/xrpld/app/tx/detail/CashCheck.cpp index 8b5ef79b6d..e9545b0074 100644 --- a/src/xrpld/app/tx/detail/CashCheck.cpp +++ b/src/xrpld/app/tx/detail/CashCheck.cpp @@ -212,7 +212,7 @@ CashCheck::preclaim(PreclaimContext const& ctx) if (!sleTrustLine) { // We can only create a trust line if the issuer does not - // have requireAuth set. + // have lsfRequireAuth set. return tecNO_AUTH; } diff --git a/src/xrpld/app/tx/detail/MPTokenAuthorize.h b/src/xrpld/app/tx/detail/MPTokenAuthorize.h index b8c1b2e91c..ef35e78e76 100644 --- a/src/xrpld/app/tx/detail/MPTokenAuthorize.h +++ b/src/xrpld/app/tx/detail/MPTokenAuthorize.h @@ -26,7 +26,7 @@ namespace ripple { struct MPTAuthorizeArgs { - XRPAmount const& priorBalance{}; + XRPAmount const& priorBalance; uint192 const& mptIssuanceID; AccountID const& accountID; std::uint32_t flags{}; diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.cpp b/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.cpp index a171cfdd3b..8e0d1ef6c9 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.cpp +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.cpp @@ -109,6 +109,9 @@ MPTokenIssuanceCreate::create( if (args.metadata) (*mptIssuance)[sfMPTokenMetadata] = *args.metadata; + if (args.domainId) + (*mptIssuance)[sfDomainID] = *args.domainId; + view.insert(mptIssuance); } diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.h b/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.h index 6e6c197187..a91490e6a2 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.h +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceCreate.h @@ -35,6 +35,7 @@ struct MPTCreateArgs std::optional assetScale{}; std::optional transferFee{}; std::optional const& metadata{}; + std::optional domainId{}; }; class MPTokenIssuanceCreate : public Transactor diff --git a/src/xrpld/app/tx/detail/VaultCreate.cpp b/src/xrpld/app/tx/detail/VaultCreate.cpp index 73f503f7b3..65876d6cb4 100644 --- a/src/xrpld/app/tx/detail/VaultCreate.cpp +++ b/src/xrpld/app/tx/detail/VaultCreate.cpp @@ -146,6 +146,7 @@ VaultCreate::doApply() .sequence = 1, .flags = mptFlags, .metadata = tx[~sfMPTokenMetadata], + .domainId = tx[~sfDomainID], }); if (!maybeShare) return maybeShare.error(); @@ -156,8 +157,6 @@ VaultCreate::doApply() vault->at(sfOwner) = ownerId; vault->at(sfAccount) = pseudoId; vault->at(sfAsset) = tx[sfAsset]; - if (tx.isFieldPresent(sfDomainID)) - vault->setFieldH256(sfDomainID, tx.getFieldH256(sfDomainID)); // Leave default values for AssetTotal and AssetAvailable, both zero. if (auto value = tx[~sfAssetMaximum]) vault->at(sfAssetMaximum) = *value; diff --git a/src/xrpld/app/tx/detail/VaultDeposit.cpp b/src/xrpld/app/tx/detail/VaultDeposit.cpp index 9c6d95c1aa..607f87f095 100644 --- a/src/xrpld/app/tx/detail/VaultDeposit.cpp +++ b/src/xrpld/app/tx/detail/VaultDeposit.cpp @@ -23,7 +23,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -52,21 +54,20 @@ VaultDeposit::preclaim(PreclaimContext const& ctx) if (!vault) return tecOBJECT_NOT_FOUND; - // Only the VaultDeposit transaction is subject to this permission check. - if (vault->getFlags() == tfVaultPrivate && - ctx.tx[sfAccount] != vault->at(sfOwner)) + auto const account = ctx.tx[sfAccount]; + if (vault->getFlags() == tfVaultPrivate && account != vault->at(sfOwner)) { - // Similar to credential::valid call inside Payment::prelaim (different - // overload), if we do not see authorised credentials in preclaim, we do - // not progress to doApply. This means that any expired credentials are - // only deleted *if* we pass this check here in preclaim. - if (auto const domain = vault->at(~sfVaultID)) - { - if (auto const err = - credentials::valid(ctx.view, *domain, ctx.tx[sfAccount]); - !isTesSuccess(err)) - return err; - } + auto const err = requireAuth( + ctx.view, MPTIssue(vault->at(sfMPTokenIssuanceID)), account); + return err; + + // The above will perform authorization check based on DomainID stored + // in MPTokenIssuance. Had this been a regular MPToken, it would also + // allow use of authorization granted by the issuer explicitly, but + // Vault does not have an MPT issuer (instead it uses pseudo-account). + // + // If we passed the above check then we also need to do similar check + // inside doApply(), in order to check for expired credentials. } return tesSUCCESS; @@ -79,17 +80,6 @@ VaultDeposit::doApply() if (!vault) return tecOBJECT_NOT_FOUND; - auto const dst = ctx_.tx[sfAccount]; - auto const src = vault->at(sfOwner); - - if (vault->getFlags() & lsfVaultPrivate) - { - // TODO move DomainID from vault to MPTokenIssuance - if (auto const err = verifyDomain(ctx_, src, dst, vault); - !isTesSuccess(err)) - return err; - } - auto const assets = ctx_.tx[sfAmount]; Asset const& asset = vault->at(sfAsset); if (assets.asset() != asset) @@ -107,30 +97,17 @@ VaultDeposit::doApply() } // Make sure the depositor can hold shares. - auto share = (*vault)[sfMPTokenIssuanceID]; - auto maybeToken = findToken(view(), MPTIssue(share), account_); - if (!maybeToken) - { - if (maybeToken.error() == tecNO_LINE) - { - if (auto ter = MPTokenAuthorize::authorize( - view(), - j_, - {.priorBalance = mPriorBalance, - .mptIssuanceID = share, - .accountID = account_})) - return ter; - } - else if (maybeToken.error() != tesSUCCESS) - { - return maybeToken.error(); - } - } + MPTIssue const mptIssue((*vault)[sfMPTokenIssuanceID]); + if (auto const err = + verifyAuth(ctx_.view(), mptIssue, account_, mPriorBalance, j_); + !isTesSuccess(err)) + return err; // Compute exchange before transferring any amounts. auto const shares = assetsToSharesDeposit(view(), vault, assets); XRPL_ASSERT( - shares.asset() != assets.asset(), "do not mix up assets and shares"); + shares.asset() != assets.asset(), + "ripple::VaultDeposit::doApply : assets are not shares"); vault->at(sfAssetTotal) += assets; vault->at(sfAssetAvailable) += assets; diff --git a/src/xrpld/ledger/View.h b/src/xrpld/ledger/View.h index 7ed40bc33a..a186a66101 100644 --- a/src/xrpld/ledger/View.h +++ b/src/xrpld/ledger/View.h @@ -573,27 +573,43 @@ struct TokenDescriptor std::shared_ptr issuance; }; -[[nodiscard]] Expected -findToken( - ReadView const& view, - MPTIssue const& mptIssue, - AccountID const& account); - [[nodiscard]] TER requireAuth(ReadView const& view, Issue const& issue, AccountID const& account); /** Check if the account lacks required authorization. + * * Return tecNO_AUTH or tecNO_LINE if it does * and tesSUCCESS otherwise. */ [[nodiscard]] TER requireAuth(ReadView const& view, Issue const& issue, AccountID const& account); + +/** Check if the account lacks required authorization. + * + * Does not check for expired credentials, hence must be followed by + * verifyAuth from doApply + */ [[nodiscard]] TER requireAuth( ReadView const& view, MPTIssue const& mptIssue, AccountID const& account); +/** Check if the account lacks required authorization. + * + * Called from doApply - it will check for expired (and delete if found any) + * credentials maching DomainID set in MPTIssuance. Must be called if + * requireAuth(...MPTIssue...) check passed in preclaim. Will create MPToken + * if needed, on the basis of non-expired credentals found. + */ +[[nodiscard]] TER +verifyAuth( + ApplyView& view, + MPTIssue const& mptIssue, + AccountID const& account, + XRPAmount const& priorBalance, + beast::Journal j); + /** Check if the destination account is allowed * to receive MPT. Return tecNO_AUTH if it doesn't * and tesSUCCESS otherwise. diff --git a/src/xrpld/ledger/detail/View.cpp b/src/xrpld/ledger/detail/View.cpp index 8b622e0369..61aaee3761 100644 --- a/src/xrpld/ledger/detail/View.cpp +++ b/src/xrpld/ledger/detail/View.cpp @@ -19,15 +19,20 @@ #include // TODO: Move the helper out of the `app` module. +#include #include #include +#include #include #include #include #include #include +#include +#include #include #include +#include #include #include #include @@ -341,6 +346,8 @@ accountHolds( auto const sleMpt = view.read(keylet::mptoken(mptIssue.getMptID(), account)); + + // TODO check authorization per DomainID stored in MPTokenIssuance if (!sleMpt) amount.clear(mptIssue); else if ( @@ -1197,7 +1204,8 @@ removeEmptyHolding( return MPTokenAuthorize::authorize( view, journal, - {.mptIssuanceID = mptID, + {.priorBalance = {}, + .mptIssuanceID = mptID, .accountID = accountID, .flags = tfMPTUnauthorize}); } @@ -1603,6 +1611,7 @@ rippleCreditMPT( } else return tecNO_AUTH; + // TODO check authorization per DomainID stored in MPTokenIssuance } if (uReceiverID == issuer) @@ -1627,6 +1636,7 @@ rippleCreditMPT( } else return tecNO_AUTH; + // TODO check authorization per DomainID stored in MPTokenIssuanced } return tesSUCCESS; } @@ -2031,29 +2041,42 @@ requireAuth(ReadView const& view, Issue const& issue, AccountID const& account) return tesSUCCESS; } -[[nodiscard]] Expected -findToken( +[[nodiscard]] Expected static findToken( ReadView const& view, MPTIssue const& mptIssue, AccountID const& account) { auto const mptID = keylet::mptIssuance(mptIssue.getMptID()); auto const sleIssuance = view.read(mptID); - if (!sleIssuance) return Unexpected(tecOBJECT_NOT_FOUND); auto const mptIssuer = sleIssuance->getAccountID(sfIssuer); - // Issuer won't have mptoken, i.e. "the operation failed succcessfully" - if (mptIssuer == account) - return Unexpected(tesSUCCESS); + if (mptIssuer == account) // Issuer won't have MPToken + return {TokenDescriptor{.token = nullptr, .issuance = sleIssuance}}; auto const mptokenID = keylet::mptoken(mptID.key, account); auto const sleToken = view.read(mptokenID); - // if account has no MPToken, fail if (!sleToken) - return Unexpected(tecNO_LINE); + { + auto const maybeDomainID = sleIssuance->at(~sfDomainID); + if (!maybeDomainID) + return Unexpected(tecNO_AUTH); + + auto const slePD = + view.read(keylet::permissionedDomain(*maybeDomainID)); + if (!slePD) + return Unexpected(tecOBJECT_NOT_FOUND); + + if (auto const err = credentials::valid(view, *maybeDomainID, account); + !isTesSuccess(err)) + return Unexpected(err); + + // No token but there are credentials matching DomainID, so a token + // could be created & authorized if the required creds are not expired + return {TokenDescriptor{.token = nullptr, .issuance = sleIssuance}}; + } return {TokenDescriptor{.token = sleToken, .issuance = sleIssuance}}; } @@ -2065,28 +2088,108 @@ requireAuth( AccountID const& account) { auto maybeToken = findToken(view, mptIssue, account); - // Whatever reason why we could not find if (!maybeToken) { - // Convert tecNO_LINE to useful error - if (maybeToken.error() == tecNO_LINE) - return tecNO_AUTH; - // Note, error() is tesSUCCESS if no authorization was needed return maybeToken.error(); } + else if (maybeToken->token == nullptr) + { + // Either account is issuer and no authorization is needed, or it has + // credentials maching DomainID stored in MPT issuance. + // Note: the credentials on which basis we return tesSUCCESS might have + // expired; the user must call verifyAuth to check with ApplyView. + return tesSUCCESS; + } auto sleToken = maybeToken->token; auto sleIssuance = maybeToken->issuance; // mptoken must be authorized if issuance enabled requireAuth - if (sleIssuance->getFieldU32(sfFlags) & lsfMPTRequireAuth && + if ((sleIssuance->getFieldU32(sfFlags) & lsfMPTRequireAuth) && !(sleToken->getFlags() & lsfMPTAuthorized)) return tecNO_AUTH; return tesSUCCESS; } +[[nodiscard]] TER +verifyAuth( + ApplyView& view, + MPTIssue const& mptIssue, + AccountID const& account, + XRPAmount const& priorBalance, // for MPToken authorization + beast::Journal j) +{ + auto const mptIssuanceID = mptIssue.getMptID(); + auto const sleIssuance = view.read(keylet::mptIssuance(mptIssuanceID)); + if (!sleIssuance) + return tefINTERNAL; // Should have called requireAuth earlier + + if (account == sleIssuance->at(sfIssuer)) + return tesSUCCESS; // Won't create MPToken for the token issuer + + auto const sleToken = view.read(keylet::mptoken(mptIssuanceID, account)); + bool const domainCheck = + (sleToken && sleToken->getFlags() & lsfMPTDomainCheck); + + bool authorizedByDomain = false; + if (domainCheck || sleToken == nullptr) + { + // We check DomainID if: + // 1. Token not found or + // 2. Token found and has lsfMPTDomainCheck flag + auto const maybeDomainID = sleIssuance->at(~sfDomainID); + authorizedByDomain = maybeDomainID.has_value() && + verifyDomain(view, account, *maybeDomainID, j) == tesSUCCESS; + } + + if (authorizedByDomain && sleToken == nullptr) // && !domainCheck, implied + { + // Create MPToken with the lsfMPTDomainCheck flag set + if (auto const err = MPTokenAuthorize::authorize( + view, + j, + { + .priorBalance = priorBalance, + .mptIssuanceID = mptIssuanceID, + .accountID = account, + .flags = 0, + }); + !isTesSuccess(err)) + return err; + + auto sleMpt = view.peek(keylet::mptoken(mptIssuanceID, account)); + XRPL_ASSERT(sleMpt, "ripple::verifyAuth : found new MPToken"); + std::uint32_t const flags = sleMpt->getFieldU32(sfFlags); + sleMpt->setFieldU32(sfFlags, flags | lsfMPTDomainCheck); + view.update(sleMpt); + + return tesSUCCESS; + } + else if (!authorizedByDomain && domainCheck) // && sleToken, implied + { + // We will only delete MPToken here if it has 0 balance + [[maybe_unused]] auto _ = MPTokenAuthorize::authorize( + view, + j, + { + .priorBalance = priorBalance, + .mptIssuanceID = mptIssuanceID, + .accountID = account, + .flags = tfMPTUnauthorize, + }); + + return tecNO_PERMISSION; + } + else if (authorizedByDomain || sleToken) + { + return tesSUCCESS; + } + + return tecNO_PERMISSION; +} + TER canTransfer( ReadView const& view,