Skip to content

Commit

Permalink
Make XRPAmount constructor explicit:
Browse files Browse the repository at this point in the history 
Remove the implicit conversion from int64 to XRPAmount. The motivation for this
was noticing that many calls to `to_string` with an integer parameter type were
calling the wrong `to_string` function. Since the calls were not prefixed with
`std::`, and there is no ADL to call `std::to_string`, this was converting the
int to an `XRPAmount` and calling `to_string(XRPAmount)`.

Since `to_string(XRPAmount)` did the same thing as `to_string(int)` this error
went undetected.
  • Loading branch information
@seelabs
seelabs committed Jan 9, 2020
1 parent e3b5b80 commit 761bb57
Show file tree
Hide file tree
Showing 23 changed files with 87 additions and 77 deletions.
2 changes: 1 addition & 1 deletion src/ripple/app/consensus/RCLValidations.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ handleNewValidation(Application& app,
{
auto const seq = val->getFieldU32(sfLedgerSequence);
dmp(j.warn(),
"already validated sequence at or past " + to_string(seq));
"already validated sequence at or past " + std::to_string(seq));
}

if (val->isTrusted() && outcome == ValStatus::current)
Expand Down
4 changes: 2 additions & 2 deletions src/ripple/app/ledger/impl/InboundLedger.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ InboundLedger::~InboundLedger ()
"Acquire " << mHash << " abort " <<
((getTimeouts () == 0) ? std::string() :
(std::string ("timeouts:") +
to_string (getTimeouts ()) + " ")) <<
std::to_string (getTimeouts ()) + " ")) <<
mStats.get ();
}
}
Expand Down Expand Up @@ -463,7 +463,7 @@ void InboundLedger::done ()
(mFailed ? " fail " : " ") <<
((getTimeouts () == 0) ? std::string() :
(std::string ("timeouts:") +
to_string (getTimeouts ()) + " ")) <<
std::to_string (getTimeouts ()) + " ")) <<
mStats.get ();

