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(base): add base chain support #5044

Merged
merged 24 commits into from
Nov 26, 2024
Merged

feat(base): add base chain support #5044

merged 24 commits into from
Nov 26, 2024

Conversation

alfetopito
Copy link
Collaborator

@alfetopito alfetopito commented Oct 24, 2024

Summary

Initial Base chain support.

Still not 100% functional as backend/solvers are not 100% up.
It has a feature flag so should be safe to merge even without full support.

To Test

  1. Full app test on Base network

@alfetopito alfetopito self-assigned this Oct 24, 2024
Copy link

vercel bot commented Oct 24, 2024

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

Name Status Preview Updated (UTC)
cosmos ✅ Ready (Inspect) Visit Preview Nov 26, 2024 1:03pm
cowfi ✅ Ready (Inspect) Visit Preview Nov 26, 2024 1:03pm
explorer-dev ✅ Ready (Inspect) Visit Preview Nov 26, 2024 1:03pm
swap-dev ✅ Ready (Inspect) Visit Preview Nov 26, 2024 1:03pm
widget-configurator ✅ Ready (Inspect) Visit Preview Nov 26, 2024 1:03pm

Copy link

socket-security bot commented Oct 24, 2024

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report↗︎

@alfetopito alfetopito force-pushed the feat/base branch 3 times, most recently from 3450c83 to 2be0d47 Compare October 31, 2024 16:21
@elena-zh
Copy link
Contributor

elena-zh commented Nov 7, 2024

Hey @alfetopito , great job!
I've found some issues/improvements, most of them we discussed in this thread. I will list here some new ones and some questions that would be nice to address later (not to forget about it):

  1. (nitpick) On Explorer, the chain color looks very similar to the one we use for Arbitrum. Might it be changed a bit?
    diff color

  2. Surplus tooltip: it would be great to change a date for 'November 2024' (or December)
    now

  3. There is no validation on a minimum chunk size for a TWAP order

  4. In general, AFAIU, TWAP orders are not working yet

  5. (might be an edge case): I created a TWAP order along with Safe modification in 1 TX, but the warning remained to be displayed until I refreshed the page. Order creation TX was signed with sponsored funds by Safe (e.g. not executed though connected wallet). Could you please check whether the same issue happens to you?
    image

  6. Is this OK that a limit price may be slighty recalculated on an order placement? See, I set it to 27, but the order shows 29.0051
    image

  7. (does not work yet): Hooks simulation fails in Base (will be fixed when bff changes are merged)

  8. (discuss later): Hooks support in Base (not working)
    image

  9. (discuss later): COW token liquidity in Base

  10. (discuss later): CoW AMM pools in Base

@alfetopito
Copy link
Collaborator Author

Hey @alfetopito , great job! I've found some issues/improvements, most of them we discussed in this thread. I will list here some new ones and some questions that would be nice to address later (not to forget about it):

1. (nitpick) On Explorer, the chain color looks very similar to the one we use for Arbitrum. Might it be changed a bit?
   ![diff color](https://private-user-images.githubusercontent.com/70885163/383962379-c3b19a66-aab4-4c5b-8614-1d50c4f7b599.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzA5OTQ3MjgsIm5iZiI6MTczMDk5NDQyOCwicGF0aCI6Ii83MDg4NTE2My8zODM5NjIzNzktYzNiMTlhNjYtYWFiNC00YzViLTg2MTQtMWQ1MGM0ZjdiNTk5LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDExMDclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMTA3VDE1NDcwOFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWQyYTQzMDhiZGIyZDU1NDIyM2ZkY2YwOWFhMGJkZDM2YzExNmUwOGMzYjIyMjNhYjljNmI5NDgzN2Y2YWQ5YTEmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.OzBn1f0P8Je0jrt32XvdOwQdHpE7oSCG3vE0I4sfmAE)

The color code was taken from their brand guide https://github.com/base-org/brand-kit.
Can't help it if it's too close to Arbitrum's brand color 🤷

2. Surplus tooltip: it would be great to change a date for 'November 2024' (or December)
   ![now](https://private-user-images.githubusercontent.com/70885163/383962657-b4c6f4cb-eb43-4170-946b-861b020f6f5c.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzA5OTQ3MjgsIm5iZiI6MTczMDk5NDQyOCwicGF0aCI6Ii83MDg4NTE2My8zODM5NjI2NTctYjRjNmY0Y2ItZWI0My00MTcwLTk0NmItODYxYjAyMGY2ZjVjLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDExMDclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMTA3VDE1NDcwOFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTc4NWM4NmUyYWY1YzUyMWY3OGZkNGVjN2FhMzBiMGIyOTMwOGU5ZjQ3ODhhNjA4Yjg3OGJkZjE2ZDU4NzMzYTkmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.hrFko4_eBg13SlBDVMApVg-c3DIf8Z9kUv7AOSbBuP8)

Fixed

3. There is no validation on a minimum chunk size for a TWAP order

It was a bug, fixed.

4. In general, AFAIU, TWAP orders are not working yet

Correct, we need backend PROD to be up first.

5. (might be an edge case): I created a TWAP order along with Safe modification in 1 TX, but the warning remained to be displayed until I refreshed the page. Order creation TX was signed with sponsored funds by Safe (e.g. not executed though connected wallet). Could you please check whether the same issue happens to you?
   ![image](https://private-user-images.githubusercontent.com/70885163/383963311-eb9cbf10-f4fb-4e49-85b7-f3cc81496690.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzA5OTQ3MjgsIm5iZiI6MTczMDk5NDQyOCwicGF0aCI6Ii83MDg4NTE2My8zODM5NjMzMTEtZWI5Y2JmMTAtZjRmYi00ZTQ5LTg1YjctZjNjYzgxNDk2NjkwLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDExMDclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMTA3VDE1NDcwOFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTA1NmQ1NjZjNjI5ZmQ4YWJkMmU2YTQ2Njk5NTQ5YzAyM2Q2NTBiN2RhNzhlYTU2MjI1MWFiY2M2MWJhYjA0YTgmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0._xE8hCSmEskYYSEaKpquKqTXKkxFYZu06Tw7VTmYkm8)

6. Is this OK that a limit price may be slighty recalculated on an order placement? See, I set it to 27, but the [order ](https://explorer-dev-git-feat-base-cowswap.vercel.app/base/orders/0xab351c233b5784059edb161b0586eed57a9339c4bc0ecb54033cfb214ee3d32c9fa3c00a92ec5f96b1ad2527ab41b3932efeda58672c9f0e?tab=overview) shows 29.0051
   ![image](https://private-user-images.githubusercontent.com/70885163/383965753-d76c58bb-b1ea-46af-b63e-2c9fb990664e.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzA5OTQ3MjgsIm5iZiI6MTczMDk5NDQyOCwicGF0aCI6Ii83MDg4NTE2My8zODM5NjU3NTMtZDc2YzU4YmItYjFlYS00NmFmLWI2M2UtMmM5ZmI5OTA2NjRlLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDExMDclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMTA3VDE1NDcwOFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWMwYzY3OTNlY2FjM2YxZjRmNThmYWYzMWJiMjMwZjU5MzM4NTAzNTgxNTdmNGQxYmEyYzI4OGVlOTg4OTBjNWMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.Ddvw0Rq9bUhZGXDnHyIMFhZjt5NhIL2masi7tTcVNKk)

Shouldn't be. Does this happen only on Base?

7. (does not work yet): Hooks simulation fails in Base (will be fixed when bff changes are merged)

8. (discuss later): Hooks support in Base (not working)
   ![image](https://private-user-images.githubusercontent.com/70885163/383964607-70a55f2c-896c-40a6-82c9-edc96240b4e4.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzA5OTQ3MjgsIm5iZiI6MTczMDk5NDQyOCwicGF0aCI6Ii83MDg4NTE2My8zODM5NjQ2MDctNzBhNTVmMmMtODk2Yy00MGE2LTgyYzktZWRjOTYyNDBiNGU0LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDExMDclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMTA3VDE1NDcwOFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTE0MmQyOWQ0OTEwMGU3M2M1OTU3NDllYmU0YzQzY2UyODk2OGIwYjc0MTljNjUzOWQ1OGZkZTA4NjRjZGY1OTMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.nLqlmYRKuZ6IUMJCPOMQMhkr7WTzObOwNPcwCGgZygQ)

9. (discuss later): COW token [liquidity ](https://cowservices.slack.com/archives/C06G1E3AU77/p1729784468627909)in Base

10. (discuss later): [CoW AMM pools](https://cowservices.slack.com/archives/C07M5DZHMHC/p1730450112639629) in Base

@elena-zh
Copy link
Contributor

elena-zh commented Nov 8, 2024

Hey @alfetopito ,

2. Surplus tooltip: it would be great to change a date for 'November 2024' (or December)

Fixed

Looks good

3. There is no validation on a minimum chunk size for a TWAP order

It was a bug, fixed.

Unfortunately, not fixed
image

Shouldn't be. Does this happen only on Base?

No :( It is a regression bug. I've opened #5077 task for this.

Thanks!

Copy link
Contributor

@anxolin anxolin left a comment

Choose a reason for hiding this comment

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

Amazing. It looks great.

I will merge this to DEVELOP, and iterate to add the missing parts. Mostly token stuff, twap stuff and small bugs reported by Elena.

@@ -29,7 +41,7 @@ export function SurplusCard() {
const surplusUsdAmount = useUsdAmount(showSurplusAmount ? surplusAmount : undefined).value
const native = useNativeCurrency()
const nativeSymbol = native.symbol || 'ETH'
const isArbitrumOne = native.chainId === SupportedChainId.ARBITRUM_ONE
const startDate = START_DATE[native.chainId as SupportedChainId]
Copy link
Contributor

Choose a reason for hiding this comment

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

now is cleaner :)

[SupportedChainId.SEPOLIA]: '',
}

const PANCAKE_CHAINS = {
[SupportedChainId.MAINNET]: 'eth',
[SupportedChainId.GNOSIS_CHAIN]: '',
[SupportedChainId.ARBITRUM_ONE]: 'arb',
[SupportedChainId.BASE]: 'base',
[SupportedChainId.SEPOLIA]: '',
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: im dont love to rely on empty string. I prefer to make the type solicit by allowing undefined or null

[SupportedChainId.SEPOLIA]: GNO_SEPOLIA,
}

const SDAI_GNOSIS_CHAIN_ADDRESS = '0xaf204776c7245bf4147c2612bf6e5972ee483701'
const GBPE_GNOSIS_CHAIN_ADDRESS = '0x5cb9073902f2035222b9749f8fb0c9bfe5527108'

// Not used for fees
Copy link
Contributor

Choose a reason for hiding this comment

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

As this is a const file, and you are referring to the business logic. I would not add this comment here

@@ -11,6 +11,8 @@ const ENS_REGISTRAR_ADDRESSES: Record<SupportedChainId, string | null> = {
[SupportedChainId.GNOSIS_CHAIN]: null,
[SupportedChainId.SEPOLIA]: '0x00000000000C2E074eC69A0dFb2997BA6C7d2e1e',
[SupportedChainId.ARBITRUM_ONE]: null,
[SupportedChainId.BASE]: null,
// TODO: use mainnet registrar for all chains https://docs.ens.domains/learn/deployments, which means being connected to mainnet additionally to the other chain
Copy link
Contributor

Choose a reason for hiding this comment

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

created this #5133

Copy link

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@cowprotocol/[email protected] network +1 4.27 MB cowprotocol_dev
npm/@cowprotocol/[email protected] None 0 0 B

🚮 Removed packages: npm/@cowprotocol/[email protected], npm/@cowprotocol/[email protected]

View full report↗︎

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.

3 participants