Repository navigation
Conversation
…dation gaps interface_to_flag_list encoded any flag name it did not recognise as 0. That included the library's own LoanSetFlag, LoanManageFlag and AMMClawbackFlag names, whose enums were not exported, and the global TF_INNER_BATCH_TXN for every transaction type. A dict flag set to True that the transaction type does not define now raises XRPLModelException (#1016); the three enums are exported and global flags are looked up for every type. LoanSet now rejects the values LoanSet::preflight rejects with temINVALID: a grace_period outside 60..payment_interval (60 if omitted), a payment_total or principal_requested of 0 or less, a loan_origination_fee outside 0..principal_requested, and negative service, late and close payment fees. VaultCreate rejects a withdrawal_policy other than 1. DGE-8226 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 25 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (8)
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.
The diff makes dict-form transaction flags fail closed instead of silently encoding to Flags: 0, exports previously-missing Loan/AMMClawback flag enums, and tightens LoanSet/VaultCreate validation to match rippled's preflight checks. The logic was cross-checked against the extensive new unit tests (bounds for grace_period, principal_requested, loan_origination_fee, fee fields, withdrawal_policy, and flag-name rejection) and is internally consistent: Decimal parsing is used correctly for string amount fields, integer fields are compared directly, the global TransactionFlag set is correctly merged with per-type flags before validation, and the exception messages match what the tests assert. No correctness, security, or resource-leak issues were found in the added/changed lines.
This branch has not been deployed
1 participant
High Level Overview of Change
Truethat the transaction type doesn't define now raises instead of encoding asFlags: 0. The globalTF_INNER_BATCH_TXNnow works in dict form for every transaction type.LoanSetFlag,LoanManageFlagandAMMClawbackFlag(and theirFlagInterfaces) are exported, so their dict-form flags take effect.LoanSetandVaultCreatevalidation now rejects the values rippled rejects in preflight.Jira: DGE-8226
Fixes #1016
Context of Change
Dict-form flags (#1016).
interface_to_flag_listturned any flag name it didn't recognise into0, with no error. This dates from #363. The transaction still validated, signed and submitted, but without the flag the caller asked for. Three things led there:Unexported enums. The flag table is built from the
*Flagenums exported byxrpl.models.transactions.LoanSetFlag,LoanManageFlagandAMMClawbackFlagweren't exported, so even the library's own names were dropped. These calls succeed on the ledger but don't do what the caller asked:LoanManage(flags={"TF_LOAN_DEFAULT": True})Flags: 0tesSUCCESS(LoanManage.cpp:414-423)LoanSet(flags={"TF_LOAN_OVER_PAYMENT": True})Flags: 0AMMClawback(flags={"TF_CLAW_TWO_ASSETS": True})Flags: 0Global flag lookup. The global
TransactionFlagwas never consulted.{"TF_INNER_BATCH_TXN": True}encoded as 0 for every type, andBatch.__post_init__adds exactly that to inner transactions whose flags are a dict.Unrecognised names. Rippled-style names (
tfImmediateOrCancel) and typos also became 0.The table now includes the global flags for every transaction type. A flag set to
Truethat isn't valid for the type raises anXRPLModelExceptionthat lists the valid names. A name set toFalsechanges nothing on the ledger, so it is still ignored. xrpl.js already rejects unknown flag names.LoanSetvalidation now followsLoanSet::preflight, which returnstemINVALIDfor each of these:grace_periodgrace_period≤payment_interval(60 if omitted)grace_period > payment_interval, and only when both were setpayment_totalprincipal_requestedloan_origination_feeprincipal_requestedloan_service_fee,late_payment_fee,close_payment_feeThe amount fields are number strings, so they are compared as
Decimal. A value that doesn't parse as a finite number is left to the existing serialization error. Fractional or NaN values in the integer fields need no new check here, because the model's type check already rejects floats. TheLoanSetchecks date from #866.VaultCreateaccepted anywithdrawal_policy. rippled only accepts 1 (vaultStrategyFirstComeFirstServe) and otherwise returnstemMALFORMED(VaultCreate.cpp:67-71).Trueis rejected too, the same wayvault_kindalready rejects booleans.The xrpl.js half of DGE-8226 is XRPLF/xrpl.js#3533.
Type of Change
Code that passed an unrecognised flag name set to
Trueused to sign a transaction without that flag; it now raises.test_transaction_has_no_flagsasserted the old behaviour (anOfferCancelgivenPaymentChannelClaim'sTF_CLOSEsigned withFlags: 0), so it now expects the error. The stricterLoanSetandVaultCreatechecks only reject values rippled already rejects.Did you update CHANGELOG.md?
Test Plan
test_better_transaction_flags.py:LoanSet,LoanManageandAMMClawbackdict flags resolve to their bits.TF_INNER_BATCH_TXNresolves forPaymentandAccountDelete.Trueraises, both rippled-style and a typo.Falseis ignored.test_loan_set.py: each new bound at and past its edge, andgrace_periodwith and withoutpayment_interval.test_vault_create.py:withdrawal_policyof 0, 2 andTrueis rejected.False, fails onmainand passes with the fix.rippleci/xrpld:develop --standalone(xrpld 3.4.0-rc1). So no existing test relies on an unknown flag name or on a value the new checks reject.mypy --strictpass.🤖 Generated with Claude Code