Nialexsan/wbtc weth #108
Nialexsan/wbtc weth #108
Conversation
This reverts commit 7d6c2bb.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| strategy: Type, | ||
| collateral: Type | ||
| ): Bool { | ||
| let composerConfig = self.configs[composer]! |
There was a problem hiding this comment.
Suggested fix: The hasConfig function uses force-unwrap (!) on optional values which can panic. Should be changed to use optional binding:
access(all) view fun hasConfig(
composer: Type,
strategy: Type,
collateral: Type
): Bool {
if let composerConfig = self.configs[composer] {
if let strategyConfig = composerConfig[strategy] {
return strategyConfig[collateral] != nil
}
}
return false
}
|
Additional changes needed:
These changes are staged locally and can be reviewed. |
| self.config = {} | ||
|
|
||
| let moetType = Type<@MOET.Vault>() | ||
| let moetEVMAddress = FlowEVMBridgeConfig.getEVMAddressAssociated(with: Type<@MOET.Vault>()) |
There was a problem hiding this comment.
this is a safety check to be sure that moet evm address exists,
it will panic on the next line if it doesn't
There was a problem hiding this comment.
Maybe make that more obvious by not introducing an unused variable:
if FlowEVMBridgeConfig.getEVMAddressAssociated(with: Type<@MOET.Vault>()) == nil {
panic("Could not find EVM address for \(moetType.identifier) - ensure the asset is onboarded to the VM Bridge")
}
turbolent
left a comment
turbolent
left a comment
There was a problem hiding this comment.
Nice, just a few minor suggestions. Only reviewed the Cadence code from a code quality perspective, IDK about the logic details
| } | ||
| /// Executed when a Strategy is burned, cleaning up the Strategy's stored AutoBalancer | ||
| access(contract) fun burnCallback() { | ||
| FlowYieldVaultsAutoBalancers._cleanupAutoBalancer(id: self.id()!) |
There was a problem hiding this comment.
This will fail if there is no ID set. Is that intended?
| strategy: Type, | ||
| collateral: Type | ||
| ): Bool { | ||
| let composerConfig = self.configs[composer] ?? nil |
There was a problem hiding this comment.
Here and for the other ?? nil occurrences: The lookup already returns nil if there is no element, so nil ?? nil is just nil:
| let composerConfig = self.configs[composer] ?? nil | |
| let composerConfig = self.configs[composer] |
| let composerConfig = self.configs[composer] ?? nil | ||
| if composerConfig == nil { | ||
| return false | ||
| } | ||
| let strategyConfig = composerConfig![strategy] ?? nil | ||
| if strategyConfig == nil { | ||
| return false | ||
| } | ||
| return strategyConfig![collateral] != nil |
There was a problem hiding this comment.
Avoid force-unwrapping and use if-let instead:
| let composerConfig = self.configs[composer] ?? nil | |
| if composerConfig == nil { | |
| return false | |
| } | |
| let strategyConfig = composerConfig![strategy] ?? nil | |
| if strategyConfig == nil { | |
| return false | |
| } | |
| return strategyConfig![collateral] != nil | |
| if let composerConfig = self.configs[composer] { | |
| if let strategyConfig = composerConfig[strategy] { | |
| return strategyConfig[collateral] != nil | |
| } | |
| } | |
| return false |
If there will be many elements in self.configs[composer] and composerConfig[strategy], you might want to consider taking references to the entries, so that the large dictionaries aren't copied.
| return strategyConfig![collateral] != nil | ||
| } | ||
|
|
||
| access(all) view fun getSupportedComposers(): {Type: Bool} { |
There was a problem hiding this comment.
Don't know if this is used elsewhere outside of this contract, but inside of this contract the result is always just used for a lookup, so the construction of the dictionary is unnecessary.
Maybe consider replacing this, or at least adding isSupportedComposer(_ type: Type): Bool function which and calling it instead of self.getSupportedComposers()[type] == true:
fun isSupportedComposer(_ type: Type): Bool {
return type == Type<@mUSDFStrategyComposer>()
}| pre { | ||
| self.getSupportedComposers()[type] == true: | ||
| "Unsupported StrategyComposer \(type.identifier) requested" | ||
| (&self.configs[type] as &{Type: {Type: FlowYieldVaultsStrategiesV1_1.CollateralConfig}}?) != nil: |
There was a problem hiding this comment.
AFAICS there should be no need to take a reference on the LHS:
| (&self.configs[type] as &{Type: {Type: FlowYieldVaultsStrategiesV1_1.CollateralConfig}}?) != nil: | |
| self.configs[type] != nil: |
| for stratType in config.keys { | ||
| let newPerCollateral = config[stratType]! | ||
| let existingPerCollateral = mergedComposerConfig[stratType] ?? {} | ||
| var mergedPerCollateral: {Type: FlowYieldVaultsStrategiesV1_1.CollateralConfig} = existingPerCollateral |
There was a problem hiding this comment.
Type annotation is unnecessary, it can be inferred:
| var mergedPerCollateral: {Type: FlowYieldVaultsStrategiesV1_1.CollateralConfig} = existingPerCollateral | |
| var mergedPerCollateral = existingPerCollateral |
| ) | ||
|
|
||
| // Wrap into the nested config expected by upsertConfigFor | ||
| let singleCollateralConfig: {Type: {Type: FlowYieldVaultsStrategiesV1_1.CollateralConfig}} = { |
There was a problem hiding this comment.
Type annotation is probably unnecessary and can be inferred:
| let singleCollateralConfig: {Type: {Type: FlowYieldVaultsStrategiesV1_1.CollateralConfig}} = { | |
| let singleCollateralConfig = { |
| self.configs = { | ||
| Type<@mUSDFStrategyComposer>(): { | ||
| Type<@mUSDFStrategy>(): {} | ||
| } | ||
| } as {Type: {Type: {Type: FlowYieldVaultsStrategiesV1_1.CollateralConfig}}} |
There was a problem hiding this comment.
Maybe only annotate the type of what cannot be inferred, i.e. the inner empty dictionary:
| self.configs = { | |
| Type<@mUSDFStrategyComposer>(): { | |
| Type<@mUSDFStrategy>(): {} | |
| } | |
| } as {Type: {Type: {Type: FlowYieldVaultsStrategiesV1_1.CollateralConfig}}} | |
| self.configs = { | |
| Type<@mUSDFStrategyComposer>(): { | |
| Type<@mUSDFStrategy>(): {} as {Type: FlowYieldVaultsStrategiesV1_1.CollateralConfig} | |
| } | |
| } |
| self.config = {} | ||
|
|
||
| let moetType = Type<@MOET.Vault>() | ||
| let moetEVMAddress = FlowEVMBridgeConfig.getEVMAddressAssociated(with: Type<@MOET.Vault>()) |
There was a problem hiding this comment.
Maybe make that more obvious by not introducing an unused variable:
if FlowEVMBridgeConfig.getEVMAddressAssociated(with: Type<@MOET.Vault>()) == nil {
panic("Could not find EVM address for \(moetType.identifier) - ensure the asset is onboarded to the VM Bridge")
}
| 0.8 \ | ||
| 1.0 \ | ||
| 0.8 \ | ||
| 0.0 \ | ||
| 0.04 \ | ||
| 0.4 \ | ||
| 1_000_000.0 \ | ||
| 1_000_000.0 \ |
There was a problem hiding this comment.
@liobrasil just a sanity check
do these params look correct for this config:
- collateral factor: 0.8
- borrow factor: 1.0
- Kink interest curve
- optimal utilization 80%
- base rate 0%
- slope1: 4
- slope2: 40
- deposit rate: 1_000_000
- deposit capacity: 1_000_000
There was a problem hiding this comment.
Aave Rate Strategy (Volatile One)
- Source: https://github.com/aave/risk-v3/blob/main/liquidity-risk/borrow-interest-rate.md#rate-strategy-volatile-one
- Optimal utilization: 45%
- Base rate: 0%
- Slope1: 4
- Slope2: 300
- Takeaway: steep curve beyond 45% utilization → strongly disincentivizes excessive borrowing to preserve liquidity
- Question for @Gornutz: should we adjust our optimal utilization or slope2?
Collateral / Borrow Factors (WBTC)
- Euler: CF 88% & BF 91% (https://forum.euler.finance/t/eip-1-promote-wbtc-to-collateral-tier/27)
- Aave: LTV 73% (https://aave.com/docs/resources/parameters)
- Reminder: in Euler, LTV ≈ CF × BF
Deposit Capacity (WBTC)
- Current defaults feel too large for initial mainnet testing
- Proposal:
depositRate = 0.006 WBTC/hr - Proposal:
depositCapacityCap = 0.1 WBTC - USD equivalents depend on BTC spot (e.g., at $90k/BTC this is ≈ $540/hr and ≈ $9000 cap)
|
on pause due to non-upgradable changes in FlowCreditMarket |
Closes: #???
on pause due to non-upgradable changes in FlowCreditMarket
Description
testnet txns for vaults with the collaterals:
FlowYieldVaultsStrategies diff:
main...2d4e129#diff-3ca82892e3f3687d53c4df502a69e2b07e6f5a0038466d9792121979b51d2f32
For contributor use:
masterbranchFiles changedin the Github PR explorer