XRPLF
DRAFT: Lending Protocol implementation XLS-66
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)
- [ ]
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl) - [ ] Peer protocol change (must be backward compatible or bump the peer protocol version)
Codecov Report
:x: Patch coverage is 90.63685% with 247 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 79.0%. Comparing base (c9f17dd) to head (3ca1593).
:warning: Report is 1 commits behind head on develop.
Additional details and impacted files
@@ Coverage Diff @@
## develop #5270 +/- ##
=========================================
+ Coverage 78.6% 79.0% +0.5%
=========================================
Files 818 839 +21
Lines 68938 71377 +2439
Branches 8240 8339 +99
=========================================
+ Hits 54177 56422 +2245
- Misses 14761 14955 +194
| Files with missing lines | Coverage Δ | |
|---|---|---|
| include/xrpl/basics/Number.h | 100.0% <ø> (ø) |
|
| include/xrpl/json/json_value.h | 98.5% <ø> (ø) |
|
| include/xrpl/ledger/ApplyView.h | 100.0% <ø> (ø) |
|
| include/xrpl/ledger/View.h | 100.0% <100.0%> (ø) |
|
| include/xrpl/protocol/Asset.h | 96.3% <100.0%> (+0.8%) |
:arrow_up: |
| 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.7% <100.0%> (+1.2%) |
:arrow_up: |
| include/xrpl/protocol/STObject.h | 93.1% <ø> (ø) |
|
| ... and 50 more |
... and 13 files with indirect coverage changes
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
Merge memo: history rewritten and base branch changed to develop after #5224 has been merged to develop
Initial stage of reconciliation in commit https://github.com/XRPLF/rippled/commit/f041036df1634afdc89cdba12f0a589ac8463f5f
(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 https://github.com/XRPLF/rippled/commit/527e0c916fc31893b476c2cd3bd785e267af4a1c
(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
@dangell7 @shawnxie999 I have merged develop at 0a34b5c691 (ie. with #5060 and #5404 changes) into this open PR, in commit https://github.com/XRPLF/rippled/pull/5270/commits/cc83ea8eb6618ee65c18067bd2ac3ddf61e8cecc (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
There are build errors in the latest commit:
/User/rippled/src/test/app/Loan_test.cpp:1816:25: error: use of undeclared identifier 'json' 1816 | createJson, json(sfCounterpartySignature, Json::objectValue)); | ^ /User/rippled/src/test/app/Loan_test.cpp:1829:25: error: use of undeclared identifier 'json' 1829 | createJson, json(sfCounterpartySignature, counterpartyJson)); | ^
There are build errors in the latest commit:
/User/rippled/src/test/app/Loan_test.cpp:1816:25: error: use of undeclared identifier 'json' 1816 | createJson, json(sfCounterpartySignature, Json::objectValue)); | ^ /User/rippled/src/test/app/Loan_test.cpp:1829:25: error: use of undeclared identifier 'json' 1829 | createJson, json(sfCounterpartySignature, counterpartyJson)); | ^
Unity build strikes again. I'll have it fixed ASAP.
Hi @ximinez
The PR does not build due to the unused variable error. Could you please take a look?
1136 | auto const& fields = getPseudoAccountFields();
Details here
I don't normally rewrite history on a non-draft PR, but the tip had a build error, and I want to provide a single commit as the resolution for the issue it's addressing. For reference:
$ git diff 1bf68f7 1628756
diff --git a/src/xrpld/app/misc/LendingHelpers.h b/src/xrpld/app/misc/LendingHelpers.h
index ee921851bc..022e9557e9 100644
--- a/src/xrpld/app/misc/LendingHelpers.h
+++ b/src/xrpld/app/misc/LendingHelpers.h
@@ -278,7 +278,7 @@ computePeriodicPaymentParts(
if (roundedPayment > interest)
return roundedPayment;
auto newPayment = roundedPayment;
- if (asset.native() || !asset.holds<Issue>())
+ if (asset.native() || !asset.template holds<Issue>())
{
// integral types, just add one
++newPayment;
There are some test failures in the latest commit. That's because I added support for an extra command line option, and didn't update the corresponding tests in RPCCall.
I'll have to update the tests when I get back, but the failures are safe to ignore for now.
There are errors in Vault_test:
#70 failed: Vault_test.cpp(3506)
ripple.app.Vault Scale redeem exact
#7 failed: Vault_test.cpp(3531)
ripple.app.Vault Scale redeem with rounding
#9 failed: Vault_test.cpp(3566)
ripple.app.Vault Scale redeem exact
#7 failed: Vault_test.cpp(3593)
ripple.app.Vault Scale redeem rest
ripple.app.Vault Scale withdraw overflow
#66 failed: Vault_test.cpp(3654)
ripple.app.Vault Scale withdraw exact
#7 failed: Vault_test.cpp(3682)
ripple.app.Vault Scale withdraw insignificant amount
ripple.app.Vault Scale withdraw with rounding assets
#9 failed: Vault_test.cpp(3729)
ripple.app.Vault Scale withdraw with rounding shares up
#7 failed: Vault_test.cpp(3758)
ripple.app.Vault Scale withdraw with rounding shares down
#7 failed: Vault_test.cpp(3787)
ripple.app.Vault Scale withdraw tiny amount
#7 failed: Vault_test.cpp(3810)
ripple.app.Vault Scale withdraw rest
ripple.app.Vault Scale clawback overflow
#66 failed: Vault_test.cpp(3873)
ripple.app.Vault Scale clawback exact
#7 failed: Vault_test.cpp(3901)
ripple.app.Vault Scale clawback insignificant amount
ripple.app.Vault Scale clawback with rounding assets
#7 failed: Vault_test.cpp(3941)
ripple.app.Vault Scale clawback with rounding shares up
#7 failed: Vault_test.cpp(3971)
ripple.app.Vault Scale clawback with rounding shares down
#7 failed: Vault_test.cpp(4001)
ripple.app.Vault Scale clawback tiny amount
#7 failed: Vault_test.cpp(4025)
There are build errors in the latest changes:
In file included from /Users/user/Work/Projects/rippled/build/CMakeFiles/rippled.dir/Unity/unity_21_cxx.cxx:34:
In file included from /Users/user/Work/Projects/rippled/src/test/app/Loan_test.cpp:22:
/Users/user/Work/Projects/rippled/src/xrpld/app/misc/LendingHelpers.h:328:15: error: 'ValueProxy' is a private member of 'ripple::STObject'
328 | STObject::ValueProxy<STNumber>& principalOutstandingField,
| ^
/Users/user/Work/Projects/rippled/build/modules/xrpl.libxrpl.protocol/xrpl/protocol/STObject.h:545:17: note: declared private here
545 | class STObject::ValueProxy : public Proxy<T>
| ^
In file included from /Users/user/Work/Projects/rippled/build/CMakeFiles/rippled.dir/Unity/unity_21_cxx.cxx:34:
In file included from /Users/user/Work/Projects/rippled/src/test/app/Loan_test.cpp:22:
/Users/user/Work/Projects/rippled/src/xrpld/app/misc/LendingHelpers.h:329:15: error: 'ValueProxy' is a private member of 'ripple::STObject'
329 | STObject::ValueProxy<STUInt32>& paymentRemainingField,
| ^
/Users/user/Work/Projects/rippled/build/modules/xrpl.libxrpl.protocol/xrpl/protocol/STObject.h:545:17: note: declared private here
545 | class STObject::ValueProxy : public Proxy<T>
| ^
In file included from /Users/user/Work/Projects/rippled/build/CMakeFiles/rippled.dir/Unity/unity_21_cxx.cxx:34:
In file included from /Users/user/Work/Projects/rippled/src/test/app/Loan_test.cpp:22:
/Users/user/Work/Projects/rippled/src/xrpld/app/misc/LendingHelpers.h:330:15: error: 'ValueProxy' is a private member of 'ripple::STObject'
330 | STObject::ValueProxy<STUInt32>& prevPaymentDateField,
| ^
/Users/user/Work/Projects/rippled/build/modules/xrpl.libxrpl.protocol/xrpl/protocol/STObject.h:545:17: note: declared private here
545 | class STObject::ValueProxy : public Proxy<T>
| ^
In file included from /Users/user/Work/Projects/rippled/build/CMakeFiles/rippled.dir/Unity/unity_21_cxx.cxx:34:
In file included from /Users/user/Work/Projects/rippled/src/test/app/Loan_test.cpp:22:
/Users/user/Work/Projects/rippled/src/xrpld/app/misc/LendingHelpers.h:331:15: error: 'ValueProxy' is a private member of 'ripple::STObject'
331 | STObject::ValueProxy<STUInt32>& nextDueDateField,
| ^
/Users/user/Work/Projects/rippled/build/modules/xrpl.libxrpl.protocol/xrpl/protocol/STObject.h:545:17: note: declared private here
545 | class STObject::ValueProxy : public Proxy<T>
| ^
4 errors generated.
[69/86] Building CXX object CMakeFiles/rippled.dir/Unity/unity_7_cxx.cxx.o
In file included from /Users/user/Work/Projects/rippled/build/CMakeFiles/rippled.dir/Unity/unity_7_cxx.cxx:19:
In file included from /Users/user/Work/Projects/rippled/src/xrpld/app/tx/detail/LoanBrokerCoverClawback.cpp:22:
/Users/user/Work/Projects/rippled/src/xrpld/app/misc/LendingHelpers.h:328:15: error: 'ValueProxy' is a private member of 'ripple::STObject'
328 | STObject::ValueProxy<STNumber>& principalOutstandingField,
| ^
/Users/user/Work/Projects/rippled/build/modules/xrpl.libxrpl.protocol/xrpl/protocol/STObject.h:545:17: note: declared private here
545 | class STObject::ValueProxy : public Proxy<T>
| ^
In file included from /Users/user/Work/Projects/rippled/build/CMakeFiles/rippled.dir/Unity/unity_7_cxx.cxx:19:
In file included from /Users/user/Work/Projects/rippled/src/xrpld/app/tx/detail/LoanBrokerCoverClawback.cpp:22:
/Users/user/Work/Projects/rippled/src/xrpld/app/misc/LendingHelpers.h:329:15: error: 'ValueProxy' is a private member of 'ripple::STObject'
329 | STObject::ValueProxy<STUInt32>& paymentRemainingField,
| ^
/Users/user/Work/Projects/rippled/build/modules/xrpl.libxrpl.protocol/xrpl/protocol/STObject.h:545:17: note: declared private here
545 | class STObject::ValueProxy : public Proxy<T>
| ^
In file included from /Users/user/Work/Projects/rippled/build/CMakeFiles/rippled.dir/Unity/unity_7_cxx.cxx:19:
In file included from /Users/user/Work/Projects/rippled/src/xrpld/app/tx/detail/LoanBrokerCoverClawback.cpp:22:
/Users/user/Work/Projects/rippled/src/xrpld/app/misc/LendingHelpers.h:330:15: error: 'ValueProxy' is a private member of 'ripple::STObject'
330 | STObject::ValueProxy<STUInt32>& prevPaymentDateField,
| ^
/Users/user/Work/Projects/rippled/build/modules/xrpl.libxrpl.protocol/xrpl/protocol/STObject.h:545:17: note: declared private here
545 | class STObject::ValueProxy : public Proxy<T>
| ^
In file included from /Users/user/Work/Projects/rippled/build/CMakeFiles/rippled.dir/Unity/unity_7_cxx.cxx:19:
In file included from /Users/user/Work/Projects/rippled/src/xrpld/app/tx/detail/LoanBrokerCoverClawback.cpp:22:
/Users/user/Work/Projects/rippled/src/xrpld/app/misc/LendingHelpers.h:331:15: error: 'ValueProxy' is a private member of 'ripple::STObject'
331 | STObject::ValueProxy<STUInt32>& nextDueDateField,
| ^
/Users/user/Work/Projects/rippled/build/modules/xrpl.libxrpl.protocol/xrpl/protocol/STObject.h:545:17: note: declared private here
545 | class STObject::ValueProxy : public Proxy<T>
| ^
4 errors generated.
[74/86] Building CXX object CMakeFiles/rippled.dir/Unity/unity_5_cxx.cxx.o
Build fails:
[76/86] Building CXX object CMakeFiles/rippled.dir/Unity/unity_3_cxx.cxx.o
FAILED: CMakeFiles/rippled.dir/Unity/unity_3_cxx.cxx.o
In file included from /Users/user/Work/Projects/rippled-landing/build/CMakeFiles/rippled.dir/Unity/unity_3_cxx.cxx:22:
/Users/user/Work/Projects/rippled-landing/src/xrpld/app/misc/detail/LendingHelpers.cpp:30:22: error: no member named 'isEnabled' in 'ripple::VaultCreate'
30 | VaultCreate::isEnabled(ctx);
| ~~~~~~~~~~~~~^
1 error generated.
[84/86] Building CXX object CMakeFiles/rippled.dir/Unity/unity_36_cxx.cxx.o
ninja: build stopped: subcommand failed.
Missing tests or should add LCOV_EXCL_LINE
@gregtatcam I have a task to add more test coverage. Most of those items can be tested, they just aren't yet.
@dangell7 I've addressed your comments. Could you re-review ASAP, so I can resolve the issue with unsigned commits, and get this thing merged?
Commit message:
Implement Lending Protocol (unsupported) (#5270)
- Spec: XLS-66
- Introduces amendment "LendingProtocol", but leaves it UNSUPPORTED to
allow for standalone testing, future development work, and potential
bug fixes.
- AccountInfo RPC will indicate the type of pseudo-account when
appropriate.
- Refactors and improves several existing classes and functional areas,
including Number, STAmount, STObject, json_value, Asset, directory
handling, View helper functions, and unit test helpers.