Add Goerli support #170

henrypalacios · 2022-07-25T12:56:09Z

Summary

Closes #136

Add goerli network to cow-sdk instance constants (Update cow-sdk library)
Add the option in the network-list menu in the header UI
Create and verify new route prefix network /goerli
Enable/check routes for /goerli suffixes such as:
- /search,
- /address,
- /orders,
- /tx
Fill in the home page data from the goerli network subgraph

coveralls · 2022-07-25T13:01:06Z

Pull Request Test Coverage Report for Build 2828116247

5 of 7 (71.43%) changed or added relevant lines in 4 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.07%) to 43.824%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/state/network/updater.tsx	1	2	50.0%
src/utils/miscellaneous.ts	1	2	50.0%

Totals
Change from base Build 2707860945:	-0.07%
Covered Lines:	2158
Relevant Lines:	4174

💛 - Coveralls

github-actions · 2022-07-25T13:01:40Z

🔭 Explorer: Explorer test app
🔒 Safe Swap App: Trade test app (add to your safe apps)
📈 Trade: Trade test app
📚 Storybook: Component stories

package.json

ramirotw · 2022-07-26T20:02:49Z

src/const.ts

  },
 )

-export const COW_SDK = [Network.MAINNET, Network.RINKEBY, Network.GNOSIS_CHAIN].reduce<
+export const COW_SDK = [Network.MAINNET, Network.RINKEBY, Network.GNOSIS_CHAIN, Network.GOERLI].reduce<


with the latest version we can omit this and have just 1 instance instead of 4.

Then we can pass the chainId as an options object to each method of the cowApi class

ramirotw · 2022-07-26T20:02:58Z

src/const.ts

@@ -241,7 +242,7 @@ export const COW_SDK = [Network.MAINNET, Network.RINKEBY, Network.GNOSIS_CHAIN].
  return acc
 }, {})

-export const COW_SDK_DEV = [Network.MAINNET, Network.RINKEBY, Network.GNOSIS_CHAIN].reduce<
+export const COW_SDK_DEV = [Network.MAINNET, Network.RINKEBY, Network.GNOSIS_CHAIN, Network.GOERLI].reduce<


same as above

henrypalacios · 2022-07-27T02:41:00Z

with the latest version we can omit this and have just 1 instance instead of 4.
Then we can pass the chainId as an options object to each method of the cowApi class

☝️ Thanks for feedback @ramirotw, I'm aware of it. But as I mentioned, I was focused first on achieving full functionality of the routes.

At this point I'm stuck looking for the bug responsible for not sending the requests to barn 🤔

elena-zh · 2022-07-27T09:51:02Z

Hey @henrypalacios , I have not tested the PR yet, just opened it and decided to leave this note for you: Gorli network color in Cowswap is a light-blue, so it would be nice, I think, to make colors consistent in both applications

And this issue might not be related to you current implementation, but 24h transactions show 'Infininty' %

package.json

ramirotw · 2022-08-08T21:05:22Z

When the sdk is published I'll ping you @elena-zh to test again

alfetopito · 2022-08-09T17:36:19Z

Just a note: there was an issue with the backend node being indexed. It's being addressed but expect it to not have data for a couple of minutes/hours

alfetopito

Nice!

One small thing I noticed and I hope I did the suggestion in the right place in the code. The network name in the UI should be Görli

Besides that can't really test much until backend has finished with the fix

src/api/operator/operatorApi.ts

src/components/NetworkSelector/index.tsx

add the thingy on top of the `o` Co-authored-by: Leandro Boscariol <[email protected]>

anxolin

It would have been nice to split this one in two:

Update to SDK 1.0.0
Add Goerli

No need to do it now, but the reason is:

Makes it easier to review, only Goerli changes here. So better reviews, give more code assurance
The PR would be easily serve as documentation on how to add a new network. We will soon need to add other networks, so it's nice to have some reference PRs for it

I had an issue testing this, not sure if I did it right. It's probably related to the issue of the indexing of the backend?

I tried to trade in Goerli using barn, but we weren't able to find the order:
https://pr170--explorer.review.gnosisdev.com/goerli/search/0x3251328616d152ecd178850af413eff15bb284b29b143d9cebe9f7870b8ce5d979063d9173c09887d536924e2f6eadbabac099f562f3829f/

Last question, why there's no volumes in the top traded tokens?

alfetopito

Looks good, and I can see orders now!

But, it looks like the appData section in the order details broke in this PR https://pr170--explorer.review.gnosisdev.com/goerli/orders/0x125ba4899f3283620f14fff030920524c67f57dd1c9a7c53bfbcc2c5d0ef79f15b0abe214ab7875562adee331deff0fe1912fe4262f385a1

It looks fine on dev, so probably something with the API changes

elena-zh · 2022-08-10T11:13:59Z

Hey @henrypalacios , @ramirotw , nice PR!

Some issues:

Seems that @anxolin has reported it above

I tried to trade in Goerli using barn, but we weren't able to find the order:
https://pr170--explorer.review.gnosisdev.com/goerli/search/0x3251328616d152ecd178850af413eff15bb284b29b143d9cebe9f7870b8ce5d979063d9173c09887d536924e2f6eadbabac099f562f3829f/

So I also noticed, that BARN and PROD APIs are mixed up in this PR: I see orders from Prod in users' details page in Dev explorer, and do not see orders from Dev and Barn there

2. Then, when I press on the last batch ID in Gorly dashboard, open transaction details and navigate to an order details page, I see not found page, however, an order should be opened (see the video)
3. There are no links to contracts at the bottom of the page when Gorly is selected

4. When search for a transaction from a different network (the app is opened in Mainnet, search for Gorli transaction), amounts are not loaded (currently, no issues on Prod with this)

I also have faced a known issue Top tokens for another chain may be displayed when switch a network from an order details page #151 when I was navigated to Gorli main page, but Tokens page form GC was displayed there, so it would be nice to fix it in the near future...

Thanks!

ramirotw · 2022-08-18T13:48:17Z

Changes included in PR #182

Add option in network-list menu UI

b2d02fe

henrypalacios added app:Explorer Explorer App Protofire labels Jul 25, 2022

henrypalacios self-assigned this Jul 25, 2022

Fix error on path route /goerli

e53ed60

ramirotw reviewed Jul 26, 2022

View reviewed changes

henrypalacios changed the title ~~Add option in network-list menu UI~~ Add Goerli option in network-list menu UI Jul 26, 2022

henrypalacios changed the title ~~Add Goerli option in network-list menu UI~~ Add Goerli option in network-list menu Jul 26, 2022

henrypalacios added 2 commits July 26, 2022 18:19

Using cow-sdk 1.0.0

705d9ae

Using cow-sdk 1.0.0

e730713

henrypalacios force-pushed the 136-add-goerli branch 2 times, most recently from af592de to 38be36b Compare July 27, 2022 01:18

Using cow-sdk 1.0.0

cb7a75c

henrypalacios force-pushed the 136-add-goerli branch from 38be36b to cb7a75c Compare July 27, 2022 02:03

ramirotw added 5 commits August 1, 2022 08:43

bump sdk to rc.1

644561b

apply latest sdk changes

0cf6341

add missing networks

f2d886b

add missing networks

408fe5a

reuse % diff function

c23d518

ramirotw reviewed Aug 8, 2022

View reviewed changes

package.json Outdated Show resolved Hide resolved

ramirotw added 2 commits August 8, 2022 17:57

add goerli to network search list

d123f35

change network selector color

5f613f6

bump cow sdk

001c1a8

ramirotw changed the title ~~Add Goerli option in network-list menu~~ Add Goerli support Aug 9, 2022

bump sdk to 1.0.0-RC.4

558f200

ramirotw marked this pull request as ready for review August 9, 2022 17:32

ramirotw requested review from alfetopito and elena-zh August 9, 2022 17:32

alfetopito reviewed Aug 9, 2022

View reviewed changes

src/api/operator/operatorApi.ts Show resolved Hide resolved

src/components/NetworkSelector/index.tsx Outdated Show resolved Hide resolved

ramirotw and others added 2 commits August 9, 2022 16:36

Update src/components/NetworkSelector/index.tsx

3a523b2

add the thingy on top of the `o` Co-authored-by: Leandro Boscariol <[email protected]>

fix css classes

aad434d

anxolin reviewed Aug 10, 2022

View reviewed changes

alfetopito reviewed Aug 10, 2022

View reviewed changes

alfetopito mentioned this pull request Aug 11, 2022

136 add goerli 2 #182

Merged

3 tasks

ramirotw closed this Aug 18, 2022

alfetopito deleted the 136-add-goerli branch August 19, 2022 14:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Goerli support #170

Add Goerli support #170

henrypalacios commented Jul 25, 2022 •

edited by ramirotw

Loading

coveralls commented Jul 25, 2022 •

edited

Loading

github-actions bot commented Jul 25, 2022

ramirotw Jul 26, 2022

ramirotw Jul 26, 2022

henrypalacios commented Jul 27, 2022

elena-zh commented Jul 27, 2022

ramirotw commented Aug 8, 2022

alfetopito commented Aug 9, 2022

alfetopito left a comment

anxolin left a comment

alfetopito left a comment •

edited

Loading

elena-zh commented Aug 10, 2022 •

edited

Loading

ramirotw commented Aug 18, 2022

Add Goerli support #170

Add Goerli support #170

Conversation

henrypalacios commented Jul 25, 2022 • edited by ramirotw Loading

Summary

coveralls commented Jul 25, 2022 • edited Loading

Pull Request Test Coverage Report for Build 2828116247

💛 - Coveralls

github-actions bot commented Jul 25, 2022

ramirotw Jul 26, 2022

Choose a reason for hiding this comment

ramirotw Jul 26, 2022

Choose a reason for hiding this comment

henrypalacios commented Jul 27, 2022

elena-zh commented Jul 27, 2022

ramirotw commented Aug 8, 2022

alfetopito commented Aug 9, 2022

alfetopito left a comment

Choose a reason for hiding this comment

anxolin left a comment

Choose a reason for hiding this comment

alfetopito left a comment • edited Loading

Choose a reason for hiding this comment

elena-zh commented Aug 10, 2022 • edited Loading

ramirotw commented Aug 18, 2022

henrypalacios commented Jul 25, 2022 •

edited by ramirotw

Loading

coveralls commented Jul 25, 2022 •

edited

Loading

alfetopito left a comment •

edited

Loading

elena-zh commented Aug 10, 2022 •

edited

Loading