ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-12-06 09:17:57 +00:00
Code Issues Packages Projects Releases Wiki Activity

Improve checks in VaultSet VaultCreate and VaultDeposit

Browse Source
This commit is contained in:
Bronek Kozicki
2025-03-19 16:26:44 +00:00
parent d77fc57c6a
commit 6db2144912
7 changed files with 88 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
include/xrpl/protocol/detail/ledger_entries.macro
View File

@@ -473,7 +473,7 @@ LEDGER_ENTRY(ltVAULT, 0x0083, Vault, vault, ({
{sfOwnerNode, soeREQUIRED},
{sfOwner, soeREQUIRED},
{sfAccount, soeREQUIRED},
{sfData, soeDEFAULT},
{sfData, soeOPTIONAL},
{sfAsset, soeREQUIRED},
{sfAssetTotal, soeDEFAULT},
{sfAssetAvailable, soeDEFAULT},

3
src/test/app/Vault_test.cpp
View File

@@ -117,6 +117,7 @@ class Vault_test : public beast::unit_test::suite
{
testcase(prefix + " fail to update because wrong owner");
auto tx = vault.set({.owner = issuer, .id = keylet.key});
tx[sfAssetMaximum] = asset(50).number();
env(tx, ter(tecNO_PERMISSION));
}
@@ -576,7 +577,7 @@ class Vault_test : public beast::unit_test::suite
testcase("global lock");
mptt.set({.account = issuer, .flags = tfMPTLock});
auto [tx, keylet] = vault.create({.owner = owner, .asset = asset});
env(tx, ter(tecLOCKED));
env(tx, ter(tecFROZEN));
});
testCase([this](

10
src/xrpld/app/tx/detail/VaultCreate.cpp
View File

@@ -62,9 +62,7 @@ VaultCreate::preflight(PreflightContext const& ctx)
if (auto const domain = ctx.tx[~sfDomainID])
{
if (!ctx.rules.enabled(featurePermissionedDomains))
return temDISABLED;
else if (*domain == beast::zero)
if (*domain == beast::zero)
return temMALFORMED;
else if ((ctx.tx.getFlags() & tfVaultPrivate) == 0)
return temMALFORMED; // DomainID only allowed on private vaults
@@ -99,8 +97,6 @@ VaultCreate::preclaim(PreclaimContext const& ctx)
auto issuance = ctx.view.read(keylet::mptIssuance(mptID));
if (!issuance)
return tecNO_ENTRY;
if (issuance->getFlags() & lsfMPTLocked)
return tecLOCKED;
if ((issuance->getFlags() & lsfMPTCanTransfer) == 0)
return tecNO_AUTH;
}
@@ -160,6 +156,10 @@ VaultCreate::doApply()
if (txFlags & tfVaultPrivate)
mptFlags |= lsfMPTRequireAuth;
// Note, here we are **not** creating an MPToken for the assets held in the
// vault. That MPToken or TrustLine/RippleState is created above, in
// addEmptyHolding. Here we are creating MPTokenIssuance for the shares in
// the vault
auto maybeShare = MPTokenIssuanceCreate::create(
view(),
j_,

50
src/xrpld/app/tx/detail/VaultDeposit.cpp
View File

@@ -57,20 +57,25 @@ VaultDeposit::preclaim(PreclaimContext const& ctx)
if (!vault)
return tecOBJECT_NOT_FOUND;
auto account = ctx.tx[sfAccount];
auto const account = ctx.tx[sfAccount];
auto const assets = ctx.tx[sfAmount];
auto asset = vault->at(sfAsset);
if (assets.asset() != asset)
auto const vaultAsset = vault->at(sfAsset);
if (assets.asset() != vaultAsset)
return tecWRONG_ASSET;
// Cannot deposit inside Vault an Asset frozen for the depositor
if (isFrozen(ctx.view, account, asset))
if (isFrozen(ctx.view, account, vaultAsset))
return tecFROZEN;
auto const share = MPTIssue(vault->at(sfMPTokenIssuanceID));
// Cannot deposit if the shares of the vault are frozen
if (isFrozen(ctx.view, account, share))
return tecFROZEN;
// Defensive check, given above `if (asset.asset() != vaultAsset)`
if (share == assets.asset())
return tecWRONG_ASSET;
if ((vault->getFlags() & tfVaultPrivate) && account != vault->at(sfOwner))
{
// The authorization check below is based on DomainID stored in
@@ -78,8 +83,7 @@ VaultDeposit::preclaim(PreclaimContext const& ctx)
// would allow authorization granted by the issuer explicitly, but Vault
// does not have an MPT issuer (instead it uses pseudo-account, which is
// blackholed and cannot create any transactions).
auto const err = requireAuth(
ctx.view, MPTIssue(vault->at(sfMPTokenIssuanceID)), account);
auto const err = requireAuth(ctx.view, share, account);
// As per requireAuth spec, we suppress tecEXPIRED error here, so we can
// delete any expired credentials inside doApply.
@@ -87,6 +91,25 @@ VaultDeposit::preclaim(PreclaimContext const& ctx)
return err;
}
if (assets.holds<MPTIssue>())
{
auto mptID = assets.get<MPTIssue>().getMptID();
auto issuance = ctx.view.read(keylet::mptIssuance(mptID));
if (!issuance)
return tecNO_ENTRY;
if ((issuance->getFlags() & lsfMPTCanTransfer) == 0)
return tecNO_AUTH;
}
if (accountHolds(
ctx.view,
account,
vaultAsset,
FreezeHandling::fhZERO_IF_FROZEN,
AuthHandling::ahZERO_IF_UNAUTHORIZED,
ctx.j) < assets)
return tecINSUFFICIENT_FUNDS;
return tesSUCCESS;
}
@@ -95,21 +118,9 @@ VaultDeposit::doApply()
{
auto const vault = view().peek(keylet::vault(ctx_.tx[sfVaultID]));
if (!vault)
return tecOBJECT_NOT_FOUND;
return tefINTERNAL;
auto const assets = ctx_.tx[sfAmount];
Asset const& asset = vault->at(sfAsset);
if (accountHolds(
view(),
account_,
asset,
FreezeHandling::fhZERO_IF_FROZEN,
AuthHandling::ahZERO_IF_UNAUTHORIZED,
j_) < assets)
{
return tecINSUFFICIENT_FUNDS;
}
// Make sure the depositor can hold shares.
auto const mptIssuanceID = (*vault)[sfMPTokenIssuanceID];
@@ -118,7 +129,6 @@ VaultDeposit::doApply()
return tefINTERNAL;
auto const& vaultAccount = vault->at(sfAccount);
MPTIssue const mptIssue(mptIssuanceID);
// Note, vault owner is always authorized
if ((vault->getFlags() & tfVaultPrivate) && account_ != vault->at(sfOwner))

52
src/xrpld/app/tx/detail/VaultSet.cpp
View File

@@ -20,8 +20,10 @@
#include <xrpld/app/tx/detail/VaultSet.h>
#include <xrpld/ledger/View.h>
#include <xrpl/protocol/Asset.h>
#include <xrpl/protocol/Feature.h>
#include <xrpl/protocol/Indexes.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STNumber.h>
#include <xrpl/protocol/TER.h>
#include <xrpl/protocol/TxFlags.h>
@@ -40,8 +42,10 @@ VaultSet::preflight(PreflightContext const& ctx)
if (auto const ter = preflight1(ctx))
return ter;
if (ctx.tx.getFlags() & tfUniversalMask)
return temINVALID_FLAG;
if (auto const data = ctx.tx[~sfData])
{
if (data->empty() || data->length() > maxDataPayloadLength)
@@ -49,13 +53,13 @@ VaultSet::preflight(PreflightContext const& ctx)
}
auto const domain = ctx.tx[~sfDomainID];
if (domain)
{
if (!ctx.rules.enabled(featurePermissionedDomains))
return temDISABLED;
else if (*domain == beast::zero)
return temMALFORMED;
}
if (domain && *domain == beast::zero)
return temMALFORMED;
if (!ctx.tx.isFieldPresent(sfDomainID) &&
!ctx.tx.isFieldPresent(sfAssetMaximum) &&
!ctx.tx.isFieldPresent(sfData))
return temMALFORMED;
return preflight2(ctx);
}
@@ -63,8 +67,7 @@ VaultSet::preflight(PreflightContext const& ctx)
TER
VaultSet::preclaim(PreclaimContext const& ctx)
{
auto const id = ctx.tx[sfVaultID];
auto const vault = ctx.view.read(keylet::vault(id));
auto const vault = ctx.view.read(keylet::vault(ctx.tx[sfVaultID]));
if (!vault)
return tecOBJECT_NOT_FOUND;
@@ -72,9 +75,29 @@ VaultSet::preclaim(PreclaimContext const& ctx)
if (ctx.tx[sfAccount] != vault->at(sfOwner))
return tecNO_PERMISSION;
// We can only set domain if private flag was originally set
if (auto const domain = ctx.tx[~sfDomainID])
auto const mptIssuanceID = (*vault)[sfMPTokenIssuanceID];
auto const sleIssuance = ctx.view.read(keylet::mptIssuance(mptIssuanceID));
if (!sleIssuance)
return tefINTERNAL;
auto const domain = ctx.tx[~sfDomainID];
auto const oldDomain = sleIssuance->at(~sfDomainID);
auto const data = ctx.tx[~sfData];
auto const oldData = vault->at(~sfData);
auto const assetMax = ctx.tx[~sfAssetMaximum];
auto const oldAssetMax = vault->at(sfAssetMaximum);
int const changes = //
(domain && (domain != oldDomain)) + //
(data && (data != oldData)) + //
(assetMax && (*assetMax != oldAssetMax));
// This transaction would change nothing
if (!changes)
return tecNO_PERMISSION;
if (domain)
{
// We can only set domain if private flag was originally set
if ((vault->getFlags() & tfVaultPrivate) == 0)
return tecINVALID_DOMAIN;
@@ -83,12 +106,7 @@ VaultSet::preclaim(PreclaimContext const& ctx)
if (!sleDomain)
return tecNO_ENTRY;
// Sanity checks, all this should be enforced by VaultCreate
auto const mptIssuanceID = (*vault)[sfMPTokenIssuanceID];
auto const sleIssuance =
ctx.view.read(keylet::mptIssuance(mptIssuanceID));
if (!sleIssuance)
return tefINTERNAL;
// Sanity check only, this should be enforced by VaultCreate
if ((sleIssuance->getFlags() & lsfMPTRequireAuth) == 0)
return tefINTERNAL;
}

2
src/xrpld/ledger/View.h
View File

@@ -160,7 +160,7 @@ isAnyFrozen(
MPTIssue const& mptIssue);
/*
We do not have use a case for these (yet ?)
We do not have a use case for these (yet ?)
[[nodiscard]] bool
isAnyFrozen(

25
src/xrpld/ledger/detail/View.cpp
View File

@@ -302,28 +302,31 @@ isVaultPseudoAccountFrozen(ReadView const& view, MPTIssue const& mptShare)
auto const mptIssuance =
view.read(keylet::mptIssuance(mptShare.getMptID()));
XRPL_ASSERT(
mptIssuance,
"ripple::isVaultPseudoAccountFrozen : non-null MPTokenIssuance");
if (!mptIssuance)
if (mptIssuance == nullptr)
{
UNREACHABLE(
"ripple::isVaultPseudoAccountFrozen : null MPTokenIssuance");
return false;
}
auto const issuer = mptIssuance->getAccountID(sfIssuer);
auto const mptIssuer = view.read(keylet::account(issuer));
XRPL_ASSERT(
mptIssuer,
"ripple::isVaultPseudoAccountFrozen : non-null MPToken issuer");
if (!mptIssuer)
if (mptIssuer == nullptr)
{
UNREACHABLE("ripple::isVaultPseudoAccountFrozen : null MPToken issuer");
return false;
}
if (!mptIssuer->isFieldPresent(sfVaultID))
return false; // not a Vault pseudo-account
return false; // not a Vault pseudo-account, common case
auto const vault =
view.read(keylet::vault(mptIssuer->getFieldH256(sfVaultID)));
XRPL_ASSERT(vault, "ripple::isVaultPseudoAccountFrozen : non-null vault");
if (!vault)
if (vault == nullptr)
{
UNREACHABLE("ripple::isVaultPseudoAccountFrozen : null vault");
return false;
}
Asset const asset{vault->at(sfAsset)};
return isFrozen(view, issuer, asset);