fix: accept a multisigned Account in the LoanSet counterparty helpers

[kuan121]

High Level Overview of Change

sign_loan_set_by_counterparty and combine_loanset_counterparty_signers now accept a LoanSet whose Account is multisigned.

Jira: DGE-8438

Context of Change

Both helpers refuse to add the counterparty's signature until the Account has signed, and they detected that with txn_signature. A multisigned transaction has an empty signing_pub_key and a signers list but no txn_signature, so a loan broker whose account is protected by a signer list got Transaction must be first signed by first party. for a fully signed LoanSet, and could not originate a loan.

XLS-66 allows Signers on the initiating side (§3.8.3), and rippled checks the two signatures independently (LoanSet::checkSign).

The helpers now treat the Account as signed when it has a txn_signature or a non-empty signers list. The old signing_pub_key is None check is dropped: Transaction.signing_pub_key defaults to "", so it was never None. Nothing else changes: the counterparty signs the same fields as before, which include the Account's (empty) SigningPubKey and exclude Signers. A LoanSet the Account has not signed is still rejected.

The check dates from #900, which added the helpers. xrpl.js has the same bug and is fixed in XRPLF/xrpl.js#3531.

Type of Change

• Bug fix (non-breaking change which fixes an issue)

Did you update CHANGELOG.md?

• Yes

Test Plan

• Unit tests in tests/unit/transaction/test_counterparty_signer.py, on a LoanSet multisigned by its Account:
  • A single-key counterparty signs it.
  • Two counterparty signers sign it and are combined.
  • In both cases the Account's signers are kept, and each counterparty signature verifies against the counterparty signing bytes.
  • A LoanSet whose Account has not signed still raises in both helpers.
• Integration test test_loan_set_txn_multisigned_loan_broker in tests/integration/transactions/test_lending_protocol.py: a loan broker with a 2-of-2 signer list originates a loan to a borrower with a 2-of-2 signer list, through all four client types, and rippled creates the Loan.
• Other tests:
  • The new unit and integration tests fail on main with Transaction must be first signed by first party. and pass with the fix.
  • I ran them locally against rippleci/xrpld:develop --standalone (xrpld 3.4.0-rc1) with this repo's .ci-config. The full unit suite, the lending integration suite (8 tests), flake8 and mypy --strict pass.

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: XRPLF/xrpl-py/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 3a8e8c48-e083-4a5d-b760-f845cd8d8b41

📥 Commits

Reviewing files that changed from the base of the PR and between 7a81134 and faf962a.

📒 Files selected for processing (4)

• CHANGELOG.md
• tests/integration/transactions/test_lending_protocol.py
• tests/unit/transaction/test_counterparty_signer.py
• xrpl/transaction/counterparty_signer.py

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

---

Walkthrough

The counterparty signing helpers now accept a LoanSet signed by the first party through a nonempty signer list. New unit and integration tests cover signature retention, missing signatures, and loan creation with a loan broker.

Changes

LoanSet counterparty signing

Layer / File(s)	Summary
First-party signature validation xrpl/transaction/counterparty_signer.py, tests/unit/transaction/test_counterparty_signer.py	Both helpers accept a transaction signature or a nonempty signer list as proof of first-party signing. Unit tests check single-key and multisigned counterparty signatures, retained signer data, and errors when the first party has not signed.
Multisigned LoanSet integration tests/integration/transactions/test_lending_protocol.py, CHANGELOG.md	The integration test submits a LoanSet multisigned by the issuer and signed by the borrower’s counterparties, then checks the accepted loan references the created loan broker. The changelog records the helper support and prior error.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: pdp2121

Merge Risk: ⚪ Minimal · up to faf96

The LoanSet signing change is mergeable after normal checks; no PR-introduced issue remains identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main fix: support multisigned Accounts in the LoanSet counterparty helpers.
Description check	✅ Passed	The description covers the change, context, bug behavior, affected helpers, bug-fix classification, changelog update, linked issue, and detailed unit and integration test plans. It is complete and ali…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[xrplf-ai-reviewer]

Clean, well-scoped bug fix: _is_signed_by_first_party now checks txn_signature is not None or bool(signers) instead of relying on signing_pub_key is None (which could never be true). Both call sites (sign_loan_set_by_counterparty and combine_loanset_counterparty_signers) were updated consistently, the dropped signing_pub_key is None check was correctly identified as dead code, and the new unit/integration tests cover the single-sign, multi-sign+combine, and still-unsigned-account rejection paths. No correctness, security, or consistency issues found in the added lines.