diff --git a/src/xrpld/app/tx/detail/VaultCreate.cpp b/src/xrpld/app/tx/detail/VaultCreate.cpp index aca44a581e..44b4257ed1 100644 --- a/src/xrpld/app/tx/detail/VaultCreate.cpp +++ b/src/xrpld/app/tx/detail/VaultCreate.cpp @@ -97,12 +97,12 @@ VaultCreate::calculateBaseFee(ReadView const& view, STTx const& tx) TER VaultCreate::preclaim(PreclaimContext const& ctx) { - auto asset = ctx.tx[sfAsset]; + auto vaultAsset = ctx.tx[sfAsset]; auto account = ctx.tx[sfAccount]; - if (asset.holds()) + if (vaultAsset.holds()) { - auto mptID = asset.get().getMptID(); + auto mptID = vaultAsset.get().getMptID(); auto issuance = ctx.view.read(keylet::mptIssuance(mptID)); if (!issuance) return tecOBJECT_NOT_FOUND; @@ -113,14 +113,14 @@ VaultCreate::preclaim(PreclaimContext const& ctx) // Check for pseudo-account issuers - we do not want a vault to hold such // assets (e.g. MPT shares to other vaults or AMM LPTokens) as they would be // impossible to clawback (should the need arise) - if (!asset.native()) + if (!vaultAsset.native()) { - if (isPseudoAccount(ctx.view, asset.getIssuer())) + if (isPseudoAccount(ctx.view, vaultAsset.getIssuer())) return tecWRONG_ASSET; } // Cannot create Vault for an Asset frozen for the vault owner - if (isFrozen(ctx.view, account, asset)) + if (isFrozen(ctx.view, account, vaultAsset)) return tecFROZEN; if (auto const domain = ctx.tx[~sfDomainID]) diff --git a/src/xrpld/app/tx/detail/VaultDeposit.cpp b/src/xrpld/app/tx/detail/VaultDeposit.cpp index 93b41df4df..433d6d2f69 100644 --- a/src/xrpld/app/tx/detail/VaultDeposit.cpp +++ b/src/xrpld/app/tx/detail/VaultDeposit.cpp @@ -67,22 +67,22 @@ VaultDeposit::preclaim(PreclaimContext const& ctx) return tecWRONG_ASSET; auto const mptIssuanceID = vault->at(sfShareMPTID); - auto const share = MPTIssue(mptIssuanceID); - if (share == assets.asset()) - return tefINTERNAL; + auto const vaultShare = MPTIssue(mptIssuanceID); + if (vaultShare == assets.asset()) + return tefINTERNAL; // LCOV_EXCL_LINE auto const sleIssuance = ctx.view.read(keylet::mptIssuance(mptIssuanceID)); if (!sleIssuance) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE if (sleIssuance->isFlag(lsfMPTLocked)) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE // Cannot deposit inside Vault an Asset frozen for the depositor if (isFrozen(ctx.view, account, vaultAsset)) return vaultAsset.holds() ? tecFROZEN : tecLOCKED; // Cannot deposit if the shares of the vault are frozen - if (isFrozen(ctx.view, account, share)) + if (isFrozen(ctx.view, account, vaultShare)) return tecLOCKED; if (vault->isFlag(tfVaultPrivate) && account != vault->at(sfOwner)) @@ -106,24 +106,11 @@ VaultDeposit::preclaim(PreclaimContext const& ctx) return tecNO_AUTH; } - if (auto const ter = std::visit( - [&](TIss const& issue) -> TER { - return requireAuth(ctx.view, issue, account); - }, - vaultAsset.value()); + // Source MPToken must exist (if asset is an MPT) + if (auto const ter = requireAuth(ctx.view, vaultAsset, account); !isTesSuccess(ter)) return ter; - if (assets.holds()) - { - auto mptID = assets.get().getMptID(); - auto issuance = ctx.view.read(keylet::mptIssuance(mptID)); - if (!issuance) - return tecOBJECT_NOT_FOUND; - if ((issuance->getFlags() & lsfMPTCanTransfer) == 0) - return tecNO_AUTH; - } - if (accountHolds( ctx.view, account, diff --git a/src/xrpld/app/tx/detail/VaultWithdraw.cpp b/src/xrpld/app/tx/detail/VaultWithdraw.cpp index 7e21141b84..5bc0da7e73 100644 --- a/src/xrpld/app/tx/detail/VaultWithdraw.cpp +++ b/src/xrpld/app/tx/detail/VaultWithdraw.cpp @@ -63,9 +63,9 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx) return tecNO_ENTRY; auto const assets = ctx.tx[sfAmount]; - auto const asset = vault->at(sfAsset); - auto const share = vault->at(sfShareMPTID); - if (assets.asset() != asset && assets.asset() != share) + auto const vaultAsset = vault->at(sfAsset); + auto const vaultShare = vault->at(sfShareMPTID); + if (assets.asset() != vaultAsset && assets.asset() != vaultShare) return tecWRONG_ASSET; // Enforce valid withdrawal policy @@ -95,29 +95,18 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx) if (!ctx.view.exists(keylet::depositPreauth(dstAcct, account))) return tecNO_PERMISSION; } - - if (assets.holds()) - { - if (auto const ter = canTransfer( - ctx.view, assets.get(), account, dstAcct); - !isTesSuccess(ter)) - return ter; - } } - if (auto const ter = std::visit( - [&](TIss const& issue) -> TER { - return requireAuth(ctx.view, issue, dstAcct); - }, - asset.value()); + // Destination MPToken must exist (if asset is an MPT) + if (auto const ter = requireAuth(ctx.view, vaultAsset, dstAcct); !isTesSuccess(ter)) return ter; // Cannot withdraw from a Vault an Asset frozen for the destination account - if (isFrozen(ctx.view, dstAcct, asset)) - return asset.holds() ? tecFROZEN : tecLOCKED; + if (isFrozen(ctx.view, dstAcct, vaultAsset)) + return vaultAsset.holds() ? tecFROZEN : tecLOCKED; - if (isFrozen(ctx.view, account, share)) + if (isFrozen(ctx.view, account, vaultShare)) return tecLOCKED; return tesSUCCESS; @@ -128,12 +117,12 @@ VaultWithdraw::doApply() { auto const vault = view().peek(keylet::vault(ctx_.tx[sfVaultID])); if (!vault) - return tefINTERNAL; // Enforced in preclaim + return tefINTERNAL; // LCOV_EXCL_LINE auto const mptIssuanceID = (*vault)[sfShareMPTID]; auto const sleIssuance = view().read(keylet::mptIssuance(mptIssuanceID)); if (!sleIssuance) - return tefINTERNAL; + return tefINTERNAL; // LCOV_EXCL_LINE // Note, we intentionally do not check lsfVaultPrivate flag on the Vault. If // you have a share in the vault, it means you were at some point authorized diff --git a/src/xrpld/ledger/View.h b/src/xrpld/ledger/View.h index bd2c5f05a9..2044357898 100644 --- a/src/xrpld/ledger/View.h +++ b/src/xrpld/ledger/View.h @@ -701,6 +701,12 @@ transferXRP( STAmount const& amount, beast::Journal j); +/* Check if MPToken exists: + * - StrongAuth - before checking lsfMPTRequireAuth is set + * - WeakAuth - after checking if lsfMPTRequireAuth is set + */ +enum class MPTAuthType : bool { StrongAuth = true, WeakAuth = false }; + /** Check if the account lacks required authorization. * * Return tecNO_AUTH or tecNO_LINE if it does @@ -711,22 +717,46 @@ requireAuth(ReadView const& view, Issue const& issue, AccountID const& account); /** Check if the account lacks required authorization. * - * This will also check for expired credentials. If it is called directly - * from preclaim, the user should convert result tecEXPIRED to tesSUCCESS and - * proceed to also check permissions with enforceMPTokenAuthorization inside - * doApply. This will ensure that any expired credentials are deleted. + * This will also check for expired credentials. If it is called directly + * from preclaim, the user should convert result tecEXPIRED to tesSUCCESS and + * proceed to also check permissions with enforceMPTokenAuthorization inside + * doApply. This will ensure that any expired credentials are deleted. * - * requireAuth check is recursive for MPT shares in a vault, descending to - * assets in the vault, up to maxAssetCheckDepth recursion depth. This is - * purely defensive, as we currently do not allow such vaults to be created. + * requireAuth check is recursive for MPT shares in a vault, descending to + * assets in the vault, up to maxAssetCheckDepth recursion depth. This is + * purely defensive, as we currently do not allow such vaults to be created. + * + * If StrongAuth then return tecNO_AUTH if MPToken doesn't exist or + * lsfMPTRequireAuth is set and MPToken is not authorized. If WeakAuth then + * return tecNO_AUTH if lsfMPTRequireAuth is set and MPToken doesn't exist or is + * not authorized (explicitly or via credentials, if DomainID is set in + * MPTokenIssuance). Consequently, if WeakAuth and lsfMPTRequireAuth is *not* + * set, this function will return true even if MPToken does *not* exist. */ [[nodiscard]] TER requireAuth( ReadView const& view, MPTIssue const& mptIssue, AccountID const& account, + MPTAuthType authType = MPTAuthType::StrongAuth, int depth = 0); +[[nodiscard]] TER inline requireAuth( + ReadView const& view, + Asset const& asset, + AccountID const& account, + MPTAuthType authType = MPTAuthType::StrongAuth) +{ + return std::visit( + [&](TIss const& issue_) { + if constexpr (std::is_same_v) + return requireAuth(view, issue_, account); + else + return requireAuth(view, issue_, account, authType); + }, + asset.value()); +} + /** Enforce account has MPToken to match its authorization. * * Called from doApply - it will check for expired (and delete if found any) @@ -767,7 +797,8 @@ canTransfer( ReadView const& view, MPTIssue const& mptIssue, AccountID const& from, - AccountID const& to); + AccountID const& to, + int depth = 0); /** Deleter function prototype. Returns the status of the entry deletion * (if should not be skipped) and if the entry should be skipped. The status diff --git a/src/xrpld/ledger/detail/View.cpp b/src/xrpld/ledger/detail/View.cpp index c1062d056c..630c1ec10a 100644 --- a/src/xrpld/ledger/detail/View.cpp +++ b/src/xrpld/ledger/detail/View.cpp @@ -501,7 +501,8 @@ accountHolds( if (zeroIfUnauthorized == ahZERO_IF_UNAUTHORIZED && view.rules().enabled(featureSingleAssetVault)) { - if (auto const err = requireAuth(view, mptIssue, account); + if (auto const err = requireAuth( + view, mptIssue, account, MPTAuthType::StrongAuth); !isTesSuccess(err)) amount.clear(mptIssue); } @@ -2278,6 +2279,7 @@ requireAuth( ReadView const& view, MPTIssue const& mptIssue, AccountID const& account, + MPTAuthType authType, int depth) { auto const mptID = keylet::mptIssuance(mptIssue.getMptID()); @@ -2314,7 +2316,8 @@ requireAuth( if constexpr (std::is_same_v) return requireAuth(view, issue, account); else - return requireAuth(view, issue, account, depth + 1); + return requireAuth( + view, issue, account, authType, depth + 1); }, asset.value()); !isTesSuccess(err)) @@ -2324,6 +2327,11 @@ requireAuth( auto const mptokenID = keylet::mptoken(mptID.key, account); auto const sleToken = view.read(mptokenID); + + // if account has no MPToken, fail + if (!sleToken && authType == MPTAuthType::StrongAuth) + return tecNO_AUTH; + // Note, this check is not amendment-gated because DomainID will be always // empty **unless** writing to it has been enabled by an amendment auto const maybeDomainID = sleIssuance->at(~sfDomainID); @@ -2331,27 +2339,24 @@ requireAuth( { XRPL_ASSERT( sleIssuance->getFieldU32(sfFlags) & lsfMPTRequireAuth, - "ripple::requireAuth issuance requires authorization"); - // We ignore error from validDomain if we found sleToken, as it could - // belong to someone who is explicitly authorized e.g. a vault owner. - // err = tefINTERNAL | tecOBJECT_NOT_FOUND | tecNO_AUTH | tecEXPIRED + "ripple::requireAuth : issuance requires authorization"); + // ter = tefINTERNAL | tecOBJECT_NOT_FOUND | tecNO_AUTH | tecEXPIRED if (auto const ter = credentials::validDomain(view, *maybeDomainID, account); - isTesSuccess(ter) || sleToken == nullptr) + isTesSuccess(ter)) + return ter; // Note: sleToken might be null + else if (!sleToken) return ter; + // We ignore error from validDomain if we found sleToken, as it could + // belong to someone who is explicitly authorized e.g. a vault owner. } - // if account has no MPToken, fail - if (!sleToken) - return tecNO_AUTH; - // mptoken must be authorized if issuance enabled requireAuth if (sleIssuance->isFlag(lsfMPTRequireAuth) && - !(sleToken->isFlag(lsfMPTAuthorized))) + (!sleToken || (!(sleToken->getFlags() & lsfMPTAuthorized)))) return tecNO_AUTH; - // We are authorized by permissioned domain. - return tesSUCCESS; + return tesSUCCESS; // Note: sleToken might be null } [[nodiscard]] TER @@ -2450,7 +2455,8 @@ canTransfer( ReadView const& view, MPTIssue const& mptIssue, AccountID const& from, - AccountID const& to) + AccountID const& to, + int depth) { auto const mptID = keylet::mptIssuance(mptIssue.getMptID()); auto const sleIssuance = view.read(mptID);