assert (mComplete || mFailed);
Expand Down
10 changes: 5 additions & 5 deletions src/ripple/app/misc/impl/TxQ.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,7 @@ TxQ::apply(Application& app, OpenView& view,
auto const potentialTotalSpend = multiTxn->fee +
std::min(balance - std::min(balance, reserve),
multiTxn->potentialSpend);
assert(potentialTotalSpend > 0);
assert(potentialTotalSpend > XRPAmount{0});
sleBump->setFieldAmount(sfBalance,
balance - potentialTotalSpend);
sleBump->setFieldU32(sfSequence, tSeq);
Expand Down Expand Up @@ -1516,11 +1516,11 @@ TxQ::doRPC(Application& app) const
auto& levels = ret[jss::levels] = Json::objectValue;

ret[jss::ledger_current_index] = view->info().seq;
ret[jss::expected_ledger_size] = to_string(metrics.txPerLedger);
ret[jss::current_ledger_size] = to_string(metrics.txInLedger);
ret[jss::current_queue_size] = to_string(metrics.txCount);
ret[jss::expected_ledger_size] = std::to_string(metrics.txPerLedger);
ret[jss::current_ledger_size] = std::to_string(metrics.txInLedger);
ret[jss::current_queue_size] = std::to_string(metrics.txCount);
if (metrics.txQMaxSize)
ret[jss::max_queue_size] = to_string(*metrics.txQMaxSize);
ret[jss::max_queue_size] = std::to_string(*metrics.txQMaxSize);

levels[jss::reference_level] = to_string(metrics.referenceFeeLevel);
levels[jss::minimum_level] = to_string(metrics.minProcessingFeeLevel);
Expand Down
4 changes: 2 additions & 2 deletions src/ripple/app/tx/impl/InvariantCheck.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ XRPBalanceChecks::visitEntry(
return true;

// Can't have a negative balance (0 is OK)
if (drops < 0)
if (drops < XRPAmount{0})
return true;

return false;
Expand Down Expand Up @@ -262,7 +262,7 @@ NoZeroEscrow::visitEntry(
if (!amount.native())
return true;

if (amount.xrp() <= 0)
if (amount.xrp() <= XRPAmount{0})
return true;

if (amount.xrp() >= INITIAL_XRP)
Expand Down
1 change: 1 addition & 0 deletions src/ripple/basics/XRPAmount.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ class XRPAmount
}

constexpr
explicit
XRPAmount (value_type drops)
: drops_ (drops)
{
Expand Down
1 change: 1 addition & 0 deletions src/ripple/protocol/STAmount.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ class STAmount
STAmount (SField const& name, Issue const& issue,
std::uint64_t mantissa = 0, int exponent = 0, bool negative = false);

explicit
STAmount (std::uint64_t mantissa = 0, bool negative = false);

STAmount (Issue const& issue, std::uint64_t mantissa = 0, int exponent = 0,
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/protocol/SystemParameters.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ systemName ()
/** Number of drops in the genesis account. */
constexpr
XRPAmount
INITIAL_XRP{ 100'000'000'000 * DROPS_PER_XRP };
INITIAL_XRP{100'000'000'000 * DROPS_PER_XRP };

/** Returns true if the amount does not exceed the initial XRP in existence. */
inline
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/rpc/impl/TransactionSign.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1154,7 +1154,7 @@ Json::Value transactionSubmitMultiSigned (
<< " field. Fees must be specified in XRP.";
return RPC::make_error (rpcINVALID_PARAMS, err.str ());
}
if (fee <= 0)
if (fee <= STAmount{0})
{
std::ostringstream err;
err << "Invalid " << sfFee.fieldName
Expand Down
30 changes: 12 additions & 18 deletions src/test/app/LoadFeeTrack_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,10 @@ class LoadFeeTrack_test : public beast::unit_test::suite
return f;
}();

BEAST_EXPECT(scaleFeeLoad(FeeUnit64{ 0 }, l, fees, false) ==
XRPAmount{ 0 });
BEAST_EXPECT(scaleFeeLoad(FeeUnit64{ 10000 }, l, fees, false) ==
XRPAmount{ 10000 });
BEAST_EXPECT(scaleFeeLoad(FeeUnit64{ 1 }, l, fees, false) ==
XRPAmount{ 1 });
BEAST_EXPECT(scaleFeeLoad(FeeUnit64{0}, l, fees, false) == XRPAmount{0});
BEAST_EXPECT(
scaleFeeLoad(FeeUnit64{10000}, l, fees, false) == XRPAmount{10000});
BEAST_EXPECT(scaleFeeLoad(FeeUnit64{1}, l, fees, false) == XRPAmount{1});
}
{
Fees const fees = [&]() {
Expand All @@ -58,12 +56,10 @@ class LoadFeeTrack_test : public beast::unit_test::suite
return f;
}();

BEAST_EXPECT(scaleFeeLoad(FeeUnit64{ 0 }, l, fees, false) ==
XRPAmount{ 0 });
BEAST_EXPECT(scaleFeeLoad(FeeUnit64{ 10000 }, l, fees, false) ==
XRPAmount{ 100000 });
BEAST_EXPECT(scaleFeeLoad(FeeUnit64{ 1 }, l, fees, false) ==
XRPAmount{ 10 });
BEAST_EXPECT(scaleFeeLoad(FeeUnit64{0}, l, fees, false) == XRPAmount{0});
BEAST_EXPECT(
scaleFeeLoad(FeeUnit64{10000}, l, fees, false) == XRPAmount{100000});
BEAST_EXPECT(scaleFeeLoad(FeeUnit64{1}, l, fees, false) == XRPAmount{10});
}
{
Fees const fees = [&]()
Expand All @@ -76,12 +72,10 @@ class LoadFeeTrack_test : public beast::unit_test::suite
return f;
}();

BEAST_EXPECT(scaleFeeLoad(FeeUnit64{ 0 }, l, fees, false) ==
XRPAmount{ 0 });
BEAST_EXPECT(scaleFeeLoad(FeeUnit64{ 10000 }, l, fees, false) ==
XRPAmount{ 1000 });
BEAST_EXPECT(scaleFeeLoad(FeeUnit64{ 1 }, l, fees, false) ==
XRPAmount{ 0 });
BEAST_EXPECT(scaleFeeLoad(FeeUnit64{0}, l, fees, false) == XRPAmount{0});
BEAST_EXPECT(
scaleFeeLoad(FeeUnit64{10000}, l, fees, false) == XRPAmount{1000});
BEAST_EXPECT(scaleFeeLoad(FeeUnit64{1}, l, fees, false) == XRPAmount{0});
}
}
};
Expand Down
2 changes: 1 addition & 1 deletion src/test/app/PayChan_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ struct PayChan_test : public beast::unit_test::suite
// Try to claim more than authorized
auto const preBob = env.balance (bob);
STAmount const authAmt = chanBal + XRP (500);
STAmount const reqAmt = authAmt + 1;
STAmount const reqAmt = authAmt + STAmount{1};
assert (reqAmt <= chanAmt);
auto const sig =
signClaimAuth (alice.pk (), alice.sk (), chan, authAmt);
Expand Down
48 changes: 24 additions & 24 deletions src/test/app/SHAMapStore_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class SHAMapStore_test : public beast::unit_test::suite
{
cfg->LEDGER_HISTORY = deleteInterval;
auto& section = cfg->section(ConfigSection::nodeDatabase());
section.set("online_delete", to_string(deleteInterval));
section.set("online_delete", std::to_string(deleteInterval));
return cfg;
}

Expand Down Expand Up @@ -182,7 +182,7 @@ class SHAMapStore_test : public beast::unit_test::suite
store.rendezvous();

auto ledger = env.rpc("ledger", "validated");
BEAST_EXPECT(goodLedger(env, ledger, to_string(ledgerSeq++)));
BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq++)));

