fix: defi aggregated opportunities uniqBy id first by gomesalexandre · Pull Request #6628 · shapeshift/web

gomesalexandre · 2024-04-03T10:57:02Z

Description

By calling uniqBy by id first, we effectively fix the issue with opportunities having a wrong amount displayed - the issue was that multiple assets can have the same assetId / contractAddres (e.g ethAssetId), which means we would pick the first opportunity we find for said asset, which could yield unpredictable results.

Pull Request Type

🐛 Bug fix (Non-breaking Change: Fixes an issue)
🛠️ Chore (Non-breaking Change: Doc updates, pkg upgrades, typos, etc..)
💅 New Feature (Breaking/Non-breaking Change)

Issue (if applicable)

closes Stakewise DeFi balances off by orders of magnitude #6608

Risk

High Risk PRs Require 2 approvals

Low - ensure opportunities meta/user data still looks sane

What protocols, transaction types or contract interactions might be affected by this PR?

Testing

Opportunity metadata still looks sane against develop
Testing with willy's wallet, Stakewise DeFi balances off by orders of magnitude #6608 cannot be repro'd in this diff.

Engineering

☝🏽

Operations

☝🏽

Screenshots (if applicable)

gomesalexandre · 2024-04-03T10:57:16Z

fix: defi aggregated opportunities uniqBy id first #6628 👈
fix: deep merge opportunities accounts #6627
develop

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @gomesalexandre and the rest of your teammates on Graphite

…set" This reverts commit 1ed1a59.

0xApotheosis

gm, good catch

* fix: zapperAssetToMaybeAssetId handle base-token as native asset * fix: filter unique opportunities by id first * Revert "fix: zapperAssetToMaybeAssetId handle base-token as native asset" This reverts commit 1ed1a59.

