Skip to content

Commit 3439226

Browse files
fix(shield-swap): swap_many takes expected_out; share the position decoder
Two findings from a second review pass. swap_many's refusal message told callers to "quote it yourself and pass expected_out" — and swap_many had no expected_out parameter, so the advice was impossible to follow. It now accepts one, which both makes the message true and gives callers with their own price source (or an unreachable API) a way through. find_position_plaintext matched any dict whose `pool` field equalled the target, so a record of another type carrying that field would be returned as a position and then spent as one. That is the same defect fixed in _owned_from_record last commit; its sibling was left behind. Both now go through decode_position_record, with a test using a lookalike record. The agent page passed its compactness budget again. It had been squeezed to 134 chars of headroom, so the cap moves 24k → 26k deliberately rather than a third squeeze, with the reason recorded alongside the earlier raises. The new paragraph is also written tighter; the page sits at 24130 with 1870 spare. 237 shield-swap (+2), pyright clean.
1 parent ccf0e49 commit 3439226

7 files changed

Lines changed: 67 additions & 16 deletions

File tree

‎shield-swap-sdk/AGENTS.md‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ The scan catches positions the journal never saw (account used from
6666
another machine, journal lost); it needs a registered record
6767
provider and is skipped silently without one.
6868

69-
### `swap_many(self, *, pool_key: 'str', token_in_id: 'str', amount_in: 'int', count: 'int', slippage_bps: 'int' = 50, record_wait_seconds: 'float' = 120.0, account: 'Any' = None) -> 'SwapBatchReport'`
69+
### `swap_many(self, *, pool_key: 'str', token_in_id: 'str', amount_in: 'int', count: 'int', slippage_bps: 'int' = 50, expected_out: 'Optional[int]' = None, record_wait_seconds: 'float' = 120.0, account: 'Any' = None) -> 'SwapBatchReport'`
7070

7171
*count* private swaps of *amount_in* each, with reserved counters.
7272

@@ -78,6 +78,11 @@ becomes claimable (it stays in ``still_pending``). A failed
7878
broadcast burns its counter and the batch continues; failures are
7979
reported, not raised. Requires ``from_profile()``.
8080

81+
*expected_out* (base units) skips the route quote. Without it the batch
82+
quotes once and refuses rather than falling back to a spot estimate,
83+
which ignores the pool fee and would revert every swap after paying for
84+
its proof.
85+
8186
### `collect_all(self, account: 'Any' = None) -> 'CollectReport'`
8287

8388
Claim every finalized swap and collect owed fees on open positions.

‎shield-swap-sdk/python/aleo_shield_swap/AGENTS.md‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ The scan catches positions the journal never saw (account used from
6666
another machine, journal lost); it needs a registered record
6767
provider and is skipped silently without one.
6868

69-
### `swap_many(self, *, pool_key: 'str', token_in_id: 'str', amount_in: 'int', count: 'int', slippage_bps: 'int' = 50, record_wait_seconds: 'float' = 120.0, account: 'Any' = None) -> 'SwapBatchReport'`
69+
### `swap_many(self, *, pool_key: 'str', token_in_id: 'str', amount_in: 'int', count: 'int', slippage_bps: 'int' = 50, expected_out: 'Optional[int]' = None, record_wait_seconds: 'float' = 120.0, account: 'Any' = None) -> 'SwapBatchReport'`
7070

7171
*count* private swaps of *amount_in* each, with reserved counters.
7272

@@ -78,6 +78,11 @@ becomes claimable (it stays in ``still_pending``). A failed
7878
broadcast burns its counter and the batch continues; failures are
7979
reported, not raised. Requires ``from_profile()``.
8080

81+
*expected_out* (base units) skips the route quote. Without it the batch
82+
quotes once and refuses rather than falling back to a spot estimate,
83+
which ignores the pool fee and would revert every swap after paying for
84+
its proof.
85+
8186
### `collect_all(self, account: 'Any' = None) -> 'CollectReport'`
8287

8388
Claim every finalized swap and collect owed fees on open positions.

‎shield-swap-sdk/python/aleo_shield_swap/_core.py‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -188,16 +188,17 @@ def decode_position_record(plaintext: str) -> Optional[dict[str, Any]]:
188188

189189

190190
def find_position_plaintext(records: Any, pool_key: str) -> Optional[str]:
191-
"""First unspent PositionNFT plaintext whose ``pool`` matches, or None."""
191+
"""First unspent PositionNFT plaintext whose ``pool`` matches, or None.
192+
193+
Uses :func:`decode_position_record`, so a record of another type that
194+
happens to carry a matching ``pool`` field is not returned as a position.
195+
"""
192196
for rec in records:
193197
plaintext = record_plaintext(rec)
194198
if not plaintext:
195199
continue
196-
try:
197-
decoded = parse_plaintext(plaintext)
198-
except (ValueError, TypeError):
199-
continue
200-
if isinstance(decoded, dict) and decoded.get("pool") == pool_key:
200+
decoded = decode_position_record(plaintext)
201+
if decoded is not None and decoded.get("pool") == pool_key:
201202
return plaintext
202203
return None
203204

