Taker(api2): support manual maker selection

[Abhay349]

fixes- #695

[stark-3k]

fixes- #695

Can you explain the changes that you made?

[Abhay349]

Thanks for the review!
I’m still learning the coinswap codebase, so I’ll try to explain in simple terms what I changed.
I have added a small optional feature in taker/api2 that allows the taker to manually specify which makers to use for a swap. If this option is not provided, the existing behavior (automatic maker selection from the offerbook) remains exactly the same.

To support this, I have:
-Added an optional manually_selected_makers field to SwapParams

-Added validation so the number of selected makers must match maker_count

-Ensured manually selected makers are present in the offerbook and not marked as bad.

-Reused the existing GetOffer ->SwapDetails -> Ack flow without changing protocol logic.

I tried to keep the change minimal and avoided touching unrelated logic. All builds and tests pass locally.
Please let me know if I misunderstood any part or if you’d like this done differently, I’m happy to update it.

[stark-3k]

I tried to keep the change minimal and avoided touching unrelated logic. All builds and tests pass locally. Please let me know if I misunderstood any part or if you’d like this done differently, I’m happy to update it.

Are you sure that you've run the tests? Because the tests shouldn't compile with the changes that you've made.

[Abhay349]

Sorry for the oversight earlier, I missed updating a few test files initially.
I’ve fixed them now, and all tests are passing.

Thanks!

[stark-3k]

can you also add a test to check manual maker selection?

[Abhay349]

@stark-3k thanks for detailed feedback, I have started working on above issues and will update u ASAP..

[Abhay349]

Hi @stark-3k,
While writing the taproot manual maker selection test, I noticed that MakerAddress is intentionally opaque (there’s no public constructor or FromStr).
I just wanted to confirm the intended approach here: should integration tests first let the offerbook discover the makers and then reuse those MakerAddress values when setting SwapParams.manually_selected_makers, instead of trying to construct the addresses directly?

[stark-3k]

Hi @stark-3k, While writing the taproot manual maker selection test, I noticed that MakerAddress is intentionally opaque (there’s no public constructor or FromStr). I just wanted to confirm the intended approach here: should integration tests first let the offerbook discover the makers and then reuse those MakerAddress values when setting SwapParams.manually_selected_makers, instead of trying to construct the addresses directly?

Yes, the taker first discovers the makers the manual selection happens among the makers present in the offerbook

[Abhay349]

Hi @stark-3k ,
I’ve updated the implementation based on your feedback and added an integration test for manual maker selection (Taproot).
- The test follows the existing api2 flow and uses only public APIs (do_coinswap)
- No direct access to offerbook internals or private methods
- Maker discovery happens via the normal background offer sync
- Ensured makers are funded, create fidelity bonds, and complete the full swap
- manually_selected_makers.len() is validated against maker_count
Happy to iterate on further feedback.

Thanks

[codecov]

Codecov Report

❌ Patch coverage is 76.47059% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.17%. Comparing base (776b75d) to head (a892fd8).
⚠️ Report is 83 commits behind head on master.

Files with missing lines	Patch %	Lines
src/taker/api2.rs	77.38%	19 Missing ⚠️
src/bin/taker.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #697      +/-   ##
==========================================
+ Coverage   68.87%   78.17%   +9.29%     
==========================================
  Files          35       49      +14     
  Lines        4932    15321   +10389     
==========================================
+ Hits         3397    11977    +8580     
- Misses       1535     3344    +1809     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Abhay349]

Closed this PR as this issue can be solved by major refactoring as discuused in meet.