Skip to content

[OpenAPI Codegen] Update scaffolding models from rippled-api-spec v0.1.0-b1 #832

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

pdp2121
Copy link
Collaborator

@pdp2121 pdp2121 commented Apr 14, 2025

This PR includes newly generated scaffolding models and tests based on the rippled-api-spec release v0.1.0-b1.

First Release of OpenAPI Codegen Integration for xrpl-py

Overview

This pull request represents the first release of our OpenAPI code generation effort for scaffolding models and validations in xrpl-py. We invite maintainers and contributors to review the implementation and process, and share feedback so we can continue to refine and improve the workflow.

Source codes

Why OpenAPI?

The OpenAPI and AsyncAPI specifications serve as the single source of truth for the rippled API, covering the JSON-RPC and WebSocket clients, respectively. These specs define all existing endpoints, requests, transactions, flags, error codes, and more.
Our goal is for rippled maintainers to incorporate the specs into their development workflow, enabling downstream consumers—such as client libraries and Clio—to be notified early of changes. This helps ensure consistency across the ecosystem and reduces manual maintenance efforts.

Codegen Workflow

The workflow follows these steps:

  • Spec Update: A rippled developer or spec maintainer adds a feature or fix and updates the spec accordingly.

  • Release: A new version of the spec is published to GitHub.

  • PR Generation: A new pull request is automatically created in xrpl-py as part of CI/CD, reflecting changes to models and tests under the openapi-codegen folder.

  • Review & Integration: xrpl-py maintainers review the changes, make any necessary adjustments, and move the updated code into the main models and tests folders.

Due to the complexity of the rippled API and the need to avoid disrupting ongoing maintenance of xrpl-py, we do not use the default OpenAPI generator directly. Instead, we produce tailored scaffolding code aligned with the existing codebase. This approach allows maintainers to review and incrementally adopt changes while using the tool as both a change notifier and code generator.

Generated Code Structure

Models

  • Request bodies – Note that response models are not strictly typed in the current version of xrpl-py, but this may change in future iterations.

  • Transaction bodies – Including any dependent models such as Currency, XChainBridge, and PriceOracle.

Validations (get_errors methods)

Validation logic is generated based on a predefined set of rules listed in the validation-rules-appendix. We're encouraging rippled maintainers to contribute new, reusable validation rules that apply to multiple transactions or fields. Rules that are overly complex or unique to a single transaction are typically not generated and must be added manually. This limitation stems from the generalization required for automation and the constraints of the OpenAPI specification.

Unit Tests

Unit tests are generated for transactions (not for requests, which are simpler and not heavily validated). Each transaction includes:

  • A successful test case with valid input

  • A test for each constraint defined in the validation appendix

  • Tests for required fields and edge cases (e.g., exceeding maxLength)

  • Recursive validation to ensure constraints on nested objects are also covered

Test cases are named descriptively and are automatically generated to provide meaningful coverage.

Included in This Beta Release

  • 4 request models (remaining requests will be included in a future release)

  • 47 transaction models (pseudo-transactions will be added in the next release)

Known Limitations

  1. The codegen tool is designed to be low-maintenance, requiring updates only for:
  • New validation rules

  • Major structural changes in the rippled API

  1. Default OpenAPI generation is insufficient, as it lacks the flexibility needed to match the xrpl-py structure. Our generator produces customized scaffolding code instead.

  2. Complex static validations that cannot be generalized are not generated and must be manually implemented.

  3. Integration tests are not generated, as each transaction requires a unique setup that cannot be effectively automated.

  4. definitions.json are not included at the moment since its generation is already covered by a separate effort.

Copy link
Contributor

coderabbitai bot commented Apr 14, 2025

Caution

Review failed

Failed to post review comments.

Walkthrough

This change introduces a comprehensive suite of new data models and unit tests for XRPL (XRP Ledger) transaction types, request types, and supporting entities. It adds Python dataclasses and enums representing all major XRPL transaction types (such as payments, offers, escrow, AMM, NFT, credential, cross-chain, and more), their validation logic, and associated flag enumerations. Supporting models for currencies, token amounts, memos, paths, and bridges are included. The request model layer is also implemented, covering account and ledger queries and transaction submission. Extensive unit tests are provided for each transaction type, verifying field constraints, validation logic, and error handling.

Changes