‎shield-swap-sdk/python/aleo_shield_swap/client.py‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -987,6 +987,7 @@ def swap_many(
987987
amount_in: int,
988988
count: int,
989989
slippage_bps: int = 50,
990+
expected_out: Optional[int] = None,
990991
record_wait_seconds: float = 120.0,
991992
account: Any = None,
992993
) -> SwapBatchReport:
@@ -999,18 +1000,25 @@ def swap_many(
9991000
becomes claimable (it stays in ``still_pending``). A failed
10001001
broadcast burns its counter and the batch continues; failures are
10011002
reported, not raised. Requires ``from_profile()``.
1003+
1004+
*expected_out* (base units) skips the route quote. Without it the batch
1005+
quotes once and refuses rather than falling back to a spot estimate,
1006+
which ignores the pool fee and would revert every swap after paying for
1007+
its proof.
10021008
"""
10031009
if self.journal is None:
10041010
raise ValueError("swap_many() needs a journal — construct with "
10051011
"ShieldSwap.from_profile().")
10061012
acct = self._account(account)
1007-
# Quote once for the batch: a spot estimate ignores the pool fee, so
1008-
# min-out would exceed the real output and finalize would reject.
1013+
# One quote for the whole batch unless the caller supplied one: a spot
1014+
# estimate ignores the pool fee, so min-out would exceed the real output
1015+
# and finalize would reject.
10091016
pool = self.get_pool(pool_key)
10101017
token_out_id = pool.token1 if token_in_id == pool.token0 else pool.token0
1011-
expected_out = self._quote_expected_out(
1012-
token_in_id=token_in_id, token_out_id=token_out_id,
1013-
amount_in=amount_in)
1018+
if expected_out is None:
1019+
expected_out = self._quote_expected_out(
1020+
token_in_id=token_in_id, token_out_id=token_out_id,
1021+
amount_in=amount_in)
10141022
if expected_out is None and slippage_bps < 10_000:
10151023
# Falling back to the spot estimate would set amount_out_min above
10161024
# what the pool can actually pay (spot ignores the fee), so every

‎shield-swap-sdk/tests/test_core.py‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,3 +178,16 @@ def test_extract_tx_id_handles_all_dps_shapes():
178178
"execution": {}}}) == "at1c"
179179
with pytest.raises(ValueError, match="Cannot find"):
180180
extract_tx_id({"transaction": {"type": "execute"}})
181+
182+
183+
def test_find_position_plaintext_rejects_a_lookalike_record():
184+
"""A non-PositionNFT record carrying a matching `pool` must not be returned
185+
as a position — it would be spent as one."""
186+
from aleo_shield_swap._core import find_position_plaintext
187+
lookalike = "{ owner: aleo1x.private, pool: 5field.private, amount: 9u128.private }"
188+
position = ("{ owner: aleo1x.private, withdrawal: aleo1y.private, "
189+
"token_id: 1field.private, token0_id: 2field.private, "
190+
"token1_id: 3field.private, pool: 5field.private, "
191+
"tick_lower: -60i32.private, tick_upper: 60i32.private }")
192+
recs = [{"record_plaintext": lookalike}, {"record_plaintext": position}]
193+
assert find_position_plaintext(recs, "5field") == position

‎shield-swap-sdk/tests/test_gen_context.py‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,11 @@ def test_committed_page_is_current():
4242

4343

4444
def test_page_stays_compact():
45-
# ~6k tokens — cheap context, enforced. Raised 20k → 22k with the
45+
# ~6.5k tokens — cheap context, enforced. Raised 20k → 22k with the
4646
# router-dispatch surface (wrapper_proofs / withdrawal params); 22k → 24k
4747
# when get_pools/get_tokens/derive_pool_key/derive_tick_key picked up full
48-
# docstrings (they rendered blank before).
49-
assert len(_render()) < 24_000
48+
# docstrings (they rendered blank before); 24k → 26k for the swap/swap_many
49+
# footgun warnings (build-time counter reservation, refusing an unusable
50+
# quote). 24k had been squeezed to 134 chars of headroom, which any further
51+
# edit broke — this is deliberate room, not another squeeze.
52+
assert len(_render()) < 26_000

‎shield-swap-sdk/tests/test_swap_many.py‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,3 +210,19 @@ def get_route(self, **_):
210210
with pytest.raises(NotAuthenticatedError):
211211
fresh._quote_expected_out(token_in_id="t0", token_out_id="t1",
212212
amount_in=10**6)
213+
214+
215+
def test_swap_many_accepts_a_caller_supplied_quote(dex, monkeypatch):
216+
"""expected_out skips the route quote — the escape hatch the refusal
217+
message points callers at, so it must actually exist."""
218+
called = {"n": 0}
219+
220+
def _never(self, **kw):
221+
called["n"] += 1
222+
return None
223+
224+
monkeypatch.setattr(ShieldSwap, "_quote_expected_out", _never)
225+
dex.swap_many(pool_key="5field", token_in_id="t0", amount_in=10**6,
226+
count=1, expected_out=990_000)
227+
assert called["n"] == 0, "should not have quoted"
228+
assert dex.journal.counter_cursor() == 1

0 commit comments

Comments
 (0)