ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-11-18 18:15:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

Catch up the consequences of Number changes

Browse Source 
- Change the Number::maxIntValue to all 9's.
- Add integral() to Asset (copied from Lending)
- Add toNumber() functions to STAmount, MPTAmount, XRPAmount to allow
  explicit conversions with enforcement options.
- Add optional Number::EnforceInteger options to STAmount and STNumber
  ctors, conversions, etc. IOUs are never checked.
- Update Vault transactors, and helper functions, to check restrictions.
- Fix and add Vault tests.
This commit is contained in:
Ed Hennis
2025-11-05 18:15:49 -05:00
parent cb6df196dc
commit 0175dd70db
15 changed files with 274 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
include/xrpl/basics/Number.h
View File

@@ -53,7 +53,8 @@ public:
constexpr static rep maxMantissa = minMantissa * 10 - 1;
static_assert(maxMantissa == 9'999'999'999'999'999LL);
constexpr static rep maxIntValue = minMantissa / 10;
constexpr static rep maxIntValue = maxMantissa / 10;
static_assert(maxIntValue == 999'999'999'999'999LL);
// The range for the exponent when normalized
constexpr static int minExponent = -32768;

6
include/xrpl/protocol/Asset.h
View File

@@ -84,6 +84,12 @@ public:
return holds<Issue>() && get<Issue>().native();
}
bool
integral() const
{
return !holds<Issue>() || get<Issue>().native();
}
friend constexpr bool
operator==(Asset const& lhs, Asset const& rhs);

8
include/xrpl/protocol/MPTAmount.h
View File

@@ -62,11 +62,17 @@ public:
explicit constexpr
operator bool() const noexcept;
operator Number() const noexcept
operator Number() const
{
return {value(), Number::strong};
}
Number
toNumber(Number::EnforceInteger enforce) const
{
return {value(), enforce};
}
/** Return the sign of the amount */
constexpr int
signum() const noexcept;

67
include/xrpl/protocol/STAmount.h
View File

