Sponsored Content
Skip to content

feat: Add --definitions flag and artifact - #6858

Merged
bthomee merged 50 commits into
XRPLF:developfrom
pdp2121:add-definitions-artifact
Apr 22, 2026
Merged

feat: Add --definitions flag and artifact#6858
bthomee merged 50 commits into
XRPLF:developfrom
pdp2121:add-definitions-artifact

Conversation

@pdp2121

@pdp2121 pdp2121 commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

High Level Overview of Change

Adds a --definitions CLI flag to xrpld that outputs server definitions as JSON and exits, without starting the server.
CI is updated to export and upload server_definitions.json as a build artifact on every run.

Context of Change

This is particularly valuable for SDK maintainers (e.g. xrpl.js, xrpl-py) who need up-to-date definitions early in a feature's development cycle, before a Docker image is available or a branch is merged to develop.

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)

Before / After

Before: Generating definitions.json for downstream SDKs required either running a live server_definitions RPC call against a running node, or maintaining a custom regex-based script to extract definitions from source.

After: SDK maintainers can build and use --definitions flag to generate the json file, or simply download the server-definitions artifact directly from any CI run.

Test Plan

CI passes and artifact generated.

@pdp2121 pdp2121 changed the title feat: add --definitions flag and artifact feat: Add --definitions flag and artifact Apr 10, 2026
Comment thread src/xrpld/rpc/handlers/server_info/ServerDefinitions.cpp Outdated
@mvadari
mvadari requested a review from mathbunnyru April 10, 2026 21:21
@codecov

codecov Bot commented Apr 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.6%. Comparing base (1c6cdc6) to head (5c177a2).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #6858     +/-   ##
=========================================
- Coverage     82.6%   82.6%   -0.0%     
=========================================
  Files         1010    1010             
  Lines        79566   79568      +2     
  Branches      7540    7532      -8     
=========================================
  Hits         65704   65704             
- Misses       13862   13864      +2     
Files with missing lines Coverage Δ
src/xrpld/app/main/Main.cpp 80.7% <100.0%> (+2.0%) ⬆️
...pld/rpc/handlers/server_info/ServerDefinitions.cpp 98.3% <100.0%> (+<0.1%) ⬆️

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

Comment thread src/xrpld/rpc/handlers/server_info/ServerDefinitions.cpp Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a new way to export XRPL server definitions without running a node, and wires CI to publish those definitions as an artifact for downstream SDK consumers.

Changes:

  • Add --definitions CLI flag to output server definitions JSON and exit.
  • Refactor server_definitions handler to reuse a shared static definitions instance and expose getStaticServerDefinitions().
  • Update CI workflow to generate and upload server_definitions.json as an artifact on a specific public release build.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/xrpld/rpc/handlers/server_info/ServerDefinitions.h Declares getStaticServerDefinitions() for non-RPC consumers.
src/xrpld/rpc/handlers/server_info/ServerDefinitions.cpp Centralizes the static definitions instance and implements getStaticServerDefinitions().
src/xrpld/app/main/Main.cpp Adds --definitions option to emit definitions JSON and exit.
src/test/rpc/ServerDefinitions_test.cpp Adds unit test coverage for getStaticServerDefinitions() and hash consistency with the RPC handler.
.github/workflows/reusable-build-test-config.yml Exports and uploads server_definitions.json artifact in CI for a designated build.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/xrpld/app/main/Main.cpp Outdated
Comment thread .github/workflows/reusable-build-test-config.yml Outdated
@pdp2121
pdp2121 requested a review from mvadari April 13, 2026 15:44
Comment thread .github/workflows/reusable-build-test-config.yml Outdated
Comment thread .github/workflows/reusable-build-test-config.yml Outdated
Comment thread src/xrpld/rpc/handlers/server_info/ServerDefinitions.h Outdated
Comment thread src/xrpld/app/main/Main.cpp Outdated
Comment thread src/test/rpc/ServerDefinitions_test.cpp Outdated
Comment thread src/xrpld/rpc/handlers/server_info/ServerDefinitions.cpp Outdated
@pdp2121
pdp2121 requested a review from mvadari April 20, 2026 17:37
@mvadari

mvadari commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

@pdp2121 is this PR good to go now?

@pdp2121

pdp2121 commented Apr 20, 2026

Copy link
Copy Markdown
Contributor Author

@mvadari Yes it should be. Could you help with merging it? Thank you!

@mathbunnyru mathbunnyru added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label Apr 20, 2026
@mathbunnyru

Copy link
Copy Markdown
Contributor

@mvadari Yes it should be. Could you help with merging it? Thank you!

I added the "Ready to merge" label, which means that next time our authors will be taking a look at what to merge will likely add it to a merge queue.

@bthomee
bthomee added this pull request to the merge queue Apr 22, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 22, 2026
@bthomee
bthomee added this pull request to the merge queue Apr 22, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 22, 2026
@pdp2121

pdp2121 commented Apr 22, 2026

Copy link
Copy Markdown
Contributor Author

There seems to be a flaky test not related to this PR

@mathbunnyru

Copy link
Copy Markdown
Contributor

There seems to be a flaky test not related to this PR

I agree, but it's a bit suspicious it failed 3 times in a row. I restarted the workflow, let's see if it fails again

@pdp2121

pdp2121 commented Apr 22, 2026

Copy link
Copy Markdown
Contributor Author

It passed now. My worry is that it will fail in merge queue since it's flaky.
The flaky test is PerfLog_test
cc @bthomee

@mvadari

mvadari commented Apr 22, 2026

Copy link
Copy Markdown
Contributor

@mathbunnyru it's failing on one of my other PRs as well

@bthomee
bthomee added this pull request to the merge queue Apr 22, 2026
@bthomee

bthomee commented Apr 22, 2026

Copy link
Copy Markdown
Collaborator

@mathbunnyru it's failing on one of my other PRs as well

@mvadari @mathbunnyru I think one of the PRs that was merged today might be to blame. Any thoughts?

Merged via the queue into XRPLF:develop with commit bd1b126 Apr 22, 2026
3 checks passed
marek-foss-neti pushed a commit to marek-foss-neti/rippled that referenced this pull request May 5, 2026
Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com>
@mvadari mvadari added this to the 3.2.0 milestone May 20, 2026
beartec-jpg pushed a commit to beartec-jpg/FalconLedger that referenced this pull request Jun 1, 2026
Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants