Repository navigation
Conversation
rippled declares these fields soeDEFAULT, so it leaves them out of the JSON when their value is 0, but xrpl.js typed them as always present: MPToken.MPTAmount, LoanBroker.DebtMaximum, AMM.TradingFee and the TradingFee / DiscountedFee of its vote and auction slots, and Loan's PaymentRemaining, PrincipalOutstanding, TotalValueOutstanding, NextPaymentDueDate and GracePeriod. Code such as `loan.PaymentRemaining === 0` type-checked but was never true. Integration tests now assert each field is absent once it is 0, where the existing flows can reach that state. DGE-8229, #3154 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (8)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review. WalkthroughSeveral ledger model fields are now optional because rippled can omit them when their value is zero. Integration tests check selected omissions for AMM, Loan, LoanBroker, and MPToken ledger entries. ChangesLedger field optionality
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to This change marks ledger fields that rippled omits when their value is 0 as optional in the type definitions. Runtime behavior does not change. Code that reads these fields as always present may need to handle undefined, and the changelog documents this. Merge risk is minimal. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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. Comment |
![xrplf-ai-reviewer[bot]](https://avatars.githubusercontent.com/u/67929741?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is a clean, well-scoped type-correctness fix: ten ledger-object fields that rippled genuinely omits when their value is 0 are marked optional in the TypeScript models (AMM, Loan, LoanBroker, MPToken), matching rippled's actual JSON output. The accompanying integration-test updates correctly add assertions that these fields are undefined once the ledger reaches a zero-value state, and existing reads of the now-optional fields are updated with narrowing casts in places where the test context guarantees the value is still non-zero. I didn't find any lines in the diff that misuse the new optionality, introduce incorrect assertions, or regress runtime behavior — the change is type-only as described, and the test plan corroborates it against a live node. No correctness, security, or consistency issues worth flagging in the changed lines.
This branch has not been deployed
1 participant
High Level Overview of Change
Marks the 10 ledger object fields that rippled omits when their value is 0 as optional.
Jira: DGE-8229
Fixes #3154
Context of Change
rippled declares these fields
SoeDefaultinledger_entries.macroandInnerObjectFormats.cpp. When the value is 0, the field is absent from the object's JSON. xrpl.js typed them as always present, so code likeloan.PaymentRemaining === 0type-checked but was never true on a repaid loan, because the field isn't there.MPTokenMPTAmountLoanBrokerDebtMaximumAMMTradingFee,VoteSlots[].VoteEntry.TradingFee,AuctionSlot.DiscountedFeeLoanPaymentRemaining,PrincipalOutstanding,TotalValueOutstanding,NextPaymentDueDate,GracePeriodThat is every
SoeDefaultfield that xrpl.js still typed as required. The others are already optional, as #3153 did forVault. No RPC response model needs the same change:amm_infoalways writestrading_feeanddiscounted_fee, as 0 when the ledger field is absent (AMMInfo.cpp:201-228).Type of Change
This changes types only; runtime behaviour is the same. TypeScript code that reads these fields with
strictNullChecksmust now handleundefined, as six lines inlendingProtocol.test.tshad to. That matches what rippled returns, and it is the same kind of change as #3153.Did you update HISTORY.md?
Test Plan
Integration tests now check, against a real node, that each field is absent once its value is 0. The checks use existing flows where they reach that state:
lendingProtocol.test.ts:TotalValueOutstandingorNextPaymentDueDate. It already asserted the same forPrincipalOutstandingandPaymentRemaining.DebtMaximum: '0'has noDebtMaximum.mptokenAuthorize.test.ts: a newly authorizedMPTokenwith a zero balance has noMPTAmount.ammVote.test.ts: once every LP votes 0, the AMM has noTradingFee, and its auction slot and vote entries have no fee fields.Loan.GracePeriodgets no integration check, becauseLoanSetrequires at least 60, so a real loan never stores 0.All pass against
rippleci/xrpld:develop --standalone(xrpld 3.4.0-rc1). Thexrplunit suite (134 suites, 1343 tests) and lint pass, andtscoversrcandtestreports no new errors.🤖 Generated with Claude Code