diff --git a/src/ripple/app/misc/impl/TxQ.cpp b/src/ripple/app/misc/impl/TxQ.cpp index 07668c1a66..2d67685529 100644 --- a/src/ripple/app/misc/impl/TxQ.cpp +++ b/src/ripple/app/misc/impl/TxQ.cpp @@ -290,6 +290,8 @@ TxQ::MaybeTx::apply(Application& app, OpenView& view, beast::Journal j) { // If the rules or flags change, preflight again assert(pfresult); + STAmountSO stAmountSO{view.rules().enabled(fixSTAmountCanonicalize)}; + if (pfresult->rules != view.rules() || pfresult->flags != flags) { JLOG(j.debug()) << "Queued transaction " << txID @@ -728,6 +730,8 @@ TxQ::apply( ApplyFlags flags, beast::Journal j) { + STAmountSO stAmountSO{view.rules().enabled(fixSTAmountCanonicalize)}; + // See if the transaction paid a high enough fee that it can go straight // into the ledger. if (auto directApplied = tryDirectApply(app, view, tx, flags, j)) diff --git a/src/ripple/app/tx/impl/Transactor.cpp b/src/ripple/app/tx/impl/Transactor.cpp index 5c777afd14..bc7a98308a 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -746,6 +746,8 @@ Transactor::operator()() { JLOG(j_.trace()) << "apply: " << ctx_.tx.getTransactionID(); + STAmountSO stAmountSO{view().rules().enabled(fixSTAmountCanonicalize)}; + #ifdef DEBUG { Serializer ser; diff --git a/src/ripple/app/tx/impl/apply.cpp b/src/ripple/app/tx/impl/apply.cpp index e423332756..7ccba78261 100644 --- a/src/ripple/app/tx/impl/apply.cpp +++ b/src/ripple/app/tx/impl/apply.cpp @@ -113,6 +113,8 @@ apply( ApplyFlags flags, beast::Journal j) { + STAmountSO stAmountSO{view.rules().enabled(fixSTAmountCanonicalize)}; + auto pfresult = preflight(app, view.rules(), tx, flags, j); auto pcresult = preclaim(pfresult, app, view); return doApply(pcresult, app, view); diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index cb7a1c06ab..05506614f4 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -114,7 +114,9 @@ class FeatureCollections "fixAmendmentMajorityCalc", // Fix Amendment majority calculation "NegativeUNL", "TicketBatch", - "FlowSortStrands"}; + "FlowSortStrands", + "fixSTAmountCanonicalize", + }; std::vector features; boost::container::flat_map featureToIndex; @@ -373,6 +375,7 @@ extern uint256 const fixAmendmentMajorityCalc; extern uint256 const featureNegativeUNL; extern uint256 const featureTicketBatch; extern uint256 const featureFlowSortStrands; +extern uint256 const fixSTAmountCanonicalize; } // namespace ripple diff --git a/src/ripple/protocol/STAmount.h b/src/ripple/protocol/STAmount.h index 1f70f5f022..01bd4934b3 100644 --- a/src/ripple/protocol/STAmount.h +++ b/src/ripple/protocol/STAmount.h @@ -21,6 +21,7 @@ #define RIPPLE_PROTOCOL_STAMOUNT_H_INCLUDED #include +#include #include #include #include @@ -471,6 +472,32 @@ isXRP(STAmount const& amount) return isXRP(amount.issue().currency); } +// Since `canonicalize` does not have access to a ledger, this is needed to put +// the low-level routine stAmountCanonicalize on an amendment switch. Only +// transactions need to use this switchover. Outside of a transaction it's safe +// to unconditionally use the new behavior. +extern LocalValue stAmountCanonicalizeSwitchover; + +/** RAII class to set and restore the STAmount canonicalize switchover. + */ + +class STAmountSO +{ +public: + explicit STAmountSO(bool v) : saved_(*stAmountCanonicalizeSwitchover) + { + *stAmountCanonicalizeSwitchover = v; + } + + ~STAmountSO() + { + *stAmountCanonicalizeSwitchover = saved_; + } + +private: + bool saved_; +}; + } // namespace ripple #endif diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 17386f7a5a..eaf1cbf714 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -134,6 +134,7 @@ detail::supportedAmendments() //"NegativeUNL", // Commented out to prevent automatic enablement "TicketBatch", "FlowSortStrands", + "fixSTAmountCanonicalize", }; return supported; } @@ -188,7 +189,8 @@ uint256 const fixAmendmentMajorityCalc = *getRegisteredFeature("fixAmendmentMajorityCalc"), featureNegativeUNL = *getRegisteredFeature("NegativeUNL"), featureTicketBatch = *getRegisteredFeature("TicketBatch"), - featureFlowSortStrands = *getRegisteredFeature("FlowSortStrands"); + featureFlowSortStrands = *getRegisteredFeature("FlowSortStrands"), + fixSTAmountCanonicalize = *getRegisteredFeature("fixSTAmountCanonicalize"); // The following amendments have been active for at least two years. Their // pre-amendment code has been removed and the identifiers are deprecated. diff --git a/src/ripple/protocol/impl/STAmount.cpp b/src/ripple/protocol/impl/STAmount.cpp index 37a010c3f3..40132e3b43 100644 --- a/src/ripple/protocol/impl/STAmount.cpp +++ b/src/ripple/protocol/impl/STAmount.cpp @@ -34,6 +34,8 @@ namespace ripple { +LocalValue stAmountCanonicalizeSwitchover(true); + static const std::uint64_t tenTo14 = 100000000000000ull; static const std::uint64_t tenTo14m1 = tenTo14 - 1; static const std::uint64_t tenTo17 = tenTo14 * 1000; @@ -662,13 +664,23 @@ STAmount::canonicalize() // native currency amounts should always have an offset of zero mIsNative = true; - if (mValue == 0) + // log(2^64,10) ~ 19.2 + if (mValue == 0 || mOffset <= -20) { + mValue = 0; mOffset = 0; mIsNegative = false; return; } + if (*stAmountCanonicalizeSwitchover) + { + // log(cMaxNativeN, 10) == 17 + if (mOffset > 17) + Throw( + "Native currency amount out of range"); + } + while (mOffset < 0) { mValue /= 10; @@ -677,6 +689,14 @@ STAmount::canonicalize() while (mOffset > 0) { + if (*stAmountCanonicalizeSwitchover) + { + // N.B. do not move the overflow check to after the + // multiplication + if (mValue > cMaxNativeN) + Throw( + "Native currency amount out of range"); + } mValue *= 10; --mOffset; }