diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 28f73b5f1d..b94335224f 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -45,10 +45,11 @@ class Number // of -maxMantissa to maxMantissa. Values larger than this will be // truncated before the decimal point, rendering the value inaccurate. // 3. In "operator rep()", which explicitly converts the number into a - // 64-bit integer, if the Number is not representable(), AND one of the - // SingleAssetVault (or LendingProtocol, coming soon) amendments are - // enabled, the operator will throw a "std::overflow_error" as if the - // number had overflowed the limits of the 64-bit integer range. + // 64-bit integer, if the integer value grows larger than maxMantissa + // while it's being computed, AND one of the SingleAssetVault (or + // LendingProtocol, coming soon) amendments are enabled, the operator + // will throw a "std::overflow_error" as if the number had overflowed the + // limits of the 64-bit integer range. // // The Number is usually only going to be checked in transactions, based on // the specific transaction logic, and is entirely context dependent. @@ -159,10 +160,11 @@ public: * "mixed mode" more convenient, e.g. MPTAmount + Number. 3. In "operator rep()", which explicitly converts the number into a - 64-bit integer, if the Number is not representable(), AND one of the - SingleAssetVault (or LendingProtocol, coming soon) amendments are - enabled, the operator will throw a "std::overflow_error" as if the - number had overflowed the limits of the 64-bit integer range. + 64-bit integer, if the integer value grows larger than maxMantissa + while it's being computed, AND one of the SingleAssetVault (or + LendingProtocol, coming soon) amendments are enabled, the operator + will throw a "std::overflow_error" as if the number had overflowed + the limits of the 64-bit integer range. */ explicit operator rep() const; // round to nearest, even on tie diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index 8bf3b6e283..715b8d3388 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -970,17 +970,17 @@ class Vault_test : public beast::unit_test::suite env(tx, ter(temMALFORMED)); } - // accepted range from 0 to 13 + // accepted range from 0 to 15 { auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); - tx[sfScale] = 13; + tx[sfScale] = 15; env(tx); env.close(); auto const sleVault = env.le(keylet); if (!BEAST_EXPECT(sleVault)) return; - BEAST_EXPECT((*sleVault)[sfScale] == 13); + BEAST_EXPECT((*sleVault)[sfScale] == 15); } { @@ -3696,7 +3696,8 @@ class Vault_test : public beast::unit_test::suite testCase(15, [&, this](Env& env, Data d) { testcase("MPT fractional deposits are supported"); - // Deposits large than Number::maxIntValue are invalid + // Deposits that result in share amounts larger than + // Number::maxIntValue are invalid { auto tx = d.vault.deposit( {.depositor = d.depositor, diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 14a7010878..0c570dc320 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -761,6 +761,7 @@ public: BEAST_EXPECT(!a.getLimited()); BEAST_EXPECT(a.fits()); BEAST_EXPECT(a.representable()); + checkInt(a, -100); // If there's any interaction with an integer, the value // becomes an integer. This is not always what the value is // being used for, so it's up to the context to check or not @@ -769,59 +770,72 @@ public: BEAST_EXPECT(a.getLimited()); BEAST_EXPECT(a.fits()); BEAST_EXPECT(a.representable()); + checkInt(a, 3600); } { Number a{100, true}; BEAST_EXPECT(a.getLimited()); BEAST_EXPECT(a.fits()); BEAST_EXPECT(a.representable()); + checkInt(a, 100); a = Number{1, 15}; BEAST_EXPECT(!a.fits()); BEAST_EXPECT(a.representable()); + checkInt(a, 1'000'000'000'000'000); // The false in the assigned value does not override the // flag in "a" a = Number{1, 30, false}; BEAST_EXPECT(a.getLimited()); BEAST_EXPECT(!a.fits()); BEAST_EXPECT(!a.representable()); + checkInt(a, 0, "Number::operator rep() overflow"); a = -100; BEAST_EXPECT(a.getLimited()); BEAST_EXPECT(a.fits()); BEAST_EXPECT(a.representable()); + checkInt(a, -100); a *= Number{1, 13}; BEAST_EXPECT(a.getLimited()); BEAST_EXPECT(!a.fits()); BEAST_EXPECT(a.representable()); + checkInt(a, -1'000'000'000'000'000); a *= Number{1, 3}; BEAST_EXPECT(a.getLimited()); BEAST_EXPECT(!a.fits()); BEAST_EXPECT(!a.representable()); + checkInt(a, -1'000'000'000'000'000'000); // Intermittent value precision can be lost, but the result // will be rounded, so that's fine. a /= Number{1, 5}; BEAST_EXPECT(a.getLimited()); BEAST_EXPECT(a.fits()); BEAST_EXPECT(a.representable()); + checkInt(a, -10'000'000'000'000); a = Number{1, 14} - 3; BEAST_EXPECT(a.getLimited()); BEAST_EXPECT(a.fits()); BEAST_EXPECT(a.representable()); + checkInt(a, 99'999'999'999'997); a += 1; BEAST_EXPECT(a.getLimited()); BEAST_EXPECT(a.fits()); BEAST_EXPECT(a.representable()); + checkInt(a, 99'999'999'999'998); ++a; BEAST_EXPECT(a.getLimited()); BEAST_EXPECT(a.fits()); BEAST_EXPECT(a.representable()); + checkInt(a, 99'999'999'999'999); a++; BEAST_EXPECT(a.getLimited()); BEAST_EXPECT(!a.fits()); BEAST_EXPECT(a.representable()); + checkInt(a, 100'000'000'000'000); a = Number{5, true}; BEAST_EXPECT(a.getLimited()); BEAST_EXPECT(a.fits()); BEAST_EXPECT(a.representable()); + checkInt(a, 5); auto const maxInt = std::numeric_limits::max(); a = maxInt; diff --git a/src/xrpld/app/tx/detail/VaultDeposit.cpp b/src/xrpld/app/tx/detail/VaultDeposit.cpp index ad9355d43e..8789933ea4 100644 --- a/src/xrpld/app/tx/detail/VaultDeposit.cpp +++ b/src/xrpld/app/tx/detail/VaultDeposit.cpp @@ -287,7 +287,7 @@ VaultDeposit::doApply() { // It's easy to hit this exception from Number with large enough // Scale so we avoid spamming the log and only use debug here. - JLOG(j_.warn()) // + JLOG(j_.debug()) // << "VaultDeposit: integer overflow error in total assets with" << " scale=" << (int)vault->at(sfScale).value() // << ", assetsTotal=" << vault->at(sfAssetsTotal).value()