Conversation
Adds TIP-1027 spec for optional recipient parameter on StablecoinDEX swap functions. Caller is treated as logical sender for TIP-403/TIP-1025 policy checks. Includes DEX-balance credit special case for recipient == DEX. Co-Authored-By: Daniel Robinson <1187252+danrobinson@users.noreply.github.com>
decofe
requested review from
0xKitsune,
dankrad,
danrobinson,
howydev and
legion2002
as code owners
Caller acquiring a new token via swap-to-DEX-balance must be policy-checked as both sender and recipient, unlike maker fills which were pre-checked at order placement. Co-Authored-By: Daniel Robinson <1187252+danrobinson@users.noreply.github.com>
Issuers must be able to freeze all DEX trading of their token by blacklisting the DEX address. The privileged settlement path must check isAuthorizedSender(policyId, DEX) on both the external-recipient and DEX-balance-credit paths. Co-Authored-By: Daniel Robinson <1187252+danrobinson@users.noreply.github.com>
…fy msg.sender semantics - Pause check on DEX-balance credit path (can't acquire paused tokens) - TIP-1025 isAddressTransferAuthorized(caller, caller, tokenOut) on DEX-credit path - Clarify that policies see msg.sender, not end-user-through-router - Add tokenIn-recipient invariant - Add test cases for TIP-1025 self/self, pause, and router semantics Co-Authored-By: Daniel Robinson <1187252+danrobinson@users.noreply.github.com>
DEX-balance credit path checks isAddressTransferAuthorized(DEX, caller) not (caller, caller) — tokens come from DEX custody. Co-Authored-By: Daniel Robinson <1187252+danrobinson@users.noreply.github.com>
External recipient path uses normal ITIP20.transfer(recipient) — no manual policy enforcement or privileged settlement needed. DEX-credit path checks pause, DEX-as-sender (issuer freeze), and caller-as-recipient. Co-Authored-By: Daniel Robinson <1187252+danrobinson@users.noreply.github.com>
- Explicitly scope out TIP-1025 from DEX-credit path - Add atomicity requirement (settlement failure reverts entire swap) - Narrow recipient==msg.sender equivalence claim - Soften gas-cost invariant to semantics-only - Add settlement-only-change invariant Co-Authored-By: Daniel Robinson <1187252+danrobinson@users.noreply.github.com>
|
This PR has been marked stale due to 7 days of inactivity. |
… policy, TIP-1028 tests Co-authored-by: Daniel Robinson <1187252+danrobinson@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019cf74a-a0bd-70b8-9b3d-0a67f027b304
…ed tests Co-authored-by: Daniel Robinson <1187252+danrobinson@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019cf74a-a0bd-70b8-9b3d-0a67f027b304
|
|
||
| When the recipient is the DEX itself, the output tokens are credited to the caller's internal DEX balance (`balances[msg.sender][tokenOut] += amountOut`) instead of performing a TIP-20 transfer. This enables atomic swap-into-balance flows — for example, swapping and immediately placing a limit order without a round-trip transfer. | ||
|
|
||
| Since no TIP-20 transfer occurs on this path, the DEX manually enforces equivalent checks: |
There was a problem hiding this comment.
| Since no TIP-20 transfer occurs on this path, the DEX manually enforces equivalent checks: | |
| Since no transfer of the TIP-20 output token occurs on this path, the DEX manually enforces equivalent checks: |
There was a problem hiding this comment.
Applied — updated to "Since no transfer of the TIP-20 output token occurs on this path".
Co-authored-by: Daniel Robinson <1187252+danrobinson@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019cf74a-a0bd-70b8-9b3d-0a67f027b304
|
|
||
| ### External Recipient | ||
|
|
||
| For `WithMemo` variants with an external recipient, the DEX calls `ITIP20(tokenOut).transferWithMemo(recipient, amountOut, memo)`. This emits the standard TIP-20 `TransferWithMemo` event on the output token with `from = StablecoinDEX, to = recipient`. The memo is attached to the output settlement — the transfer that delivers value to the recipient. |
There was a problem hiding this comment.
Should mention that, as usual, the TransferWithMemo event happens immediately after the Transfer event
There was a problem hiding this comment.
Done — the memo section now specifies that both a Transfer event and a TransferWithMemo event are emitted, with TransferWithMemo immediately after Transfer. Test cases also updated.
|
|
||
| ### DEX-Recipient (Internal Balance Credit) | ||
|
|
||
| When `recipient == address(StablecoinDEX)` and a memo is provided (via the `WithMemo` variants), the memo is attached to the **input** transfer instead — the TIP-20 transfer from the caller to the DEX for `tokenIn`. The DEX uses `transferFromWithMemo` (or the equivalent internal path) to pull input tokens with the memo. This is because no TIP-20 output transfer occurs on this path, so the memo annotates the caller's deposit action. |
There was a problem hiding this comment.
Should mention the same about there being both a Transfer event and a TransferWithMemo event. Doesn't need to specify what function the DEX calls for this.
There was a problem hiding this comment.
Updated — now describes both events (Transfer + TransferWithMemo) and ordering without specifying which DEX function call produces them.
|
|
||
| ## Event Emission | ||
|
|
||
| The `OrderFilled` event's `taker` field remains `msg.sender` — the taker is the party that initiated the trade, not the recipient of the output. The TIP-20 `Transfer` event shows `from = StablecoinDEX, to = recipient`, which accurately reflects the on-chain token movement. Attribution of who initiated the swap is available via the `OrderFilled` event's `taker` field. |
There was a problem hiding this comment.
Can remove the final sentence of this paragraph
Consistent with multi-hop swaps and order fills, which don't pause-check intermediate/internal balance credits. Pause is enforced on withdraw. Co-authored-by: Daniel Robinson <1187252+danrobinson@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019cf74a-a0bd-70b8-9b3d-0a67f027b304
Co-authored-by: Daniel Robinson <1187252+danrobinson@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019cf74a-a0bd-70b8-9b3d-0a67f027b304
|
|
||
| ## Errors | ||
|
|
||
| One new error is added: |
There was a problem hiding this comment.
Why does this add an event? Should it be just an error thrown by TIP-20, same as any time someone tries to transfer to address(0)?
There was a problem hiding this comment.
Good point — dropped the custom InvalidRecipient error. The DEX validates recipient != address(0) upfront and reverts with TIP-20's existing InvalidRecipient() error. No new error type needed.
- Use Dan's suggested wording for DEX-recipient intro - Specify Transfer + TransferWithMemo event pairs and ordering - Remove implementation details (function names) from memo section - Remove redundant sentence from Event Emission - Trim motivation to payments + DEX balance deposits - Drop custom InvalidRecipient error, reuse TIP-20's - Fix invariant 2 (pause check was already removed) Co-authored-by: Daniel Robinson <1187252+danrobinson@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019cf74a-a0bd-70b8-9b3d-0a67f027b304
- Fix contradiction: implementation outline said 'check pause' for
DEX-recipient but detailed spec says no pause check
- Handle WithMemo + DEX-recipient when swap is funded from internal
balance (no TIP-20 transfer → no TransferWithMemo emitted)
- Require upfront recipient validation before any fills/mutations
- Mirror TIP-20 check_recipient: reject address(0) and TIP-20 prefixes
- Add test cases for swapExactAmountOutTo{WithMemo} variants
- Add test for paused token succeeding on DEX-recipient path
- Add test for DEX-recipient memo with full internal funding
- Split pause test into external-recipient (reverts) vs DEX-recipient
(succeeds)
Co-authored-by: Daniel Robinson <1187252+danrobinson@users.noreply.github.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019cf74a-a0bd-70b8-9b3d-0a67f027b304
…-funded DEX-recipient swaps Ensures WithMemo variants always emit a TransferWithMemo event, even when the swap is fully funded from internal DEX balance. Co-authored-by: Daniel Robinson <1187252+danrobinson@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019cf74a-a0bd-70b8-9b3d-0a67f027b304
|
|
||
| ## New Functions | ||
|
|
||
| Four new swap functions are added to the `IStablecoinDEX` interface — two base functions and two `WithMemo` variants: |
There was a problem hiding this comment.
feel like this should be WithRecipient and WithMemo.. the To suffix is really confusing
There was a problem hiding this comment.
+1, we should update this IMO.
There was a problem hiding this comment.
Ah interesting idea, but WithRecipient and WithMemo sounds like the one with the memo doesn't have a recipient, but it does.
An alternative would just be overloading the function with 1-2 arguments and not changing the name. How do we feel about overloading?
|
|
||
| When the recipient is the DEX itself, the output tokens are credited to the caller's internal DEX balance (`balances[msg.sender][tokenOut] += amountOut`) instead of performing a TIP-20 transfer. This enables atomic swap-into-balance flows — for example, swapping and immediately placing a limit order without a round-trip transfer. | ||
|
|
||
| Since no transfer of the TIP-20 output token occurs on this path, the DEX manually enforces policy checks: |
There was a problem hiding this comment.
If no transfer occurs, why do we need to enforce 403? We don't check 403 for other dex operations that might result in blocked funds getting stuck
| ### DEX-Recipient (Internal Balance Credit) | ||
|
|
||
| When `recipient == address(StablecoinDEX)` and a memo is provided (via the `WithMemo` variants), the memo is attached to the **input** transfer instead, since no TIP-20 output transfer occurs on this path. | ||
|
|
||
| If the input is pulled (fully or partially) from the caller's external wallet, the external portion emits both a `Transfer` event and a `TransferWithMemo` event (with the `TransferWithMemo` immediately after). If the swap is funded entirely from the caller's internal DEX balance, the DEX emits a zero-value `Transfer` and `TransferWithMemo` event on `tokenIn` (with `from = msg.sender, to = StablecoinDEX, amount = 0`) to preserve the memo. | ||
|
|
||
| For the non-memo `To` variants with DEX-recipient, no `TransferWithMemo` is emitted regardless of funding source. |
There was a problem hiding this comment.
this is super wonky, just revert if you're trying to swap with a memo into the dex or something
There was a problem hiding this comment.
i concur. do we have a clear usecase/user for why we're building these somewhat wonky semantics?
There was a problem hiding this comment.
You're right, let's just have this one revert; the special cases just kinda piled up a bit
| /// @param amountIn Amount of tokenIn to sell | ||
| /// @param minAmountOut Minimum amount of tokenOut the recipient must receive | ||
| /// @param recipient Address to receive the output tokens | ||
| /// @param memo Arbitrary 32-byte memo attached to the output transfer |
There was a problem hiding this comment.
but it's input transfer if recipient = DEX.
There was a problem hiding this comment.
Yep good point, should say that
| /// @param amountOut Exact amount of tokenOut the recipient must receive | ||
| /// @param maxAmountIn Maximum amount of tokenIn to spend | ||
| /// @param recipient Address to receive the output tokens | ||
| /// @param memo Arbitrary 32-byte memo attached to the output transfer |
There was a problem hiding this comment.
but it's input transfer if recipient = DEX.
|
|
||
| 14. **Exact-out memo on external recipient**: `swapExactAmountOutToWithMemo` emits a `TransferWithMemo` event on `tokenOut` immediately after the `Transfer` event. | ||
|
|
||
| 15. **Exact-out memo on DEX-recipient**: `swapExactAmountOutToWithMemo(..., DEX_ADDRESS, memo)` follows the same memo rules as the exact-in variant (memo on input transfer if external funding occurs, no memo if fully internal-funded). |
There was a problem hiding this comment.
contradicts invariant 7 that says withMemo variants must always emit a TransferWithMemo?
|
|
||
| ## Motivation | ||
|
|
||
| Today, swapping and sending to another address requires two transactions: a swap (output goes to the caller) followed by a transfer to the intended recipient. This costs extra gas, increases latency, and prevents atomic swap-and-pay flows. |
There was a problem hiding this comment.
dumb q: we do have native batching in tempo tx? why eat all this complexity instead of just doing that?
There was a problem hiding this comment.
- Not everyone can use Tempo txes.
- There's a cost to initialize the temporary balance (250k gas)—if we eventually get refunds for storage that is cleared in the same slot that could help, but it's still worse.
- The caller may not be authorized to receive the token. I think this is an interesting edge case that adds a nice capability to TIP-1015.
|
|
||
| ### External Recipient | ||
|
|
||
| For `WithMemo` variants with an external recipient, the output settlement emits both a `Transfer` event and a `TransferWithMemo` event on the output token, with `from = StablecoinDEX, to = recipient`. The `TransferWithMemo` event is emitted immediately after the `Transfer` event. |
There was a problem hiding this comment.
won't this be confusing if the recipient expects to see a transfer from the sender but sees from from DEX?
decofe
left a comment
decofe
left a comment
There was a problem hiding this comment.
TIP-1027 Review
protocolVersion → T5
Frontmatter says protocolVersion: TBD. Should be T5.
Security / Correctness Issues
-
Paused-token bypass on DEX-recipient path is a meaningful weakening of pause semantics. The TIP explicitly allows
swapExactAmountInTo(..., DEX_ADDRESS)to succeed whentokenOutis paused, with the rationale that pause is enforced onwithdraw. But this means a user can acquire economic exposure to a paused token via internal DEX balance. If an issuer pauses a token expecting to freeze all new acquisition, this is a bypass. The TIP acknowledges this is intentional, but it deserves a more prominent callout in the spec — possibly a dedicated "Security Considerations" section — so issuers understand the trade-off. -
Synthetic zero-value
Transfer+TransferWithMemoevents are dangerous for indexers. Whenrecipient == DEXand the swap is fully internally funded, the TIP requires emitting zero-value events ontokenInto preserve the memo. Off-chain systems (explorers, accounting tools, compliance) will index these as real token movements. Consider:- Using a distinct event type (e.g.,
MemoAttached) instead of a zero-valueTransfer - Or at minimum, documenting that indexers MUST ignore zero-value
Transferevents from DEX-recipient paths
- Using a distinct event type (e.g.,
-
Sender identity in TIP-403 checks on external-recipient path. The spec says output settlement uses
ITIP20(tokenOut).transfer(recipient, amountOut)— a transfer from the DEX. The DEX is the sender for TIP-403 purposes. This means:- The DEX (not the caller) must be an authorized sender
- If the DEX is blacklisted as sender on
tokenOut, all swap-and-send to external recipients fails - This is probably correct, but should be stated explicitly so it is not misimplemented as a caller-is-sender check
Suggested changes
-protocolVersion: TBD
+protocolVersion: T5…rsion T5 Co-Authored-By: Daniel Robinson <1187252+danrobinson@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019d68f2-72e9-746f-aa40-1ab00dc19763
|
Addressed in a1f615c: added pause check on the DEX-recipient internal balance credit path, updated protocolVersion to T5. Paused |
|
This PR has been marked stale due to 7 days of inactivity. |
|
|
||
| When `recipient == address(StablecoinDEX)` and a memo is provided (via the `WithMemo` variants), the memo is attached to the **input** transfer instead, since no TIP-20 output transfer occurs on this path. | ||
|
|
||
| If the input is pulled (fully or partially) from the caller's external wallet, the external portion emits both a `Transfer` event and a `TransferWithMemo` event (with the `TransferWithMemo` immediately after). If the swap is funded entirely from the caller's internal DEX balance, the DEX emits a zero-value `Transfer` and `TransferWithMemo` event on `tokenIn` (with `from = msg.sender, to = StablecoinDEX, amount = 0`) to preserve the memo. |
There was a problem hiding this comment.
when the swap is funded entirely from internal DEX balance, who actually emits the zero-value Transfer / TransferWithMemo "on tokenIn"? events are scoped to the emitting contract, so the DEX can't just log them on the token's address, either:
- the DEX calls
tokenIn.transferWithMemo(self, 0, memo)(which means TIP-20 has to allow zero-value memo transfers - something else I'm missing
same question applies to invariant 7 and test case 12.
tempoxyz-bot
left a comment
tempoxyz-bot
left a comment
There was a problem hiding this comment.
👁️ Cyclops Review
PR #3016 adds a draft TIP for swap-and-send functions (target: T5). The diff is docs-only, but the spec makes security claims about the existing T2 StablecoinDEX that are demonstrably false. Three verified findings describe real pre-existing bugs in live T2 code that this spec's rationale relies upon.
🚨 [HIGH] StablecoinDEX swaps and orders bypass access-key spending limits
File: crates/precompiles/src/tip20/mod.rs:616-685
DEX input debits never enforce access-key spending caps. _transfer_from() does not call authorize_transfer(), and internal-balance debits have no keychain accounting. A compromised access key can drain any DEX-approved or internal balance far beyond its configured cap. TIP-1027's WithMemo design would widen this surface by explicitly using transferFromWithMemo on the input leg.
Fix: Charge access-key spending limits in _transfer_from() whenever from == tx_origin, and mirror in the DEX internal-balance debit path.
✅ Verified with test (test_access_key_limit_bypass_via_dex_swap).
Reviewer Callouts
- ⚡ Fee-token inference:
crates/revm/src/common.rs:174-190only decodes legacy swap selectors. New selectors must be added when TIP-1027 is implemented. - ⚡ Zero-value memo emission mechanism: If implemented via
transferFromWithMemo(msg.sender, DEX, 0, memo), it would trigger pause/policy checks ontokenIn, creating asymmetric behavior vs. non-memo variant. - ⚡ TIP-1028 not yet in repo: Referenced multiple times but no
tip-1028.mdexists. - ⚡ Check ordering: Spec checks policy before pause, but TIP-20
transferchecks pause first.
jenpaff
added
cyclops
tempoxyz-bot
left a comment
tempoxyz-bot
left a comment
There was a problem hiding this comment.
👁️ Cyclops Review
This PR adds a Draft TIP-1027 specification for swap-and-send. No runtime code is changed. The spec is internally consistent, but the audit surfaced pre-existing issues that this TIP either amplifies or relies on incorrectly.
🚨 [SECURITY] Sender-blacklisted makers can still trade through pre-escrowed orders
Severity: Medium | Status: Verified with passing test
File: crates/precompiles/src/stablecoin_dex/mod.rs:655-724
cancel_stale_order() correctly classifies an order as stale once the maker loses sender authorization, but fill_order() / partial_fill_order() never revalidate that condition. A sender-blacklisted maker's pre-escrowed orders can still be filled, crediting the maker with the opposite asset which they then withdraw. TIP-1027 lines 133–141 assume placement-time policy checks are sufficient for existing maker-fill credits — this is incorrect after post-placement policy changes.
Recommended Fix: Recompute the stale condition before matching any order. If the maker is no longer authorized as a sender on the escrowed token, skip or auto-cancel the order. Quote paths should also exclude stale liquidity.
⚠️ [ISSUE] Access-key spend limits bypassed on DEX external-funding path — TIP-1027 recipient amplifies to direct theft
Severity: High (once TIP-1027 is implemented)
File: crates/precompiles/src/tip20/mod.rs:616-685
TIP-1027 claims the new *To functions reuse the same input-debit logic (a "settlement-only change"). However, the current transfer_from / transfer_from_with_memo paths used by the DEX never consult AccountKeychain spend limits, while transfer() and system_transfer_from() do. Today this is an over-broad self-trade. With TIP-1027's recipient parameter, a compromised limited access key could spend the user's full DEX allowance and redirect output to an attacker address.
Recommended Fix: Add AccountKeychain::authorize_transfer() / check_and_update_spending_limit() to the transfer_from and transfer_from_with_memo code paths before TIP-1027 is implemented.
⚠️ [ISSUE] Post-pause bids create fresh paused-token positions through existing fill path
Severity: Medium
File: crates/precompiles/src/stablecoin_dex/mod.rs:421-438
TIP-1027 adds a pause check on the DEX-recipient path to prevent paused-token position buildup. However, existing place() never checks paused() on the base token, and fill_order() credits paused-token internal balances without a pause gate. The invariant TIP-1027 claims to establish does not hold for existing paths.
Recommended Fix: Extend the pause gate to place() for the base token and block swap input consumption from internal paused-token balances.
Reviewer Callouts
- ⚡ Stale-order matching: Confirm whether stale orders should be auto-cancelled on encounter or skipped, and ensure quote paths stop advertising unfillable liquidity.
- ⚡ Implementation must inherit
check_not_paused()route validation: New external-recipient swap functions must run throughvalidate_and_build_route()(per SIGP-203) so paused-token routing fails early. - ⚡ TIP-1028 carve-out: When TIP-1028 lands, ensure withdraw-time enforcement is actually implemented for DEX-balance credits.
- ⚡ Access-key gap is pre-existing: The
transfer_fromspend-limit bypass exists today on legacy swaps; TIP-1027 just makes it exploitable for theft viarecipient. - ⚡ Fee-token inference and ABI updates: When implemented, update selector-sensitive consumers together (
crates/revm/src/common.rs, dispatch, contracts ABI, Solidity reference interface).
|
|
||
| Unlike the existing maker-fill path (where makers' internal balances are credited without checking pause, because the maker opted in before the pause by placing an order), the swap-and-send caller is newly acquiring a token position. Pause MUST be enforced to prevent users from building up positions in a paused token. | ||
|
|
||
| This is different from the existing maker-fill path (where `_fillOrder` credits `balances[maker]` without re-checking policies). In that case, the maker was already policy-checked when they placed the order. Here, the caller is newly acquiring a token position via swap, so we must verify they are authorized to hold it. |
There was a problem hiding this comment.
💡 [SUGGESTION] Spec references wrong error name
The actual interface at interfaces/ITIP20.sol:9 defines ContractPaused, not TokenPaused. This pseudocode would not compile as written.
Recommended Fix: Change ITIP20.TokenPaused() to ITIP20.ContractPaused().
|
|
||
| TIP-1028 address-level receive policies do not apply to the DEX-credit path, since no TIP-20 transfer occurs. Address-level receive policies govern actual token transfers, not internal DEX balance accounting. | ||
|
|
||
| ## Output Settlement (External Recipient) |
There was a problem hiding this comment.
This paragraph claims the DEX-recipient pause check prevents users from "building up positions in a paused token." However, existing place() never checks paused() on the base token, and fill_order() credits paused-token internal balances without a pause gate. A fresh post-pause bidder can still acquire paused-token positions through the existing matching engine (crates/precompiles/src/stablecoin_dex/mod.rs:421-438, 667-724).
The invariant this paragraph claims to establish does not hold for existing paths.
|
|
||
| ## Output Settlement (External Recipient) | ||
|
|
||
| For external recipients, the DEX calls `ITIP20(tokenOut).transfer(recipient, amountOut)` (or `transferWithMemo(recipient, amountOut, memo)` for the `WithMemo` variants) — a normal TIP-20 transfer from the DEX. This is identical to how existing swap functions settle output, just with `recipient` instead of `msg.sender`. All standard TIP-20 checks (TIP-403 token-level policies, TIP-1028 address-level receive policies, pause state, recipient validity) are enforced automatically by the TIP-20 transfer. |
There was a problem hiding this comment.
🚨 [SECURITY] Placement-time policy check assumption is incorrect for stale orders (verified)
This paragraph assumes the maker "was already policy-checked when they placed the order" so re-checking is unnecessary. However, cancel_stale_order() recognizes that sender authorization can change after placement, while fill_order() / partial_fill_order() never revalidate. A sender-blacklisted maker's pre-escrowed orders can still be matched, crediting the maker with the opposite asset which they then withdraw.
This was reproduced with a passing test. See crates/precompiles/src/stablecoin_dex/mod.rs:655-724 and :1096-1123.
Recommended Fix: Recompute the stale condition before matching any order.
| - Otherwise: settle via `ITIP20(tokenOut).transfer(recipient, amountOut)` for base variants, or `ITIP20(tokenOut).transferWithMemo(recipient, amountOut, memo)` for `WithMemo` variants. | ||
|
|
||
| The existing `swapExactAmountIn` and `swapExactAmountOut` remain unchanged — they are not refactored to delegate to the new functions, preserving identical behavior for existing callers. | ||
|
|
There was a problem hiding this comment.
Reusing the current input-debit logic is not safe once recipient can be arbitrary. The DEX external-funding path uses TIP20::transfer_from(), which does not consult AccountKeychain spend limits (unlike transfer() and system_transfer_from() which do). Today this is bounded because output returns to msg.sender. With recipient, a compromised limited access key could spend the user's full pre-approved DEX allowance and redirect swap output to an attacker.
See crates/precompiles/src/tip20/mod.rs:616-685 vs :603-613 and :642-657.
Recommended Fix: Add AccountKeychain::authorize_transfer() to the transfer_from path before implementing TIP-1027.
0xKitsune
left a comment
0xKitsune
left a comment
There was a problem hiding this comment.
IMO the "credit to internal balance" functionality should be its own function (maybe swapAndDeposit?) instead of a special case where you pass the DEX address as the recipient.
Right now, depending on whether you pass the DEX address or a regular address, the function does two different things with different policy checks, no actual token transfer, the memo gets attached to a different leg, and TIP-1028 doesn't apply.
Splitting it into two functions means each does one thing clearly. swapAndSend always transfers to the recipient, and swapAndDeposit doesn't need a memo since nothing is moving externally.
If swapAndDeposit is useful, I think we could consider it for another TIP.
|
|
||
| ## New Functions | ||
|
|
||
| Four new swap functions are added to the `IStablecoinDEX` interface — two base functions and two `WithMemo` variants: |
There was a problem hiding this comment.
+1, we should update this IMO.
| 21. **Recipient address-level receive policy**: If `recipient` has a whitelist receive policy that does not include the DEX address, the swap-and-send reverts (enforced by TIP-20 `transfer`). | ||
|
|
||
| 22. **Recipient token set**: If `recipient` has a token set that does not include `tokenOut`, the swap-and-send reverts. |
There was a problem hiding this comment.
Is this purposeful or the use of the ESCROW_ADDRESS in #3791 was added later and the tokens should be redirected there?
9 participants
Adds TIP-1027: optional
recipientparameter onswapExactAmountIn/swapExactAmountOut, so callers can swap and send output to a different address in one call.Key design decisions:
swapExactAmountInTo,swapExactAmountOutTo) — existing functions unchangedrecipient == DEXcredits the caller's internal DEX balance (no TIP-20 transfer)recipient == address(0)revertsCompatible with TIP-1015 (compound policies) and TIP-1028 (address-level receive policies).
Prompted by: Dan