diff --git a/src/ripple/app/tx/impl/Escrow.cpp b/src/ripple/app/tx/impl/Escrow.cpp index 096ab4d68..6ba2f90ec 100644 --- a/src/ripple/app/tx/impl/Escrow.cpp +++ b/src/ripple/app/tx/impl/Escrow.cpp @@ -267,6 +267,7 @@ EscrowCreate::doApply() ctx_.view(), sleLine, amount, + 1, ctx_.journal, DryRun); @@ -345,6 +346,7 @@ EscrowCreate::doApply() ctx_.view(), sleLine, amount, + 1, ctx_.journal, WetRun); @@ -570,6 +572,7 @@ EscrowFinish::doApply() sle, // src account sled, // dst account amount, // xfer amount + -1, j_, DryRun // dry run ); @@ -620,6 +623,7 @@ EscrowFinish::doApply() sle, // src account sled, // dst account amount, // xfer amount + -1, j_, WetRun // wet run; ); @@ -708,6 +712,7 @@ EscrowCancel::doApply() ctx_.view(), sleLine, -amount, + -1, ctx_.journal, DryRun); result != tesSUCCESS) @@ -753,6 +758,7 @@ EscrowCancel::doApply() ctx_.view(), sleLine, -amount, + -1, ctx_.journal, WetRun); diff --git a/src/ripple/app/tx/impl/PayChan.cpp b/src/ripple/app/tx/impl/PayChan.cpp index 466f8dca4..3b4a836e6 100644 --- a/src/ripple/app/tx/impl/PayChan.cpp +++ b/src/ripple/app/tx/impl/PayChan.cpp @@ -138,6 +138,7 @@ closeChannel( view, sleLine, -amount, + -1, j, DryRun); @@ -189,6 +190,7 @@ closeChannel( view, sleLine, -amount, + -1, j, WetRun); @@ -312,6 +314,7 @@ PayChanCreate::preclaim(PreclaimContext const& ctx) ctx.view, sleLine, amount, + 1, ctx.j, DryRun); @@ -417,6 +420,7 @@ PayChanCreate::doApply() ctx_.view(), sleLine, amount, + 1, ctx_.journal, WetRun); @@ -506,6 +510,7 @@ PayChanFund::doApply() ctx_.view(), sleLine, amount, + 1, ctx_.journal, DryRun); @@ -587,6 +592,7 @@ PayChanFund::doApply() ctx_.view(), sleLine, amount, + 1, ctx_.journal, WetRun); @@ -776,6 +782,7 @@ PayChanClaim::doApply() sleSrcAcc, sled, reqDelta, + 0, ctx_.journal, WetRun); diff --git a/src/ripple/ledger/View.h b/src/ripple/ledger/View.h index 070507c0e..2b97b2d61 100644 --- a/src/ripple/ledger/View.h +++ b/src/ripple/ledger/View.h @@ -475,6 +475,7 @@ trustAdjustLockedBalance( V& view, S& sleLine, STAmount const& deltaAmt, + int deltaLockCount, // if +1 lockCount is increased, -1 is decreased, 0 unchanged beast::Journal const& j, R dryRun) { @@ -542,26 +543,43 @@ trustAdjustLockedBalance( return tecUNFUNDED_PAYMENT; } - STAmount lockedBalance {sfLockedBalance, deltaAmt.issue()}; + STAmount priorLockedBalance {sfLockedBalance, deltaAmt.issue()}; if (sleLine->isFieldPresent(sfLockedBalance)) - lockedBalance = + priorLockedBalance = high ? -(*sleLine)[sfLockedBalance] : (*sleLine)[sfLockedBalance]; - lockedBalance += deltaAmt; + uint32_t priorLockCount = 0; + if (sleLine->isFieldPresent(sfLockCount)) + priorLockCount = sleLine->getFieldU32(sfLockCount); - if (lockedBalance > balance) + uint32_t finalLockCount = priorLockCount + deltaLockCount; + STAmount finalLockedBalance = priorLockedBalance + deltaAmt; + + if (finalLockedBalance > balance) { JLOG(j.trace()) << "trustAdjustLockedBalance: " << "lockedBalance(" - << lockedBalance + << finalLockedBalance << ") > balance(" << balance << ") = true\n"; return tecUNFUNDED_PAYMENT; } - if (lockedBalance < beast::zero) + if (finalLockedBalance < beast::zero) + return tecINTERNAL; + + // check if there is significant precision loss + if (!isAddable(balance, deltaAmt) || + !isAddable(priorLockedBalance, deltaAmt) || + !isAddable(finalLockedBalance, balance)) + return tecPRECISION_LOSS; + + // sanity check possible overflows on the lock counter + if ((deltaLockCount > 0 && priorLockCount > finalLockCount) || + (deltaLockCount < 0 && priorLockCount < finalLockCount) || + (deltaLockCount == 0 && priorLockCount != finalLockCount)) return tecINTERNAL; // we won't update any SLEs if it is a dry run @@ -570,11 +588,17 @@ trustAdjustLockedBalance( if constexpr(std::is_same::value && std::is_same>::value) { - if (lockedBalance == beast::zero) + if (finalLockedBalance == beast::zero || finalLockCount == 0) + { sleLine->makeFieldAbsent(sfLockedBalance); + sleLine->makeFieldAbsent(sfLockCount); + } else + { sleLine-> - setFieldAmount(sfLockedBalance, high ? -lockedBalance : lockedBalance); + setFieldAmount(sfLockedBalance, high ? -finalLockedBalance : finalLockedBalance); + sleLine->setFieldU32(sfLockCount, finalLockCount); + } view.update(sleLine); } @@ -752,6 +776,7 @@ trustTransferLockedBalance( S& sleSrcAcc, S& sleDstAcc, STAmount const& amount, // issuer, currency are in this field + int deltaLockCount, // -1 decrement, +1 increment, 0 unchanged beast::Journal const& j, R dryRun) { @@ -820,10 +845,10 @@ trustTransferLockedBalance( return tecNO_LINE; // can't transfer a locked balance that does not exist - if (!sleSrcLine->isFieldPresent(sfLockedBalance)) + if (!sleSrcLine->isFieldPresent(sfLockedBalance) || !sleSrcLine->isFieldPresent(sfLockCount)) { JLOG(j.trace()) - << "trustTransferLockedBalance could not find sfLockedBalance on source line"; + << "trustTransferLockedBalance could not find sfLockedBalance/sfLockCount on source line"; return tecUNFUNDED_PAYMENT; } @@ -836,6 +861,8 @@ trustTransferLockedBalance( STAmount priorLockedBalance = srcHigh ? -((*sleSrcLine)[sfLockedBalance]) : (*sleSrcLine)[sfLockedBalance]; + uint32_t priorLockCount = (*sleSrcLine)[sfLockCount]; + AccountID srcIssuerAccID = sleSrcLine->getFieldAmount(srcHigh ? sfLowLimit : sfHighLimit).getIssuer(); @@ -853,6 +880,19 @@ trustTransferLockedBalance( STAmount finalLockedBalance = priorLockedBalance - amount; + uint32_t finalLockCount = priorLockCount + deltaLockCount; + + // check if there is significant precision loss + if (!isAddable(priorBalance, amount) || + !isAddable(priorLockedBalance, amount)) + return tecPRECISION_LOSS; + + // sanity check possible overflows on the lock counter + if ((deltaLockCount > 0 && priorLockCount > finalLockCount) || + (deltaLockCount < 0 && priorLockCount < finalLockCount) || + (deltaLockCount == 0 && priorLockCount != finalLockCount)) + return tecINTERNAL; + // this should never happen but defensively check it here before updating sle if (finalBalance < beast::zero || finalLockedBalance < beast::zero) { @@ -865,10 +905,16 @@ trustTransferLockedBalance( { sleSrcLine->setFieldAmount(sfBalance, srcHigh ? -finalBalance : finalBalance); - if (finalLockedBalance == beast::zero) + if (finalLockedBalance == beast::zero || finalLockCount == 0) + { sleSrcLine->makeFieldAbsent(sfLockedBalance); + sleSrcLine->makeFieldAbsent(sfLockCount); + } else + { sleSrcLine->setFieldAmount(sfLockedBalance, srcHigh ? -finalLockedBalance : finalLockedBalance); + sleSrcLine->setFieldU32(sfLockCount, finalLockCount); + } } } @@ -958,6 +1004,10 @@ trustTransferLockedBalance( << "trustTransferLockedBalance would increase dest line above limit without permission"; return tecPATH_DRY; } + + // check if there is significant precision loss + if (!isAddable(priorBalance, dstAmt)) + return tecPRECISION_LOSS; if constexpr(!dryRun) sleDstLine->setFieldAmount(sfBalance, dstHigh ? -finalBalance : finalBalance); diff --git a/src/ripple/protocol/SField.h b/src/ripple/protocol/SField.h index 95a315be4..7e4c4ccf1 100644 --- a/src/ripple/protocol/SField.h +++ b/src/ripple/protocol/SField.h @@ -400,6 +400,7 @@ extern SF_UINT32 const sfMintedNFTokens; extern SF_UINT32 const sfBurnedNFTokens; extern SF_UINT32 const sfHookStateCount; extern SF_UINT32 const sfEmitGeneration; +extern SF_UINT32 const sfLockCount; // 64-bit integers (common) extern SF_UINT64 const sfIndexNext; diff --git a/src/ripple/protocol/STAmount.h b/src/ripple/protocol/STAmount.h index 806ef75a1..c956f96ed 100644 --- a/src/ripple/protocol/STAmount.h +++ b/src/ripple/protocol/STAmount.h @@ -526,6 +526,29 @@ isFakeXRP(STAmount const& amount) return isFakeXRP(amount.issue().currency); } +/** returns true iff adding or subtracting results in less than or equal to 0.01% precision loss **/ +inline bool +isAddable(STAmount const& amt1, STAmount const& amt2) +{ + if (amt1 == beast::zero || amt2 == beast::zero) + return true; + + static const STAmount one {IOUAmount{1, 0}, noIssue()}; + static const STAmount maxLoss {IOUAmount{1, -4}, noIssue()}; + + STAmount A = amt1; + STAmount B = amt2; + + A.setIssue(noIssue()); + B.setIssue(noIssue()); + + STAmount lhs = divide((A - B) + B, A, noIssue()) - one; + STAmount rhs = divide((B - A) + A, B, noIssue()) - one; + + return ((rhs.negative() ? -rhs : rhs) + (lhs.negative() ? -lhs : lhs)) <= maxLoss; +} + + // 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 diff --git a/src/ripple/protocol/TER.h b/src/ripple/protocol/TER.h index 38342f0c1..50157ec32 100644 --- a/src/ripple/protocol/TER.h +++ b/src/ripple/protocol/TER.h @@ -289,6 +289,7 @@ enum TECcodes : TERUnderlyingType { tecINSUFFICIENT_FUNDS = 159, tecOBJECT_NOT_FOUND = 160, tecINSUFFICIENT_PAYMENT = 161, + tecPRECISION_LOSS = 162, }; //------------------------------------------------------------------------------ diff --git a/src/ripple/protocol/impl/LedgerFormats.cpp b/src/ripple/protocol/impl/LedgerFormats.cpp index 22bde8840..311a85445 100644 --- a/src/ripple/protocol/impl/LedgerFormats.cpp +++ b/src/ripple/protocol/impl/LedgerFormats.cpp @@ -106,6 +106,7 @@ LedgerFormats::LedgerFormats() {sfHighQualityIn, soeOPTIONAL}, {sfHighQualityOut, soeOPTIONAL}, {sfLockedBalance, soeOPTIONAL}, + {sfLockCount, soeOPTIONAL}, }, commonFields); diff --git a/src/ripple/protocol/impl/SField.cpp b/src/ripple/protocol/impl/SField.cpp index 7e10d02da..b27e9f614 100644 --- a/src/ripple/protocol/impl/SField.cpp +++ b/src/ripple/protocol/impl/SField.cpp @@ -150,6 +150,7 @@ CONSTRUCT_TYPED_SFIELD(sfMintedNFTokens, "MintedNFTokens", UINT32, CONSTRUCT_TYPED_SFIELD(sfBurnedNFTokens, "BurnedNFTokens", UINT32, 44); CONSTRUCT_TYPED_SFIELD(sfHookStateCount, "HookStateCount", UINT32, 45); CONSTRUCT_TYPED_SFIELD(sfEmitGeneration, "EmitGeneration", UINT32, 46); +CONSTRUCT_TYPED_SFIELD(sfLockCount, "LockCount", UINT32, 47); // 64-bit integers (common) CONSTRUCT_TYPED_SFIELD(sfIndexNext, "IndexNext", UINT64, 1);