chore: update polygon and base contract addresses #330
Conversation
fadeev
requested review from
andresaiello,
brewmaster012,
lumtis,
CharlieMc0,
skosito and
fbac
as code owners
WalkthroughWalkthroughThe recent updates enhance the JSON configuration files and TypeScript type definitions for the ZetaChain network and blockchain asset management. New sections have been added for testnet and mainnet assets, optimizing their organization and specificity. Type definitions have been expanded for better coverage of blockchain parameters, ensuring clearer system interactions. Additionally, logic adjustments in the tasks file ensure that the correct endpoint is utilized for the base_sepolia chain, improving overall functionality. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (5)
- v1/data/addresses.json (2 hunks)
- v1/data/addresses.mainnet.json (1 hunks)
- v1/data/addresses.testnet.json (3 hunks)
- v1/lib/types.ts (1 hunks)
- v1/tasks/addresses.ts (2 hunks)
Additional context used
Gitleaks
v1/data/addresses.json
9-9: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
Additional comments not posted (16)
v1/lib/types.ts (3)
1-1: Enhance type safety with comprehensive symbols.The expanded
ParamSymboltype now covers a broader range of tokens, which enhances type safety and flexibility. Ensure all usages ofParamSymbolare updated to accommodate these new values.
2-2: Improved specificity in chain names.The
ParamChainNametype now includes additional blockchain networks, providing more precise context for operations. Verify that these changes are correctly reflected in the rest of the codebase.
3-3: Expanded operational roles forParamType.The
ParamTypetype now includes additional operational roles, which should improve clarity and alignment with system functionality. Ensure these roles are correctly used throughout the codebase.v1/data/addresses.json (2)
3-11: Verify the newbase_testnetconfiguration.The new
base_testnetsection provides detailed configuration for various components. Ensure these addresses are correct and intended for public use.Tools
Gitleaks
9-9: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
9-9: Potential exposure of sensitive information.Line 9 may contain a generic API key or sensitive information. Double-check that this address is safe to include in the public configuration.
Tools
Gitleaks
9-9: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
v1/data/addresses.mainnet.json (2)
348-360: New entry for USDT on POL added.The addition of USDT on POL expands the dataset for omnichain assets. Ensure this entry is accurate and consistent with other entries.
361-373: New entry for USDC on POL added.The addition of USDC on POL enhances the dataset for omnichain assets. Verify the accuracy and consistency of this entry with other similar entries.
v1/data/addresses.testnet.json (7)
174-186: New asset entry is consistent and correctly formatted.The entry for the ZRC20 asset on the ZetaChain testnet is properly structured and includes all necessary details.
327-333: New asset entry is consistent and correctly formatted.The entry for the connector asset on the base_sepolia chain is properly structured and includes all necessary details.
334-340: New asset entry is consistent and correctly formatted.The entry for the erc20Custody asset on the base_sepolia chain is properly structured and includes all necessary details.
341-347: New asset entry is consistent and correctly formatted.The entry for the pauser asset on the base_sepolia chain is properly structured and includes all necessary details.
348-354: New asset entry is consistent and correctly formatted.The entry for the tss asset on the base_sepolia chain is properly structured and includes all necessary details.
355-361: New asset entry is consistent and correctly formatted.The entry for the tssUpdater asset on the base_sepolia chain is properly structured and includes all necessary details.
362-368: New asset entry is consistent and correctly formatted.The entry for the zetaToken asset on the base_sepolia chain is properly structured and includes all necessary details.
v1/tasks/addresses.ts (2)
210-211: Correct endpoint logic for base_sepolia.The conditional logic correctly maps "base_sepolia" to "base_testnet" to ensure the correct endpoint is used.
236-237: Correct endpoint logic for base_sepolia.The conditional logic correctly maps "base_sepolia" to "base_testnet" to ensure the correct endpoint is used.
fadeev
changed the title
| | "USDT.BSC" | ||
| | "USDT.ETH"; | ||
|
|
||
| export type ParamChainName = ZetaProtocolNetwork; |
There was a problem hiding this comment.
Can we update ZetaProtocolNetwork directly and leave this 1:1 type? We are using this on Hub. Also this already includes base_testnet if this is the only change for this type.
There was a problem hiding this comment.
On chain it's stored as base_sepolia, in the networks package it's base_testnet. I don't expect we change what's on chain, so I propose we change what's in the networks.
There was a problem hiding this comment.
I see, is it too much effort to rename all base_testnet instances in favor of base_sepolia in both repositories? I think we should have only one, and agree we should match what's already on chain
There was a problem hiding this comment.
PR to update the networks package to "base_sepolia" is above.
This PR updates the types in this repo to "base_sepolia". No effort at all.
There was a problem hiding this comment.
Amazing! Hadn't seen the comment above with the other PR due to a github sync delay.
My original suggestion still applies with a further update then. We should keep ParamChainName = ZetaProtocolNetwork and update base_testnet to base_sepolia in address.tools.ts.
There was a problem hiding this comment.
types.ts is generated automatically from the addresses JSON. address.tools.ts is a legacy types file maintained manually. Consider switching to types.ts as it ensures that types actually reflect the values that are in the addresses JSON. Let me know if something's missing from types.ts.
There was a problem hiding this comment.
That makes sense. We can definitively switch to use that types.ts file directly. Do you think we can remove the address.tools.ts to avoid duplication?
There was a problem hiding this comment.
I'm taking a look and seems some constants from address.tools.ts are being used in other places from this repo. Maybe we can make a new issue/PR to refactor this separately.
There was a problem hiding this comment.
I don't think anyone's going to be refactoring v1 contracts as they're all being replaced by v2.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- v1/data/addresses.mainnet.json (4 hunks)
- v1/data/addresses.testnet.json (8 hunks)
- v1/tasks/addresses.ts (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- v1/data/addresses.mainnet.json
- v1/tasks/addresses.ts
Additional comments not posted (7)
v1/data/addresses.testnet.json (7)
174-186: Entry forzeta_testnetis well-structured.The new ZRC20 asset entry for
zeta_testnetis correctly formatted and includes all necessary fields.
327-333: Entry forbase_sepoliaconnector is correctly formatted.The new connector asset entry for
base_sepoliais well-structured and includes all required fields.
334-340: Entry forbase_sepoliaERC20 custody is well-structured.The new ERC20 custody asset entry for
base_sepoliais correctly formatted and includes all required fields.
341-347: Entry forbase_sepoliapauser is correctly formatted.The new pauser asset entry for
base_sepoliais well-structured and includes all required fields.
348-354: Entry forbase_sepoliaTSS is well-structured.The new TSS asset entry for
base_sepoliais correctly formatted and includes all required fields.
355-361: Entry forbase_sepoliaTSS updater is correctly formatted.The new TSS updater asset entry for
base_sepoliais well-structured and includes all required fields.
362-368: Entry forbase_sepoliazetaToken is well-structured.The new zetaToken asset entry for
base_sepoliais correctly formatted and includes all required fields.
| @@ -1,5 +1,15 @@ | |||
| { | |||
| "ccm": { | |||
| "base_testnet": { | |||
There was a problem hiding this comment.
Should this be updated to base_sepolia?
| "base_testnet": { | |
| "base_sepolia": { |
There was a problem hiding this comment.
This is a legacy file that hasn't been updated in ages. This file as well as address.tools.ts and some other files are just there, because they may have been used by the protocol contracts deploy scripts.
There was a problem hiding this comment.
Updated files are addresses.mainnet.json and addresses.testnet.json.
There was a problem hiding this comment.
Got it, just wondering about this file being updated in this PR, we can ignore this
base_sepoliaandbase_testnetSummary by CodeRabbit
New Features
Bug Fixes
Documentation