feat: Integrate Permissioned Domain & Credential Checks for Lending Protocol#6517
feat: Integrate Permissioned Domain & Credential Checks for Lending Protocol#6517a1q123456 wants to merge 32 commits into
Conversation
* adds sfMemoData field to VaultDelete transaction
Co-authored-by: Ed Hennis <ed@ripple.com>
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #6517 +/- ##
=======================================
Coverage 82.4% 82.4%
=======================================
Files 1011 1011
Lines 76911 76988 +77
Branches 8965 8965
=======================================
+ Hits 63343 63415 +72
- Misses 13559 13564 +5
Partials 9 9
... and 4 files with indirect coverage changes 🚀 New features to boost your workflow:
|
| return tecOBJECT_NOT_FOUND; // LCOV_EXCL_LINE | ||
| } | ||
|
|
||
| if (auto const ter = credentials::validDomain(ctx.view, *domainID, borrower); |
There was a problem hiding this comment.
@ximinez, I'd like to draw your attention to this code here. The call to validDomain is correct, but the way expired credentials are deleted isn't great. Should we add a call to verifyValidDomain to delete expired credentials?
Edit: I think so.. I don't see the design changing any time soon.
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com>
Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com>
Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com>
Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com>
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
![xrplf-ai-reviewer[bot]](https://avatars.githubusercontent.com/u/67929741?s=60&v=4){kind=small}
There was a problem hiding this comment.
Scanned through the diff
One confirmed copy-paste bug (tfLoanSetMask returned from LoanBrokerSet::getFlagsMask), a TOCTOU inconsistency between preclaim/doApply credential validators, a missing amendment guard in doApply, and two invariant coverage gaps — see inline comments.
Review by ReviewBot 🤖
Review by Claude Opus 4.6 · Prompt: V12
There was a problem hiding this comment.
Several correctness and security issues flagged inline: a reserve-burning usability gap (private broker without DomainID), a missing zero-value guard in the broker creation path, a potential divergence between verifyValidDomain/validDomain semantics, an invariant that passes on zero-valued DomainID, and a weak test assertion. See inline comments for details.
Review by Claude Opus 4.6 · Prompt: V12
|
This PR has conflicts, please resolve them in order for the PR to be reviewed. |
…erimissioned-domain-to-lending
a1q123456
force-pushed
the
a1q123456/adding-perimissioned-domain-to-lending
branch
from
708e4d6 to
2ad2175
Compare
|
All conflicts have been resolved. Assigned reviewers can now start or resume their review. |
a1q123456
changed the title
|
This PR has conflicts, please resolve them in order for the PR to be reviewed. |
3 participants
High Level Overview of Change
Context of Change
XRPLF/XRPL-Standards#484
Type of Change
.gitignore, formatting, dropping support for older tooling)API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)