@@ -40,6 +40,12 @@ private:
exponent_type mOffset;
bool mIsNegative;
// The Enforce integer setting is not stored or serialized. If set, it is
// used during automatic conversions to Number. If not set, the default
// behavior is used. It can also be overridden when coverting by using
// toNumber().
std::optional<Number::EnforceInteger> enforceConversion_;
public:
using value_type = STAmount;
@@ -135,9 +141,28 @@ public:
STAmount(A const& asset, int mantissa, int exponent = 0);
template <AssetType A>
STAmount(A const& asset, Number const& number)
STAmount(
A const& asset,
Number const& number,
std::optional<Number::EnforceInteger> enforce = std::nullopt)
: STAmount(asset, number.mantissa(), number.exponent())
{
enforceConversion_ = enforce;
if (!enforce)
{
// Use the default conversion behavior
[[maybe_unused]]
Number const n = *this;
}
else if (enforce == Number::strong)
{
// Throw if it's not valid
if (!validNumber())
{
Throw<std::overflow_error>(
"STAmount::STAmount integer Number lost precision");
}
}
}
// Legacy support for new-style amounts
@@ -145,6 +170,17 @@ public:
STAmount(XRPAmount const& amount);
STAmount(MPTAmount const& amount, MPTIssue const& mptIssue);
operator Number() const;
Number
toNumber(Number::EnforceInteger enforce) const;
void
setIntegerEnforcement(std::optional<Number::EnforceInteger> enforce);
std::optional<Number::EnforceInteger>
integerEnforcement() const noexcept;
bool
validNumber() const noexcept;
//--------------------------------------------------------------------------
//
@@ -155,6 +191,9 @@ public:
int
exponent() const noexcept;
bool
integral() const noexcept;
bool
native() const noexcept;
@@ -435,6 +474,12 @@ STAmount::exponent() const noexcept
return mOffset;
}
inline bool
STAmount::integral() const noexcept
{
return mAsset.integral();
}
inline bool
STAmount::native() const noexcept
{
@@ -510,6 +555,8 @@ inline STAmount::operator bool() const noexcept
inline STAmount::operator Number() const
{
if (enforceConversion_)
return toNumber(*enforceConversion_);
if (native())
return xrp();
if (mAsset.holds<MPTIssue>())
@@ -517,6 +564,17 @@ inline STAmount::operator Number() const
return iou();
}
inline Number
STAmount::toNumber(Number::EnforceInteger enforce) const
{
if (native())
return xrp().toNumber(enforce);
if (mAsset.holds<MPTIssue>())
return mpt().toNumber(enforce);
// It doesn't make sense to enforce limits on IOUs
return iou();
}
inline STAmount&
STAmount::operator=(beast::Zero)
{
@@ -538,6 +596,11 @@ STAmount::operator=(Number const& number)
mValue = mIsNegative ? -number.mantissa() : number.mantissa();
mOffset = number.exponent();
canonicalize();
// Convert it back to a Number to check that it's valid
[[maybe_unused]]
Number n = *this;
return *this;
}
@@ -553,7 +616,7 @@ STAmount::clear()
{
// The -100 is used to allow 0 to sort less than a small positive values
// which have a negative exponent.
mOffset = native() ? 0 : -100;
mOffset = integral() ? 0 : -100;
mValue = 0;
mIsNegative = false;
}

12
include/xrpl/protocol/STNumber.h
View File

@@ -56,6 +56,18 @@ public:
bool
isDefault() const override;
/// Sets the flag on the underlying number
void
setIntegerEnforcement(Number::EnforceInteger enforce);
/// Gets the flag value on the underlying number
Number::EnforceInteger
integerEnforcement() const noexcept;
/// Checks the underlying number
bool
valid() const noexcept;
operator Number() const
{
return value_;

1
include/xrpl/protocol/SystemParameters.h
View File

@@ -23,6 +23,7 @@ systemName()
/** Number of drops in the genesis account. */
constexpr XRPAmount INITIAL_XRP{100'000'000'000 * DROPS_PER_XRP};
static_assert(INITIAL_XRP.drops() == 100'000'000'000'000'000);
/** Returns true if the amount does not exceed the initial XRP in existence. */
inline bool

6
include/xrpl/protocol/XRPAmount.h
View File

@@ -146,6 +146,12 @@ public:
return {drops(), Number::weak};
}
Number
toNumber(Number::EnforceInteger enforce) const
{
return {value(), enforce};
}
/** Return the sign of the amount */
constexpr int
signum() const noexcept

23
src/libxrpl/ledger/View.cpp
View File

@@ -2878,13 +2878,17 @@ assetsToSharesDeposit(
Number const assetTotal = vault->at(sfAssetsTotal);
STAmount shares{vault->at(sfShareMPTID)};
shares.setIntegerEnforcement(Number::weak);
if (assetTotal == 0)
return STAmount{
shares.asset(),
Number(assets.mantissa(), assets.exponent() + vault->at(sfScale))
.truncate()};
.truncate(),
Number::weak};
Number const shareTotal = issuance->at(sfOutstandingAmount);
Number const shareTotal{
unsafe_cast<std::int64_t>(issuance->at(sfOutstandingAmount)),
Number::strong};
shares = (shareTotal * (assets / assetTotal)).truncate();
return shares;
}
@@ -2906,6 +2910,7 @@ sharesToAssetsDeposit(
Number const assetTotal = vault->at(sfAssetsTotal);
STAmount assets{vault->at(sfAsset)};
assets.setIntegerEnforcement(Number::weak);
if (assetTotal == 0)
return STAmount{
assets.asset(),
@@ -2913,7 +2918,9 @@ sharesToAssetsDeposit(
shares.exponent() - vault->at(sfScale),
false};
Number const shareTotal = issuance->at(sfOutstandingAmount);
Number const shareTotal{
unsafe_cast<std::int64_t>(issuance->at(sfOutstandingAmount)),
Number::strong};
assets = assetTotal * (shares / shareTotal);
return assets;
}
@@ -2937,9 +2944,12 @@ assetsToSharesWithdraw(
Number assetTotal = vault->at(sfAssetsTotal);
assetTotal -= vault->at(sfLossUnrealized);
STAmount shares{vault->at(sfShareMPTID)};
shares.setIntegerEnforcement(Number::weak);
if (assetTotal == 0)
return shares;
Number const shareTotal = issuance->at(sfOutstandingAmount);
Number const shareTotal{
unsafe_cast<std::int64_t>(issuance->at(sfOutstandingAmount)),
Number::strong};
Number result = shareTotal * (assets / assetTotal);
if (truncate == TruncateShares::yes)
result = result.truncate();
@@ -2965,9 +2975,12 @@ sharesToAssetsWithdraw(
Number assetTotal = vault->at(sfAssetsTotal);
assetTotal -= vault->at(sfLossUnrealized);
STAmount assets{vault->at(sfAsset)};
assets.setIntegerEnforcement(Number::weak);
if (assetTotal == 0)
return assets;
Number const shareTotal = issuance->at(sfOutstandingAmount);
Number const shareTotal{
unsafe_cast<std::int64_t>(issuance->at(sfOutstandingAmount)),
Number::strong};
assets = assetTotal * (shares / shareTotal);
return assets;
}

19
src/libxrpl/protocol/STAmount.cpp
View File

@@ -255,6 +255,25 @@ STAmount::move(std::size_t n, void* buf)
return emplace(n, buf, std::move(*this));
}
void
STAmount::setIntegerEnforcement(std::optional<Number::EnforceInteger> enforce)
{
enforceConversion_ = enforce;
}
std::optional<Number::EnforceInteger>
STAmount::integerEnforcement() const noexcept
{
return enforceConversion_;
}
bool
STAmount::validNumber() const noexcept
{
Number n = toNumber(Number::EnforceInteger::weak);
return n.valid();
}
//------------------------------------------------------------------------------
//
// Conversion

18
src/libxrpl/protocol/STNumber.cpp
View File

@@ -94,6 +94,24 @@ STNumber::isDefault() const
return value_ == Number();
}
void
STNumber::setIntegerEnforcement(Number::EnforceInteger enforce)
{
value_.setIntegerEnforcement(enforce);
}
Number::EnforceInteger
STNumber::integerEnforcement() const noexcept
{
return value_.integerEnforcement();
}
bool
STNumber::valid() const noexcept
{
return value_.valid();
}
std::ostream&
operator<<(std::ostream& out, STNumber const& rhs)
{

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

@@ -3597,7 +3597,32 @@ class Vault_test : public beast::unit_test::suite
});
testCase(18, [&, this](Env& env, Data d) {
testcase("Scale deposit overflow on second deposit");
testcase("MPT scale deposit overflow");
// The computed number of shares can not be represented as an MPT
// without truncation
{
auto tx = d.vault.deposit(
{.depositor = d.depositor,
.id = d.keylet.key,
.amount = d.asset(5)});
env(tx, ter{tecPRECISION_LOSS});
env.close();
}
});
testCase(14, [&, this](Env& env, Data d) {
testcase("MPT scale deposit overflow on first deposit");
auto tx = d.vault.deposit(
{.depositor = d.depositor,
.id = d.keylet.key,
.amount = d.asset(10)});
env(tx, ter{tecPRECISION_LOSS});
env.close();
});
testCase(14, [&, this](Env& env, Data d) {
testcase("MPT scale deposit overflow on second deposit");
{
auto tx = d.vault.deposit(
@@ -3618,8 +3643,8 @@ class Vault_test : public beast::unit_test::suite
}
});
testCase(18, [&, this](Env& env, Data d) {
testcase("Scale deposit overflow on total shares");
testCase(14, [&, this](Env& env, Data d) {
testcase("No MPT scale deposit overflow on total shares");
{
auto tx = d.vault.deposit(
@@ -3635,7 +3660,7 @@ class Vault_test : public beast::unit_test::suite
{.depositor = d.depositor,
.id = d.keylet.key,
.amount = d.asset(5)});
env(tx, ter{tecPATH_DRY});
env(tx);
env.close();
}
});
@@ -3919,6 +3944,28 @@ class Vault_test : public beast::unit_test::suite
testCase(18, [&, this](Env& env, Data d) {
testcase("Scale withdraw overflow");
{
auto tx = d.vault.deposit(
{.depositor = d.depositor,
.id = d.keylet.key,
.amount = d.asset(5)});
env(tx, ter{tecPRECISION_LOSS});
env.close();
}
{
auto tx = d.vault.withdraw(
{.depositor = d.depositor,
.id = d.keylet.key,
.amount = STAmount(d.asset, Number(10, 0))});
env(tx, ter{tecPRECISION_LOSS});
env.close();
}
});
testCase(14, [&, this](Env& env, Data d) {
testcase("MPT scale withdraw overflow");
{
auto tx = d.vault.deposit(
{.depositor = d.depositor,
@@ -4137,6 +4184,29 @@ class Vault_test : public beast::unit_test::suite
testCase(18, [&, this](Env& env, Data d) {
testcase("Scale clawback overflow");
{
auto tx = d.vault.deposit(
{.depositor = d.depositor,
.id = d.keylet.key,
.amount = d.asset(5)});
env(tx, ter(tecPRECISION_LOSS));
env.close();
}
{
auto tx = d.vault.clawback(
{.issuer = d.issuer,
.id = d.keylet.key,
.holder = d.depositor,
.amount = STAmount(d.asset, Number(10, 0))});
env(tx, ter{tecPRECISION_LOSS});
env.close();
}
});
testCase(14, [&, this](Env& env, Data d) {
testcase("MPT Scale clawback overflow");
{
auto tx = d.vault.deposit(
{.depositor = d.depositor,

4
src/test/basics/Number_test.cpp
View File

@@ -858,7 +858,7 @@ public:
{
BEAST_EXPECT(e.what() == "Number::operator= integer overflow"s);
// The throw is done _after_ the number is updated.
BEAST_EXPECT((a == Number{2, 14}));
BEAST_EXPECT((a == Number::maxIntValue * 2));
BEAST_EXPECT(!a.valid());
}
try
@@ -870,7 +870,7 @@ public:
{
BEAST_EXPECT(e.what() == "Number::Number integer overflow"s);
// The Number doesn't get updated because the ctor throws
BEAST_EXPECT((a == Number{2, 14}));
BEAST_EXPECT((a == Number::maxIntValue * 2));
BEAST_EXPECT(!a.valid());
}
a = Number(1, 10);

19
src/xrpld/app/tx/detail/VaultClawback.cpp
View File

@@ -71,9 +71,13 @@ VaultClawback::preclaim(PreclaimContext const& ctx)
}
Asset const vaultAsset = vault->at(sfAsset);
if (auto const amount = ctx.tx[~sfAmount];
amount && vaultAsset != amount->asset())
return tecWRONG_ASSET;
if (auto const amount = ctx.tx[~sfAmount])
{
if (vaultAsset != amount->asset())
return tecWRONG_ASSET;
else if (!amount->validNumber())
return tecPRECISION_LOSS;
}
if (vaultAsset.native())
{
@@ -157,6 +161,8 @@ VaultClawback::doApply()
MPTIssue const share{mptIssuanceID};
STAmount sharesDestroyed = {share};
STAmount assetsRecovered;
assetsRecovered.setIntegerEnforcement(Number::weak);
sharesDestroyed.setIntegerEnforcement(Number::weak);
try
{
if (amount == beast::zero)
@@ -169,6 +175,9 @@ VaultClawback::doApply()
AuthHandling::ahIGNORE_AUTH,
j_);
if (!sharesDestroyed.validNumber())
return tecPRECISION_LOSS;
auto const maybeAssets =
sharesToAssetsWithdraw(vault, sleIssuance, sharesDestroyed);
if (!maybeAssets)
@@ -184,6 +193,8 @@ VaultClawback::doApply()
if (!maybeShares)
return tecINTERNAL; // LCOV_EXCL_LINE
sharesDestroyed = *maybeShares;
if (!sharesDestroyed.validNumber())
return tecPRECISION_LOSS;
}
auto const maybeAssets =
@@ -192,6 +203,8 @@ VaultClawback::doApply()
return tecINTERNAL; // LCOV_EXCL_LINE
assetsRecovered = *maybeAssets;
}
if (!assetsRecovered.validNumber())
return tecPRECISION_LOSS;
// Clamp to maximum.
if (assetsRecovered > *assetsAvailable)

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

@@ -42,6 +42,9 @@ VaultDeposit::preclaim(PreclaimContext const& ctx)
if (assets.asset() != vaultAsset)
return tecWRONG_ASSET;
if (!assets.validNumber())
return tecPRECISION_LOSS;
if (vaultAsset.native())
; // No special checks for XRP
else if (vaultAsset.holds<MPTIssue>())
@@ -217,6 +220,7 @@ VaultDeposit::doApply()
}
STAmount sharesCreated = {vault->at(sfShareMPTID)}, assetsDeposited;
sharesCreated.setIntegerEnforcement(Number::weak);
try
{
// Compute exchange before transferring any amounts.
@@ -227,14 +231,14 @@ VaultDeposit::doApply()
return tecINTERNAL; // LCOV_EXCL_LINE
sharesCreated = *maybeShares;
}
if (sharesCreated == beast::zero)
if (sharesCreated == beast::zero || !sharesCreated.validNumber())
return tecPRECISION_LOSS;
auto const maybeAssets =
sharesToAssetsDeposit(vault, sleIssuance, sharesCreated);
if (!maybeAssets)
return tecINTERNAL; // LCOV_EXCL_LINE
else if (*maybeAssets > amount)
else if (*maybeAssets > amount || !maybeAssets->validNumber())
{
// LCOV_EXCL_START
JLOG(j_.error()) << "VaultDeposit: would take more than offered.";
@@ -260,13 +264,22 @@ VaultDeposit::doApply()
sharesCreated.asset() != assetsDeposited.asset(),
"ripple::VaultDeposit::doApply : assets are not shares");
vault->at(sfAssetsTotal) += assetsDeposited;
vault->at(sfAssetsAvailable) += assetsDeposited;
auto assetsTotalProxy = vault->at(sfAssetsTotal);
auto assetsAvailableProxy = vault->at(sfAssetsAvailable);
if (vault->at(sfAsset).value().integral())
{
assetsTotalProxy.value().setIntegerEnforcement(Number::weak);
assetsAvailableProxy.value().setIntegerEnforcement(Number::weak);
}
assetsTotalProxy += assetsDeposited;
assetsAvailableProxy += assetsDeposited;
if (!assetsTotalProxy->valid() || !assetsAvailableProxy->valid())
return tecLIMIT_EXCEEDED;
view().update(vault);
// A deposit must not push the vault over its limit.
auto const maximum = *vault->at(sfAssetsMaximum);
if (maximum != 0 && *vault->at(sfAssetsTotal) > maximum)
if (maximum != 0 && *assetsTotalProxy > maximum)
return tecLIMIT_EXCEEDED;
// Transfer assets from depositor to vault.

11
src/xrpld/app/tx/detail/VaultWithdraw.cpp
View File

@@ -50,6 +50,9 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx)
if (assets.asset() != vaultAsset && assets.asset() != vaultShare)
return tecWRONG_ASSET;
if (!assets.validNumber())
return tecPRECISION_LOSS;
if (vaultAsset.native())
; // No special checks for XRP
else if (vaultAsset.holds<MPTIssue>())
@@ -154,6 +157,8 @@ VaultWithdraw::doApply()
MPTIssue const share{mptIssuanceID};
STAmount sharesRedeemed = {share};
STAmount assetsWithdrawn;
assetsWithdrawn.setIntegerEnforcement(Number::weak);
sharesRedeemed.setIntegerEnforcement(Number::weak);
try
{
if (amount.asset() == vaultAsset)
@@ -167,13 +172,15 @@ VaultWithdraw::doApply()
sharesRedeemed = *maybeShares;
}
if (sharesRedeemed == beast::zero)
if (sharesRedeemed == beast::zero || !sharesRedeemed.validNumber())
return tecPRECISION_LOSS;
auto const maybeAssets =
sharesToAssetsWithdraw(vault, sleIssuance, sharesRedeemed);
if (!maybeAssets)
return tecINTERNAL; // LCOV_EXCL_LINE
assetsWithdrawn = *maybeAssets;
if (!assetsWithdrawn.validNumber())
return tecPRECISION_LOSS;
}
else if (amount.asset() == share)
{
@@ -184,6 +191,8 @@ VaultWithdraw::doApply()
if (!maybeAssets)
return tecINTERNAL; // LCOV_EXCL_LINE
assetsWithdrawn = *maybeAssets;
if (!assetsWithdrawn.validNumber())
return tecPRECISION_LOSS;
}
else
return tefINTERNAL; // LCOV_EXCL_LINE