Repository navigation
Conversation
…ed without it rippled rejects a LoanSet without CounterpartySignature in preflight with temBAD_SIGNER, which xrpl.js surfaced as "No signer may duplicate account or other signers". submitRequest now throws a ValidationError that names CounterpartySignature and signLoanSetByCounterparty 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 (4)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review. WalkthroughSubmission validation now raises ChangesLoanSet Signature Validation
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~12 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The LoanSet submission change is mergeable after normal checks; no unresolved issue is established. 🚥 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 |
There was a problem hiding this comment.
The change adds a client-side pre-check in submitRequest that throws a clear ValidationError when a top-level LoanSet lacks CounterpartySignature and isn't an inner Batch transaction (checked via tfInnerBatchTxn), replacing the opaque temBAD_SIGNER rippled error. The logic correctly mirrors rippled's LoanSet::preflight condition, and the top-level TransactionType check naturally excludes Batch submissions from the new check (since the submitted blob's type is Batch, not LoanSet), consistent with the stated intent that inner LoanSets aren't checked. HISTORY.md is updated, and unit/integration tests cover the unsigned, counterparty-signed, and Batch-inner-LoanSet cases. I did not find correctness, security, or consistency issues in the added code worth flagging.
High Level Overview of Change
submitandsubmitAndWaitnow throw a clearValidationErrorwhen 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 throughsignLoanSetByCounterparty. A developer who forgets the second step and submits gets:rippled returns
temBAD_SIGNERfromLoanSet::preflightwhenCounterpartySignatureis missing outside a Batch (LoanSet.cpp:94-97), and the generic text for that code points at duplicate signers that don't exist. Three teams at the Lending Protocol hackathon lost time to this.submitRequest, which bothsubmitandsubmitAndWaitcall, now throws before sending: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.validate()andWallet.sign()are unchanged: theAccountsigns before the counterparty does, so a LoanSet withoutCounterpartySignatureis valid until it is submitted.Type of Change
One behaviour change: for this input,
submitused to resolve withengine_result: temBAD_SIGNER, and it now rejects without sending anything. rippled rejects such a LoanSet in every case, so no working code path changes.ValidationErrorextendsXrplError, sosubmitAndWaitcallers catchingXrplErrorstill catch it.The existing multi-signing step in
lendingProtocol.test.tsthat assertedtemBAD_SIGNERfor an unsigned-by-counterparty LoanSet now asserts the new error.Did you update HISTORY.md?
Test Plan
Unit tests in
test/client/submit.test.ts:Accountthrows the new error, whether submitted as a blob or signed duringsubmitwith a wallet.CounterpartySignatureis submitted.Integration: the updated step in
test/integration/transactions/lendingProtocol.test.ts.The unit test fails on
mainand passes with the fix. I ran it locally againstrippleci/xrpld:develop --standalone(xrpld 3.4.0-rc1), together with the lending,submit,submitAndWaitand Batch integration suites. Thexrplunit suite (134 suites, 1346 tests) and lint also pass.🤖 Generated with Claude Code