8000 Lending Protocol implementation XLS-66 by ximinez · Pull Request #5270 · XRPLF/rippled · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Lending Protocol implementation XLS-66 #5270

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.

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

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from

Conversation

ximinez
Copy link
Collaborator
@ximinez ximinez commented Jan 31, 2025

High Level Overview of Change

Context of Change

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Performance (increase or change in throughput and/or latency)
  • Tests (you added tests for code that already exists, or your new feature included in this PR)
  • Documentation update
  • Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
  • Release

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)

@ximinez ximinez force-pushed the ximinez/lending-XLS-66 branch 4 times, most recently from 1240135 to 6f5d1ad Compare February 4, 2025 19:35
Copy link
codecov bot commented Feb 4, 2025

Codecov Report

Attention: Patch coverage is 92.52336% with 160 lines in your changes missing coverage. Please review.

Project coverage is 79.3%. Comparing base (c2f3e2e) to head (186821f).

Files with missing lines Patch % Lines
src/xrpld/app/misc/LendingHelpers.h 77.5% 36 Missing ⚠️
src/xrpld/app/tx/detail/InvariantCheck.cpp 91.0% 20 Missing ⚠️
src/xrpld/app/tx/detail/LoanBrokerDelete.cpp 82.6% 12 Missing ⚠️
src/xrpld/app/tx/detail/LoanSet.cpp 94.2% 12 Missing ⚠️
src/libxrpl/protocol/STParsedJSON.cpp 86.5% 10 Missing ⚠️
src/xrpld/app/tx/detail/LoanManage.cpp 94.3% 9 Missing ⚠️
src/xrpld/app/tx/detail/LoanPay.cpp 93.2% 8 Missing ⚠️
src/xrpld/app/tx/detail/LoanDelete.cpp 88.5% 7 Missing ⚠️
src/xrpld/app/tx/detail/Transactor.cpp 91.4% 7 Missing ⚠️
...rc/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp 90.0% 6 Missing ⚠️
... and 9 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #5270     +/-   ##
=========================================
+ Coverage     79.1%   79.3%   +0.2%     
=========================================
  Files          816     837     +21     
  Lines        71605   72871   +1266     
  Branches      8237    8305     +68     
=========================================
+ Hits         56626   57793   +1167     
- Misses       14979   15078     +99     
Files with missing lines Coverage Δ
include/xrpl/basics/base_uint.h 96.9% <100.0%> (+<0.1%) ⬆️
include/xrpl/protocol/Indexes.h 100.0% <100.0%> (ø)
include/xrpl/protocol/Protocol.h 100.0% <100.0%> (ø)
include/xrpl/protocol/SField.h 100.0% <ø> (ø)
include/xrpl/protocol/STAmount.h 95.8% <100.0%> (+0.3%) ⬆️
include/xrpl/protocol/STLedgerEntry.h 100.0% <ø> (ø)
include/xrpl/protocol/STObject.h 93.1% <ø> (ø)
include/xrpl/protocol/STTx.h 100.0% <100.0%> (ø)
include/xrpl/protocol/STValidation.h 69.5% <ø> (ø)
include/xrpl/protocol/XRPAmount.h 98.9% <ø> (ø)
... and 143 more