File(s) Change Summary
xrpl/openapi-codegen/models/transactions/*.py, xrpl/openapi-codegen/models/transactions/types/transaction_type.py, xrpl/openapi-codegen/models/transactions/__init__.py Added dataclass models for all XRPL transaction types (e.g., Payment, OfferCreate, EscrowCreate, AMM, NFT, Credential, Cross-chain, etc.), including field definitions, validation logic, and enums for transaction/flag types.
xrpl/openapi-codegen/models/requests/*.py, xrpl/openapi-codegen/models/requests/__init__.py Introduced request models for XRPL API methods (e.g., AccountInfo, AccountChannels, AccountLines, ServerInfo), including base classes, type aliases, and union types for submit modes.
xrpl/openapi-codegen/models/*.py Added supporting models: Currency, IssuedCurrency, XRP, TokenAmount, Memo, PathStep, PriceData, XChainBridge, and credential-related models.
xrpl/openapi-codegen/test/transactions/*.py Added comprehensive unit tests for each transaction model, covering required fields, validation errors, flag logic, and correct construction. Each test asserts correct behavior and error messages.
xrpl/openapi-codegen/models/sign_and_submit_mode_*.py, xrpl/openapi-codegen/models/submit_only_mode.py Added models for transaction signing and submission modes, supporting both sign-and-submit and submit-only workflows.
pyproject.toml, .flake8, mypy.ini Updated coverage, linting, and type-checking configurations to exclude the xrpl/openapi-codegen directory from coverage, linting, and type checks.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant TransactionModel
    participant Validation
    participant Exception

    User->>TransactionModel: Construct transaction with fields
    TransactionModel->>Validation: Validate fields (e.g., required, type, constraints)
    alt Validation passes
        Validation-->>TransactionModel: No errors
        TransactionModel-->>User: Instance created, is_valid() returns True
    else Validation fails
        Validation-->>Exception: Raise XRPLModelException with error message
        Exception-->>User: Exception raised, test asserts error
    end
Loading

Possibly related PRs

  • XRPLF/xrpl-py#759: Introduces credential-related transactions and ledger objects, including CredentialCreate, CredentialAccept, and CredentialDelete, as well as modifications to transactions like AccountDelete to support credentials. The new tests in this PR directly relate to the credential functionality from Credentials #759.
  • XRPLF/xrpl-py#708: Enhances base model validation logic with type checking for model parameters. This PR’s tests for specific transaction models relate at the validation framework level.

Suggested reviewers

  • mvadari
  • khancode

Poem

In the warren of code, a rabbit hopped,
Adding models and tests that never stopped.
Transactions and tokens, credentials anew,
Each field and flag with a test to review.
Now XRPL's logic is covered with care,
Bugs beware—validation is everywhere!
🐇✨

Tip

⚡💬 Agentic Chat (Pro Plan, General Availability)
  • We're introducing multi-step agentic chat in review comments and issue comments, within and outside of PR's. This feature enhances review and issue discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments and add commits to existing pull requests.
✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, 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 generate docstrings to generate docstrings for this 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 for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@pdp2121
Copy link
Collaborator Author

pdp2121 commented Apr 14, 2025

We will fix the workflow to ignore the scaffolding folder for linter so that all checks will pass

@@ -79,6 +79,7 @@ build-backend = "poetry.core.masonry.api"
[tool.coverage.run]
branch = true
source = ["xrpl"]
omit = ["xrpl/openapi-codegen/*"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This won't be needed if the openapi-codegen folder is at the top level

@@ -1,2 +1,2 @@
[mypy]
exclude = dist
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wouldn't exclude the folder, if there's something wrong with the typing then the codegen probably needs to be fixed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes I will fix the CI/CD pipeline on the codegen end to fix linting issues before publishing the code

Comment on lines +10 to +11
exclude =
xrpl/openapi-codegen/*
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wouldn't exclude the folder, if there's something wrong with the typing then the codegen probably needs to be fixed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same here, will fix the pipeline

(Required) The address of the account to authorize.
"""

def _get_errors(self: AuthAccount) -> Dict[str, str]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

_get_errors shouldn't be included if there's nothing to include

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great catch, I think this is a spec error specific to this object (probably we added a validation rule with incorrect name for this object


@require_kwargs_on_init
@dataclass(frozen=True)
class AuthAccount(BaseModel):
Copy link
Collaborator

@mvadari mvadari Apr 15, 2025

Choose a reason for hiding this comment

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

This needs to use NestedModel - all inner object types will need it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agree. Will fix in the next version

@require_kwargs_on_init
@dataclass(frozen=True)
class IssuedCurrency(BaseModel):
currency: Optional[str] = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't all types have a description?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, we will fix after revising the spec

@require_kwargs_on_init
@dataclass(frozen=True)
class Memo(BaseModel):
memo_data: Optional[str] = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: if a type doesn't have a description, can there be a newline between the class line and the first param? Easier to read that way

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agree. But all of them should have a description imo so probably will need to add to the spec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants