From 471b7f88b5b045e1f84f4e68c5f035c409edca33 Mon Sep 17 00:00:00 2001 From: Mo Morsi Date: Thu, 16 Jul 2020 20:59:16 -0400 Subject: [PATCH] Changes baseFee, reserveBase, reserveIncrement to XRPAmount Addresses weird custom serialization of these fields, see issues #3165 and #3502 on github. Work in progress, submitting for initial feedback, will require an amendment to transaction from the old representation to the new. Tests updated to pass. --- src/ripple/app/ledger/Ledger.cpp | 6 ++--- src/ripple/app/misc/FeeVoteImpl.cpp | 25 +++++++++---------- src/ripple/app/misc/NetworkOPs.cpp | 6 ++--- src/ripple/app/tx/impl/Change.cpp | 8 +++--- src/ripple/proto/org/xrpl/rpc/v1/common.proto | 6 ++--- src/ripple/protocol/SField.h | 6 ++--- src/ripple/protocol/impl/SField.cpp | 6 ++--- src/ripple/rpc/impl/GRPCHelpers.cpp | 6 ++--- src/test/app/PseudoTx_test.cpp | 6 ++--- src/test/jtx/impl/Env.cpp | 2 +- src/test/nodestore/DatabaseShard_test.cpp | 4 +-- 11 files changed, 40 insertions(+), 41 deletions(-) diff --git a/src/ripple/app/ledger/Ledger.cpp b/src/ripple/app/ledger/Ledger.cpp index 3801d994441..7de34910b04 100644 --- a/src/ripple/app/ledger/Ledger.cpp +++ b/src/ripple/app/ledger/Ledger.cpp @@ -549,16 +549,16 @@ Ledger::setup(Config const& config) // VFALCO NOTE Why getFieldIndex and not isFieldPresent? if (sle->getFieldIndex(sfBaseFee) != -1) - fees_.base = sle->getFieldU64(sfBaseFee); + fees_.base = sle->getFieldAmount(sfBaseFee).xrp(); if (sle->getFieldIndex(sfReferenceFeeUnits) != -1) fees_.units = sle->getFieldU32(sfReferenceFeeUnits); if (sle->getFieldIndex(sfReserveBase) != -1) - fees_.reserve = sle->getFieldU32(sfReserveBase); + fees_.reserve = sle->getFieldAmount(sfReserveBase).xrp(); if (sle->getFieldIndex(sfReserveIncrement) != -1) - fees_.increment = sle->getFieldU32(sfReserveIncrement); + fees_.increment = sle->getFieldAmount(sfReserveIncrement).xrp(); } } catch (SHAMapMissingNode const&) diff --git a/src/ripple/app/misc/FeeVoteImpl.cpp b/src/ripple/app/misc/FeeVoteImpl.cpp index e2dc2e40712..66ad2c62c25 100644 --- a/src/ripple/app/misc/FeeVoteImpl.cpp +++ b/src/ripple/app/misc/FeeVoteImpl.cpp @@ -122,7 +122,7 @@ FeeVoteImpl::doValidation(Fees const& lastFees, STValidation& v) << "Voting for base fee of " << target_.reference_fee; if (auto const f = target_.reference_fee.dropsAs()) - v.setFieldU64(sfBaseFee, *f); + v.setFieldAmount(sfBaseFee, XRPAmount(*f)); } if (lastFees.accountReserve(0) != target_.account_reserve) @@ -131,7 +131,7 @@ FeeVoteImpl::doValidation(Fees const& lastFees, STValidation& v) << "Voting for base reserve of " << target_.account_reserve; if (auto const f = target_.account_reserve.dropsAs()) - v.setFieldU32(sfReserveBase, *f); + v.setFieldAmount(sfReserveBase, XRPAmount(*f)); } if (lastFees.increment != target_.owner_reserve) @@ -140,7 +140,7 @@ FeeVoteImpl::doValidation(Fees const& lastFees, STValidation& v) << "Voting for reserve increment of " << target_.owner_reserve; if (auto const f = target_.owner_reserve.dropsAs()) - v.setFieldU32(sfReserveIncrement, *f); + v.setFieldAmount(sfReserveIncrement, XRPAmount(*f)); } } @@ -169,11 +169,10 @@ FeeVoteImpl::doVoting( if (val->isFieldPresent(sfBaseFee)) { using xrptype = XRPAmount::value_type; - auto const vote = val->getFieldU64(sfBaseFee); - if (vote <= std::numeric_limits::max() && - isLegalAmount(XRPAmount{unsafe_cast(vote)})) - baseFeeVote.addVote( - XRPAmount{unsafe_cast(vote)}); + auto const vote = val->getFieldAmount(sfBaseFee).xrp(); + if (vote.drops() <= std::numeric_limits::max() && + isLegalAmount(vote)) + baseFeeVote.addVote(vote); else // Invalid amounts will be treated as if they're // not provided. Don't throw because this value is @@ -188,7 +187,7 @@ FeeVoteImpl::doVoting( if (val->isFieldPresent(sfReserveBase)) { baseReserveVote.addVote( - XRPAmount{val->getFieldU32(sfReserveBase)}); + val->getFieldAmount(sfReserveBase).xrp()); } else { @@ -198,7 +197,7 @@ FeeVoteImpl::doVoting( if (val->isFieldPresent(sfReserveIncrement)) { incReserveVote.addVote( - XRPAmount{val->getFieldU32(sfReserveIncrement)}); + val->getFieldAmount(sfReserveIncrement).xrp()); } else { @@ -231,9 +230,9 @@ FeeVoteImpl::doVoting( [seq, baseFee, baseReserve, incReserve, feeUnits](auto& obj) { obj[sfAccount] = AccountID(); obj[sfLedgerSequence] = seq; - obj[sfBaseFee] = baseFee; - obj[sfReserveBase] = baseReserve; - obj[sfReserveIncrement] = incReserve; + obj[sfBaseFee] = XRPAmount(baseFee); + obj[sfReserveBase] = XRPAmount(baseReserve); + obj[sfReserveIncrement] = XRPAmount(incReserve); obj[sfReferenceFeeUnits] = feeUnits.fee(); }); diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 6a19cc66bbb..908ca877467 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -2087,13 +2087,13 @@ NetworkOPsImp::pubValidation(std::shared_ptr const& val) jvObj[jss::load_fee] = *loadFee; if (auto const baseFee = (*val)[~sfBaseFee]) - jvObj[jss::base_fee] = static_cast(*baseFee); + jvObj[jss::base_fee] = (*baseFee).getJson(JsonOptions::none); if (auto const reserveBase = (*val)[~sfReserveBase]) - jvObj[jss::reserve_base] = *reserveBase; + jvObj[jss::reserve_base] = (*reserveBase).getJson(JsonOptions::none); if (auto const reserveInc = (*val)[~sfReserveIncrement]) - jvObj[jss::reserve_inc] = *reserveInc; + jvObj[jss::reserve_inc] = (*reserveInc).getJson(JsonOptions::none); for (auto i = mStreamMaps[sValidations].begin(); i != mStreamMaps[sValidations].end();) diff --git a/src/ripple/app/tx/impl/Change.cpp b/src/ripple/app/tx/impl/Change.cpp index 9563790d86e..7702370369e 100644 --- a/src/ripple/app/tx/impl/Change.cpp +++ b/src/ripple/app/tx/impl/Change.cpp @@ -229,12 +229,12 @@ Change::applyFee() view().insert(feeObject); } - feeObject->setFieldU64(sfBaseFee, ctx_.tx.getFieldU64(sfBaseFee)); + feeObject->setFieldAmount(sfBaseFee, ctx_.tx.getFieldAmount(sfBaseFee)); feeObject->setFieldU32( sfReferenceFeeUnits, ctx_.tx.getFieldU32(sfReferenceFeeUnits)); - feeObject->setFieldU32(sfReserveBase, ctx_.tx.getFieldU32(sfReserveBase)); - feeObject->setFieldU32( - sfReserveIncrement, ctx_.tx.getFieldU32(sfReserveIncrement)); + feeObject->setFieldAmount(sfReserveBase, ctx_.tx.getFieldAmount(sfReserveBase)); + feeObject->setFieldAmount( + sfReserveIncrement, ctx_.tx.getFieldAmount(sfReserveIncrement)); view().update(feeObject); diff --git a/src/ripple/proto/org/xrpl/rpc/v1/common.proto b/src/ripple/proto/org/xrpl/rpc/v1/common.proto index cc0a0aa14b5..c3ebba12126 100644 --- a/src/ripple/proto/org/xrpl/rpc/v1/common.proto +++ b/src/ripple/proto/org/xrpl/rpc/v1/common.proto @@ -123,13 +123,13 @@ message ReferenceFeeUnits message ReserveBase { // in drops - uint32 value = 1; + CurrencyAmount value = 1; } message ReserveIncrement { // in drops - uint32 value = 1; + CurrencyAmount value = 1; } message Sequence @@ -185,7 +185,7 @@ message TransferRate message BaseFee { // in drops - uint64 value = 1 [jstype=JS_STRING]; + CurrencyAmount value = 1; } message BookNode diff --git a/src/ripple/protocol/SField.h b/src/ripple/protocol/SField.h index e0f78d5ec50..56c6febe37a 100644 --- a/src/ripple/protocol/SField.h +++ b/src/ripple/protocol/SField.h @@ -381,8 +381,6 @@ extern SF_U32 const sfLastLedgerSequence; extern SF_U32 const sfTransactionIndex; extern SF_U32 const sfOperationLimit; extern SF_U32 const sfReferenceFeeUnits; -extern SF_U32 const sfReserveBase; -extern SF_U32 const sfReserveIncrement; extern SF_U32 const sfSetFlag; extern SF_U32 const sfClearFlag; extern SF_U32 const sfSignerQuorum; @@ -396,7 +394,6 @@ extern SF_U64 const sfIndexNext; extern SF_U64 const sfIndexPrevious; extern SF_U64 const sfBookNode; extern SF_U64 const sfOwnerNode; -extern SF_U64 const sfBaseFee; extern SF_U64 const sfExchangeRate; extern SF_U64 const sfLowNode; extern SF_U64 const sfHighNode; @@ -447,6 +444,9 @@ extern SF_Amount const sfHighLimit; extern SF_Amount const sfFee; extern SF_Amount const sfSendMax; extern SF_Amount const sfDeliverMin; +extern SF_Amount const sfBaseFee; +extern SF_Amount const sfReserveBase; +extern SF_Amount const sfReserveIncrement; // currency amount (uncommon) extern SF_Amount const sfMinimumOffer; diff --git a/src/ripple/protocol/impl/SField.cpp b/src/ripple/protocol/impl/SField.cpp index 558635bee5d..e8e5c126864 100644 --- a/src/ripple/protocol/impl/SField.cpp +++ b/src/ripple/protocol/impl/SField.cpp @@ -108,8 +108,6 @@ SF_U32 const sfLastLedgerSequence(access, STI_UINT32, 27, "LastLedgerSequence"); SF_U32 const sfTransactionIndex(access, STI_UINT32, 28, "TransactionIndex"); SF_U32 const sfOperationLimit(access, STI_UINT32, 29, "OperationLimit"); SF_U32 const sfReferenceFeeUnits(access, STI_UINT32, 30, "ReferenceFeeUnits"); -SF_U32 const sfReserveBase(access, STI_UINT32, 31, "ReserveBase"); -SF_U32 const sfReserveIncrement(access, STI_UINT32, 32, "ReserveIncrement"); SF_U32 const sfSetFlag(access, STI_UINT32, 33, "SetFlag"); SF_U32 const sfClearFlag(access, STI_UINT32, 34, "ClearFlag"); SF_U32 const sfSignerQuorum(access, STI_UINT32, 35, "SignerQuorum"); @@ -123,7 +121,6 @@ SF_U64 const sfIndexNext(access, STI_UINT64, 1, "IndexNext"); SF_U64 const sfIndexPrevious(access, STI_UINT64, 2, "IndexPrevious"); SF_U64 const sfBookNode(access, STI_UINT64, 3, "BookNode"); SF_U64 const sfOwnerNode(access, STI_UINT64, 4, "OwnerNode"); -SF_U64 const sfBaseFee(access, STI_UINT64, 5, "BaseFee"); SF_U64 const sfExchangeRate(access, STI_UINT64, 6, "ExchangeRate"); SF_U64 const sfLowNode(access, STI_UINT64, 7, "LowNode"); SF_U64 const sfHighNode(access, STI_UINT64, 8, "HighNode"); @@ -180,6 +177,9 @@ SF_Amount const sfHighLimit(access, STI_AMOUNT, 7, "HighLimit"); SF_Amount const sfFee(access, STI_AMOUNT, 8, "Fee"); SF_Amount const sfSendMax(access, STI_AMOUNT, 9, "SendMax"); SF_Amount const sfDeliverMin(access, STI_AMOUNT, 10, "DeliverMin"); +SF_Amount const sfBaseFee(access, STI_AMOUNT, 11, "BaseFee"); +SF_Amount const sfReserveBase(access, STI_AMOUNT, 12, "ReserveBase"); +SF_Amount const sfReserveIncrement(access, STI_AMOUNT, 13, "ReserveIncrement"); // currency amount (uncommon) SF_Amount const sfMinimumOffer(access, STI_AMOUNT, 16, "MinimumOffer"); diff --git a/src/ripple/rpc/impl/GRPCHelpers.cpp b/src/ripple/rpc/impl/GRPCHelpers.cpp index 7b68f9fe331..e394e7e3d26 100644 --- a/src/ripple/rpc/impl/GRPCHelpers.cpp +++ b/src/ripple/rpc/impl/GRPCHelpers.cpp @@ -829,7 +829,7 @@ template void populateBaseFee(T& to, STObject const& from) { - populateProtoPrimitive( + populateProtoAmount( [&to]() { return to.mutable_base_fee(); }, from, sfBaseFee); } @@ -847,7 +847,7 @@ template void populateReserveBase(T& to, STObject const& from) { - populateProtoPrimitive( + populateProtoAmount( [&to]() { return to.mutable_reserve_base(); }, from, sfReserveBase); } @@ -855,7 +855,7 @@ template void populateReserveIncrement(T& to, STObject const& from) { - populateProtoPrimitive( + populateProtoAmount( [&to]() { return to.mutable_reserve_increment(); }, from, sfReserveIncrement); diff --git a/src/test/app/PseudoTx_test.cpp b/src/test/app/PseudoTx_test.cpp index d76b66f0a99..35756f4c19d 100644 --- a/src/test/app/PseudoTx_test.cpp +++ b/src/test/app/PseudoTx_test.cpp @@ -34,9 +34,9 @@ struct PseudoTx_test : public beast::unit_test::suite res.emplace_back(STTx(ttFEE, [&](auto& obj) { obj[sfAccount] = AccountID(); obj[sfLedgerSequence] = seq; - obj[sfBaseFee] = 0; - obj[sfReserveBase] = 0; - obj[sfReserveIncrement] = 0; + obj[sfBaseFee] = STAmount(); + obj[sfReserveBase] = STAmount(); + obj[sfReserveIncrement] = STAmount(); obj[sfReferenceFeeUnits] = 0; })); diff --git a/src/test/jtx/impl/Env.cpp b/src/test/jtx/impl/Env.cpp index 855dfe7bbf0..3fd9dadfe5f 100644 --- a/src/test/jtx/impl/Env.cpp +++ b/src/test/jtx/impl/Env.cpp @@ -137,7 +137,7 @@ Env::close( if (resp["result"]["status"] != std::string("success")) { JLOG(journal.error()) - << "Env::close() failed: " << resp["result"]["status"] + << "Env::close() failed: " << resp << std::endl; res = false; } diff --git a/src/test/nodestore/DatabaseShard_test.cpp b/src/test/nodestore/DatabaseShard_test.cpp index c4606ecd7b4..87606648fe8 100644 --- a/src/test/nodestore/DatabaseShard_test.cpp +++ b/src/test/nodestore/DatabaseShard_test.cpp @@ -1130,8 +1130,8 @@ class DatabaseShard_test : public TestBase { using namespace test::jtx; - std::string ripemd160Key("4CFA8985836B549EC99D2E9705707F488DC91E4E"), - ripemd160Dat("8CC61F503C36339803F8C2FC652C1102DDB889F1"); + std::string ripemd160Key("633EA054C1D6619B6B67B3BE4149829C0D49AC2A"), + ripemd160Dat("5CC8725822A017FDD62F7AAFD5052F4ADE7D8167"); for (int i = 0; i < 2; i++) {