* add watch asset functionality * feat: regenerate asset data 04/02/2024 (#6620) * feat: disable sym deposits after RUNE (#6622) * fix: don't bork asset generation on idle API failures (#6619) * feat: collect 0x trade surplus (#6625) * feat: bottom sheet dialog (#6623) * add bottom sheet dialog * try out vaul * hook up send * Create SubPage.tsx * adjusting dialog header * Update DialogCloseButton.tsx * add new action menu * updates * update more height stuff * use 100dvh instead of vh * use 100vh * anchor footer at the bottom of the screen * update new asset search * Update SelectAssets.tsx * add min height for regular modals * Update MobileNavBar.tsx * remove extra stuff not needed for PR * remove more stuff not for this PR * Update AddressInput.tsx * Update AddressInput.tsx * Update Dialog.tsx * Update SelectAssets.tsx * fix for dialog height * Update AssetSearch.tsx --------- Co-authored-by: gomes <17035424+gomesalexandre@users.noreply.github.com> * chore: use unchained proxy endpoints over nownodes proxy (#6606) * feat: use proxy validation endpoint (#6626) * feat: use proxy validation endpoint * chore: improve fn name * update endpoint * fix: csp * fix: tests * chore: use shapeshift proxy csp * chore: fail open --------- Co-authored-by: kaladinlight <35275952+kaladinlight@users.noreply.github.com> * feat: many migrations (#6631) * fix: remove portal on account dropdown (#6635) * account down remove portal * Update StepRow.tsx * feat: cow affiliate fees (#6621) * chore: update app translations (#6638) * chore: invalidate outdated translations * Translate main.json via GitLocalize * Translate main.json via GitLocalize * Translate main.json via GitLocalize * Translate main.json via GitLocalize * Translate main.json via GitLocalize * Translate main.json via GitLocalize * Translate main.json via GitLocalize * Translate main.json via GitLocalize * Translate main.json via GitLocalize * Translate main.json via GitLocalize --------- Co-authored-by: Hellhound13 <hellhound13@gmx.at> Co-authored-by: Jpanam <jorgepana26@hotmail.com> Co-authored-by: yuki <yuki@beyondclub.xyz> Co-authored-by: guiribabrb <guilherme.rb.barbosa@gmail.com> Co-authored-by: Markus Meyer <djarmy@gmail.com> Co-authored-by: Romko <29romko29@gmail.com> Co-authored-by: tonyjiang12399 <327904087@qq.com> * feat: cow quote/unsigned order build cleanup (#6639) * feat: getFullAppData tests * feat: cow quote/unsigned order build cleanup * fix: deep merge opportunities accounts (#6627) * feat: regenerate asset data 04/04/2024 (#6636) Co-authored-by: GitHub Action <action@github.com> * fix: defi aggregated opportunities uniqBy id first (#6628) * fix: zapperAssetToMaybeAssetId handle base-token as native asset * fix: filter unique opportunities by id first * Revert "fix: zapperAssetToMaybeAssetId handle base-token as native asset" This reverts commit 1ed1a59. * feat: lending loan summary repayment lock (#6642) * fix: mobile dashboard routing (#6637) * feat: thorchain lp disable max deposits (#6645) * feat: uni-v2 pools static generation (#6629) * feat: uni-v2 pools static generation * feat: revert DynamicLpAssets flag * feat: regen * feat: cleanup asset upserty bits * feat: regen * feat: regenerate asset data 04/08/2024 (#6648) * feat: thorchain LP charts (#6624) * add home page * trigger ci * Update AssetActions.tsx * feat: tag foxy as expired opportunity (#6651) * fix: empty my position pools page (#6656) Update YourPositions.tsx * Update src/pages/Home/Home.tsx Co-authored-by: gomes <17035424+gomesalexandre@users.noreply.github.com> * Update src/state/slices/preferencesSlice/preferencesSlice.ts Co-authored-by: gomes <17035424+gomesalexandre@users.noreply.github.com> * rename to assetIds * rework the avatar * more comments * add top assets default * feat: naming --------- Co-authored-by: woodenfurniture <125113430+woodenfurniture@users.noreply.github.com> Co-authored-by: gomes <17035424+gomesalexandre@users.noreply.github.com> Co-authored-by: Apotheosis <0xapotheosis@gmail.com> Co-authored-by: kevin <35275952+kaladinlight@users.noreply.github.com> Co-authored-by: firebomb1 <88804546+firebomb1@users.noreply.github.com> Co-authored-by: Hellhound13 <hellhound13@gmx.at> Co-authored-by: Jpanam <jorgepana26@hotmail.com> Co-authored-by: yuki <yuki@beyondclub.xyz> Co-authored-by: guiribabrb <guilherme.rb.barbosa@gmail.com> Co-authored-by: Markus Meyer <djarmy@gmail.com> Co-authored-by: Romko <29romko29@gmail.com> Co-authored-by: tonyjiang12399 <327904087@qq.com> Co-authored-by: GitHub Action <action@github.com> Co-authored-by: Apotheosis <97164662+0xApotheosis@users.noreply.github.com>

* feat: regenerate asset data 04/02/2024 (#6620) * feat: disable sym deposits after RUNE (#6622) * fix: don't bork asset generation on idle API failures (#6619) * feat: collect 0x trade surplus (#6625) * feat: bottom sheet dialog (#6623) * add bottom sheet dialog * try out vaul * hook up send * Create SubPage.tsx * adjusting dialog header * Update DialogCloseButton.tsx * add new action menu * updates * update more height stuff * use 100dvh instead of vh * use 100vh * anchor footer at the bottom of the screen * update new asset search * Update SelectAssets.tsx * add min height for regular modals * Update MobileNavBar.tsx * remove extra stuff not needed for PR * remove more stuff not for this PR * Update AddressInput.tsx * Update AddressInput.tsx * Update Dialog.tsx * Update SelectAssets.tsx * fix for dialog height * Update AssetSearch.tsx --------- Co-authored-by: gomes <17035424+gomesalexandre@users.noreply.github.com> * chore: use unchained proxy endpoints over nownodes proxy (#6606) * feat: use proxy validation endpoint (#6626) * feat: use proxy validation endpoint * chore: improve fn name * update endpoint * fix: csp * fix: tests * chore: use shapeshift proxy csp * chore: fail open --------- Co-authored-by: kaladinlight <35275952+kaladinlight@users.noreply.github.com> * feat: many migrations (#6631) * fix: remove portal on account dropdown (#6635) * account down remove portal * Update StepRow.tsx * feat: cow affiliate fees (#6621) * chore: update app translations (#6638) * chore: invalidate outdated translations * Translate main.json via GitLocalize * Translate main.json via GitLocalize * Translate main.json via GitLocalize * Translate main.json via GitLocalize * Translate main.json via GitLocalize * Translate main.json via GitLocalize * Translate main.json via GitLocalize * Translate main.json via GitLocalize * Translate main.json via GitLocalize * Translate main.json via GitLocalize --------- Co-authored-by: Hellhound13 <hellhound13@gmx.at> Co-authored-by: Jpanam <jorgepana26@hotmail.com> Co-authored-by: yuki <yuki@beyondclub.xyz> Co-authored-by: guiribabrb <guilherme.rb.barbosa@gmail.com> Co-authored-by: Markus Meyer <djarmy@gmail.com> Co-authored-by: Romko <29romko29@gmail.com> Co-authored-by: tonyjiang12399 <327904087@qq.com> * feat: cow quote/unsigned order build cleanup (#6639) * feat: getFullAppData tests * feat: cow quote/unsigned order build cleanup * fix: deep merge opportunities accounts (#6627) * feat: regenerate asset data 04/04/2024 (#6636) Co-authored-by: GitHub Action <action@github.com> * fix: defi aggregated opportunities uniqBy id first (#6628) * fix: zapperAssetToMaybeAssetId handle base-token as native asset * fix: filter unique opportunities by id first * Revert "fix: zapperAssetToMaybeAssetId handle base-token as native asset" This reverts commit 1ed1a59. * feat: lending loan summary repayment lock (#6642) * fix: mobile dashboard routing (#6637) * feat: thorchain lp disable max deposits (#6645) * feat: uni-v2 pools static generation (#6629) * feat: uni-v2 pools static generation * feat: revert DynamicLpAssets flag * feat: regen * feat: cleanup asset upserty bits * feat: regen * feat: regenerate asset data 04/08/2024 (#6648) * feat: thorchain LP charts (#6624) * feat: tag foxy as expired opportunity (#6651) * fix: empty my position pools page (#6656) Update YourPositions.tsx * feat: add first pass WarningAcknowledgement component * chore: use translations * chore: wire up savers deposit * chore: tidy * feat: use warningAcknowledgement in trade quote confirm * feat: use warningAcknowledgement in trade quote confirm * feat: use warningAcknowledgement in trade lp deposit * fix: trade quote input warning logic * chore: use dimmed background * chore: component a little closer to mock * chore: component tweaks * chore: style fixes * chore: use cancel translation * fix: trade confirm warning * Update WarningAcknowledgement.tsx * Update WarningAcknowledgement.tsx * feat: chart styles and loading state (#6655) * fix: thor token repayments (#6676) * fix: don't render back button on modals initial routes (#6675) * chore: use translations & constants * chore: address review feedback * fix: unbork trade widget size * fix: show minimum trade amount when unsafe * fix: trade confirm width * chore: trigger CI * fix: typo * chore: extract motion style to constant * fix: add no min fallback message --------- Co-authored-by: woodenfurniture <125113430+woodenfurniture@users.noreply.github.com> Co-authored-by: gomes <17035424+gomesalexandre@users.noreply.github.com> Co-authored-by: reallybeard <89934888+reallybeard@users.noreply.github.com> Co-authored-by: kevin <35275952+kaladinlight@users.noreply.github.com> Co-authored-by: firebomb1 <88804546+firebomb1@users.noreply.github.com> Co-authored-by: Hellhound13 <hellhound13@gmx.at> Co-authored-by: Jpanam <jorgepana26@hotmail.com> Co-authored-by: yuki <yuki@beyondclub.xyz> Co-authored-by: guiribabrb <guilherme.rb.barbosa@gmail.com> Co-authored-by: Markus Meyer <djarmy@gmail.com> Co-authored-by: Romko <29romko29@gmail.com> Co-authored-by: tonyjiang12399 <327904087@qq.com> Co-authored-by: GitHub Action <action@github.com>

gomesalexandre mentioned this pull request Apr 3, 2024

fix: deep merge opportunities accounts #6627

Merged

3 tasks

gomesalexandre marked this pull request as ready for review April 3, 2024 10:59

gomesalexandre requested a review from a team as a code owner April 3, 2024 10:59

Base automatically changed from fix_defi_opportunities to develop April 5, 2024 00:42

gomesalexandre added 3 commits April 5, 2024 13:56

fix: zapperAssetToMaybeAssetId handle base-token as native asset

41f34c7

fix: filter unique opportunities by id first

f4ee85e

Revert "fix: zapperAssetToMaybeAssetId handle base-token as native as…

a81e722

…set" This reverts commit 1ed1a59.

0xApotheosis force-pushed the fix_defi_opportunities_2 branch from c4dcf92 to a81e722 Compare April 5, 2024 02:56

0xApotheosis self-assigned this Apr 5, 2024

0xApotheosis approved these changes Apr 5, 2024

View reviewed changes

0xApotheosis merged commit 88b3612 into develop Apr 5, 2024

0xApotheosis deleted the fix_defi_opportunities_2 branch April 5, 2024 03:06

0xApotheosis mentioned this pull request Apr 7, 2024

chore: release v1.571.0 #6646

Merged

0xApotheosis mentioned this pull request Apr 10, 2024

chore: release v1.572.0 #6657

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: defi aggregated opportunities uniqBy id first#6628

fix: defi aggregated opportunities uniqBy id first#6628
0xApotheosis merged 3 commits intodevelopfrom
fix_defi_opportunities_2

gomesalexandre commented Apr 3, 2024 •

edited

Loading

Uh oh!

gomesalexandre commented Apr 3, 2024 •

edited

Loading

Uh oh!

0xApotheosis left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

gomesalexandre commented Apr 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull Request Type

Issue (if applicable)

Risk

Testing

Engineering

Operations

Screenshots (if applicable)

Uh oh!

gomesalexandre commented Apr 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xApotheosis left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gomesalexandre commented Apr 3, 2024 •

edited

Loading

gomesalexandre commented Apr 3, 2024 •

edited

Loading