-
Notifications
You must be signed in to change notification settings - Fork 106
[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
base: main
Are you sure you want to change the base?
Conversation
Caution Review failedFailed to post review comments. WalkthroughThis 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
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
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
Documentation and Community
|
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/*"] |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
exclude = | ||
xrpl/openapi-codegen/* |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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]: |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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): |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
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 ofxrpl-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
New validation rules
Major structural changes in the rippled API
Default OpenAPI generation is insufficient, as it lacks the flexibility needed to match the xrpl-py structure. Our generator produces customized scaffolding code instead.
Complex static validations that cannot be generalized are not generated and must be manually implemented.
Integration tests are not generated, as each transaction requires a unique setup that cannot be effectively automated.
definitions.json
are not included at the moment since its generation is already covered by a separate effort.