test(key-wallet): cover the change-address guard

[ZocoLini]

This started as a fix: set_funding derived a change address unconditionally, discarding one the caller had set and burning a pool address to do it. #925 rewrote that path into add_funding / fund_from four days later and carried the guard with it, so the fix has been in dev since 7 August — which is why there is no production change left here.

What #925 did not bring was a test. The guard reads as a redundant is_none next to an unconditional assignment, and every existing test funds first and sets the address after, an order that never reaches it. Removing it would compile, pass the suite, and silently override the caller's address while advancing the change pool for one that is then
discarded.

Folded into set_funding_skips_reserved_utxos, whose fixture already had what it needed, plus a note on add_funding stating the ordering contract. Verified by removing the guard: the new assertion fails.

Two findings from the earlier review are deliberately not addressed; reasoning in the thread.

Summary by CodeRabbit

• Bug Fixes

  • Preserved explicitly configured change addresses during transaction funding.
  • Prevented unnecessary consumption of pooled change addresses when a change address is already set.
  • Ensured funding uses the caller’s selected change address instead of deriving a different address.

• Documentation

  • Clarified the priority of explicitly configured change addresses during funding.

• Tests

  • Added regression coverage confirming the configured address is preserved and no pooled address is consumed.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 54de23b4-dbe1-4d28-a2f1-630fca7eb076

📥 Commits

Reviewing files that changed from the base of the PR and between 9c89c68 and 8262e04.

📒 Files selected for processing (1)

• key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs

💤 Files with no reviewable changes (1)

• key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Changes

Funding change address

Layer / File(s)	Summary
Change address selection and regression coverage key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs	add_funding preserves an explicit change address and derives one only when none is set. Documentation describes this precedence. A regression test verifies that the change-address pool is not consumed.

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

Merge Risk: ⚪ Minimal · up to 8262e

The change preserves configured change addresses during transaction funding. No concrete merge-blocking risk is identified in the supplied context.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the added regression coverage for the change-address guard. It emphasizes the test rather than the underlying fix, but it remains clearly related to the pull request cha…

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/tx-builder-address

---

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.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs`:
- Around line 127-129: Update TransactionBuilder::set_funding so it does not
call next_change_address or advance the change pool when initializing
change_addr; defer derivation until the build path has the final address, or
otherwise ensure set_change_address explicitly prevents and replaces any derived
address. Preserve the existing behavior for callers that do not provide a change
address, and add regression coverage for
set_funding(...).set_change_address(...).build_unsigned().
- Around line 127-129: Validate explicit change addresses in set_funding by
comparing the address network with funds_acc.network()/acc.network before
preserving change_addr, and return a new BuilderError variant on mismatch. Keep
matching explicit addresses unchanged, update
set_funding_keeps_an_explicit_change_address to use a matching-network
fixture/address, and add a regression test covering rejection of a cross-network
address.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 16ccc41c-dfd2-40f9-a50f-b927f63181a9

📥 Commits

Reviewing files that changed from the base of the PR and between 9cbe4e7 and 9c89c68.

📒 Files selected for processing (1)

• key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.30%. Comparing base (354a1cb) to head (ef0cbe9).

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #920      +/-   ##
==========================================
+ Coverage   77.27%   77.30%   +0.03%     
==========================================
  Files         329      329              
  Lines       83624    83642      +18     
==========================================
+ Hits        64617    64662      +45     
+ Misses      19007    18980      -27     

Flag	Coverage Δ
core	78.25% <ø> (ø)
ffi	51.88% <ø> (ø)
rpc	20.00% <ø> (ø)
spv	92.14% <ø> (+0.13%)	⬆️
wallet	79.80% <100.00%> (+0.01%)	⬆️

Files with missing lines	Coverage Δ
.../wallet/managed_wallet_info/transaction_builder.rs	92.76% <100.00%> (+0.07%)	⬆️

... and 7 files with indirect coverage changes

[github-actions]

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.