-
Notifications
You must be signed in to change notification settings - Fork 15
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
[VEN-2516] Deploy weETH Oracle #175
Changes from all commits
4ca37b9
6145e08
8a43973
c1164e8
4b0a2d7
8b80c1c
30baca8
0ac8a93
1a3466a
da6e458
16e8733
524fe52
6356a75
93778bd
752da30
6055f9c
cbff0bf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
// SPDX-License-Identifier: BSD-3-Clause | ||
pragma solidity 0.8.25; | ||
|
||
import "../interfaces/IEtherFiLiquidityPool.sol"; | ||
import "@openzeppelin/contracts/access/Ownable.sol"; | ||
|
||
contract MockEtherFiLiquidityPool is IEtherFiLiquidityPool, Ownable { | ||
/// @notice The amount of eETH per weETH scaled by 1e18 | ||
uint256 public amountPerShare; | ||
|
||
constructor() Ownable() {} | ||
|
||
function setAmountPerShare(uint256 _amountPerShare) external onlyOwner { | ||
amountPerShare = _amountPerShare; | ||
} | ||
|
||
function amountForShare(uint256 _share) external view override returns (uint256) { | ||
return (_share * amountPerShare) / 1e18; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
import { ethers } from "hardhat"; | ||
import { DeployFunction } from "hardhat-deploy/dist/types"; | ||
import { HardhatRuntimeEnvironment } from "hardhat/types"; | ||
|
||
import { ADDRESSES, assets } from "../helpers/deploymentConfig"; | ||
|
||
const func: DeployFunction = async ({ getNamedAccounts, deployments, network }: HardhatRuntimeEnvironment) => { | ||
const { deploy } = deployments; | ||
const { deployer } = await getNamedAccounts(); | ||
|
||
const resilientOracle = await ethers.getContract("ResilientOracle"); | ||
const proxyOwnerAddress = network.live ? ADDRESSES[network.name].timelock : deployer; | ||
|
||
let { EtherFiLiquidityPool, weETH, eETH } = ADDRESSES[network.name]; | ||
|
||
if (!EtherFiLiquidityPool) { | ||
// deploy mock liquidity pool | ||
await deploy("MockEtherFiLiquidityPool", { | ||
from: deployer, | ||
contract: "MockEtherFiLiquidityPool", | ||
args: [], | ||
log: true, | ||
autoMine: true, // speed up deployment on local network (ganache, hardhat), no effect on live networks | ||
skipIfAlreadyDeployed: true, | ||
}); | ||
|
||
const mockEtherFiLiquidityPool = await ethers.getContract("MockEtherFiLiquidityPool"); | ||
EtherFiLiquidityPool = mockEtherFiLiquidityPool.address; | ||
await mockEtherFiLiquidityPool.transferOwnership(proxyOwnerAddress); | ||
} | ||
|
||
await deploy("WeETHOracle", { | ||
from: deployer, | ||
log: true, | ||
deterministicDeployment: false, | ||
args: [EtherFiLiquidityPool, weETH, eETH, resilientOracle.address], | ||
proxy: { | ||
owner: proxyOwnerAddress, | ||
proxyContract: "OptimizedTransparentProxy", | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have added skipIfAlreadyDeployed to this deployment. I think without skipIfAlreadyDeployed=true the contracts are getting re-deployed. So I think we should have a separate issue to include this flag in all the deployment scripts. |
||
skipIfAlreadyDeployed: true, | ||
}); | ||
}; | ||
|
||
export default func; | ||
func.tags = ["weETH"]; | ||
func.skip = async (hre: HardhatRuntimeEnvironment) => hre.network.name !== "ethereum" && hre.network.name !== "sepolia"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's enable the deployment to hardhat too There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. script 4 is also skipped on hardhat There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even this I think we should review for all the scripts and fix them if we want to enable for hardhat There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. VEN-2547 |
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 would also deploy the mocks tokens for
weETH
andeETH
if the address is not available in theADDRESSES
mapping (for example on hardhat). /cc @coreyarThere 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 highly recommend not using address mappings at all and instead fetching deployments
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 structure deployment scripts into 3 types of scripts for each deployment, (mocks, deploy, setup)
for example there could be
deploy/6-mocks-weETH-oracle.ts ( mocks wouldn't run in the right order but a name like
dependencies
would )deploy/6-deploy-weETH-oracle.ts
deploy/6-setup-weETH-oracle.ts
Where setup receives an id so that it is only run once and mocks isn't deployed on live networks ( it would deploy the token contracts Jesus mentions) deploy only deploys contracts and setup doesn't deploy and only calls methods on contracts
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.
Right now this deployment script is following the pattern of the other deployment scripts in this repo.
I agree with your comments but I think we should make these changes in all the deployment scripts and not just this one to keep consistency. I would suggest we create a separate ticket and PR for this changes and this it itself requires a lot of code fixes.
wdyt @coreyar @chechu ?
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.
VEN-2547. We can leave this as it is in this PR, and review it in the new task.