BEAST_EXPECT(store.getLastRotated() == ledgerSeq - 1);
return ledgerSeq;
Expand Down Expand Up @@ -230,19 +230,19 @@ class SHAMapStore_test : public beast::unit_test::suite

for (auto i = firstSeq + 1; i < deleteInterval + firstSeq; ++i)
{
env.fund(XRP(10000), noripple("test" + to_string(i)));
env.fund(XRP(10000), noripple("test" + std::to_string(i)));
env.close();

ledgerTmp = env.rpc("ledger", "current");
BEAST_EXPECT(goodLedger(env, ledgerTmp, to_string(i)));
BEAST_EXPECT(goodLedger(env, ledgerTmp, std::to_string(i)));
}
BEAST_EXPECT(store.getLastRotated() == lastRotated);

for (auto i = 3; i < deleteInterval + lastRotated; ++i)
{
ledgers.emplace(std::make_pair(i,
env.rpc("ledger", to_string(i))));
BEAST_EXPECT(goodLedger(env, ledgers[i], to_string(i), true) &&
ledgers.emplace(
std::make_pair(i, env.rpc("ledger", std::to_string(i))));
BEAST_EXPECT(goodLedger(env, ledgers[i], std::to_string(i), true) &&
getHash(ledgers[i]).length());
}

Expand All @@ -255,7 +255,7 @@ class SHAMapStore_test : public beast::unit_test::suite
env.close();

auto ledger = env.rpc("ledger", "current");
BEAST_EXPECT(goodLedger(env, ledger, to_string(deleteInterval + 4)));
BEAST_EXPECT(goodLedger(env, ledger, std::to_string(deleteInterval + 4)));
}

store.rendezvous();
Expand All @@ -275,13 +275,13 @@ class SHAMapStore_test : public beast::unit_test::suite
env.close();

ledgerTmp = env.rpc("ledger", "current");
BEAST_EXPECT(goodLedger(env, ledgerTmp, to_string(i + 3)));
BEAST_EXPECT(goodLedger(env, ledgerTmp, std::to_string(i + 3)));

ledgers.emplace(std::make_pair(i,
env.rpc("ledger", to_string(i))));
ledgers.emplace(
std::make_pair(i, env.rpc("ledger", std::to_string(i))));
BEAST_EXPECT(store.getLastRotated() == lastRotated ||
i == lastRotated + deleteInterval - 2);
BEAST_EXPECT(goodLedger(env, ledgers[i], to_string(i), true) &&
BEAST_EXPECT(goodLedger(env, ledgers[i], std::to_string(i), true) &&
getHash(ledgers[i]).length());
}

Expand Down Expand Up @@ -320,7 +320,7 @@ class SHAMapStore_test : public beast::unit_test::suite
env.close();

auto ledger = env.rpc("ledger", "validated");
BEAST_EXPECT(goodLedger(env, ledger, to_string(ledgerSeq), true));
BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq), true));
}

store.rendezvous();
Expand All @@ -335,7 +335,7 @@ class SHAMapStore_test : public beast::unit_test::suite
env.close();

auto ledger = env.rpc("ledger", "validated");
BEAST_EXPECT(goodLedger(env, ledger, to_string(ledgerSeq++), true));
BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq++), true));
}

store.rendezvous();
Expand All @@ -351,7 +351,7 @@ class SHAMapStore_test : public beast::unit_test::suite
env.close();

auto ledger = env.rpc("ledger", "validated");
BEAST_EXPECT(goodLedger(env, ledger, to_string(ledgerSeq), true));
BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq), true));
}

store.rendezvous();
Expand Down Expand Up @@ -389,7 +389,7 @@ class SHAMapStore_test : public beast::unit_test::suite
env.close();

auto ledger = env.rpc("ledger", "validated");
BEAST_EXPECT(goodLedger(env, ledger, to_string(ledgerSeq), true));
BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq), true));
}

store.rendezvous();
Expand All @@ -398,7 +398,7 @@ class SHAMapStore_test : public beast::unit_test::suite
BEAST_EXPECT(lastRotated == store.getLastRotated());

