fix: add new fields to `server_definitions` by pdp2121 · Pull Request #3227 · XRPLF/xrpl.js

pdp2121 · 2026-03-12T17:28:40Z

High Level Overview of Change

Next rippled release will introduce new fields to server_definitions (already merged to develop):

TRANSACTION_FORMATS: Describes the fields and their optionality for each transaction type, including common fields shared across all transactions.
LEDGER_ENTRY_FORMATS: Describes the fields and their optionality for each ledger entry type, including common fields shared across all ledger entries.
TRANSACTION_FLAGS: Maps transaction type names to their supported flags and flag values.
LEDGER_ENTRY_FLAGS: Maps ledger entry type names to their flags and flag values.
ACCOUNT_SET_FLAGS: Maps AccountSet flag names (asf flags) to their numeric values.

See feat: Add Formats and Flags to server_definitions

This will allow our CI tests to pass.

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Refactor (non-breaking change that only restructures code)
Tests (You added tests for code that already exists, or your new feature included in this PR)
Documentation Updates
Release

Did you update HISTORY.md?

Yes
No, this change does not impact library users

Test Plan

All CI tests pass.

Future Tasks

Update our scripts to fetch this data from rippled codebase into definitions.json

coderabbitai · 2026-03-12T17:28:58Z

Walkthrough

Added five new fields to the ServerDefinitionsResponse type (flags and formats) and corresponding never guards in the union alternative. Integration tests were expanded with helper assertions to validate the new fields. HISTORY was updated to document the additions.

Changes

Cohort / File(s)	Summary
Type Definition Updates `packages/xrpl/src/models/methods/serverDefinitions.ts`	Added `ACCOUNT_SET_FLAGS`, `LEDGER_ENTRY_FLAGS`, `LEDGER_ENTRY_FORMATS`, `TRANSACTION_FLAGS`, and `TRANSACTION_FORMATS` to the first branch of `ServerDefinitionsResponse`. Added corresponding `...?: never` guards in the other union branch.
Integration Tests `packages/xrpl/test/integration/requests/serverDefinitions.test.ts`	Introduced helper assertion functions and extended the `server_definitions` test to assert presence and structure of the five new fields (`ACCOUNT_SET_FLAGS`, `LEDGER_ENTRY_FLAGS`, `LEDGER_ENTRY_FORMATS`, `TRANSACTION_FLAGS`, `TRANSACTION_FORMATS`).
Changelog `packages/xrpl/HISTORY.md`	Added an "Unreleased" entry documenting the five new `ServerDefinitionsResponse` fields.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

khancode
ckeshava
achowdhry-ripple

Poem

🐰 I hopped through types and found new light,
Flags and formats dancing bright,
Tests check every nested part,
A tiny change from my quick heart,
Cheers to code that hops just right! 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: adding new fields to server_definitions, which matches the primary objective of the PR.
Description check	✅ Passed	The pull request description provides a clear overview of changes, context, and rationale, with proper templating and marked checkboxes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings (stacked PR)
📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix-server-definitions

📝 Coding Plan

Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

🧹 Nitpick comments (1)

packages/xrpl/test/integration/requests/serverDefinitions.test.ts (1)
157-163: Consider updating the "with same hash" test to also check for the absence of the new fields.

For consistency with the type's second union branch (which has never guards for all 10 optional fields), this assertion should verify that the 5 new fields are also absent in the cached response.
Proposed fix
       assert.doesNotHaveAnyKeys(result, [
         'FIELDS',
         'LEDGER_ENTRY_TYPES',
         'TRANSACTION_RESULTS',
         'TRANSACTION_TYPES',
         'TYPES',
+        'ACCOUNT_SET_FLAGS',
+        'LEDGER_ENTRY_FLAGS',
+        'LEDGER_ENTRY_FORMATS',
+        'TRANSACTION_FLAGS',
+        'TRANSACTION_FORMATS',
       ])
