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

Uniswap doesn't have the required data to calculate yield #775

Open
dmatora opened this issue Feb 16, 2022 · 9 comments
Open

Uniswap doesn't have the required data to calculate yield #775

dmatora opened this issue Feb 16, 2022 · 9 comments

Comments

@dmatora
Copy link

dmatora commented Feb 16, 2022

Getting Uniswap doesn't have the required data to calculate yield error when running yarn test

@danielattilasimon
Copy link
Collaborator

Hey @dmatora!

The reason for the error is that the tests take place in a local blockchain environment where Uniswap (more specifically its subgraph) isn't available. The same error can be seen in the browser console if you try to use the frontend on any of the Ethereum testnets.

We should have probably made this a warning instead, as it's actually harmless. For example see this passing CI run where the same error is present:
https://github.com/liquity/dev/runs/4287514833?check_suite_focus=true#step:8:461

@dmatora
Copy link
Author

dmatora commented Feb 17, 2022

Shouldn't happen if i checkout mainnet-coverage branch, right?

@danielattilasimon
Copy link
Collaborator

The error is expected to show up regardless of branch when running yarn test, but it doesn't cause the test to fail. Are the tests failing on your end? In that case, there might be a different root cause.

@dmatora
Copy link
Author

dmatora commented Feb 17, 2022

Switching branch does solve the issue for me (at least when using fresh Linux box for new build), but the person who asked me for help (running windows) says he is still getting the issue even when using specified branch

@danielattilasimon
Copy link
Collaborator

danielattilasimon commented Feb 17, 2022

Ah, so on the main branch, the tests are failing for you? Can you show the last 100 or so lines of output?

I double checked by running the tests locally just now (on main branch), and they are passing on my end, even though the Uniswap error is visible:

  console.error
    Uniswap doesn't have the required data to calculate yield

      31 |         setLqtyPrice(lqtyPriceUSD);
      32 |       } catch (error) {
    > 33 |         console.error(error);
         |                 ^
      34 |       }
      35 |     })();
      36 |   }, [lqtyTokenAddress]);

      at src/components/Stability/Yield.tsx:33:17

  console.log
    Estimated TX cost: 698,079

      at src/components/Trove/ExpensiveTroveChangeWarning.tsx:49:19

  console.log
    Start monitoring tx 0x21efda249de2d774317e313bd101148f52886da9180b386cf7d6970dd740275d

      at src/components/Transaction.tsx:364:15

  console.log
    Block #25 1-confirms tx 0x21efda249de2d774317e313bd101148f52886da9180b386cf7d6970dd740275d

      at waitForConfirmation (src/components/Transaction.tsx:312:19)

  console.log
    Finish monitoring tx 0x21efda249de2d774317e313bd101148f52886da9180b386cf7d6970dd740275d

      at waitForConfirmation (src/components/Transaction.tsx:313:19)

  console.log
    Logs of tx 0x21efda249de2d774317e313bd101148f52886da9180b386cf7d6970dd740275d:
      priceFeed.LastGoodPriceUpdated({ _lastGoodPrice: 200e18 })
      troveManager.BaseRateUpdated({ _baseRate: 0 })
      troveManager.LastFeeOpTimeUpdated({ _lastFeeOpTime: 0.000000001645073499e18 })
      lqtyStaking.F_LUSDUpdated({ _F_LUSD: 0 })
      lusdToken.Transfer({ from: address(0), to: lqtyStaking, value: 10.045e18 })
      troveManager.TroveSnapshotsUpdated({ _L_ETH: 0, _L_LUSDDebt: 0 })
      troveManager.TotalStakesUpdated({ _newTotalStakes: 20e18 })
      sortedTroves.NodeAdded({ _id: user, _NICR: 0.901288617400728691e18 })
      borrowerOperations.TroveCreated({ _borrower: user, arrayIndex: 0 })
      activePool.ActivePoolETHBalanceUpdated({ _ETH: 20e18 })
      activePool.ActivePoolLUSDDebtUpdated({ _LUSDDebt: 2019.045e18 })
      lusdToken.Transfer({ from: address(0), to: user, value: 2009e18 })
      activePool.ActivePoolLUSDDebtUpdated({ _LUSDDebt: 2219.045e18 })
      lusdToken.Transfer({ from: address(0), to: gasPool, value: 200e18 })
      borrowerOperations.TroveUpdated({ _borrower: user, _debt: 2219.045e18, _coll: 20e18, stake: 20e18, operation: 0 })
      borrowerOperations.LUSDBorrowingFeePaid({ _borrower: user, _LUSDFee: 10.045e18 })

      at waitForConfirmation (src/components/Transaction.tsx:317:21)

 PASS  src/App.test.tsx (11.279 s)
  ✓ there's no smoke (2604 ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        12.048 s
Ran all test suites.

@dmatora
Copy link
Author

dmatora commented Feb 17, 2022

Well, it says Tests: 1 passed, 1 total even if you run yarn workspace @liquity/dev-frontend test --watchAll=false --forceExit --detectOpenHandles without starting frontend server and it crashes with Error: connect ECONNREFUSED 127.0.0.1:80
Btw, any clue why test expects frontend on port 80 instead of 3000?

@danielattilasimon
Copy link
Collaborator

The frontend checks for an optional config.json (looks like this: https://devui.liquity.org/config.json), however when running the frontend in React Testing Library, this file won't be present; in fact the frontend assets aren't even served via HTTP.

So when the frontend checks for the config file (happens here) there isn't even an HTTP server to connect to, hence Error: connect ECONNREFUSED 127.0.0.1:80.

Ideally, we should have mocked out any network requests from the test. However, the error isn't fatal, and the frontend proceeds with the default configuration, which is fine for the purpose of this test.

If in the end it says Tests: 1 passed, 1 total, then it's working as expected.

@dmatora
Copy link
Author

dmatora commented Feb 17, 2022

I'm using docker run --name liquity -d --rm -p 80:80 liquity/dev-frontend to workaround the issue

@danielattilasimon
Copy link
Collaborator

No workaround is needed. Both the Uniswap yield error and ECONNREFUSED are expected to show up during a successful run of the tests. They can be ignored. It would be nice if they didn't, but since they are harmless, we didn't prioritize eliminating them.

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

No branches or pull requests

3 participants
@dmatora @danielattilasimon and others