search

XRPLF / xrpl-py

A Python library to interact with the XRP Ledger (XRPL) blockchain
ISC License
151 stars 86 forks source link

Verifiable Credentials #759

Open ckeshava opened 1 month ago

ckeshava commented 1 month ago

High Level Overview of Change

Python Client SDK implementation for https://github.com/XRPLF/XRPL-Standards/tree/master/XLS-0070d-credentials

Corresponding c++ changes: https://github.com/XRPLF/rippled/pull/5103/files

Context of Change

Following changes are introduced in this PR: (Quoting Verbatim from the XLS specification document)

Type of Change

Did you update CHANGELOG.md?

Test Plan

Note: The integration tests will not pass the CI because we don't have an appropriate docker container with the custom branch. At the moment, integration tests are to be run with custom builds of rippled.

ckeshava commented 1 month ago

@mvadari thanks for the early feedback

coderabbitai[bot] commented 1 month ago

Walkthrough

The pull request introduces significant enhancements to the XRPL framework, focusing on credential management. It includes updates to configuration files, new transaction types, and expanded test coverage for various credential-related transactions. Key changes involve the addition of features in the configuration, new models for credential transactions (create, accept, delete), and comprehensive unit and integration tests to validate these functionalities. The updates ensure that the system can handle credential transactions effectively while maintaining backward compatibility.

Changes

File Path Change Summary
.ci-config/rippled.cfg Added new features: fixAMMv1_1, Credentials, NFTokenMintOffer, fixNFTokenPageLinks, fixInnerObjTemplate2, fixEnforceNFTokenTrustline, fixReducedOffersV2.
tests/integration/transactions/test_credential.py Introduced tests for credential transactions, including creation, acceptance, and deletion, along with a utility function to check credential presence.
tests/integration/transactions/test_deposit_preauth.py Enhanced tests for DepositPreauth, including renaming methods and adding new tests for authorization and credential handling.
tests/unit/models/requests/test_deposit_authorized.py Added a new test class TestDepositAuthorized to validate the DepositAuthorized request.
tests/unit/models/transactions/test_account_delete.py Introduced tests for the AccountDelete transaction model, validating credential ID constraints.
tests/unit/models/transactions/test_credential_accept.py Added tests for the CredentialAccept transaction model, validating both valid and invalid scenarios.
tests/unit/models/transactions/test_credential_create.py Created tests for the CredentialCreate class, focusing on various input validations.
tests/unit/models/transactions/test_credential_delete.py Established tests for the CredentialDelete transaction model, ensuring proper error handling.
xrpl/core/binarycodec/definitions/definitions.json Added new types and fields related to credentials, including transaction types for credential operations.
xrpl/models/requests/account_objects.py Added new enumeration value CREDENTIAL to handle credential objects in account queries.
xrpl/models/requests/deposit_authorized.py Introduced an optional field credentials to the DepositAuthorized class for credential management.
xrpl/models/transactions/__init__.py Imported new credential transaction classes: CredentialAccept, CredentialCreate, CredentialDelete.
xrpl/models/transactions/account_delete.py Enhanced AccountDelete class with a new credential_ids field and improved error handling.
xrpl/models/transactions/credential_accept.py Defined a new model for CredentialAccept transactions with required fields and error handling.
xrpl/models/transactions/credential_create.py Created a model for CredentialCreate transactions, including validation for required fields.
xrpl/models/transactions/credential_delete.py Defined a model for CredentialDelete transactions with necessary validations.
xrpl/models/transactions/deposit_preauth.py Enhanced DepositPreauth class with new credential fields and improved error validation logic.
xrpl/models/transactions/escrow_finish.py Added credential_ids field to EscrowFinish class with validation for associated credentials.
xrpl/models/transactions/payment.py Introduced credential_ids field to Payment class for managing transaction credentials.
xrpl/models/transactions/payment_channel_claim.py Added credential_ids field to PaymentChannelClaim for credential management.
xrpl/models/transactions/types/transaction_type.py Added new transaction types for credential operations: CREDENTIAL_ACCEPT, CREDENTIAL_CREATE, CREDENTIAL_DELETE.
xrpl/models/utils.py Introduced constants and functions for credential validation, enhancing error handling.
xrpl/models/transactions/did_set.py Streamlined imports and clarified error handling in the DIDSet transaction model.
.github/workflows/integration_test.yml Updated Docker image version and modified commands for integration testing workflow.
tests/unit/models/requests/test_ledger_entry.py Added tests for querying credential objects in ledger entries.
xrpl/models/requests/ledger_entry.py Introduced new Credential class and updated LedgerEntry for credential-related queries.
CONTRIBUTING.md Enhanced development setup instructions, particularly for Docker usage in integration tests.

Possibly related PRs

Suggested reviewers

🐰 In the fields where credentials grow,
New features sprout, a vibrant show!
With tests and models, all in line,
The XRPL blooms, oh how divine!
Let's hop along, with joy and cheer,
For every change, we hold so dear! 🌼

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)
🪧 Tips ### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit , please review it.` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` - `@coderabbitai help me debug CodeRabbit configuration file.` Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. ### CodeRabbit Commands (Invoked using PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. ### Other keywords and placeholders - Add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. - Add `@coderabbitai summary` to generate the high-level summary at a specific location in the PR description. - Add `@coderabbitai` anywhere in the PR title to generate the title automatically. ### Documentation and Community - Visit our [Documentation](https://docs.coderabbit.ai) for detailed information on how to use CodeRabbit. - Join our [Discord Community](http://discord.gg/coderabbit) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.
ckeshava commented 3 weeks ago

@mvadari thanks for the feedback. I believe I have addressed all your comments

khancode commented 3 weeks ago

In the PR description under Context of Change, can you list all new transactions and requests added for this feature?

ckeshava commented 2 weeks ago

Regarding the linter failures, I'd prefer to merge this PR first: https://github.com/XRPLF/xrpl-py/pull/742 That should bring around some clarity on the black linter expectations.

mvadari commented 2 weeks ago

The Docker container should be available now, to run integration tests in CI.

ckeshava commented 2 weeks ago

@mDuo13 thanks for pointing out the LedgerEntry RPC. I have included an implementation in this commit: 3814a94

ckeshava commented 1 week ago

@mDuo13 thank you. By Credential faucet, are you referring to tutorials/example-snippets ?

I discovered an unpleasant fact just now. The ledger_entry RPC accepts a Credential input and the DepositPreauth transaction also accepts a Credential inner object. Although both of them have identical names, they have different structures and are used for different purposes.

This does not hamper the usage of both the names, thanks to import aliasing. But if you guys feel strongly, I can re-name the ledger_entry RPC input to a different name.

I don't have the liberty to change the DepositPreauth transaction's input format due to binary-codec restrictions.

mvadari commented 1 week ago

I discovered an unpleasant fact just now. The ledger_entry RPC accepts a Credential input and the DepositPreauth transaction also accepts a Credential inner object. Although both of them have identical names, they have different structures and are used for different purposes.

This does not hamper the usage of both the names, thanks to import aliasing. But if you guys feel strongly, I can re-name the ledger_entry RPC input to a different name.

Doesn't matter too much, since there are other similar issues all over the codebase (e.g. the DepositPreauth transaction and the DepositPreauth ledger object).

ckeshava commented 1 week ago

Doesn't matter too much, since there are other similar issues all over the codebase (e.g. the DepositPreauth transaction and the DepositPreauth ledger object).

okay nice, that's a relief