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

feat: use new deterministic ethflow contracts #5334

Merged
merged 4 commits into from
Feb 6, 2025

Conversation

anxolin
Copy link
Contributor

@anxolin anxolin commented Jan 23, 2025

Summary

This PR is in preparation for the ETH FLOW migration. We will replace BARN and PROD eth flow contracts with some new version deployed in deterministic address (same address to all networks).

This is done to minimize the risk of sending ETH to the wrong address in case of a Dapp or Wallet bug.

Context

Tracker for the changes https://www.notion.so/cownation/tracker-ETH-Flow-Contract-Replacements-1828da5f04ca8023af57e76cf6d064d9?pvs=4

See the new deployment and more context:
cowprotocol/ethflowcontract#59

Test

Test old contracts

With he feature flag off, make sure it uses the old barn contracts:

{
  "1": "0xD02De8Da0B71E1B59489794F423FaBBa2AdC4d93",
  "100": "0xD02De8Da0B71E1B59489794F423FaBBa2AdC4d93",
  "11155111": "0x2671994c7D224ac4799ac2cf6Ef9EF187d42C69f",
  "42161": "0x6dfe75b5ddce1ade279d4fa6bd6aef3cbb6f49db",
  "5": "0xD02De8Da0B71E1B59489794F423FaBBa2AdC4d93",
  "8453": "0x3C3eA1829891BC9bEC3d06A81d5d169e52a415e3"
}

Test the new flag

If we enable the flag, it should use 0x04501b9b1d52e67f6862d157e00d13419d2d6e95 no matter which network.

Test new contracts

We could set the flag to true , but because multiple teams are using this PR, we might be affecting each other
#5358

Details on what to test

Test flows:

  • Important to test with the flag ON and OFF for all networks
  • We should test the happy path and the refunds on expiration

How to change the flag:

Copy link

vercel bot commented Jan 23, 2025

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

Name Status Preview Updated (UTC)
cowfi 🔄 Building (Inspect) Visit Preview Feb 6, 2025 3:31pm
explorer-dev 🔄 Building (Inspect) Visit Preview Feb 6, 2025 3:31pm
swap-dev 🔄 Building (Inspect) Visit Preview Feb 6, 2025 3:31pm
3 Skipped Deployments
Name Status Preview Updated (UTC)
cosmos ⬜️ Ignored (Inspect) Visit Preview Feb 6, 2025 3:31pm
sdk-tools ⬜️ Ignored (Inspect) Visit Preview Feb 6, 2025 3:31pm
widget-configurator ⬜️ Ignored (Inspect) Visit Preview Feb 6, 2025 3:31pm

@anxolin anxolin force-pushed the update-eth-flow-contracts branch from 9ec74e6 to 696a633 Compare January 30, 2025 12:55
@anxolin anxolin requested a review from a team January 30, 2025 16:06
@anxolin anxolin marked this pull request as ready for review January 30, 2025 17:29
Copy link
Contributor

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

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

LGTM!

I've retested cancellation expiration, successful cases on all chains when the flag is ON and OFF: no issues found except a know one: #4153

@anxolin anxolin force-pushed the update-eth-flow-contracts branch from 52fb6a3 to 9ba727a Compare February 6, 2025 15:30
@@ -72,7 +80,7 @@ export async function ethFlow(
throw new Error('Quote expired. Please refresh.')
}

if (contract.address !== COWSWAP_ETHFLOW_CONTRACT_ADDRESS_MAP[chainId as SupportedChainId]) {
if (contract.address !== getEthFlowContractAddresses(ethFlowEnv, useNewEthFlowContracts, chainId)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we add .toLowerCase() here?

Copy link
Contributor Author

@anxolin anxolin Feb 6, 2025

Choose a reason for hiding this comment

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

Sorry I saw this before merging. It didn't have if before, but it may be wise. I can address in develop (as this works without the change, if something, is in case a future library of ethers breaks this logic)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@anxolin anxolin merged commit f11fb61 into main Feb 6, 2025
7 of 11 checks passed
@anxolin anxolin deleted the update-eth-flow-contracts branch February 6, 2025 15:34
@github-actions github-actions bot locked and limited conversation to collaborators Feb 6, 2025
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.

4 participants