-
Notifications
You must be signed in to change notification settings - Fork 428
chore: tracking txs usd amounts to amplitude #2564
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
293a472
to
bbb8816
Compare
|
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
@grothem are we missing any places top of mind for calculating all the amount flows |
bbb8816
to
bee5e1b
Compare
rebased |
|
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
@mgrabina are we able to handle the corner case of repay max? its USD value is -1 |
![]() |
|
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
good catch! fixed now |
|
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
@@ -197,8 +200,11 @@ export const RepayActions = ({ | |||
action, | |||
txState: 'success', | |||
asset: poolAddress, | |||
amount: amountToRepay, | |||
amount: amountToRepay === '-1' ? maxAmountToRepay : amountToRepay, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mgrabina should this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think we just care about the amountUsd for that value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we currently have the same issue in amount sending -1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but is it changing anything in the actual transaction or is it just information sent to amplitude
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope transaction is sent just above
(whatsmore addTransaction only executed if tx was successful)
and addTransaction just 'saves' the tx in the state, which are used only in TransactionEventHandler just for tracking
@mgrabina repaying wSTETH left negative balance ![]() |
e.g.
