Skip to content

fix: Check credential for LoanBrokerCoverWithdraw and VaultWithdraw#7107

Open
shawnxie999 wants to merge 14 commits into
XRPLF:developfrom
shawnxie999:fix-sav-cred
Open

fix: Check credential for LoanBrokerCoverWithdraw and VaultWithdraw#7107
shawnxie999 wants to merge 14 commits into
XRPLF:developfrom
shawnxie999:fix-sav-cred

Conversation

@shawnxie999

@shawnxie999 shawnxie999 commented May 11, 2026

Copy link
Copy Markdown
Collaborator

High Level Overview of Change

Context of Change

VaultWithdraw and LoanBrokerCoverWithdraw did not accept sfCredentialIDs, so withdrawals to a destination with lsfDepositAuth and credential-only preauth failed with tecNO_PERMISSION even when the sender held valid credentials.

Fix

Add sfCredentialIDs to both transaction formats and wire up credential validation, following the pattern used by Payment. Gated by fixCleanup3_2_0 (with featureCredentials for the field).

Spec update: XRPLF/XRPL-Standards#538

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)

@shawnxie999 shawnxie999 marked this pull request as ready for review May 11, 2026 14:50
@shawnxie999 shawnxie999 changed the title Fix sav cred fix: Check credential for LoanBrokerCoverWithdraw and VaultWithdraw May 11, 2026
@shawnxie999 shawnxie999 marked this pull request as draft May 11, 2026 14:57
@shawnxie999 shawnxie999 marked this pull request as ready for review May 11, 2026 14:57
@codecov

codecov Bot commented May 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.4%. Comparing base (a389f92) to head (3fe5d43).
⚠️ Report is 83 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #7107     +/-   ##
=========================================
- Coverage     82.4%   82.4%   -0.0%     
=========================================
  Files         1011    1011             
  Lines        76913   76952     +39     
  Branches      8962    8964      +2     
=========================================
+ Hits         63348   63375     +27     
- Misses       13556   13568     +12     
  Partials         9       9             
Files with missing lines Coverage Δ
include/xrpl/ledger/View.h 100.0% <ø> (ø)
include/xrpl/protocol/detail/transactions.macro 100.0% <ø> (ø)
...col_autogen/transactions/LoanBrokerCoverWithdraw.h 100.0% <100.0%> (ø)
...xrpl/protocol_autogen/transactions/VaultWithdraw.h 100.0% <100.0%> (ø)
include/xrpl/tx/transactors/vault/VaultWithdraw.h 100.0% <ø> (ø)
src/libxrpl/ledger/View.cpp 97.4% <100.0%> (+0.6%) ⬆️
...tx/transactors/lending/LoanBrokerCoverWithdraw.cpp 97.0% <100.0%> (+0.2%) ⬆️
src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp 91.3% <100.0%> (+0.5%) ⬆️

... and 6 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.

@github-actions

Copy link
Copy Markdown

This PR has conflicts, please resolve them in order for the PR to be reviewed.

@Tapanito Tapanito self-requested a review May 15, 2026 15:22
@github-actions

Copy link
Copy Markdown

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

@github-actions

Copy link
Copy Markdown

This PR has conflicts, please resolve them in order for the PR to be reviewed.

@github-actions

Copy link
Copy Markdown

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

@ximinez ximinez added this to the 3.3.0 milestone May 20, 2026
@github-actions

Copy link
Copy Markdown

This PR has conflicts, please resolve them in order for the PR to be reviewed.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

@github-actions

Copy link
Copy Markdown

This PR has conflicts, please resolve them in order for the PR to be reviewed.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants