Feature: oft adapter step3 - deploy script and test#11
Feature: oft adapter step3 - deploy script and test#11blueogin wants to merge 62 commits intofeat/oft-adapter-step2from
Conversation
…acts, including enabling hardhat-deploy and adding new network configurations for CELO and XDC
… management structures, and add deployment and configuration scripts for cross-chain functionality
…C network, enhance GoodDollarOFTAdapter with upgrade authorization event, and modify deployment scripts for improved functionality
…ant fee deduction details
There was a problem hiding this comment.
Sorry @blueogin, your pull request is larger than the review limit of 150000 diff characters
| function _authorizeUpgrade(address newImplementation) internal override onlyOwner { | ||
| // Authorization is handled by onlyOwner modifier | ||
| // Additional checks can be added here if needed | ||
| emit AuthorizedUpgrade(newImplementation); |
There was a problem hiding this comment.
Okay, It was required for UUPSUpgradeable contract.
I will use transparent model and remove that function
There was a problem hiding this comment.
i ment that the emit event is not requred. we should use uups
There was a problem hiding this comment.
I see I will make it use UUPS
| # 6. Test bridge functionality (optional, last step) | ||
| # | ||
| # Usage: | ||
| # ./scripts/multichain-deploy/oft/configure-oft-xdc-celo.sh |
There was a problem hiding this comment.
In general scripts should be hardhat scripts in typescript. not shell.
- the script should configure a single network ie configure-oft.ts --network development-xdc
- the script should be a hardhat script so --network is available
- all config values should be read from a json file ie oft.config.json (import config from './oft.config.json')
where each network/env has its entry in the config file.
There was a problem hiding this comment.
This file is rarely used in a production environment and is intended mainly as a reference.
Also, including all configuring logic into 1 script is not ideal I think
I’ll add an OFT_CONFIGURING_GUIDE file under scripts/oft to explain the configuration steps.
There was a problem hiding this comment.
we use hadhat-deploy scripts , see messagepassingbridge
There was a problem hiding this comment.
It's now using hardhat-deploy script
| echo "" | ||
|
|
||
| # Step 7: Test bridge (optional, last step) | ||
| if [ "$SKIP_BRIDGE_TEST" != "true" ]; then |
There was a problem hiding this comment.
should be separated into a different script. ( a hardhat script)
… unused dependencies in bridge-contracts
Feature: oft adapter step3 - deploy script and test
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
…nd to transparent in GoodDollarOFTAdapter and deployment script
…pts to reference new deployment file
…nd update set-minter-burner-limits script to utilize new config
…t for cleaner implementation
…and integrating IMessagePassingBridge for limit management
| MinterBurner = await upgrades.deployProxy( | ||
| MinterBurnerFactory, | ||
| [nameServiceAddress], | ||
| { kind: "uups", initializer: "initialize" } |
There was a problem hiding this comment.
i dont understand. why this was changed. we use uups
There was a problem hiding this comment.
It's now using uups
| import fse from "fs-extra"; | ||
| import Contracts from "@gooddollar/goodprotocol/releases/deployment.json"; | ||
| import release from "../../release/deployment.json"; | ||
| import release from "../../release/deployment-oft.json"; |
There was a problem hiding this comment.
why are we not using hardhat-deploy for deployment scripts
There was a problem hiding this comment.
we are using hardhat-deploy now
|
|
||
| **Note**: Limit values can be specified in decimal format (e.g., "5000" for 5,000 G$). The scripts automatically convert them to wei (18 decimals). | ||
|
|
||
| ## Manual Configuration: Step-by-Step |
There was a problem hiding this comment.
we should have a hardhat deploy script just like messagepassingbridge has
| function _authorizeUpgrade(address newImplementation) internal override onlyOwner { | ||
| // Authorization is handled by onlyOwner modifier | ||
| // Additional checks can be added here if needed | ||
| emit AuthorizedUpgrade(newImplementation); |
There was a problem hiding this comment.
i ment that the emit event is not requred. we should use uups
packages/bridge-contracts/contracts/oft/GoodDollarOFTAdapter.sol
Outdated
Show resolved
Hide resolved
…code clarity and reduce dependencies
…te request approval mechanism to use bytes32 IDs, and implement request ID generation for improved limit management
… MinterBurner contracts
…oved clarity, implement request approval checks, and enforce limits on sending and receiving functions
…8.9 with optimizer settings and modify LayerZero config to reference the correct deployment file
…zation for enhanced fee management
…ove legacy deployment script; add set-oft-operator script for DAO governance integration
…terministic proxy pattern and improve initialization process
…ormula, IGoodDollarCustom, ISuperGoodDollar, and related interfaces
… updated before processing receive requests
…dDollarOFTAdapter, and adjust bridging amount in tests
…and production environments for GoodDollarMinterBurner and GoodDollarOFTAdapter
…dDollarOFTAdapter in development environments
…quest function to remove owner restriction in GoodDollarOFTAdapter
…to bridging configuration
…ions from OFT_CONFIGURING_GUIDE.md for clarity
…to streamline settings
…st to ensure proper allowance before quoteSend
… GoodDollar OFT contracts
| release[networkName] = {}; | ||
| } | ||
| release[networkName].GoodDollarMinterBurner = minterBurnerAddress; | ||
| await fse.writeJSON('release/deployment-oft.json', release, { spaces: 2 }); |
There was a problem hiding this comment.
hardhat deploy already writes to file. look at the help
There was a problem hiding this comment.
Now it reads contract addresses from files that are automatically generated by hardhat-deploy.
|
|
||
| // Verify GoodDollarMinterBurner implementation (no constructor args) | ||
| if (!['hardhat', 'localhost', 'develop'].includes(networkName)) { | ||
| await verifyContract(hre as any, minterBurnerImpl.address, [], 'GoodDollarMinterBurner'); |
There was a problem hiding this comment.
verify for all envs. you can exclude hardhat and localhost
There was a problem hiding this comment.
Removed develop from the helper’s skip list, so it also no longer skips verification on develop.
|
|
||
| // --- GoodDollarOFTAdapter: deploy new implementation via hardhat-deploy, then upgrade proxy --- | ||
| console.log("\nDeploying new GoodDollarOFTAdapter implementation..."); | ||
| const oftAdapterImpl = await deployments.deploy("GoodDollarOFTAdapter_Implementation_Upgrade", { |
There was a problem hiding this comment.
should impl be deterministic?
in the deploy script it is
There was a problem hiding this comment.
Now the implementation contracts are deployed deterministically with the salt of contract's bytecode
| @@ -0,0 +1,184 @@ | |||
| /*** | |||
There was a problem hiding this comment.
Okay I will rename it to set-bridge-limits.ts
| @@ -0,0 +1,249 @@ | |||
| /*** | |||
There was a problem hiding this comment.
make sure this scripts does all the steps required for full deployment.
besides DAO related actions such as giving minter rights to the minterburner
There was a problem hiding this comment.
I confirmed that all 7 steps are required
Step 1 deploys and initializes the OFT contracts, which has to happen before anything else.
Step 2 sets the adapter as operator via DAO governance
Step 3 grants MINTER_ROLE via DAO governance
Step 4 wires LayerZero connections/enforced options, an external integration step that can fail and has to be run with the correct owner/delegate permissions.
Step 5 sets bridge limits as an optional step because you may not want to set them.
Step 6 tests bridging and is optional because it depends on having balances/gas/fees and is meant as a validation step, not a core deployment requirement.
Step 7 transfers ownership to the DAO Avatar and is optional.
…cing configuration management and transaction execution
…etting bridge limits and transferring ownership, enhancing user understanding
…MinterBurner to use consistent implementation naming and enable deterministic deployments
…dDollarMinterBurner and GoodDollarOFTAdapter in deployment and upgrade scripts
…' network, enhancing clarity in network handling for GoodDollarMinterBurner and GoodDollarOFTAdapter
…tifacts for GoodDollarOFTAdapter and GoodDollarMinterBurner, improving deployment consistency and error handling
…across deployment scripts, tests, and documentation for consistency and clarity in cross-chain token management
|
Hi @sirpy |
Description
This PR implements the deployment scripts and testing infrastructure for the GoodDollar OFT (Omnichain Fungible Token) adapter system. The implementation enables cross-chain bridging of GoodDollar tokens between XDC and CELO networks using LayerZero's OFT protocol.
About #7
How Has This Been Tested?
https://layerzeroscan.com/tx/0x3575146c0e395d46b4a3e09ec3ae79e1005ea6d4461765d6dc19d6aebed512bb
https://layerzeroscan.com/tx/0xa267c36a25337ea8d45d83de83a3e83a291ba4d2eaf0fd0393d365faf3c078e8
Checklist: