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

Consolidate Tx building into a common util over all THOR domains #6683

Closed
gomesalexandre opened this issue Apr 12, 2024 · 0 comments · Fixed by #6700
Closed

Consolidate Tx building into a common util over all THOR domains #6683

gomesalexandre opened this issue Apr 12, 2024 · 0 comments · Fixed by #6700
Assignees

Comments

@gomesalexandre
Copy link
Contributor

gomesalexandre commented Apr 12, 2024

Overview

See this most recent guy which is nice and clean, properly switching over chains/Tx types to build a depositWithExpiryTx:

const handleSignTx = useCallback(() => {

We want to abstract this switching logic away into some helper function, to ensure:

  1. We avoid bugs related to e.g token Txs not being implemented
  2. We consistently call depositWithExpiry() when applicable i.e for EVMs, ensuring expiry is properly set, and facilitating Tx history parsing

References and additional details

const handleSignTx = useCallback(() => {

Acceptance Criteria

  • We have some common util for building THOR Txs with a "per-chain", or to be exact, per Tx type i.e:

    • EvmCustomTx - depositWithExpiry()
    • MsgDeposit - RUNE LP and Swap Txs
    • Send - actual sends
  • EVM transactions leverage EIP1559 if supported otherwise create legacy transactions

    • lib/utils/evm.ts was meant to be a common utility for creating evm transactions with the appropriate gas fees. utilize if possible or think of a good way to capsulate reusable evm fee logic that can be used for thorchain txs or elsewhere in the app
  • This is propagated over all THOR domains whenever applicable:

    • LP (the source of truth for this logic)
    • Savers
    • Lending
    • Swaps (if applicable)

Need By Date

No response

Screenshots/Mockups

No response

Estimated effort

No response

@gomesalexandre gomesalexandre self-assigned this Apr 12, 2024
@gomesalexandre gomesalexandre changed the title Consolidate Tx building in all THOR domains Consolidate Tx building into a common util over all THOR domains Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant