-
Notifications
You must be signed in to change notification settings - Fork 71
feat: Generated the constants file #1096
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Faisal Usmani <[email protected]>
98dcbfa
to
e78b1e9
Compare
QUOTE_TIME_BUFFER, | ||
FILL_DEADLINE_BUFFER, | ||
ARBITRUM_MAX_SUBMISSION_COST, | ||
AZERO_GAS_PRICE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably drop references to Aleph Zero because the Aleph Zero deployment is EoL now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done 791d7e7
} | ||
|
||
// Helper function to extract wrapped native tokens from TOKEN_SYMBOLS_MAP | ||
function extractWrappedNativeTokens(): { [key: string]: string } { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the constants repo we define the native token symbol for each chain - feels like we could use that here to do something a bit more dynamic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to remove this function all together: 791d7e7
function extractUsdceAddresses(): { [key: string]: string } { | ||
const result: { [key: string]: string } = {}; | ||
|
||
for (const [chainId, address] of Object.entries(USDCe)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is super reliable because the USDC symbol itself can be quite random. Zora has USDzC for example.
} | ||
|
||
// Helper function to extract WGHO addresses | ||
function extractWghoAddresses(): { [key: string]: string } { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Integrating nativeToken from the constants repo would probably also help to remove a bespoke function like this one.
if (hubChainName) { | ||
result[hubChainName] = {}; | ||
for (const [spokeChainId, addresses] of Object.entries(spokeChains)) { | ||
const spokeChainName = Object.keys(CHAIN_IDs).find( | ||
(key) => CHAIN_IDs[key as keyof typeof CHAIN_IDs] === Number(spokeChainId) | ||
); | ||
if (spokeChainName) { | ||
result[hubChainName][spokeChainName] = addresses; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thoughts on dropping the indentation by returning early if hubChainName
is not defined?
for (const [chainId, domainId] of Object.entries(CIRCLE_DOMAIN_IDs)) { | ||
const chainName = Object.keys(CHAIN_IDs).find( | ||
(key) => CHAIN_IDs[key as keyof typeof CHAIN_IDs] === Number(chainId) | ||
); | ||
if (chainName) { | ||
result[chainName] = domainId; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pattern is a bit repetitive now - is there maybe some way to refactor such that the core functionality is generic? Or are the upstream objects themselves too bespoke for that?
QUOTE_TIME_BUFFER: QUOTE_TIME_BUFFER, | ||
FILL_DEADLINE_BUFFER: FILL_DEADLINE_BUFFER, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QUOTE_TIME_BUFFER: QUOTE_TIME_BUFFER, | |
FILL_DEADLINE_BUFFER: FILL_DEADLINE_BUFFER, | |
QUOTE_TIME_BUFFER, | |
FILL_DEADLINE_BUFFER, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done: 791d7e7
script/utils/constants.json
Outdated
"LENS": -1, | ||
"BOBA": -1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooc, rather than enumerating each chain w/ -1 for no OFT EID, could we just omit chains without an OFT EID?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, done: 791d7e7
script/utils/constants.json
Outdated
"POLYGON": "0x0d500B1d8E8eF31E21C99d1Db9A6444d3ADf1270", | ||
"POLYGON_AMOY": "0xA5733b3A8e62A8faF43b0376d5fAF46E89B3033E", | ||
"ZK_SYNC": "0x5AEa5775959fBC2557Cc8789bC1bf90A239D9a91", | ||
"OPTIMISM": "0x4200000000000000000000000000000000000006", | ||
"OPTIMISM_SEPOLIA": "0x74A4A85C611679B73F402B36c0F84A7D2CcdFDa3", | ||
"BASE": "0x4200000000000000000000000000000000000006", | ||
"BASE_SEPOLIA": "0x999B45BB215209e567FaF486515af43b8353e393", | ||
"LENS": "0x6bDc36E20D267Ff0dd6097799f82e78907105e2F", | ||
"LINEA": "0xe5D7C2a44FfDDf6b295A15c148167daaAf5Cf34f", | ||
"LINEA_SEPOLIA": "0x10253594A832f967994b44f33411940533302ACb", | ||
"SCROLL_SEPOLIA": "0x5300000000000000000000000000000000000004", | ||
"SCROLL": "0x5300000000000000000000000000000000000004", | ||
"BSC": "0x2170Ed0880ac9A755fd29B2688956BD959F933F8", | ||
"UNICHAIN": "0x4200000000000000000000000000000000000006", | ||
"UNICHAIN_SEPOLIA": "0x4200000000000000000000000000000000000006", | ||
"ALEPH_ZERO": "0xB3f0eE446723f4258862D949B4c9688e7e7d35d3", | ||
"BLAST": "0x4300000000000000000000000000000000000004", | ||
"BLAST_SEPOLIA": "0x4200000000000000000000000000000000000023", | ||
"POLYGON": "0x7ceB23fD6bC0adD59E62ac25578270cFf1b9f619", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have different addresses for Polygon here - the new address is incorrect because it's WETH.
"cctpMessageTransmitter": "0x0a992d191DEeC32aFe36203Ad87D7d289a738F81", | ||
"cctpMessageTransmitter": "0x0a992d191deec32afe36203ad87d7d289a738f81", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be a good idea to split simple changes like this out to a separate PR - that'll help reduce the diff here and make it easier to review the subsequent address changes.
} | ||
|
||
// Helper function to extract USDC addresses | ||
function extractUsdcAddresses(): { [key: string]: string } { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two questions for this entire file:
- Why are we setting the keys as the chain name? I would think it's a lot easier for foundry to know the network's chainId over the name of the network itself?
- Maybe a stupid question, but these functions seem really complex for what they are doing. Why can't we just call
JSON.stringify(USDC)
and similar things for all of these other functions? I think adding this much code to just extract values from a map makes the script itself pretty rigid/hard to maintain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, I updated the scripts to fetch by chainid instead 4677066
make every a lot cleaner
Signed-off-by: Faisal Usmani <[email protected]>
Signed-off-by: Faisal Usmani <[email protected]>
Signed-off-by: Faisal Usmani <[email protected]>
56f1e0f
to
ae04d5f
Compare
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
Signed-off-by: Faisal Usmani <[email protected]>
All constants are taken from constants repo so that there is only one source of truth