mirror of
https://github.com/XRPLF/rippled.git
synced 2026-08-21 14:20:56 +00:00
docs: Correct review-noted inaccuracies in cross-scale fix comments
Address review comments on PR #8002: - LoanPay.cpp: pre-amendment description incorrectly claimed the vault pseudo-account received the raw Number. It received the vaultScale- rounded value via accountSendMulti; the real asymmetry is vault-side between sfAssetsAvailable (rounded) and sfAssetsTotal (unrounded via assetsTotalDelta). - LoanManage.cpp: replace the associateAsset-based justification for the T - A >= totalDefaultAmount guard with the actual invariant: the fields loanVaultExposure differences are whole multiples of the same immutable 10^sfLoanScale and never outgrow STAmount's 16 digits, so the promotion round-trips losslessly. Also drops the misleading "vault-associated" descriptor (the underlying fields live on the loanSle). - LoanRounding_test.cpp: the "loan scale ~ -13 vs vaultScale ~ -9" note is impossible under computeLoanProperties (which clamps loanScale to max(vaultScale, amount.exponent())). Rewrite to describe the actual loanScale == vaultScale regime the scenario exercises.
This commit is contained in:
@@ -191,9 +191,10 @@ LoanManage::defaultLoan(
|
||||
// sibling change in LoanPay for the same fix on the payment path.
|
||||
//
|
||||
// The Number-precision guard `T - A >= totalDefaultAmount` below is
|
||||
// exactly sufficient: loanVaultExposure returns a difference of vault-
|
||||
// associated STNumber fields (IOU-normalized via associateAsset), so
|
||||
// its STAmount promotion round-trips losslessly with no writeOff slack.
|
||||
// exactly sufficient: loanVaultExposure returns a difference of
|
||||
// STNumber fields that are whole multiples of the same immutable
|
||||
// `10^sfLoanScale` and never outgrow its 16 digits, so its STAmount
|
||||
// promotion round-trips losslessly with no writeOff slack.
|
||||
bool const useUnifiedAssetArithmetic = view.rules().enabled(fixCleanup3_4_0);
|
||||
|
||||
if (useUnifiedAssetArithmetic)
|
||||
|
||||
@@ -445,10 +445,14 @@ LoanPay::doApply()
|
||||
// normalization is applied symmetrically and the two balances land on
|
||||
// the same STAmount value -- which is what makes the debug invariant
|
||||
// sfAssetsAvailable == pseudo-account balance hold. Pre-amendment,
|
||||
// sfAssetsAvailable was fed the vaultScale-rounded value while the
|
||||
// pseudo-account received the raw Number, creating the same
|
||||
// cross-side asymmetry with `assetsTotalDelta` that
|
||||
// `LoanManage::defaultLoan` addresses on the default path.
|
||||
// sfAssetsAvailable and the pseudo-account both received the
|
||||
// vaultScale-rounded value (accountSendMulti was called with
|
||||
// `totalPaidToVault`, i.e. the rounded amount), but sfAssetsTotal
|
||||
// was mutated with the unrounded `assetsTotalDelta` on the very
|
||||
// next line. That vault-side asymmetry between sfAssetsAvailable
|
||||
// (rounded) and sfAssetsTotal (unrounded) is the counterpart of the
|
||||
// cross-side mismatch `LoanManage::defaultLoan` addresses on the
|
||||
// default path.
|
||||
auto const totalPaidToVault = view.rules().enabled(fixCleanup3_4_0)
|
||||
? totalPaidToVaultRaw
|
||||
: roundToAsset(asset, totalPaidToVaultRaw, vaultScale, Number::RoundingMode::Downward);
|
||||
|
||||
@@ -257,9 +257,17 @@ private:
|
||||
env(pay(issuer, borrower, asset(5'000'000'000)));
|
||||
env.close();
|
||||
|
||||
// Vault magnitude ~1e7 so vaultScale ~ -9; the loans below sit at
|
||||
// loan scale ~ -13. That 4-order gap is where storage precision
|
||||
// can differ from loan-tracked Number precision.
|
||||
// Vault magnitude ~1e7 gives a coarse vaultScale; the loans
|
||||
// below use principals sized so that
|
||||
// `computeLoanProperties(..., minimumScale=vaultScale)` clamps
|
||||
// `loanScale = std::max(vaultScale, amount.exponent())` down
|
||||
// to `vaultScale`. That equal-scale regime is precisely the
|
||||
// one where the pre-amendment paths still round vault-side
|
||||
// mutations asymmetrically (rounded on sfAssetsAvailable and
|
||||
// in the pseudo-account transfer, unrounded on sfAssetsTotal
|
||||
// via assetsTotalDelta / defaultCovered), so the dust-snap
|
||||
// and post-rounding invariant branches this test guards
|
||||
// against get maximum exposure.
|
||||
BrokerParameters const brokerParams{
|
||||
.vaultDeposit = Number{10'000'000},
|
||||
.debtMax = Number{0},
|
||||
|
||||
Reference in New Issue
Block a user