Based on learnings: While rippled server behaviors are tested upstream, this change would ensure the test's assertions remain consistent with the updated TypeScript type definitions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/xrpl/test/integration/requests/serverDefinitions.test.ts` around
lines 157 - 163, Update the "with same hash" test assertion that checks cached
responses to also assert the absence of the five new fields: add the new field
names to the assert.doesNotHaveAnyKeys call against the result variable in the
"with same hash" test in serverDefinitions.test.ts so the array currently
containing
'FIELDS','LEDGER_ENTRY_TYPES','TRANSACTION_RESULTS','TRANSACTION_TYPES','TYPES'
is extended to include the five additional optional field names to match the
TypeScript union branch that uses never guards.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/xrpl/test/integration/requests/serverDefinitions.test.ts`:
- Around line 157-163: Update the "with same hash" test assertion that checks
cached responses to also assert the absence of the five new fields: add the new
field names to the assert.doesNotHaveAnyKeys call against the result variable in
the "with same hash" test in serverDefinitions.test.ts so the array currently
containing
'FIELDS','LEDGER_ENTRY_TYPES','TRANSACTION_RESULTS','TRANSACTION_TYPES','TYPES'
is extended to include the five additional optional field names to match the
TypeScript union branch that uses never guards.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d22cc320-50f3-4eb6-b734-e7ab5bb6a939

📥 Commits

Reviewing files that changed from the base of the PR and between 928be9a and f9ef22b.

📒 Files selected for processing (2)

packages/xrpl/src/models/methods/serverDefinitions.ts
packages/xrpl/test/integration/requests/serverDefinitions.test.ts

coderabbitai

🧹 Nitpick comments (1)

packages/xrpl/test/integration/requests/serverDefinitions.test.ts (1)

147-153: Consider adding new fields to the "same hash" test.

The doesNotHaveAnyKeys assertion only checks the original 5 definition fields. For consistency, consider also verifying the new fields are absent when a matching hash is provided:

       assert.doesNotHaveAnyKeys(result, [
         'FIELDS',
         'LEDGER_ENTRY_TYPES',
         'TRANSACTION_RESULTS',
         'TRANSACTION_TYPES',
         'TYPES',
+        'ACCOUNT_SET_FLAGS',
+        'LEDGER_ENTRY_FLAGS',
+        'LEDGER_ENTRY_FORMATS',
+        'TRANSACTION_FLAGS',
+        'TRANSACTION_FORMATS',
       ])

Based on learnings, rippled's behavior is already tested in rippled's own test suite, so this is a minor consistency improvement.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/xrpl/test/integration/requests/serverDefinitions.test.ts` around
lines 147 - 153, Update the assertion in serverDefinitions.test.ts that calls
assert.doesNotHaveAnyKeys(result, [...]) so it also includes any new definition
keys added in this codebase (i.e., extend the array passed to
assert.doesNotHaveAnyKeys to include the additional fields introduced alongside
FIELDS, LEDGER_ENTRY_TYPES, TRANSACTION_RESULTS, TRANSACTION_TYPES, and TYPES);
locate the assertion by the variable name result and the
assert.doesNotHaveAnyKeys call and add the new field names to that array to
ensure they are also verified absent when a matching hash is provided.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/xrpl/test/integration/requests/serverDefinitions.test.ts`:
- Around line 147-153: Update the assertion in serverDefinitions.test.ts that
calls assert.doesNotHaveAnyKeys(result, [...]) so it also includes any new
definition keys added in this codebase (i.e., extend the array passed to
assert.doesNotHaveAnyKeys to include the additional fields introduced alongside
FIELDS, LEDGER_ENTRY_TYPES, TRANSACTION_RESULTS, TRANSACTION_TYPES, and TYPES);
locate the assertion by the variable name result and the
assert.doesNotHaveAnyKeys call and add the new field names to that array to
ensure they are also verified absent when a matching hash is provided.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2dbaedfa-f8e3-4c18-8d8a-484672f3567a

📥 Commits

Reviewing files that changed from the base of the PR and between f9ef22b and 6edf7a7.

📒 Files selected for processing (3)

packages/xrpl/HISTORY.md
packages/xrpl/src/models/methods/serverDefinitions.ts
packages/xrpl/test/integration/requests/serverDefinitions.test.ts

fix server definitions

f9ef22b

coderabbitai bot reviewed Mar 12, 2026

View reviewed changes

add HISTORY.md and fix lint

6edf7a7

coderabbitai bot reviewed Mar 12, 2026

View reviewed changes

pdp2121 requested review from achowdhry-ripple, ckeshava, kuan121 and mvadari March 12, 2026 17:45

kuan121 approved these changes Mar 12, 2026

View reviewed changes

Patel-Raj11 approved these changes Mar 12, 2026

View reviewed changes

pdp2121 merged commit f465d89 into main Mar 12, 2026
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add new fields to `server_definitions`#3227

fix: add new fields to `server_definitions`#3227
pdp2121 merged 2 commits intomainfrom
fix-server-definitions

pdp2121 commented Mar 12, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Mar 12, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

pdp2121 commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

High Level Overview of Change

Type of Change

Did you update HISTORY.md?

Test Plan

Future Tasks

Uh oh!

coderabbitai bot commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pdp2121 commented Mar 12, 2026 •

edited

Loading

coderabbitai bot commented Mar 12, 2026 •

edited

Loading