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

Create MarketPriceSelection component #496

Closed
11 tasks done
LukSteib opened this issue Oct 12, 2022 · 6 comments · Fixed by #498
Closed
11 tasks done

Create MarketPriceSelection component #496

LukSteib opened this issue Oct 12, 2022 · 6 comments · Fixed by #498
Assignees

Comments

@LukSteib
Copy link
Contributor

LukSteib commented Oct 12, 2022

Goal

Modify UI elements to support multiple callees

Context

As part of our liquidity sources stream we decided to implement multi callee support. For background discussions and overall concept please read #374

This issue tackles all of the required adjustments to the UI in order to finally enable the new feature

Tasks

  • Modify Auction type to contain new keys (see E below for a proposal)
    • note: marketData and suggestedMarketId are the new keys to be introduced
  • Modify fake function to create desired type (note that unitPrice can also be new BigNumber(NaN))
  • Create MarketPriceSelection component as shown in the mock
    • Use animation as in the BasePanel
    • Expect BigNumber.isNaN() to be true for some markets (so they should display unknown and not be selectable)
    • Use synced prop marketId to emit and receive currently selected marketId
    • Expect that suggestedMarketId might change after the component render –> meaning that
      • in case no selection was made by the user, selected market might change by itself
      • in case a selection was made by the user, it should stay fixed even if suggestedMarketId changes
  • Send marketId (suggestedMarketId if none selected) to the bidWithCallee action

Assets

Mock:
MakerDAO collateral@2x

E:

    marketUnitPrice: BigNumber
    suggestedMarketId: 'Uniswap v3'
    marketData: {
            "Uniswap v3": {
                 unitPrice: BigNumber,
                 route: string[],
            },
            "Uniswap v2": {
                 unitPrice: BigNumber,
                 route: string[],
            },
        }
    },
@aomafarag
Copy link
Contributor

aomafarag commented Oct 13, 2022

  • Send marketId (suggestedMarketId if none selected) to the bidWithCallee action

The bidWithCallee action in store/auctions has an object with two properties as a parameter: { id, alternativeDestinationAddress }. id is used to get the auction by its ID and alternativeDestinationAddress is used to specify an alternative wallet address. Then, the bidWithCallee function in core/src/auctions is called to execute the transaction. There, getCalleeAddressByCollateralType is called to get the callee address, depending on the network, the collateral type and the callee assigned to it (networkCallees[collateral.exchange.callee]).

Question: Is my task just to pass a third property in the object parameter, i.e. { id, marketId, alternativeDestinationAddress }, and not worry about the logic that will follow? This, however, will produce a build error as we would be introducing a parameter we wouldn't use. Or should this task be part of, or implemented after, #497?

@aomafarag
Copy link
Contributor

Mock: MakerDAO collateral@2x

As seen in the mock, we have the Market Difference row, whose value depends on the selected callee in Market Unit Price. This should be taken into account when refactoring the core in #497.

@valiafetisov
Copy link
Contributor

valiafetisov commented Oct 13, 2022

Very good question that I missed. I think we have to reapply calculation of the profits (basically all rows below Market Unit Price will be changed). We can only do this in the store based on the selected market. This means, you will need to pass marketId variable all the way to the store. Other solution would be to receive all those parameters inside marketData already pre-calculated, eg:

    marketUnitPrice: BigNumber
    suggestedMarketId: 'Uniswap v3'
    marketData: {
            "Uniswap v3": {
                 marketUnitPrice: BigNumber,
                 route: string[],
                 transactionGrossProfit: BigNumber,
                 transactionGrossProfitDate: Date,
                 transactionNetProfit: BigNumber,
                 // etc
            },
            "Uniswap v2": {
                 marketUnitPrice: BigNumber,
                 route: string[],
                 transactionGrossProfit: BigNumber,
                 transactionGrossProfitDate: Date,
                 transactionNetProfit: BigNumber,
                 // etc
            },
        }
    },

@valiafetisov
Copy link
Contributor

Is my task just to pass a third property in the object parameter, i.e. { id, marketId, alternativeDestinationAddress }, and not worry about the logic that will follow

Yes, you need to pass is to the dispatch, log it in the action, but not pass down to the core until the core is updated in another PR

@aomafarag
Copy link
Contributor

    marketUnitPrice: BigNumber
    suggestedMarketId: 'Uniswap v3'
    marketData: {
            "Uniswap v3": {
                 marketUnitPrice: BigNumber,
                 route: string[],
                 transactionGrossProfit: BigNumber,
                 transactionGrossProfitDate: Date,
                 transactionNetProfit: BigNumber,
                 // etc
            },
            "Uniswap v2": {
                 marketUnitPrice: BigNumber,
                 route: string[],
                 transactionGrossProfit: BigNumber,
                 transactionGrossProfitDate: Date,
                 transactionNetProfit: BigNumber,
                 // etc
            },
        }
    },

Should the marketUnitPriceToUnitPriceRatio be included in marketData as well for displaying the Market Difference?

@valiafetisov
Copy link
Contributor

yes, since this parameter also depends on the marketUnitPrice

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