YieldVaults structure#8
Conversation
8ba7f36 to
ff31ddc
Compare
ff31ddc to
af074d6
Compare
| /// - `strategy`: Any value conforming to | ||
| /// `FlowYieldVaultsInterfaces.Strategy`. | ||
| access(all) fun registerStrategy( | ||
| name: String, |
There was a problem hiding this comment.
Why introduce a plaintext naming scheme instead of relying on the built-in type system?
Human-readable naming feels like something that would be better suited for a metadata view IMO
There was a problem hiding this comment.
Types won't allow to add multiple strategies with the same Type, requiring deployment of 2 contracts for 2 strategies with different configurations.
An alternative would be IDs, which would be more efficient but less descriptive for the calling user.
There was a problem hiding this comment.
I agree with Jordan that using the Type of the Strategy to identify it is preferable here, to avoid adding a secondary identification scheme. In the near to medium term, I think we will have a small number of strategies.
2 contracts for 2 strategies with different configurations
Just noting: strategies are struct types, not contracts. If needed, we can have multiple Strategy implementations per contract.
If we find ourselves wanting to create so many different strategies that adding a type for each one feels onerous, I think that might warrant considering changing the Strategy interface to capture the full range of configurations we're trying to support.
There was a problem hiding this comment.
I strongly prefer that strategies remain immutable once they’re added to the registry. That means no changes to swappers, oracles, or any configured parameters. If something truly needs to be dynamic, it should operate within fixed, predefined bounds set at creation.
Allowing mutable parameters means we’d be changing a live system that holds significant funds, without proper testing—when there’s a safer alternative: deploy a new strategy and gradually allocate capital to it.
Mutability also makes strategies harder to trust. Even without bad intent, a simple mistake in updating a parameter could lead to loss of funds.
Keeping strategies immutable would require adding a field to the strategy contract and upgrading it whenever we want to change parameters, but that tradeoff seems worth it.
Happy to discuss this in the Q&A.
There was a problem hiding this comment.
I did not mean to suggest by my comment that strategies should be mutable. I do have some thoughts about that, but happy to agree that making them immutable is a reasonable, safe starting point. But I think that is a different conversation than how strategies are identified.
Re identification:
- We expect a small number of distinct strategies in the near-med term => creating a type for each strategy is reasonable and not onerous
- Cadence already provides a built-in identification system for types that is human-readable and unique => we should prefer using builtin features over building similar features, where possible
- Hence the suggestion to make use of the type IDs for identification
There was a problem hiding this comment.
My assumption was that we will only have a very small number of strategy types in the near-mid term,
but a reasonable amount of strategies of the same type with different configuration parameters.
If we agree that we don't want to try out different configurations of the strategies I agree a type => strategy map works. (Will add this to todays Q&A)
| /// - `name`: Name of the registered strategy. | ||
| /// | ||
| /// **Returns** A new `YieldVault` for the caller to save in storage. | ||
| access(account) fun createYieldVault(name: String): @{FlowYieldVaultsInterfaces.YieldVault} { |
There was a problem hiding this comment.
IMO access(account) should be avoided where possible. It flattens the trust boundary, and implicit access is contradictory to the capability-based security offered by the language. Wherever we can we should try to rely on explcit entitlement grants instead.
There was a problem hiding this comment.
I agree, but we can't add Entitlements to account functions.
Additionally this function will be access(all) in the future, so we should pretend already that it's access(all) already.
Will add documentation for that!
There was a problem hiding this comment.
we can't add Entitlements to account functions.
What do you think about wrapping this logic in a struct type, similar to how the core ALP state and logic is defined within the FlowALP.Pool resource? Then we can use entitlements for access control (including admin access).
For example:
entitlement Admin
entitlementParticipant
struct StrategyRegistry {
let strategies: {Type: Strategy}
access(Participant) createYieldVault() {...}
access(Admin) addStrategy() {...}
access(Admin) removeStrategy() {...}
}There was a problem hiding this comment.
Unless we plan to support multiple strategy collections, I don’t see a clear benefit to adding this.
The admin functions are already gated by an Admin resource.
To remove access(account), we could change it to access(self) and wrap the function in an additional resource. However, since the deploying account would own that resource, any other contracts deployed by the same account would also have access, resulting in effectively the same security level as access(account).
There was a problem hiding this comment.
I don’t see a clear benefit to adding this.
Here are the benefits I see:
- In Cadence, the access control primitives (capabilities and entitlements) are designed for use with struct and resource types. By implementing our state and logic around these types, we can directly use the language's access control primitives.
- For example, like I mention here, if we implement the logic in a struct/resource, we can support the "early access" allowlist-based access control requirement with just capabilities and entitlements (no secondary contract)
- In other software engineering contexts, if you have a singleton type (like a smart contract deployment with global variables), it is a good design practice to nevertheless factor out the singleton state into a component which could not be a singleton. It tends to make things like testing and future extension easier.
- We are using the approach of wrapping singleton logic in a resource type for
FlowALP.Pool. Where possible, we should try to solve the same problems using the same software engineering patterns.
05158d1 to
77fbed8
Compare
| /// - `strategy`: Any value conforming to | ||
| /// `FlowYieldVaultsInterfaces.Strategy`. | ||
| access(all) fun registerStrategy( | ||
| name: String, |
There was a problem hiding this comment.
I agree with Jordan that using the Type of the Strategy to identify it is preferable here, to avoid adding a secondary identification scheme. In the near to medium term, I think we will have a small number of strategies.
2 contracts for 2 strategies with different configurations
Just noting: strategies are struct types, not contracts. If needed, we can have multiple Strategy implementations per contract.
If we find ourselves wanting to create so many different strategies that adding a type for each one feels onerous, I think that might warrant considering changing the Strategy interface to capture the full range of configurations we're trying to support.
| /// - `name`: Name of the registered strategy. | ||
| /// | ||
| /// **Returns** A new `YieldVault` for the caller to save in storage. | ||
| access(account) fun createYieldVault(name: String): @{FlowYieldVaultsInterfaces.YieldVault} { |
There was a problem hiding this comment.
we can't add Entitlements to account functions.
What do you think about wrapping this logic in a struct type, similar to how the core ALP state and logic is defined within the FlowALP.Pool resource? Then we can use entitlements for access control (including admin access).
For example:
entitlement Admin
entitlementParticipant
struct StrategyRegistry {
let strategies: {Type: Strategy}
access(Participant) createYieldVault() {...}
access(Admin) addStrategy() {...}
access(Admin) removeStrategy() {...}
}| /// `FlowYieldVaults.strategyInfos()` so UIs can list strategies | ||
| /// without hard-coding their metadata. |
There was a problem hiding this comment.
so UIs can list strategies without hard-coding their metadata
If the goal is to enable UIs to display information about strategies without having prior knowledge about them, I think this method would benefit from a structured return type (even if most or all fields are optional).
struct StrategyInfo {
collateral: Type?
debt: Type?
yield: Type?
// ... other common properties as structured fields ...
metadata: {String: String}
}
There was a problem hiding this comment.
This assumes all strategies have collateral debt and yield.
In the original FYV, we supported, implemented and used strategies that don't have this setup.
(The strategies used on peak.money)
There was a problem hiding this comment.
Thanks for the example. I don't know enough about the strategies to say specifically what properties they are likely to have in common. But I suspect they are likely to share some common properties. Maybe not the fields I put in my example above, like you pointed out.
Again, we can make most or all of these fields optional. Then, for the strategies like peak.money that have unique properties, they don't need to be set.
If you feel like every strategy will have unique properties, then I can see the case for making this completely unstructured. But that seems unlikely to me. Let me know if my intuition is off, though!
janezpodhostnik
left a comment
There was a problem hiding this comment.
One question: The spec doesn't seem mention any limits/constraints on fungible token types. And the return vault seems to be totally agnostic on which fungible token it accepts. Is this ok? how does this work?
builds on top of #4
Problem
Users want a single, composable primitive for opening a yield-generating position without having to understand or wire up the underlying strategy (lending, LP, basis trade, …). Each strategy has its own parameters (token types, swappers, health settings); the user should not have to care.
Goal
Provide a registry (
FlowYieldVaults) that admins populate with pre-configured strategies, and that mints a yield vault for any registered name on demand. The caller picks a strategy by name; the vault captures the strategy's parameters internally and exposes a uniformFungibleToken.Provider+FungibleToken.Receiverinterface.New strategy families are added by deploying a new strategy contract whose struct conforms to
FlowYieldVaultsInterfaces.Strategy, and thenAdmin.registerStrategying an instance. Users always interact with the sameYieldVaultinterface regardless of the strategy backing it.See docs/flow_yield_vaults.md for detailed information