... and 7 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ximinez ximinez force-pushed the ximinez/lending-XLS-66 branch 3 times, most recently from 27e4c4f to 2d209c4 Compare February 8, 2025 00:58
@ximinez ximinez force-pushed the ximinez/lending-XLS-66 branch 5 times, most recently from e8fbd22 to a697138 Compare February 28, 2025 19:24
@ximinez ximinez force-pushed the ximinez/lending-XLS-66 branch 7 times, most recently from 72d979e to 441d5b5 Compare March 17, 2025 20:23
@ximinez ximinez changed the base branch from develop to ximinez/Bronek/vault March 17, 2025 22:52
@ximinez ximinez force-pushed the ximinez/Bronek/vault branch from b1e091c to 2a8861d Compare March 19, 2025 00:44
@ximinez ximinez force-pushed the ximinez/lending-XLS-66 branch 8 times, most recently from 5223459 to c7b296c Compare March 25, 2025 16:30
@ximinez ximinez force-pushed the ximinez/lending-XLS-66 branch 4 times, most recently from 502685f to 7868567 Compare May 17, 2025 16:22
LoanSet::checkSign(PreclaimContext const& ctx)
{
if (auto ret = Transactor::checkSign(ctx))
return ret;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this supposed to be `... !isTesSuccess(ret))

@Bronek Bronek changed the base branch from ximinez/Bronek/vault to develop May 21, 2025 09:42
ximinez added 5 commits May 21, 2025 11:39
- Add the LendingProtocol amendment
- Add Loan Broker and Loan ledger objects:
- Also add new SFields, Keylet functions, and an Invariant to verify no
  illegal field modification
- Update list of "constant" fields from spec
- Also add a general check for all object types for the type and index
  fields
- refactor: Check transaction flags in preflight0
- Adds a flagMask parameter to preflight1 so that it's impossible to
  forget to check flags.
- Also adds a short hash prefix to all Transactor log messages.
- refactor: Generalize Transactor preflight:
- Derived classes no longer need to explicitly check amendments, nor
  call into preflight1 or preflight2.
- implemeng LoanBrokerSet
- Transactions: LoanDelete, LoanManage, LoanDraw, LoanPay
- LoanBrokerSet creation mostly done. Need update.
- Also added a lookup table for pseudo account fields.
- Update changed field name.
- Modify modifiable fields in an update. Note there are only two.
- Add a node field to dirLink, defaulting sfOwnerNode, so other
  relationships can be updated.
- Create some helper classes for transaction fields
- Test that they work by converting some of the existing classes
- Finish creating helper classes for JTx fields
- Also change the pseudo account field lookup to a function that uses
  a switch
- Update tests, update pseudo-account checking
- Generalize some of the Invariant checks using macro files
  - Valid ledger entry type
  - Valid new account root and pseudo account check
- Enumerate transaction privileges for invariants
  - Allows them to be defined in transactions.macro instead of needing to
    scrutinize every existing Invariant class.
  - List is not necessarily comprehensive, but does cover every check
    where more than one transaction type is involved.
- Reserve a few values between Vault and Lending for future use
- Pseudo-account improvements
  - Define pseudo-account fields with an sfield flag
  - Pseudo-account invariant checks rules whenever a pseudo-account is
    created or modified.
- Move some helper functions.
- Check the regular key in the pseudo-transaction invariant check.
- Transactor::checkSign will always fail for a pseudo-account, so even
  if someone figures out how to get a good signature, it won't work.
- Fix account creation to check both amendments
- Add a validity range for sfDebtMaximum
- Change more "failed" messages. The goal here is to be able to search
  the log for "failed" and ONLY get test failures.
- NoModifiedUnmodifiableFields and ValidPseudoAccounts
- Move the Invariants_test class into the test namespace
- Clang wants an explicit ctor to emplace in a vector
- Refactor: Add a Transactor base function to make it easier to get the
  owner reserve increment as a fee.
- Refactor: Add an overload jtx::fee(increment) to pay an owner reserve.
- Initial implementation of LoanBrokerDelete
- Generalize the LoanBroker lifecycle test
- Refactor ApplyView::dirAdd to give access to low-level operations
  - Takes a page from #5362, which may turn out to be useful!
- Start writing Loan Broker invariants and tests
  - Specifically those mentioned for LoanBrokerDelete
- Move all detail namespaces to be under ripple
  - Avoids problems with namespace collisions / ambiguous symbol issues
    with unity builds, especially when adding or removing files.
- Add LoanBrokerCoverDeposit transaction
- Add LoanBrokerCoverWithdraw transaction
- Start writing tests for LoanBrokerCover*
- Add support for `Asset` and `MPTIssue` to some `jtx` helper classes
  and functions (`balance`, `expectLine`)
- Add support for pseudo-accounts to `jtx::Account` by allowing directly
  setting the AccountID without a matching key.
- Add Asset and MPTIssue support to more jtx objects / functions
  - Unfortunately, to work around some ambiguous symbol compilation
    errors, I had to change the implicit conversion from IOU to Asset to
    a conversion from IOU to PrettyAsset, and add a more explicit
    `asset()` function. This workaround only required changing two
    existing tests, so seems acceptable.
- Ensure that an account is not deleted with an XRP balance
  - Updates the AccountRootsDeletedClean invariant
- Finish up the Loan Broker tests
- Move inclusion of Transactor headers to transactions.macro
  - Only need to update in one place when adding a new transaction.
- Start implementing LoanSet transactor
  - Add some more values and functions to make it easier to work with
    basis point values / bips.
  - Fix several earlier mistakes.
- Generalize the check*Sign functions to support CounterParty
  - checkSign, checkSingleSign, and checkMultiSign in STTx and Transactor
- Start writing Loan tests
  - Required adding support for counterparty signature to jtx framework:
    arbitrary signature field destination, multiple signer callbacks
- Get Counterparty signing working
- Add more LoanSet unit tests, added LoanBroker LoanSequence field
  - LoanSequence will prevent loan key collisions
- Change Loan object indexing, fix several broken LoanSet unit tests
  - Loan objects will now only be indexed by LoanBrokerID and
    LoanSequence, which is a new field in LoanBroker. Also changes
    Loan.Sequence to Loan.LoanSequence to match up.
  - Several tests weren't working because of `PrettyAsset` scaling. Also,
    `PrettyAsset` calculations could overflow. Made that less likely by
    changing the type of `scale_`.
  - LoanSet will fail if an account tries to loan to itself.
- Ensure that an account is not deleted with a non-zero owner count
  - Updates the AccountRootsDeletedClean invariant
- Add unit tests to create a Loan successfully
  - Fix a few field initializations in LoanSet
- Refactor issuance validity check in VaultCreate
  - Utility function: canAddHolding
  - Call canAddHolding from any transactor that call addEmptyHolding
    (LoanBrokerSet, LoanSet)
- Start implementing LoanManage transaction
  - Also add a ValidLoan invariant
- Finish `LoanManage` functionality and tests, modulo LoanDraw/Pay
- Allow existing trust lines to loan brokers to be managed (by issuer)
- Implement LoanDelete, and fix a bunch of math errors in LoanManage
- Update to match latest spec: compute interest, LoanBroker reserves
- refactor: Define getFlagsMask in the base Transactor class
  - Returns tfUniversalMask for most transactors
  - Only transactors that use other flags need to override
- Implement LoanDraw, and made good progress on related tests
- Start implementing LoanPay transaction
- Implement LoanPay & most tests
- Also add an XRPL_ASSERT_PARTS, which splits the parts of the assert message
    so I don't have to remember the proper formatting.
Start writing LoanPay transaction tests
@Bronek Bronek 8000 force-pushed the ximinez/lending-XLS-66 branch from f041036 to 527e0c9 Compare May 21, 2025 10:40
@Bronek
Copy link
Collaborator
Bronek commented May 21, 2025

Merge memo: history rewritten and base branch changed to develop after #5224 has been merged to develop

Initial stage of reconciliation in commit f041036

(git)-[ximinez/lending-XLS-66] % git log --oneline --graph -24 f041036df1634afdc89cdba12f0a589ac8463f5f
*   f041036df1 Merge branch 'develop' into ximinez/lending-XLS-66
|\
| * e514de76ed (origin/develop, origin/HEAD, develop) Add single asset vault (XLS-65d) (#5224)
* |   aa0f4f7ec7 Merge branch 'vault' into ximinez/lending-XLS-66
|\ \
| * | 990d04b992 (bronek/vault, vault) Merge branch 'develop' into vault
| |\|
| * | 9a470ce11b Fix logic error from review feedback
| * | 112469dcb6 Formatting fix
| * |   e5e70398ea Merge branch 'develop' into vault
| |\ \
| * | | 14e0228c7c Review feedback
| * | | ebe388aed4 Review feedback, partial
| * | | cf553c8856 Review feedback, added logging
| * | | 3082fea734 Review feedback, partial
* | | |   793cdb3a7b Merge branch 'develop' (early part) into ximinez/lending-XLS-66
|\ \ \ \
| | |_|/
| |/| |
| * | | dd62cfcc22 fix: Update path in CODEOWNERS (#5440)
| | |/
| |/|
| * | 09690f1b38 (tag: 2.5.0-b1, origin/release) Set version to 2.5.0-b1
| * | 380ba9f1c1 Fix: Resolve slow test on macOS pipeline (#5392)
| * | c3e9380fb4 fix: Update validators-example.txt fix xrplf example URL (#5384)
| * | e3ebc253fa fix: Ensure that coverage file generation is atomic. (#5426)
* | | 7868567fd0 Lending protocol implementation (XLS-0066)
* | | 474a69044d Refactor 4: Transactor extra signing support
* | | 88c4c2c7fe Refactor 3: Transactors
* | | d15efff918 Refactor 2: STParsed Json
* | | 1d842598a5 Refactoring 1
| |/
|/|
* | 024b016d83 (origin/ximinez/Bronek/vault) Merge branch 'develop' into vault
|\|
| * c6c7c84355 Configure CODEOWNERS for changes to RPC code (#5266)

Next rebased on develop to clean the commit history (from some 220 commits), into commit 527e0c9

(git)-[ximinez/lending-XLS-66] % git log --oneline --graph -12
* 527e0c916f (HEAD -> ximinez/lending-XLS-66, origin/ximinez/lending-XLS-66) Lending protocol implementation (XLS-0066)
* fb5d94bbef Refactor 4: Transactor extra signing support
* 4fe3ec8a08 Refactor 3: Transactors
* 937b67cbc0 Refactor 2: STParsed Json
* 4e50087612 Refactoring 1
* e514de76ed (origin/develop, origin/HEAD, develop) Add single asset vault (XLS-65d) (#5224)
* dd62cfcc22 fix: Update path in CODEOWNERS (#5440)
* 09690f1b38 (tag: 2.5.0-b1, origin/release) Set version to 2.5.0-b1
* 380ba9f1c1 Fix: Resolve slow test on macOS pipeline (#5392)
* c3e9380fb4 fix: Update validators-example.txt fix xrplf example URL (#5384)
* e3ebc253fa fix: Ensure that coverage file generation is atomic. (#5426)
* c6c7c84355 Configure CODEOWNERS for changes to RPC code (#5266)

Both branches are identical: https://github.com/XRPLF/rippled/compare/f041036df1634afdc89cdba12f0a589ac8463f5f..527e0c916fc31893b476c2cd3bd785e267af4a1c

@Bronek Bronek force-pushed the ximinez/lending-XLS-66 branch from 9d9fe5f to cc83ea8 Compare June 4, 2025 18:36
@Bronek Bronek force-pushed the ximinez/lending-XLS-66 branch from cf704fb to 907cc19 Compare June 5, 2025 12:04
@Bronek
Copy link
Collaborator
Bronek commented Jun 5, 2025

@dangell7 @shawnxie999 I have merged develop at 0a34b5c (ie. with #5060 and #5404 changes) into this open PR, in commit cc83ea8 (the link will show resolution of merge conflicts).

This merge caused massive merge conflicts which I resolved by basically rewrite signature checking, to maintain both the newly added functionality in #5060 (verification of signature of internal transactions) and also the newly added functionality in this PR (support for counterparty signature in LoanSet); while also preserving some of @ximinez refactorings. This particularly shows in STTx.cpp , multisign.h , multisign.cpp and Transactor.cpp in the above listed commit. Given the high severity of any potential bug in these locations, would you mind checking this one commit ? I think I have paid attention to everything but another set of eyes would be really welcome here. @shawnxie999 you might also check how I resolved conflicts from #5404

The counterparty signature in this PR is sfCounterpartySignature, in case if you want to look where it is used / tested.

EDIT: Github does not show consolidated diffs accurately; you will see it better in command line with git show cc83ea8

@Bronek Bronek requested a review from shawnxie999 June 5, 2025 12:24
@ximinez ximinez changed the title DRAFT: Lending Protocol implementation XLS-66 Lending Protocol implementation XLS-66 Jul 2, 2025
@ximinez ximinez marked this pull request as ready for review July 2, 2025 23:04
@ximinez ximinez requested a review from a team as a code owner July 2, 2025 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Amendment DraftRunCI Normally CI does not run on draft PRs. This opts in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0