Skip to content
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

XLS-70d Credentials #5103

Merged
merged 19 commits into from
Nov 6, 2024
Merged

XLS-70d Credentials #5103

merged 19 commits into from
Nov 6, 2024

Conversation

oleks-rip
Copy link
Collaborator

@oleks-rip oleks-rip commented Aug 23, 2024

High Level Overview of Change

Implementation of Credentials feature. It extend usage of Deposit Authorization
Now account that require pre-authorization can setup DepositPreauth object with allowed credentials. And only accounts that have been authorized by the specified issuer (and get credentials from them) will be allowed to send the payments. Please check XLS-70d for detailed description of the feature.

Context of Change

  • Added new ledger object CREDENTIAL and its transactions - CredentialCreate, CredentialAccept, CredentialDelete
  • DEPOSIT_PREAUTH ledger object updated to be able to use credentials. Updated transactions: DepositPreauth,
  • Updated transactions to use credentials in their parameters: Payment, EscrowFinish, AccountDelete, PaymentChannelClaim

API Impact

  • Public API: deposit_authorized added credentials field.
  • Public API: ledger_entry added authorize_credentials to deposit_preauth field.
  • Public API: ledger_entry added credential parameter.

Copy link

codecov bot commented Aug 23, 2024

Codecov Report

Attention: Patch coverage is 97.15142% with 19 lines in your changes missing coverage. Please review.

Project coverage is 77.8%. Comparing base (c5c0e70) to head (cd9b5c9).
Report is 2 commits behind head on develop.

Files with missing lines Patch % Lines
src/xrpld/app/misc/CredentialHelpers.cpp 93.6% 8 Missing ⚠️
src/xrpld/app/tx/detail/Credentials.cpp 96.7% 6 Missing ⚠️
src/xrpld/app/tx/detail/DepositPreauth.cpp 96.4% 4 Missing ⚠️
src/xrpld/app/tx/detail/PayChan.cpp 93.8% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #5103     +/-   ##
=========================================
+ Coverage     77.7%   77.8%   +0.2%     
=========================================
  Files          779     782      +3     
  Lines        66015   66614    +599     
  Branches      8156    8163      +7     
=========================================
+ Hits         51261   51859    +598     
- Misses       14754   14755      +1     
Files with missing lines Coverage Δ
include/xrpl/protocol/ErrorCodes.h 100.0% <ø> (ø)
include/xrpl/protocol/Feature.h 100.0% <ø> (ø)
include/xrpl/protocol/HashPrefix.h 100.0% <ø> (ø)
include/xrpl/protocol/Indexes.h 100.0% <100.0%> (ø)
include/xrpl/protocol/TER.h 100.0% <ø> (ø)
include/xrpl/protocol/UintTypes.h 100.0% <100.0%> (ø)
include/xrpl/protocol/detail/ledger_entries.macro 100.0% <100.0%> (ø)
include/xrpl/protocol/detail/transactions.macro 100.0% <100.0%> (ø)
src/libxrpl/protocol/ErrorCodes.cpp 71.4% <ø> (ø)
src/libxrpl/protocol/Indexes.cpp 98.0% <100.0%> (+0.1%) ⬆️
... and 21 more

... and 15 files with indirect coverage changes

Impacted file tree graph

@oleks-rip oleks-rip force-pushed the vc_xls70 branch 2 times, most recently from d662782 to eb0ff11 Compare August 23, 2024 20:05
@gregtatcam gregtatcam self-requested a review August 26, 2024 20:42
src/xrpld/app/tx/detail/DepositPreauth.cpp Outdated Show resolved Hide resolved
src/xrpld/app/tx/detail/DepositPreauth.cpp Outdated Show resolved Hide resolved
src/xrpld/app/tx/detail/DeleteAccount.cpp Show resolved Hide resolved
src/xrpld/app/tx/detail/DeleteAccount.cpp Outdated Show resolved Hide resolved
src/xrpld/app/tx/detail/DepositPreauth.cpp Outdated Show resolved Hide resolved
src/xrpld/app/tx/detail/Payment.cpp Show resolved Hide resolved
src/xrpld/app/tx/detail/OnChainCredentials.cpp Outdated Show resolved Hide resolved
src/xrpld/app/tx/detail/OnChainCredentials.cpp Outdated Show resolved Hide resolved
src/xrpld/app/tx/detail/OnChainCredentials.cpp Outdated Show resolved Hide resolved
src/xrpld/app/tx/detail/OnChainCredentials.cpp Outdated Show resolved Hide resolved
src/libxrpl/protocol/Indexes.cpp Show resolved Hide resolved
src/xrpld/app/tx/detail/DeleteAccount.cpp Outdated Show resolved Hide resolved
src/xrpld/app/tx/detail/DeleteAccount.cpp Outdated Show resolved Hide resolved
src/xrpld/app/tx/detail/Escrow.cpp Outdated Show resolved Hide resolved
@oleks-rip oleks-rip force-pushed the vc_xls70 branch 4 times, most recently from d388182 to 003c6ef Compare September 3, 2024 12:49
review fixes
renamed malformedAuthorizedCredentials
Copy link
Collaborator

@gregtatcam gregtatcam left a comment

Choose a reason for hiding this comment

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

👍 LGTM
Nice job on this!

@mvadari
Copy link
Collaborator

mvadari commented Nov 6, 2024

@ximinez this is ready to merge.

Commit message:

Introduce Credentials support (XLS-70d): (#5103)
    
    Amendment:
    - Credentials
    
    New Transactions:
    - CredentialCreate
    - CredentialAccept
    - CredentialDelete
    
    Modified Transactions:
    - DepositPreauth
    - Payment
    - EscrowFinish
    - PaymentChannelClaim
    - AccountDelete
    
    New Object:
    - Credential

    Modified Object:
    - DepositPreauth
    
    API updates:
    - ledger_entry
    - account_objects
    - ledger_data
    - deposit_authorized
    
    Read full spec: https://github.com/XRPLF/XRPL-Standards/tree/master/XLS-0070d-credentials

@ximinez ximinez added the Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required. label Nov 6, 2024
@ximinez ximinez merged commit 8e827e3 into XRPLF:develop Nov 6, 2024
7 of 8 checks passed
This was referenced Nov 6, 2024
PeterChen13579 added a commit to XRPLF/clio that referenced this pull request Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants