Skip to content

[SIDESWAP] Fix USDt to LBTC atomic swap - dealer's suggested recv_amount#83

Open
coelhogonzalo wants to merge 2 commits into
developfrom
66-sideswap-fix
Open

[SIDESWAP] Fix USDt to LBTC atomic swap - dealer's suggested recv_amount#83
coelhogonzalo wants to merge 2 commits into
developfrom
66-sideswap-fix

Conversation

@coelhogonzalo

@coelhogonzalo coelhogonzalo commented May 27, 2026

Copy link
Copy Markdown
Collaborator

closes #66 for more details check the ticket

image image image

@coelhogonzalo coelhogonzalo changed the title 66 sideswap fix [SIDESWAP] Fix USDt to LBTC atomic swap - dealer's suggested recv_amount May 27, 2026
@coelhogonzalo coelhogonzalo changed the base branch from main to develop May 27, 2026 18:11
@coelhogonzalo coelhogonzalo requested a review from andycreed0x May 27, 2026 19:37
@coelhogonzalo

Copy link
Copy Markdown
Collaborator Author

I contacted the Blockstream team and asked if they could update the python binding to expose a Python-callable TxOut.unblind_with_ephemeral_sk(ephemeral_sk, address_blinding_pk)

Will update this PR when they reply.

@coelhogonzalo coelhogonzalo marked this pull request as ready for review June 9, 2026 13:21
@coelhogonzalo coelhogonzalo force-pushed the 66-sideswap-fix branch 2 times, most recently from 43478b0 to ae436f2 Compare June 9, 2026 14:04
@coelhogonzalo

Copy link
Copy Markdown
Collaborator Author

I contacted the Blockstream team and asked if they could update the python binding to expose a Python-callable TxOut.unblind_with_ephemeral_sk(ephemeral_sk, address_blinding_pk)

Will update this PR when they reply.

I fixed it by using wallycore directly.

@andycreed0x andycreed0x 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.

Good fix! Thanks for the PR

Comment thread src/aqua/sideswap.py
change_addr: Optional[str],
receive_ephemeral_sk: Optional[str],
change_ephemeral_sk: Optional[str],
addr_family: str = "lq",

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.

You can make this testnet-compatible with a small change. As-is, this would break if you ever want to test swaps on testnet. From Claude's analysis:

addr_family is always hardcoded to "lq", but testnet addresses use tlq. So confidential_addr_segwit_to_ec_public_key(tlq1..., "lq") will throw a raw ValueError before ever reaching the PSET.

# In _verify_pset, derive and pass addr_family:
addr_family = "tlq" if network == "testnet" else "lq"

Comment thread src/aqua/sideswap.py Outdated
recv_delta = balances.get(recv_asset, 0)
if recv_delta != recv_amount:
if recv_asset == fee_asset:
min_recv_delta = recv_amount - fee_tolerance_sats

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.

If recv_amount < fee_tolerance_sats, min_recv_delta goes negative, meaning a dealer could deliver 0 of the receive asset and still pass verification.

# Clamp to zero so the tolerance can't exceed the agreed amount
min_recv_delta = max(0, recv_amount - fee_tolerance_sats)

Comment thread src/aqua/sideswap.py
)


def unblind_dealer_outputs(

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.

The whole function is mocked out in every swap manager test.
(Something I always ask myself when reviewing: "is this test actually exercising the real functions and hitting the DB, or is everything mocked?"... might be useful for building tests.)

According to Claude:
what's missing is a test that doesn't patch this function and feeds it a real PSET ideally one honest case and one with a tampered asset.

Comment thread tests/test_sideswap.py Outdated
Comment thread src/aqua/sideswap.py Outdated
Comment thread src/aqua/sideswap.py Outdated
@marinate305

Copy link
Copy Markdown
Collaborator

Conflict in pyproject.toml appeared after PR #97 merged — likely a version bump conflict. Please rebase onto develop to resolve before merging.

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.

3 participants