refactor: remove legacy DI wrapper signatures (#656 PR A)

[mostronatorcoder]

Context

Issue #656 tracks cleanup tasks after phase-5 DI migration (#639) is complete.

This PR implements PR A from the #656 checklist: removal of legacy entrypoints.

Changes

After phase 5, all handlers have two signatures:

• *_action_with_ctx(ctx, ...) — DI entrypoint ✅
• *_action(pool, ...) — legacy wrapper ❌

This PR:

• ✅ Deletes all *_action(pool, ...) wrappers that only delegate
• ✅ Renames *_action_with_ctx → *_action (removes _with_ctx suffix)
• ✅ Updates dispatcher imports and call sites
• ✅ Fixes test signatures to use AppContext instead of raw pool
• ✅ Updates RPC service to construct AppContext via from_globals()

Files Modified (19 total)

Handlers (16):

• src/app/add_invoice.rs
• src/app/admin_add_solver.rs
• src/app/admin_cancel.rs
• src/app/admin_settle.rs
• src/app/admin_take_dispute.rs
• src/app/cancel.rs
• src/app/dispute.rs
• src/app/fiat_sent.rs
• src/app/last_trade_index.rs
• src/app/order.rs
• src/app/orders.rs
• src/app/rate_user.rs
• src/app/release.rs
• src/app/restore_session.rs
• src/app/take_buy.rs
• src/app/take_sell.rs
• src/app/trade_pubkey.rs

Dispatcher + RPC (2):

• src/app.rs (dispatcher imports and routing)
• src/rpc/service.rs (admin RPC handlers)

Auxiliary:

• Added use sqlx::{Pool, Sqlite}; to internal helper functions where needed

Impact

Before:

pub async fn order_action_with_ctx(ctx: &AppContext, ...) { ... }
pub async fn order_action(pool: &Pool<Sqlite>, ...) { ... }  // wrapper

After:

pub async fn order_action(ctx: &AppContext, ...) { ... }
// no wrapper

Diff stats:

• 19 files changed
• +145 / -263 lines
• Net: -118 lines (removed scaffolding)

Validation

✅ cargo fmt
✅ cargo clippy --all-targets --all-features -- -D warnings (0 warnings)
✅ cargo test --bin mostrod (189 passed, 0 failed)

Related

• Parent cleanup issue: cleanup: remove DI migration scaffolding after phase 5 (#639 follow-up) #656
• Original DI migration: refactor: introduce dependency injection for testability #639 (phases 1-5)

Next Steps

After merge:

• PR B: simplify dispatcher (remove pool parameter)
• PR C: eliminate remaining global accesses in handlers
• PR D (optional): finalize AppContext::from_globals()

Summary by CodeRabbit

Release Notes

• Refactor
  • Internal consolidation of application context handling. Database pool management has been optimized to use a centralized context mechanism, streamlining action handler signatures across the platform. These changes improve code consistency and maintainability without affecting user-facing functionality.

[grunch]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e5780505-a3c5-40f9-8486-b8e25f6be598

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Walkthrough

This PR refactors public action handler signatures across the application to remove explicit pool parameters and eliminate _with_ctx wrapper functions. Handlers now derive the database pool internally from AppContext via ctx.pool(), consolidating pool acquisition and simplifying the public API surface.

Changes

Cohort / File(s)	Summary
Main Application Routing src/app.rs	Updated action routing to call non-ctx handler variants (e.g., order_action instead of order_action_with_ctx). Routes unchanged for NewOrder, TakeSell, TakeBuy, FiatSent, AddInvoice, and other actions; now delegates to refactored handlers without explicit pool parameter.
Admin Action Handlers src/app/admin_add_solver.rs, src/app/admin_cancel.rs, src/app/admin_settle.rs, src/app/admin_take_dispute.rs	Removed wrapper functions and updated public signatures to remove pool parameter. Handlers now obtain pool via ctx.pool() internally; control flow preserved.
User-Facing Action Handlers src/app/add_invoice.rs, src/app/dispute.rs, src/app/fiat_sent.rs, src/app/order.rs, src/app/orders.rs, src/app/restore_session.rs, src/app/trade_pubkey.rs	Removed _with_ctx wrappers and eliminated explicit pool parameters from public function signatures. Pool now derived from context within each handler.
Trading Action Handlers src/app/take_buy.rs, src/app/take_sell.rs, src/app/cancel.rs, src/app/release.rs	Removed wrapper functions; updated signatures to drop pool parameter and obtain it from ctx.pool() internally. Tests migrated to use AppContext.
Utility Handlers src/app/last_trade_index.rs, src/app/rate_user.rs	Refactored to accept AppContext and derive pool internally. Removed _with_ctx wrappers; tests updated to use context-based calls.
RPC Service Integration src/rpc/service.rs	Updated admin action call sites to construct AppContext per-call and pass context to handlers instead of passing self.pool directly. Added #[allow(dead_code)] on pool field in AdminServiceImpl.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• refactor: add AppContext struct for dependency injection (#639 phase 1) #651 — Directly implements the AppContext-based migration by changing action handler signatures to accept AppContext and removing _with_ctx wrappers.
• refactor: wire AppContext into dispatcher and migrate cancel module (#639 phase 2) #652 — Touches the same dispatcher and action-handler wiring for AppContext; this PR continues that migration by removing _with_ctx wrappers across many handlers.
• feat: improved in a more rusty way the connectionto db pool #487 — Related refactor on DB pool acquisition; moves from explicit pool parameters to internally-obtained/global context-based pool resolution.

Suggested reviewers

• arkanoider
• Catrya

Poem

🐰 Wrapper functions fade away,
Pool flows from context, here to stay,
Cleaner signatures, less to pass,
AppContext reigns—refactoring class!
No more _with_ctx, just pure and true,
Mostro's API shines anew! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'refactor: remove legacy DI wrapper signatures' directly and clearly describes the primary change in this PR: systematically removing legacy wrapper functions and renaming DI entrypoints by dropping the _with_ctx suffix.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch cleanup/remove-legacy-wrappers-656

📝 Coding Plan

• Generate coding plan for human review comments

---

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 and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (6)

src/app/release.rs (1)

112-119: ⚠️ Potential issue | 🟡 Minor

Update the release_action docs to match the new parameters.

The signature now takes ctx and no longer takes pool, but the /// # Arguments block above still documents pool and omits ctx.

📝 Suggested doc update

-/// * `msg` - The message containing the release request and associated metadata
-/// * `event` - The unwrapped gift event containing the seller's signature and verification data
-/// * `my_keys` - Mostro node's keys used for signing events and messages
-/// * `pool` - Database connection pool for order updates
+/// * `ctx` - Application context used to resolve shared dependencies like the database pool
+/// * `msg` - The message containing the release request and associated metadata
+/// * `event` - The unwrapped gift event containing the seller's signature and verification data
+/// * `my_keys` - Mostro node's keys used for signing events and messages
 /// * `ln_client` - Lightning network client for invoice settlement

As per coding guidelines, "Document non-obvious public APIs with /// doc comments".

Also applies to: 160-167

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/app/release.rs` around lines 112 - 119, Update the doc comment for the
release_action function to reflect the new parameter list: remove the outdated
`pool` entry and add a description for the `ctx` parameter (the
execution/context object now passed into release_action); update both `/// #
Arguments` blocks that describe release_action (the earlier block and the later
block around the other occurrence) so they list `msg`, `event`, `my_keys`,
`ctx`, and `ln_client` with brief descriptions matching their roles.

src/app/order.rs (1)

56-84: ⚠️ Potential issue | 🟡 Minor

Update the order_action docs to the ctx-based API.

The /// above still lists a pool argument and the example calls the removed order_action(..., &pool) form. Please sync the parameter docs and example with order_action(&ctx, msg, &event, &my_keys).

As per coding guidelines, "Document non-obvious public APIs with /// doc comments".

Also applies to: 85-91

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/app/order.rs` around lines 56 - 84, Update the order_action documentation
and example to reflect the ctx-based API: replace the parameter list and
narrative that mention `pool` with the new signature `order_action(&ctx, msg,
&event, &my_keys)`, update the parameter section to describe `ctx` instead of
`pool`, and modify the example invocation to call `order_action(&ctx, msg,
&event, &my_keys).await?`; ensure references to `msg`, `event`, and `my_keys`
remain correct and adjust any surrounding text that described `pool` to refer to
`ctx` semantics (e.g., database access via ctx).

src/app/rate_user.rs (1)

48-58: ⚠️ Potential issue | 🟡 Minor

Sync the doc comment with the new signature.

The public docs still advertise a pool parameter, but callers now provide ctx: &AppContext. Leaving the old parameter list here points integrators to the removed API.

As per coding guidelines, "Document non-obvious public APIs with /// doc comments".

Also applies to: 71-77

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/app/rate_user.rs` around lines 48 - 58, The doc comment for the public
function rate_user is out of sync: it still documents a removed pool parameter
instead of the current ctx: &AppContext signature; update the triple-slash docs
above rate_user (and the subsequent doc block that mirrors it) to list the
correct parameters (msg, event, my_keys, ctx: &AppContext) and their meanings
and update the Returns section if the return type changed, ensuring the public
API docs accurately reflect the new function signature and not the old pool
parameter.

src/app/last_trade_index.rs (1)

14-29: ⚠️ Potential issue | 🟡 Minor

Update the parameter docs for last_trade_index.

The doc block still documents pool, but this entry point now pulls it from ctx. The public API docs are out of sync with the implementation.

As per coding guidelines, "Document non-obvious public APIs with /// doc comments".

Also applies to: 44-50

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/app/last_trade_index.rs` around lines 14 - 29, The doc comment for the
public function last_trade_index is out of sync: it still documents a `pool`
parameter though the function now obtains the DB pool from `ctx`; update the
`///` parameter docs to remove `pool`, add an entry describing `ctx` (what it
contains, e.g., DB pool and other contextual data), and ensure any other
parameter docs (e.g., `msg`, `event`, `my_keys`) remain accurate; also apply the
same update to the duplicate doc block later (the one covering lines noted in
the review) so the public API docs reflect the current function signature and
data flow.

src/app/cancel.rs (1)

348-360: ⚠️ Potential issue | 🟡 Minor

Remove the stale legacy-wrapper note.

The /// above cancel_action still says callers can use a pool-based legacy wrapper, but this PR deletes that path. That note now points readers to an API that no longer exists.

As per coding guidelines, "Document non-obvious public APIs with /// doc comments".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/app/cancel.rs` around lines 348 - 360, The doc comment for cancel_action
is outdated: remove the sentence saying callers can use the legacy
[`cancel_action`] wrapper and any mention of passing `pool` directly (that API
no longer exists) and instead leave a short, accurate note that this is the
preferred entry point and that the connection pool is extracted from AppContext;
keep the reference to the internal helper cancel_action_generic if desired but
delete the stale legacy-wrapper note.

src/app/trade_pubkey.rs (1)

25-46: ⚠️ Potential issue | 🟠 Major

Stop decrypting master_*_pubkey here.

After the phase-3/4 migration those columns are already stored in plaintext, so CryptoUtils::decrypt_data(...) will reject valid rows or compare against the wrong value. This flow should compare the stored master key directly to event.sender.

🛠️ Proposed fix

-    // Get master keys decrypted
+    // Master keys are stored in plaintext after the phase-3/4 migration.
     let (master_buyer_key, master_seller_key) = if order.master_buyer_pubkey.is_some() {
-        let master_buyer_key = CryptoUtils::decrypt_data(
-            order
-                .get_master_buyer_pubkey(None)
-                .map_err(MostroInternalErr)?
-                .to_string(),
-            None,
-        )
-        .map_err(MostroInternalErr)?;
+        let master_buyer_key = order
+            .get_master_buyer_pubkey(None)
+            .map_err(MostroInternalErr)?
+            .to_string();
         (Some(master_buyer_key), None)
     } else {
-        let master_seller_key = CryptoUtils::decrypt_data(
-            order
-                .get_master_seller_pubkey(None)
-                .map_err(MostroInternalErr)?
-                .to_string(),
-            None,
-        )
-        .map_err(MostroInternalErr)?;
+        let master_seller_key = order
+            .get_master_seller_pubkey(None)
+            .map_err(MostroInternalErr)?
+            .to_string();
         (None, Some(master_seller_key))
     };

Based on learnings, "After Phase 3 is applied, no in-memory decryption fallback is needed for these columns; direct comparison is correct."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/app/trade_pubkey.rs` around lines 25 - 46, The current code decrypts
master_buyer_pubkey/master_seller_pubkey using CryptoUtils::decrypt_data which
is no longer correct after phase-3/4; instead, remove the decryption calls and
simply read order.get_master_buyer_pubkey(None) and
order.get_master_seller_pubkey(None) (handling the Option via map or pattern
match) and assign those plaintext values to master_buyer_key/master_seller_key
(Some(value) or None) so you can directly compare them to event.sender;
eliminate any use of CryptoUtils::decrypt_data in this flow and keep the
existing MostroInternalErr mapping around the get_master_* calls.

🧹 Nitpick comments (2)

src/app/take_sell.rs (1)

206-210: Extract the repeated test-context fixture.

The same TestContextBuilder setup is copied into each async test, so any future pool/settings tweak now needs multiple edits. A small helper would keep these structural tests easier to maintain.

Also applies to: 219-219, 227-231, 239-239, 246-250, 258-258, 267-267, 273-273, 280-284, 292-292, 299-303, 309-309, 314-314

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/app/take_sell.rs` around lines 206 - 210, Extract the repeated
TestContextBuilder setup into a single helper function (e.g., new_test_context
or build_test_context) that accepts the pool (or Arc<Pool>) and returns the
built context; replace each duplicated block that calls
TestContextBuilder::new().with_pool(...).with_settings(test_settings()).build()
with a call to this helper; ensure the helper references TestContextBuilder,
test_settings, and takes/uses the same pool type (std::sync::Arc and
pool.clone() where needed) so all tests use the centralized fixture.

src/app/admin_cancel.rs (1)

16-23: Document the new AppContext-based entrypoint.

This public handler's contract changed substantially, but it still has no /// docs explaining what the caller must provide in ctx or that it mutates DB, LN, and DM state.

📝 Suggested doc stub

+/// Cancels an order from the admin dispute flow using dependencies provided by `ctx`.
+/// The caller must supply an initialized `AppContext`; this handler updates DB state,
+/// may cancel the hold invoice, and notifies the affected peers.
 pub async fn admin_cancel_action(

As per coding guidelines, "Document non-obvious public APIs with `///` doc comments".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/app/admin_cancel.rs` around lines 16 - 23, Add a concise /// doc comment
for the public handler function admin_cancel_action describing the expected
contract: that ctx: &AppContext must provide a populated DB pool/connection and
any needed config/state (e.g., authenticated user/session), what each parameter
represents (msg: Message, event: &UnwrappedGift, my_keys: &Keys, ln_client: &mut
LndConnector), that the function mutates persistent state (database), interacts
with the Lightning node (LN) and the direct message system (DM), the kinds of
side effects performed (cancelling gifts, updating DB rows, sending DM/LN
calls), and the error behavior/return type (Result<(), MostroError>) so callers
know to handle failures and any transactional expectations.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/rpc/service.rs`:
- Around line 22-23: The AdminServiceImpl is ignoring its injected DB pool by
calling AppContext::from_globals()/get_db_pool(); update the code to construct
the AppContext using the instance field pool (Arc<Pool<Sqlite>>) held on self
instead of using AppContext::from_globals(), or alternatively remove the pool
parameter/field from AdminServiceImpl::new and the struct if you intend to rely
on globals; make the same change for all sites that build AppContext (the
occurrences that currently call AppContext::from_globals()/get_db_pool()), so
the admin paths use the injected pool rather than the global.

---

Outside diff comments:
In `@src/app/cancel.rs`:
- Around line 348-360: The doc comment for cancel_action is outdated: remove the
sentence saying callers can use the legacy [`cancel_action`] wrapper and any
mention of passing `pool` directly (that API no longer exists) and instead leave
a short, accurate note that this is the preferred entry point and that the
connection pool is extracted from AppContext; keep the reference to the internal
helper cancel_action_generic if desired but delete the stale legacy-wrapper
note.

In `@src/app/last_trade_index.rs`:
- Around line 14-29: The doc comment for the public function last_trade_index is
out of sync: it still documents a `pool` parameter though the function now
obtains the DB pool from `ctx`; update the `///` parameter docs to remove
`pool`, add an entry describing `ctx` (what it contains, e.g., DB pool and other
contextual data), and ensure any other parameter docs (e.g., `msg`, `event`,
`my_keys`) remain accurate; also apply the same update to the duplicate doc
block later (the one covering lines noted in the review) so the public API docs
reflect the current function signature and data flow.

In `@src/app/order.rs`:
- Around line 56-84: Update the order_action documentation and example to
reflect the ctx-based API: replace the parameter list and narrative that mention
`pool` with the new signature `order_action(&ctx, msg, &event, &my_keys)`,
update the parameter section to describe `ctx` instead of `pool`, and modify the
example invocation to call `order_action(&ctx, msg, &event, &my_keys).await?`;
ensure references to `msg`, `event`, and `my_keys` remain correct and adjust any
surrounding text that described `pool` to refer to `ctx` semantics (e.g.,
database access via ctx).

In `@src/app/rate_user.rs`:
- Around line 48-58: The doc comment for the public function rate_user is out of
sync: it still documents a removed pool parameter instead of the current ctx:
&AppContext signature; update the triple-slash docs above rate_user (and the
subsequent doc block that mirrors it) to list the correct parameters (msg,
event, my_keys, ctx: &AppContext) and their meanings and update the Returns
section if the return type changed, ensuring the public API docs accurately
reflect the new function signature and not the old pool parameter.

In `@src/app/release.rs`:
- Around line 112-119: Update the doc comment for the release_action function to
reflect the new parameter list: remove the outdated `pool` entry and add a
description for the `ctx` parameter (the execution/context object now passed
into release_action); update both `/// # Arguments` blocks that describe
release_action (the earlier block and the later block around the other
occurrence) so they list `msg`, `event`, `my_keys`, `ctx`, and `ln_client` with
brief descriptions matching their roles.

In `@src/app/trade_pubkey.rs`:
- Around line 25-46: The current code decrypts
master_buyer_pubkey/master_seller_pubkey using CryptoUtils::decrypt_data which
is no longer correct after phase-3/4; instead, remove the decryption calls and
simply read order.get_master_buyer_pubkey(None) and
order.get_master_seller_pubkey(None) (handling the Option via map or pattern
match) and assign those plaintext values to master_buyer_key/master_seller_key
(Some(value) or None) so you can directly compare them to event.sender;
eliminate any use of CryptoUtils::decrypt_data in this flow and keep the
existing MostroInternalErr mapping around the get_master_* calls.

---

Nitpick comments:
In `@src/app/admin_cancel.rs`:
- Around line 16-23: Add a concise /// doc comment for the public handler
function admin_cancel_action describing the expected contract: that ctx:
&AppContext must provide a populated DB pool/connection and any needed
config/state (e.g., authenticated user/session), what each parameter represents
(msg: Message, event: &UnwrappedGift, my_keys: &Keys, ln_client: &mut
LndConnector), that the function mutates persistent state (database), interacts
with the Lightning node (LN) and the direct message system (DM), the kinds of
side effects performed (cancelling gifts, updating DB rows, sending DM/LN
calls), and the error behavior/return type (Result<(), MostroError>) so callers
know to handle failures and any transactional expectations.

In `@src/app/take_sell.rs`:
- Around line 206-210: Extract the repeated TestContextBuilder setup into a
single helper function (e.g., new_test_context or build_test_context) that
accepts the pool (or Arc<Pool>) and returns the built context; replace each
duplicated block that calls
TestContextBuilder::new().with_pool(...).with_settings(test_settings()).build()
with a call to this helper; ensure the helper references TestContextBuilder,
test_settings, and takes/uses the same pool type (std::sync::Arc and
pool.clone() where needed) so all tests use the centralized fixture.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 196969f8-9193-4cbe-bf4a-6b4471e0e3da

📥 Commits

Reviewing files that changed from the base of the PR and between 54d053b and 35e3f13.

📒 Files selected for processing (19)

• src/app.rs
• src/app/add_invoice.rs
• src/app/admin_add_solver.rs
• src/app/admin_cancel.rs
• src/app/admin_settle.rs
• src/app/admin_take_dispute.rs
• src/app/cancel.rs
• src/app/dispute.rs
• src/app/fiat_sent.rs
• src/app/last_trade_index.rs
• src/app/order.rs
• src/app/orders.rs
• src/app/rate_user.rs
• src/app/release.rs
• src/app/restore_session.rs
• src/app/take_buy.rs
• src/app/take_sell.rs
• src/app/trade_pubkey.rs
• src/rpc/service.rs

[mostronatorcoder]

Addressed all CodeRabbit feedback in a58682c:

RPC service fix

• ✅ AdminServiceImpl now constructs AppContext using the injected self.pool instead of calling from_globals()
• ✅ Removed #[allow(dead_code)] from pool field (now actively used)
• Applied to all 4 admin methods: call_admin_cancel, call_admin_settle, call_admin_add_solver, call_admin_take_dispute

Documentation updates

Fixed outdated doc comments to reflect current ctx-based API:

• ✅ cancel.rs: removed stale legacy wrapper mention
• ✅ last_trade_index.rs: documented ctx parameter instead of removed pool
• ✅ order.rs: updated params list and example to use &ctx
• ✅ rate_user.rs: documented ctx instead of pool
• ✅ release.rs: documented ctx parameter

All documentation now accurately reflects the current function signatures.

---

Validation:

• cargo clippy --all-targets --all-features -- -D warnings → ✅ 0 warnings

[mostronatorcoder]

Addressed second round of CodeRabbit feedback in ddf3e23:

🐛 Bug fix: trade_pubkey.rs

• ✅ Removed incorrect CryptoUtils::decrypt_data() calls
• ✅ Master keys are already plaintext after phase-3/4 encryption migration
• ✅ Now reads directly from get_master_buyer_pubkey(None) / get_master_seller_pubkey(None)

📝 Nitpick: admin_cancel.rs

• ✅ Added comprehensive doc comment for admin_cancel_action
• Documents parameters, side effects (DB/LN/DM), and error conditions

🧹 Nitpick: take_sell.rs

• ✅ Extracted build_test_context() helper function
• ✅ Replaced 5 duplicated TestContextBuilder setups with helper calls
• Cleaner test code, single source of truth for test context construction

---

Validation:

• cargo clippy --all-targets --all-features -- -D warnings → ✅ 0 warnings

found a bug

[grunch]

tACK