// This does not kick off a cleanup
canDelete = env.rpc("can_delete", to_string(
canDelete = env.rpc("can_delete", std::to_string(
ledgerSeq + deleteInterval / 2));
BEAST_EXPECT(!RPC::contains_error(canDelete[jss::result]));
BEAST_EXPECT(canDelete[jss::result][jss::can_delete] ==
Expand All @@ -414,7 +414,7 @@ class SHAMapStore_test : public beast::unit_test::suite
env.close();

auto ledger = env.rpc("ledger", "validated");
BEAST_EXPECT(goodLedger(env, ledger, to_string(ledgerSeq++), true));
BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq++), true));
}

store.rendezvous();
Expand All @@ -430,7 +430,7 @@ class SHAMapStore_test : public beast::unit_test::suite
env.close();

auto ledger = env.rpc("ledger", "validated");
BEAST_EXPECT(goodLedger(env, ledger, to_string(ledgerSeq), true));
BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq), true));
}

store.rendezvous();
Expand All @@ -442,7 +442,7 @@ class SHAMapStore_test : public beast::unit_test::suite
env.close();

auto ledger = env.rpc("ledger", "validated");
BEAST_EXPECT(goodLedger(env, ledger, to_string(ledgerSeq++), true));
BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq++), true));
}

store.rendezvous();
Expand All @@ -464,7 +464,7 @@ class SHAMapStore_test : public beast::unit_test::suite
env.close();

auto ledger = env.rpc("ledger", "validated");
BEAST_EXPECT(goodLedger(env, ledger, to_string(ledgerSeq), true));
BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq), true));
}

store.rendezvous();
Expand All @@ -476,7 +476,7 @@ class SHAMapStore_test : public beast::unit_test::suite
env.close();

auto ledger = env.rpc("ledger", "validated");
BEAST_EXPECT(goodLedger(env, ledger, to_string(ledgerSeq++), true));
BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq++), true));
}

store.rendezvous();
Expand All @@ -497,7 +497,7 @@ class SHAMapStore_test : public beast::unit_test::suite
env.close();

auto ledger = env.rpc("ledger", "validated");
BEAST_EXPECT(goodLedger(env, ledger, to_string(ledgerSeq), true));
BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq), true));
}

store.rendezvous();
Expand All @@ -509,7 +509,7 @@ class SHAMapStore_test : public beast::unit_test::suite
env.close();

auto ledger = env.rpc("ledger", "validated");
BEAST_EXPECT(goodLedger(env, ledger, to_string(ledgerSeq++), true));
BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq++), true));
}

store.rendezvous();
Expand Down
3 changes: 1 addition & 2 deletions src/test/basics/FeeUnits_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -344,8 +344,7 @@ class feeunits_test
void run() override
{
BEAST_EXPECT(INITIAL_XRP.drops() == 100'000'000'000'000'000);
BEAST_EXPECT(INITIAL_XRP ==
XRPAmount{ 100'000'000'000'000'000 });
BEAST_EXPECT(INITIAL_XRP == XRPAmount{100'000'000'000'000'000});

testTypes();
testJson();
Expand Down
7 changes: 3 additions & 4 deletions src/test/basics/XRPAmount_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,7 @@ class XRPAmount_test : public beast::unit_test::suite
{
// Explicitly test every defined function for the XRPAmount class
// since some of them are templated, but not used anywhere else.
auto make = [&](auto x) -> XRPAmount {
return x; };
auto make = [&](auto x) -> XRPAmount { return XRPAmount{x}; };

XRPAmount defaulted;
(void)defaulted;
Expand All @@ -156,8 +155,8 @@ class XRPAmount_test : public beast::unit_test::suite
test = make(targetSame);
BEAST_EXPECT(test.drops() == 200);
BEAST_EXPECT(test == targetSame);
BEAST_EXPECT(test < XRPAmount{ 1000 });
BEAST_EXPECT(test > XRPAmount{ 100 });
BEAST_EXPECT(test < XRPAmount{1000});
BEAST_EXPECT(test > XRPAmount{100});

test = std::int64_t(200);
BEAST_EXPECT(test.drops() == 200);
Expand Down
8 changes: 8 additions & 0 deletions src/test/jtx/amount.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,14 @@ operator== (PrettyAmount const& lhs,
return lhs.value() == rhs.value();
}

inline
bool
operator!= (PrettyAmount const& lhs,
PrettyAmount const& rhs)
{
return !operator==(lhs, rhs);
}

std::ostream&
operator<< (std::ostream& os,
PrettyAmount const& amount);
Expand Down
Loading

0 comments on commit 761bb57

Please sign in to comment.