Repository navigation
Conversation
…ed without it
rippled rejects a LoanSet without CounterpartySignature in preflight with
temBAD_SIGNER, whose message ("No signer may duplicate account or other
signers") points at the wrong problem. submit now raises an XRPLException
that names CounterpartySignature and sign_loan_set_by_counterparty
instead. A LoanSet flagged tfInnerBatchTxn is exempt, matching rippled.
DGE-8439
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 (3)
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review. WalkthroughAsync ChangesLoanSet Signature Validation
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Low Suggested reviewers: Merge Risk: ⚪ Minimal · up to The intended submission checks appear consistent with the transaction models. No merge-blocking issue was identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Clean, well-scoped fix: submit now raises a clear XRPLException when a top-level LoanSet lacks CounterpartySignature, mirroring rippled's preflight check and correctly excluding inner Batch transactions via the TF_INNER_BATCH_TXN flag. The added unit tests cover the three relevant cases (missing signature raises, present signature submits, inner Batch LoanSet submits). No correctness, security, or consistency issues found in the changed lines.
High Level Overview of Change
submit, and thereforesubmit_and_waitandsign_and_submit, now raises a clearXRPLExceptionwhen aLoanSetis submitted without the counterparty's signature, instead of passing on rippled's unrelatedtemBAD_SIGNERmessage.Jira: DGE-8439
Context of Change
A LoanSet is signed twice: first by the
Account, then by the counterparty throughsign_loan_set_by_counterparty. A developer who forgets the second step and submits getstemBAD_SIGNERwith rippled's generic text,No signer may duplicate account or other signers, which points at duplicate signers that don't exist. rippled returns it fromLoanSet::preflightwhenCounterpartySignatureis missing outside a Batch (LoanSet.cpp:94-97).submitnow raises before sending anything:The condition mirrors rippled's: a top-level
LoanSetwith noCounterpartySignatureand withouttfInnerBatchTxn. Inner transactions of aBatchare not checked, since rippled lets an inner LoanSet omit the field. Model validation andsignare unchanged, because theAccountsigns before the counterparty does.The same change for xrpl.js is XRPLF/xrpl.js#3532.
Type of Change
For this input,
submitused to return a response withengine_result: temBAD_SIGNER, and it now raises without sending anything. rippled rejects such a LoanSet in every case, so no working code path changes.Did you update CHANGELOG.md?
Test Plan
tests/unit/asyn/transaction/test_submit.py, with the client stubbed:Accountraises the new error and is never sent.CounterpartySignatureis sent.rippleci/xrpld:develop --standalone(xrpld 3.4.0-rc1).mainand passes with the fix.mypy --strictpass.🤖 Generated with Claude Code