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

feat(earn): prepare transactions and use on confirmation screen #6192

Draft
wants to merge 8 commits into
base: tomm/act-1389-0
Choose a base branch
from

Conversation

MuckT
Copy link
Collaborator

@MuckT MuckT commented Oct 25, 2024

Description

Handles the preparation of all earn transactions via a shared hook that's used on both EarnEnterAmount.tsx & EarnConfirmationScreen.tsx.

Test plan

  • Tested locally on iOS
  • Tested locally on Andorid
  • Unit Tests updated

Related issues

Backwards compatibility

Yes

Network scalability

N/A

Comment on lines 90 to 82
})
// @ts-expect-error prepareTransactionsResult & swapTransaction are only set when mode === 'deposit' or 'swap-deposit'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand these two lines.

  1. Seems like a red flag to do a ts-ignore here. if the return values of the different modes are that different then should they maybe be changed at the prepare transaction level?
  2. swapTransaction doesn't even seem to be used in the code.

Copy link
Collaborator Author

@MuckT MuckT Oct 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ts-expect-error is temporary; I'll remove it before moving the PR out of draft and resolving the type issues. While swapTransaction isn't currently used on this page there's a bug with the bottom sheet implementation that will likely require swap-deposit and deposit to be handled from a confirmation screen.

pool,
hooksApiUrl,
// Mode === shortcutId, except for exit which is withdraw
shortcutId: mode === 'exit' ? 'withdraw' : mode,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So exit maps to 'prepareWithdrawAndClaimTransactions' which doesn't appear to have shortcutId as a parameter. So why is this needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 98a8bd0.

@@ -144,6 +144,7 @@ function EarnEnterAmount({ route }: Props) {
}

const {
// @ts-expect-error prepareTransactionsResult & swapTransaction are only set when mode === 'deposit' or 'swap-deposit'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, the ts-expects-error is a big red flag. The underlying types should probably be changed or we just need separate functions


// Prepare the claim transactions if there are rewards positions
const claimTransactions = rewardsPositions
? await Promise.all(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this checks if rewardsPositions is falsy, but can it ever be falsy since its guarenteed to be an array?

Copy link
Collaborator Author

@MuckT MuckT Oct 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e9f2c07 & f1ac2db

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants