Skip to content

#50 Auto-resolve Liquid asset ID in sideshift send#87

Merged
marinate305 merged 7 commits into
developfrom
50-sideshift-requires-asset-id
Jun 4, 2026
Merged

#50 Auto-resolve Liquid asset ID in sideshift send#87
marinate305 merged 7 commits into
developfrom
50-sideshift-requires-asset-id

Conversation

@andycreed0x

Copy link
Copy Markdown
Collaborator

Purpose

Auto-resolve the Liquid asset ID from the deposit coin ticker in sideshift_send, eliminating the UX friction where non-L-BTC Liquid assets (e.g. USDt) required manually passing a 64-char hex --liquid-asset-id.

Closes #50 sideshift send: requires --liquid-asset-id manually for non-L-BTC assets

Main Changes

  • ✨ Added resolve_liquid_asset_id() in assets.py — single source of truth that maps a (coin, network) pair to a Liquid asset hex ID, short-circuits to None for non-Liquid networks and L-BTC, raises ValueError with actionable known-ticker list for unknown tickers, and passes explicit overrides through unchanged
  • ♻️ Refactored tools.sideshift_send to call resolve_liquid_asset_id before contacting the SideShift manager — unknown tickers fail fast before any custodial order is created
  • ♻️ Removed duplicate inline auto-resolve logic from cli/sideshift.py (now delegates to the shared helper)
  • 📝 Updated MCP tool schema description and sideshift_send prompt — AI assistants no longer need to look up liquid_asset_id manually via lw_list_assets
  • ✅ Added TestResolveLiquidAssetId (10 cases) in test_assets.py and TestToolsSideShiftSendAutoResolve (5 cases) in test_sideshift.py

Checklist

  • No hardcoded values (they should go in constants.py, .env, or our database)
  • Added/updated tests (if necessary)
  • Added/updated relevant documentation (if necessary)

@andycreed0x andycreed0x requested a review from marinate305 June 1, 2026 18:56
@marinate305

marinate305 commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Checks are failing after PR #86 merged — looks like the branch needs to be rebased onto develop to pick up the is_mnemonic_encrypted → requires_user_password rename. Happy to re-review once the tests are green.

@marinate305 marinate305 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are green and conflicts resolved. Approving.

@marinate305 marinate305 merged commit 0d37c01 into develop Jun 4, 2026
4 checks passed
@marinate305 marinate305 deleted the 50-sideshift-requires-asset-id branch June 4, 2026 02:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants