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

fix: hard code eth-flow contract addresses #3587

Merged
merged 2 commits into from
Jan 9, 2024
Merged

Conversation

shoom3301
Copy link
Collaborator

Summary

Since eth-flow contract address is static and unlikely will change in the future, to keep the code simple I hardcoded addresses.
There is also an issue with resolving version of @cowprotocol/ethflowcontract, sometimes it returns outdated files locally.

To Test

Eth-flow should work

@shoom3301 shoom3301 requested a review from a team January 8, 2024 16:01
@shoom3301 shoom3301 self-assigned this Jan 8, 2024
Copy link

vercel bot commented Jan 8, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
cosmos ✅ Ready (Inspect) Visit Preview Jan 9, 2024 11:10am
explorer ❌ Failed (Inspect) Jan 9, 2024 11:10am
swap-dev ✅ Ready (Inspect) Visit Preview Jan 9, 2024 11:10am
widget-configurator ✅ Ready (Inspect) Visit Preview Jan 9, 2024 11:10am

Comment on lines +55 to +56
prod: mapAddressToSupportedNetworks(COWSWAP_ETHFLOW_CONTRACT_PROD),
barn: mapAddressToSupportedNetworks(COWSWAP_ETHFLOW_CONTRACT_BARN),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not 100% sure.
Right now the addresses are the same, but it's not guaranteed.
See what Federico said

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I understand.
For some reason, locally, yarn wrongly resolves @cowprotocol/ethflowcontract package:
https://cowservices.slack.com/archives/C0361CDG8GP/p1704727878010159

I have no idea why it's happening, but I also faced it before and can't reproduce now.
The only way I see to solve it - hardcoding

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it has something to do with yarn.lock?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can see it contains proper data

Copy link
Collaborator

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned in the comment, I'm not very found of this solution, but it's blocking.
So, let's get it merged and revisit later when we add new networks.

@alfetopito
Copy link
Collaborator

Sadly, this broke Sepolia eth-flow https://cowservices.slack.com/archives/C0361CDD1FZ/p1704815798996489

I'll revert this change and try to fix the issue some other way.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants