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

Revert "Add a distinction between a "valid" and a "representable" Number"

Browse Source 
This reverts commit 8e56af20ee.
This commit is contained in:
Ed Hennis
2025-11-12 19:14:54 -05:00
parent b7ed35cb04
commit 2e34506835
13 changed files with 19 additions and 185 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -28,17 +28,12 @@ public:
/** Describes whether and how to enforce this number as an integer.
*
* - none: No enforcement. The value may vary freely. This is the default.
* - compatible: If the absolute value is greater than maxIntValue, valid()
* will return false. Needed for backward compatibility with XRP used in
* AMMs, and available for functions that will do their own checking. This
* is the default for automatic conversions from XRPAmount to Number.
* - weak: Like compatible, plus, if the value is unrepresentable (larger
* than maxMantissa), assignment and other operations will throw.
* - strong: Like weak, plus, if the absolute value is invalid (larger than
* maxIntValue), assignment and other operations will throw. This is the
* defalut for automatic conversions from MPTAmount to Number.
* - weak: If the absolute value is greater than maxIntValue, valid() will
* return false.
* - strong: Assignment operations will throw if the absolute value is above
* maxIntValue.
*/
enum EnforceInteger { none, compatible, weak, strong };
enum EnforceInteger { none, weak, strong };
private:
using rep = std::int64_t;
@@ -47,7 +42,8 @@ private:
// The enforcement setting is not serialized, and does not affect the
// ledger. If not "none", the value is checked to be within the valid
// integer range. See the enum description for more detail.
// integer range. With "strong", the checks will be made as automatic as
// possible.
EnforceInteger enforceInteger_ = none;
public:
@@ -57,8 +53,8 @@ public:
constexpr static rep maxMantissa = minMantissa * 10 - 1;
static_assert(maxMantissa == 9'999'999'999'999'999LL);
constexpr static rep maxIntValue = maxMantissa / 100;
static_assert(maxIntValue == 99'999'999'999'999LL);
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;
@@ -97,15 +93,6 @@ public:
bool
valid() const noexcept;
bool
representable() const noexcept;
/// Combines setIntegerEnforcement(EnforceInteger) and valid()
bool
valid(EnforceInteger enforce);
/// Because this function is const, it should only be used for one-off
/// checks
bool
valid(EnforceInteger enforce) const;
constexpr Number
operator+() const noexcept;

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

@@ -143,7 +143,7 @@ public:
operator Number() const noexcept
{
return {drops(), Number::compatible};
return {drops(), Number::weak};
}
Number

32
src/libxrpl/basics/Number.cpp
View File

@@ -227,8 +227,6 @@ Number::checkInteger(char const* what) const
{
if (enforceInteger_ == strong && !valid())
throw std::overflow_error(what);
if (enforceInteger_ == weak && !representable())
throw std::overflow_error(what);
}
void
@@ -275,20 +273,7 @@ Number::normalize()
bool
Number::valid() const noexcept
{
return valid(enforceInteger_);
}
bool
Number::valid(EnforceInteger enforce)
{
setIntegerEnforcement(enforce);
return valid();
}
bool
Number::valid(EnforceInteger enforce) const
{
if (enforce != none)
if (enforceInteger_ != none)
{
static Number const max = maxIntValue;
static Number const maxNeg = -maxIntValue;
@@ -300,21 +285,6 @@ Number::valid(EnforceInteger enforce) const
return true;
}
bool
Number::representable() const noexcept
{
if (enforceInteger_ != none)
{
static Number const max = maxMantissa;
static Number const maxNeg = -maxMantissa;
// Avoid making a copy
if (mantissa_ < 0)
return *this >= maxNeg;
return *this <= max;
}
return true;
}
Number&
Number::operator+=(Number const& y)
{

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

@@ -3454,8 +3454,6 @@ assetsToSharesDeposit(
Number const assetTotal = vault->at(sfAssetsTotal);
STAmount shares{vault->at(sfShareMPTID)};
// STAmount will ignore enforcement for IOUs, so we can set it regardless of
// type.
shares.setIntegerEnforcement(Number::weak);
if (assetTotal == 0)
return STAmount{
@@ -3488,8 +3486,6 @@ sharesToAssetsDeposit(
Number const assetTotal = vault->at(sfAssetsTotal);
STAmount assets{vault->at(sfAsset)};
// STAmount will ignore enforcement for IOUs, so we can set it regardless of
// type.
assets.setIntegerEnforcement(Number::weak);
if (assetTotal == 0)
return STAmount{
@@ -3524,8 +3520,6 @@ assetsToSharesWithdraw(
Number assetTotal = vault->at(sfAssetsTotal);
assetTotal -= vault->at(sfLossUnrealized);
STAmount shares{vault->at(sfShareMPTID)};
// STAmount will ignore enforcement for IOUs, so we can set it regardless of
// type.
shares.setIntegerEnforcement(Number::weak);
if (assetTotal == 0)
return shares;
@@ -3557,8 +3551,6 @@ sharesToAssetsWithdraw(
Number assetTotal = vault->at(sfAssetsTotal);
assetTotal -= vault->at(sfLossUnrealized);
STAmount assets{vault->at(sfAsset)};
// STAmount will ignore enforcement for IOUs, so we can set it regardless of
// type.
assets.setIntegerEnforcement(Number::weak);
if (assetTotal == 0)
return assets;

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

@@ -270,9 +270,7 @@ STAmount::integerEnforcement() const noexcept
bool
STAmount::validNumber() const noexcept
{
// compatible will not throw. IOUs will ignore the flag, and will
// always be valid.
Number n = toNumber(Number::EnforceInteger::compatible);
Number n = toNumber(Number::EnforceInteger::weak);
return n.valid();
}

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

@@ -3727,7 +3727,7 @@ class Vault_test : public beast::unit_test::suite
}
});
testCase(13, [&, this](Env& env, Data d) {
testCase(14, [&, this](Env& env, Data d) {
testcase("MPT scale deposit overflow on first deposit");
auto tx = d.vault.deposit(
{.depositor = d.depositor,
@@ -3737,7 +3737,7 @@ class Vault_test : public beast::unit_test::suite
env.close();
});
testCase(13, [&, this](Env& env, Data d) {
testCase(14, [&, this](Env& env, Data d) {
testcase("MPT scale deposit overflow on second deposit");
{
@@ -3759,7 +3759,7 @@ class Vault_test : public beast::unit_test::suite
}
});
testCase(13, [&, this](Env& env, Data d) {
testCase(14, [&, this](Env& env, Data d) {
testcase("No MPT scale deposit overflow on total shares");
{
@@ -4079,7 +4079,7 @@ class Vault_test : public beast::unit_test::suite
}
});
testCase(13, [&, this](Env& env, Data d) {
testCase(14, [&, this](Env& env, Data d) {
testcase("MPT scale withdraw overflow");
{
@@ -4320,7 +4320,7 @@ class Vault_test : public beast::unit_test::suite
}
});
testCase(13, [&, this](Env& env, Data d) {
testCase(14, [&, this](Env& env, Data d) {
testcase("MPT Scale clawback overflow");
{

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

@@ -846,87 +846,28 @@ public:
Number a{100};
BEAST_EXPECT(a.integerEnforcement() == Number::none);
BEAST_EXPECT(a.valid());
BEAST_EXPECT(a.representable());
a = Number{1, 30};
BEAST_EXPECT(a.valid());
BEAST_EXPECT(a.representable());
a = -100;
BEAST_EXPECT(a.valid());
BEAST_EXPECT(a.representable());
}
{
Number a{100, Number::compatible};
BEAST_EXPECT(a.integerEnforcement() == Number::compatible);
BEAST_EXPECT(a.valid());
BEAST_EXPECT(a.representable());
a = Number{1, 15};
BEAST_EXPECT(!a.valid());
BEAST_EXPECT(a.representable());
a = Number{1, 30, Number::none};
BEAST_EXPECT(!a.valid());
BEAST_EXPECT(!a.representable());
a = -100;
BEAST_EXPECT(a.integerEnforcement() == Number::compatible);
BEAST_EXPECT(a.valid());
BEAST_EXPECT(a.representable());
a = Number{5, Number::weak};
BEAST_EXPECT(a.integerEnforcement() == Number::weak);
BEAST_EXPECT(a.valid());
BEAST_EXPECT(a.representable());
a = Number{5, Number::strong};
BEAST_EXPECT(a.integerEnforcement() == Number::strong);
BEAST_EXPECT(a.valid());
BEAST_EXPECT(a.representable());
}
{
Number a{100, Number::weak};
BEAST_EXPECT(a.integerEnforcement() == Number::weak);
BEAST_EXPECT(a.valid());
BEAST_EXPECT(a.representable());
a = Number{1, 15};
a = Number{1, 30, Number::none};
BEAST_EXPECT(!a.valid());
BEAST_EXPECT(a.representable());
try
{
a = Number{1, 30, Number::compatible};
BEAST_EXPECT(false);
}
catch (std::overflow_error const& e)
{
BEAST_EXPECT(e.what() == "Number::operator= integer overflow"s);
// The throw is done _after_ the number is updated.
BEAST_EXPECT((a == Number{1, 30}));
}
BEAST_EXPECT(a.integerEnforcement() == Number::weak);
BEAST_EXPECT(!a.valid());
BEAST_EXPECT(!a.representable());
a = -100;
BEAST_EXPECT(a.integerEnforcement() == Number::weak);
BEAST_EXPECT(a.valid());
BEAST_EXPECT(a.representable());
a = Number{5, Number::strong};
BEAST_EXPECT(a.integerEnforcement() == Number::strong);
BEAST_EXPECT(a.valid());
BEAST_EXPECT(a.representable());
}
{
Number a{100, Number::strong};
BEAST_EXPECT(a.integerEnforcement() == Number::strong);
BEAST_EXPECT(a.valid());
BEAST_EXPECT(a.representable());
try
{
a = Number{1, 15, Number::compatible};
BEAST_EXPECT(false);
}
catch (std::overflow_error const& e)
{
BEAST_EXPECT(e.what() == "Number::operator= integer overflow"s);
// The throw is done _after_ the number is updated.
BEAST_EXPECT((a == Number{1, 15}));
}
BEAST_EXPECT(!a.valid());
BEAST_EXPECT(a.representable());
try
{
a = Number{1, 30};
@@ -939,19 +880,15 @@ public:
BEAST_EXPECT((a == Number{1, 30}));
}
BEAST_EXPECT(!a.valid());
BEAST_EXPECT(!a.representable());
a = -100;
BEAST_EXPECT(a.integerEnforcement() == Number::strong);
BEAST_EXPECT(a.valid());
BEAST_EXPECT(a.representable());
}
{
Number a{INITIAL_XRP.drops(), Number::compatible};
Number a{INITIAL_XRP.drops(), Number::weak};
BEAST_EXPECT(!a.valid());
BEAST_EXPECT(!a.representable());
a = -a;
BEAST_EXPECT(!a.valid());
BEAST_EXPECT(!a.representable());
try
{
@@ -967,7 +904,6 @@ public:
// assigned to the Number
BEAST_EXPECT(a == -INITIAL_XRP);
BEAST_EXPECT(!a.valid());
BEAST_EXPECT(!a.representable());
}
try
{
@@ -981,7 +917,6 @@ public:
// assigned to the Number
BEAST_EXPECT(a == -INITIAL_XRP);
BEAST_EXPECT(!a.valid());
BEAST_EXPECT(!a.representable());
}
a = Number::maxIntValue;
try
@@ -995,7 +930,6 @@ public:
// This time, the throw is done _after_ the number is updated.
BEAST_EXPECT(a == Number::maxIntValue + 1);
BEAST_EXPECT(!a.valid());
BEAST_EXPECT(a.representable());
}
a = -Number::maxIntValue;
try
@@ -1009,7 +943,6 @@ public:
// This time, the throw is done _after_ the number is updated.
BEAST_EXPECT(a == -Number::maxIntValue - 1);
BEAST_EXPECT(!a.valid());
BEAST_EXPECT(a.representable());
}
a = Number(1, 10);
try
@@ -1024,7 +957,6 @@ public:
// The throw is done _after_ the number is updated.
BEAST_EXPECT((a == Number{1, 20}));
BEAST_EXPECT(!a.valid());
BEAST_EXPECT(!a.representable());
}
try
{
@@ -1037,7 +969,6 @@ public:
// The throw is done _after_ the number is updated.
BEAST_EXPECT((a == Number::maxIntValue * 2));
BEAST_EXPECT(!a.valid());
BEAST_EXPECT(a.representable());
}
try
{
@@ -1050,7 +981,6 @@ public:
// The Number doesn't get updated because the ctor throws
BEAST_EXPECT((a == Number::maxIntValue * 2));
BEAST_EXPECT(!a.valid());
BEAST_EXPECT(a.representable());
}
a = Number(1, 10);
try
@@ -1064,12 +994,10 @@ public:
// The throw is done _after_ the number is updated.
BEAST_EXPECT((a == Number{1, 20}));
BEAST_EXPECT(!a.valid());
BEAST_EXPECT(!a.representable());
}
a /= Number(1, 15);
BEAST_EXPECT((a == Number{1, 5}));
BEAST_EXPECT(a.valid());
BEAST_EXPECT(a.representable());
}
}

20
src/xrpld/app/tx/detail/InvariantCheck.cpp
View File

@@ -2532,13 +2532,6 @@ ValidVault::Vault::make(SLE const& from)
self.assetsAvailable = from.at(sfAssetsAvailable);
self.assetsMaximum = from.at(sfAssetsMaximum);
self.lossUnrealized = from.at(sfLossUnrealized);
if (self.asset.integral())
{
self.assetsTotal.setIntegerEnforcement(Number::compatible);
self.assetsAvailable.setIntegerEnforcement(Number::compatible);
self.assetsMaximum.setIntegerEnforcement(Number::compatible);
self.lossUnrealized.setIntegerEnforcement(Number::compatible);
}
return self;
}
@@ -2773,19 +2766,6 @@ ValidVault::finalize(
beforeVault_.empty() || beforeVault_[0].key == afterVault.key,
"ripple::ValidVault::finalize : single vault operation");
if (!afterVault.assetsTotal.representable() ||
!afterVault.assetsAvailable.representable() ||
!afterVault.assetsMaximum.representable() ||
!afterVault.lossUnrealized.representable())
{
JLOG(j.fatal()) << "Invariant failed: vault overflowed maximum current "
"representable integer value";
XRPL_ASSERT(
enforce,
"ripple::ValidVault::finalize : vault integer limit invariant");
return !enforce; // That's all we can do here
}
auto const updatedShares = [&]() -> std::optional<Shares> {
// At this moment we only know that a vault is being updated and there
// might be some MPTokenIssuance objects which are also updated in the

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

@@ -150,11 +150,8 @@ VaultClawback::doApply()
amount.asset() == vaultAsset,
"ripple::VaultClawback::doApply : matching asset");
// Both of these values are going to be decreased in this transaction,
// so the limit doesn't really matter.
auto assetsAvailable = vault->at(sfAssetsAvailable);
auto assetsTotal = vault->at(sfAssetsTotal);
[[maybe_unused]] auto const lossUnrealized = vault->at(sfLossUnrealized);
XRPL_ASSERT(
lossUnrealized <= (assetsTotal - assetsAvailable),
@@ -164,8 +161,6 @@ VaultClawback::doApply()
MPTIssue const share{mptIssuanceID};
STAmount sharesDestroyed = {share};
STAmount assetsRecovered;
// STAmount will ignore enforcement for IOUs, so we can set it regardless of
// type.
assetsRecovered.setIntegerEnforcement(Number::weak);
sharesDestroyed.setIntegerEnforcement(Number::weak);
try

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

@@ -213,13 +213,6 @@ VaultCreate::doApply()
vault->at(sfWithdrawalPolicy) = vaultStrategyFirstComeFirstServe;
if (scale)
vault->at(sfScale) = scale;
if (asset.integral())
{
// Only the Maximum can be a non-zero value, so only it needs to be
// checked.
if (!vault->at(sfAssetsMaximum).value().valid(Number::compatible))
return tecLIMIT_EXCEEDED;
}
view().insert(vault);
// Explicitly create MPToken for the vault owner

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

@@ -220,8 +220,6 @@ VaultDeposit::doApply()
}
STAmount sharesCreated = {vault->at(sfShareMPTID)}, assetsDeposited;
// STAmount will ignore enforcement for IOUs, so we can set it regardless of
// type.
sharesCreated.setIntegerEnforcement(Number::weak);
try
{

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

@@ -147,11 +147,6 @@ VaultSet::doApply()
tx[sfAssetsMaximum] < *vault->at(sfAssetsTotal))
return tecLIMIT_EXCEEDED;
vault->at(sfAssetsMaximum) = tx[sfAssetsMaximum];
if (vault->at(sfAsset).value().integral())
{
if (!vault->at(sfAssetsMaximum).value().valid(Number::compatible))
return tecLIMIT_EXCEEDED;
}
}
if (auto const domainId = tx[~sfDomainID]; domainId)

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

@@ -207,8 +207,6 @@ VaultWithdraw::doApply()
return tecINSUFFICIENT_FUNDS;
}
// These values are only going to decrease, and can't be less than 0, so
// there's no need for integer range enforcement.
auto assetsAvailable = vault->at(sfAssetsAvailable);
auto assetsTotal = vault->at(sfAssetsTotal);
[[maybe_unused]] auto const lossUnrealized = vault->at(sfLossUnrealized);