-
Notifications
You must be signed in to change notification settings - Fork 80
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
Enable async withdrawals on superOETHb #2275
base: master
Are you sure you want to change the base?
Changes from all commits
cb1a80a
a4bd14e
c4c9c89
4940758
ca0e928
1acfd34
cffc622
410892d
a944e12
9694ee3
bf21c92
2195f8e
7a63821
6bfa0db
e3c38bb
1adfcbb
8693f3b
19baec2
10d7ff6
6f42f78
09ece7c
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 |
---|---|---|
|
@@ -163,6 +163,20 @@ contract VaultAdmin is VaultStorage { | |
emit DripperChanged(_dripper); | ||
} | ||
|
||
/** | ||
* @notice Changes the async withdrawal claim period for OETH & superOETHb | ||
* @param _delay Delay period (should be between 10 mins to 7 days). | ||
* Set to 0 to disable async withdrawals | ||
*/ | ||
function setWithdrawalClaimDelay(uint256 _delay) external onlyGovernor { | ||
require( | ||
_delay == 0 || (_delay >= 10 minutes && _delay <= 7 days), | ||
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 think it makes sense to set the max delay to something that's greater than the max validator withdraw time. Perhaps 10 days? (At least one LST has a 15 day timed withdraw) |
||
"Invalid claim delay period" | ||
); | ||
emit WithdrawalClaimDelayUpdated(_delay); | ||
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. We usually like doing events last, when possible. |
||
withdrawalClaimDelay = _delay; | ||
} | ||
|
||
/*************************************** | ||
Swaps | ||
****************************************/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
const { deployOnBaseWithGuardian } = require("../../utils/deploy-l2"); | ||
const { deployWithConfirmation } = require("../../utils/deploy"); | ||
const addresses = require("../../utils/addresses"); | ||
|
||
module.exports = deployOnBaseWithGuardian( | ||
{ | ||
deployName: "018_async_withdrawals", | ||
}, | ||
async ({ ethers }) => { | ||
const cOETHbVaultProxy = await ethers.getContract("OETHBaseVaultProxy"); | ||
const cOETHbVault = await ethers.getContractAt( | ||
"IVault", | ||
cOETHbVaultProxy.address | ||
); | ||
|
||
// Deploy new implementation | ||
const dOETHbVaultCore = await deployWithConfirmation("OETHBaseVaultCore", [ | ||
addresses.base.WETH, | ||
]); | ||
const dOETHbVaultAdmin = await deployWithConfirmation("OETHBaseVaultAdmin"); | ||
|
||
return { | ||
actions: [ | ||
{ | ||
// 1. Upgrade VaultCore | ||
contract: cOETHbVaultProxy, | ||
signature: "upgradeTo(address)", | ||
args: [dOETHbVaultCore.address], | ||
}, | ||
{ | ||
// 2. Upgrade VaultAdmin | ||
contract: cOETHbVault, | ||
signature: "setAdminImpl(address)", | ||
args: [dOETHbVaultAdmin.address], | ||
}, | ||
{ | ||
// 3. Set async claim delay to 1 day | ||
contract: cOETHbVault, | ||
signature: "setWithdrawalClaimDelay(uint256)", | ||
args: [24 * 60 * 60], // 1d | ||
}, | ||
], | ||
}; | ||
} | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
const addresses = require("../../utils/addresses"); | ||
const { deploymentWithGovernanceProposal } = require("../../utils/deploy"); | ||
|
||
module.exports = deploymentWithGovernanceProposal( | ||
{ | ||
deployName: "108_vault_upgrade", | ||
forceDeploy: false, | ||
//forceSkip: true, | ||
reduceQueueTime: true, | ||
deployerIsProposer: false, | ||
proposalId: "", | ||
}, | ||
async ({ deployWithConfirmation }) => { | ||
// Deployer Actions | ||
// ---------------- | ||
|
||
// 1. Deploy new OETH Vault Core and Admin implementations | ||
const dVaultCore = await deployWithConfirmation("OETHVaultCore", [ | ||
addresses.mainnet.WETH, | ||
]); | ||
const dVaultAdmin = await deployWithConfirmation("OETHVaultAdmin", [ | ||
addresses.mainnet.WETH, | ||
]); | ||
|
||
// 2. Connect to the OETH Vault as its governor via the proxy | ||
const cVaultProxy = await ethers.getContract("OETHVaultProxy"); | ||
const cVault = await ethers.getContractAt("IVault", cVaultProxy.address); | ||
|
||
// Governance Actions | ||
// ---------------- | ||
return { | ||
name: "Upgrade OETH Vault", | ||
actions: [ | ||
// 1. Upgrade the OETH Vault proxy to the new core vault implementation | ||
{ | ||
contract: cVaultProxy, | ||
signature: "upgradeTo(address)", | ||
args: [dVaultCore.address], | ||
}, | ||
// 2. Set OETH Vault proxy to the new admin vault implementation | ||
{ | ||
contract: cVault, | ||
signature: "setAdminImpl(address)", | ||
args: [dVaultAdmin.address], | ||
}, | ||
{ | ||
// 3. Set async claim delay to 10 minutes | ||
contract: cVault, | ||
signature: "setWithdrawalClaimDelay(uint256)", | ||
args: [10 * 60], // 10 mins | ||
}, | ||
], | ||
}; | ||
} | ||
); |
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 think we will want to eventualy change the
request.timestamp
to become an end stamp rather than a start time stamp.The current code has three awkward things:
However, our current upgrade on OETH will be atomically setting a value for this in the upgrade, and we don't have to use the zero value for now.
Given that the current zero value is dangerous, how about we also have the require nonzero in the claim? In this way we never accidentally give someone the ability to flashloan an 1:1 mint/redeem. We could then remove it at some future date once we've changed the code to make it safe.