Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Address review feedback from Lending Protocol re-review #6161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
ximinez wants to merge 14 commits into
base: ximinez/lending-XLS-66-ongoing
Choose a base branch
Loading
from
Open

Address review feedback from Lending Protocol re-review #6161

ximinez wants to merge 14 commits into from

Conversation

@ximinez
Copy link
Collaborator

@ximinez ximinez commented Dec 23, 2025 •
edited
Loading

High Level Overview of Change

  • Reduce code duplication in LoanBrokerDelete
    • Vault was being loaded twice in two different places with different
      error conditions. Bring them together.
  • Reorder "canWithdraw" parameters to put "view" first
  • Combine accountSpendable into accountHolds
  • Fix unused variable warnings: used only in asserts
    • Move the variables into an "#if !NDEBUG" block.
  • Avoid copies by taking a reference for the claw amount
  • Return function results directly
  • Fix typo for "parseLoan" in ledger_entry RPC
  • Improve some comments and unused variables
  • No need for late payment components lambda
  • Add explanatory comment for computeLoanProperties
  • Add comment linking computeRawLoanState to spec

Context of Change

#6093

@ximinez
Reduce code duplication in LoanBrokerDelete
1748b30 
- Vault was being loaded twice in two different places with different
  error conditions. Bring them together.
@ximinez ximinez mentioned this pull request Dec 23, 2025
Open
7 tasks
@ximinez ximinez marked this pull request as ready for review December 23, 2025 22:36
@ximinez ximinez requested a review from a team as a code owner December 23, 2025 22:36
@codecov
Copy link

codecov bot commented Dec 23, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 94.54545% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.1%. Comparing base (35a296d) to head (c1cb822).

Files with missing lines Patch % Lines
src/libxrpl/ledger/View.cpp 94.4% 1 Missing ⚠️
src/xrpld/app/misc/detail/LendingHelpers.cpp 75.0% 1 Missing ⚠️
src/xrpld/rpc/handlers/LedgerEntry.cpp 0.0% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                       Coverage Diff                        @@
##           ximinez/lending-XLS-66-ongoing   #6161     +/-   ##
================================================================
- Coverage                            79.2%   79.1%   -0.0%     
================================================================
  Files                                 836     836             
  Lines                               71256   71236     -20     
  Branches                             8297    8313     +16     
================================================================
- Hits                                56402   56365     -37     
- Misses                              14854   14871     +17
Files with missing lines Coverage Δ
include/xrpl/ledger/View.h 100.0% <ø> (ø)
include/xrpl/protocol/detail/ledger_entries.macro 100.0% <ø> (ø)
...rc/xrpld/app/tx/detail/LoanBrokerCoverClawback.cpp 97.8% <100.0%> (ø)
src/xrpld/app/tx/detail/LoanBrokerCoverDeposit.cpp 94.8% <100.0%> (ø)
src/xrpld/app/tx/detail/LoanBrokerDelete.cpp 86.8% <100.0%> (+3.3%) ⬆️
src/xrpld/app/tx/detail/LoanManage.cpp 89.1% <100.0%> (+1.3%) ⬆️
src/xrpld/app/tx/detail/LoanPay.cpp 95.1% <100.0%> (+<0.1%) ⬆️
src/xrpld/app/tx/detail/LoanSet.cpp 91.7% <ø> (ø)
src/xrpld/app/tx/detail/VaultDeposit.cpp 97.5% <100.0%> (ø)
src/libxrpl/ledger/View.cpp 94.5% <94.4%> (-<0.1%) ⬇️
... and 2 more

... and 5 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ximinez
Morty Proxy This is a proxified and sanitized view of the page, visit original site.