Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Swaps completing on-chain before swapper detects completion #8534

Open
gomesalexandre opened this issue Jan 10, 2025 · 2 comments
Open

Swaps completing on-chain before swapper detects completion #8534

gomesalexandre opened this issue Jan 10, 2025 · 2 comments
Labels
bug Something isn't working needs grooming

Comments

@gomesalexandre
Copy link
Contributor

gomesalexandre commented Jan 10, 2025

Overview

⚠️ before you continue reading this, note one inherent limitation: we can only be as good as upstream is. If upstream doesn't yield us a Txid, we can't really do better. This only really is possible to implement for swappers which may yield an out Txid early enough, but take much longer for the status to be complete upstream.

See #8518 (comment) for more context. Authoring this as a bug as it looks like that from a user perspective, though really is a feature, TBD with @shapeshift/product during grooming.

THOR, LI.Fi, and Chainflip Txs complete on-chain (sometimes much, much) before the swap is marked as complete. This PR is to fix this by detecting outbound Tx and marking the swap as complete:

image

CoW may also be affected, though most likely isn't, see its checkTradeStatus implementation.

By "complete on-chain", we mean Tx is seen in Tx history (potentially with 0 confirmations for UTXOs, which is fine as we update UTXO balances optimistically)

0x, Portals, and Arbitrum Bridge currently use checkEvmSwapStatus (i.e detects a given Tx is confirmed on-chain) to detect completion:

https://github.com/search?q=repo%3Ashapeshift%2Fweb%20checkEvmSwapStatus&type=code

Jupiter rely on the exact same logic for Solana:

checkTradeStatus: checkSolanaSwapStatus,

On the other hand, CoW, THOR, Li.Fi, and Chainflip rely on APIs for completion detection:

// with cow we aren't able to get the tx hash until it's already completed, so we must use the
// order uid to fetch the trades and use their existence as indicating "complete"
// https://docs.cow.fi/tutorials/how-to-submit-orders-via-the-api/6.-checking-order-status
const maybeTradesResponse = await cowService.get<CowSwapGetTradesResponse>(
`${config.REACT_APP_COWSWAP_BASE_URL}/${network}/api/v1/trades`,
{ params: { orderUid: txHash } },
)

const [{ data: txData }, { data: txStatusData }] = await Promise.all([
axios.get<ThornodeTxResponse>(
`${config.REACT_APP_THORCHAIN_NODE_URL}/lcd/thorchain/tx/${thorTxHash}`,
),
axios.get<ThornodeStatusResponse>(
`${config.REACT_APP_THORCHAIN_NODE_URL}/lcd/thorchain/tx/status/${thorTxHash}`,
),
])

// don't use lifi sdk here because all status responses are cached, negating the usefulness of polling
// i.e don't do `await getLifi().getStatus(getStatusRequest)`
const url = new URL('https://li.quest/v1/status')
const getStatusRequestParams: { [Key in keyof GetStatusRequest]: string } = {
txHash,
bridge: tool,
fromChain: fromChainId.toString(),
toChain: toChainId.toString(),
}
url.search = new URLSearchParams(getStatusRequestParams).toString()
const response = await fetch(url, { cache: 'no-store' }) // don't cache!

const maybeStatusResponse = await chainflipService.get<ChainFlipStatus>(
`${brokerUrl}/status-by-id?apiKey=${apiKey}&swapId=${swapId}`,
)

The thing is, APIs may take much longer for things to detect things as complete (e.g Li.Fi), vs. us detecting seen Tx.

References and additional details

See description

Acceptance Criteria

  • Give all swappers some love and, if possible, mark the swap as complete when the outbound Tx (i.e the one with funds) is seen in the Tx history
  • While the ACC says to mark the swap as complete, from an implementation perspective, it really applies to checkTradeStatus (which works on a hop/hash-based basis) i.e checkTradeStatus should be returning TxStatus.Confirmed as soon as we know of the buy Txid

Need By Date

No response

Screenshots/Mockups

No response

Estimated effort

No response

@gomesalexandre
Copy link
Contributor Author

gomesalexandre commented Jan 10, 2025

  • Chainflip isn't affected and we're seemingly doing as good as we can already
    // Assume as soon as we have an outbound Tx, the swap is complete.
    // Chainflip waits for 3 confirmations to assume complete (vs. 1 for us), which is turbo long.
    return {
    buyTxHash: swapEgress.transactionReference,
    status: TxStatus.Confirmed,
    message: undefined,
    }
  • THOR may be able to be improved by ditching the done checks i.e if we have a buy Tx, we're complete
    https://github.com/shapeshift/web/blob/d69056070a4effa940f26296821aff0e685427f4/packages/swapper/src/swappers/ThorchainSwapper/endpoints.ts#L655C37-L663
  • CoW may be able to be cleaned up by ditching the whole status thing, and return complete as soon as we have a buy Tx (confirm with @woodenfurniture this isn't a breaking change for limit)
    const buyTxHash = trades[0]?.txHash
    if (!buyTxHash) return createDefaultStatusResponse(undefined)
    const maybeGetOrdersResponse = await cowService.get<CowSwapGetTransactionsResponse>(
    `${config.REACT_APP_COWSWAP_BASE_URL}/${network}/api/v1/transactions/${buyTxHash}/orders`,
    )
    if (maybeGetOrdersResponse.isErr()) throw maybeGetOrdersResponse.unwrapErr()
    const {
    data: [{ status: rawStatus }],
    } = maybeGetOrdersResponse.unwrap()
    // https://api.cow.fi/docs/#/default/get_api_v1_orders__UID_
    const status = (() => {
    switch (rawStatus) {
    case 'fulfilled':
    return TxStatus.Confirmed
    case 'presignaturePending':
    case 'open':
    return TxStatus.Pending
    case 'cancelled':
    case 'expired':
    return TxStatus.Failed
    default:
    return TxStatus.Unknown
    }
    })()
    return {
    status,
    buyTxHash,
    message: rawStatus,
    }
    },
  • Same as above, Li.Fi can most likely be improved by ditching status checks and returning complete as soon as we have a buy Tx:
    const status = (() => {
    switch (statusResponse.status) {
    case 'DONE':
    return TxStatus.Confirmed
    case 'PENDING':
    return TxStatus.Pending
    case 'FAILED':
    return TxStatus.Failed
    default:
    return TxStatus.Unknown
    }
    })()
    return {
    status,
    buyTxHash: (statusResponse.receiving as ExtendedTransactionInfo)?.txHash,
    message: statusResponse.substatusMessage,
    }
    },

@AvaTech326

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs grooming
Projects
Status: Backlog
Development

No branches or pull requests

2 participants