Skip to content

fix: Correct sign-check wording in lending protocol messages - #7913

Open
tyalymov wants to merge 1 commit into
developfrom
tialymov/lending-log-non-negative-wording
Open

fix: Correct sign-check wording in lending protocol messages#7913
tyalymov wants to merge 1 commit into
developfrom
tialymov/lending-log-non-negative-wording

Conversation

@tyalymov

Copy link
Copy Markdown

High Level Overview of Change

Correct misleading sign-check wording in lending-protocol diagnostic messages. Several
vault invariant log messages and one LoanPay assert stated that a value "must be
positive" while the corresponding check only rejects negative values (zero is a valid
state). They now say "must not be negative" / "non-negative" to match the actual check.

This is diagnostic text only — there is no behavioural or consensus change.

Context of Change

Following review discussion on #7863: the vault/loan invariants use < 0 (or >= 0)
comparisons, i.e. they enforce non-negativity, but several messages read "must be
positive", which implies > 0. Zero is legitimate (e.g. an empty vault has
assetsAvailable == 0, assetsTotal == 0; assetsMaximum == 0 means "no maximum";
a vault with no impaired loans has lossUnrealized == 0), so the wording was misleading
when diagnosing invariant failures.

Reworded to match the checks:

  • VaultInvariant.cpp — "assets available / assets outstanding / assets maximum must be
    positive" → "... must not be negative" (each guards < 0).
  • LoanPay.cpp — assert description "positive vault and broker balances" → "non-negative
    vault and broker balances" (guards >= 0).

A full sweep of the lending-protocol files confirmed these are the only mismatches;
LoanInvariant / LoanBrokerInvariant already use "is negative" / "is zero or negative"
correctly, and the remaining "positive" mentions are code comments or genuinely
strict-positive checks (<= 0).

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

Test Plan

Updated the affected expectations in Invariants_test.cpp and confirmed the invariant
suite passes with the new wording. Both test sets are green (xrpld --unittest and
ctest --preset conan-debug). No logic changed, so no new tests were added.

Several vault invariant log messages and one LoanPay assert said a value
"must be positive" while the check only rejects negative values (zero is
valid). Reword to "must not be negative" / "non-negative" to match the
actual check. Log/assert text only; no behaviour change.
@tyalymov
tyalymov requested a review from Tapanito July 30, 2026 13:43

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All clear. No issues to flag.

@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@Tapanito Tapanito added the Trivial Simple change with minimal effect, or already tested. Only needs one approval. label Jul 30, 2026

@Tapanito Tapanito left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Trivial Simple change with minimal effect, or already tested. Only needs one approval.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants