Conversation
ultrasecreth
left a comment
ultrasecreth
left a comment
There was a problem hiding this comment.
The witch governance is missing, but's a really good start!
Updated yield-utils
ultrasecreth
force-pushed
the
feat/contango-wand
branch
from
8d3e078 to
f6e2b2b
Compare
|
We just need to add a |
alcueca
left a comment
alcueca
left a comment
There was a problem hiding this comment.
Only reviewed the code, not the tests yet.
|
|
||
| function deployJoin(bytes6 assetId) external auth returns (IJoin join) { | ||
| address asset = contangoCauldron.assets(assetId); | ||
| require(asset != address(0), "Asset not known to the Contango Cauldron"); |
There was a problem hiding this comment.
require(yieldLadle.joins(assetId) == IJoin(address(0)), "Join already exists").
There was a problem hiding this comment.
btw, here I'm trying to avoid overriding an existing join, but I agree we should also check there isn't one that we could copy from the yieldLadle
| witchDefaults = WitchDefaults(duration, vaultProportion, intialDiscount); | ||
| } | ||
|
|
||
| function setLineAndLimit( |
There was a problem hiding this comment.
What's the point of this function?
There was a problem hiding this comment.
have more granular access to the witch
| } | ||
|
|
||
| function setAuctioneerReward(uint256 auctioneerReward) external auth { | ||
| contangoWitch.setAuctioneerReward(auctioneerReward); |
There was a problem hiding this comment.
There should be a boundAuctioneerReward function, I guess.
| contangoWitch.setLineAndLimit(ilkId, baseId, duration, vaultProportion, collateralProportion, max); | ||
| } | ||
|
|
||
| function configureWitch(bytes6 ilkId, bytes6 baseId, uint128 max) external auth { |
There was a problem hiding this comment.
I guess this one should be named setLineAndLimit
There was a problem hiding this comment.
it was originally overloaded with that name, but that doesn't play well when you wanna get the selector to assign permissions
| } | ||
|
|
||
| /// @notice Propagate a token to the Ladle from the Yield Ladle | ||
| function addToken(address token) external auth { |
There was a problem hiding this comment.
I like copyX for when we are propagating configs from the yield ladle to the contango ladle, as you did below.
There was a problem hiding this comment.
yeah, most of the names were there and I didn't understand the pattern, so didn't change them
| compositeOracle.setSource(baseId, ilkId, yieldSpaceOracle); | ||
| } else if (yieldCauldron.assets(ilkId) != address(0)) { | ||
| DataTypes.SpotOracle memory spotOracle_ = yieldCauldron.lookupSpotOracle(baseId, ilkId); | ||
| compositeOracle.setSource(baseId, ilkId, spotOracle_.oracle); |
There was a problem hiding this comment.
Only if it is not IOracle(address(0)), I think.
There was a problem hiding this comment.
the lookup method does that already
| } | ||
|
|
||
| /// @notice Add a new series, if it exists in the Yield Cauldron | ||
| function addSeries(bytes6 seriesId) external auth returns (DataTypes.Series memory series_) { |
There was a problem hiding this comment.
As below, I think this should be copySeries
| AccessControl(address(yieldLadle.joins(USDT))).grantRole(root, address(wand)); | ||
| AccessControl(address(yieldCauldron.series(FYUSDT2306).fyToken)).grantRole(root, address(wand)); | ||
|
|
||
| wand.grantRole(wand.copySpotOracle.selector, address(this)); |
There was a problem hiding this comment.
You know that there is a grantRoles function, right?
There was a problem hiding this comment.
yes, but creating the array is not as pretty as I'd like, and this way I can group the related permissions (wand + cauldron or whatever)
|
BTW, the PR seems to fail cause it requires to fork arbitrum, I have the feeling the secret with the URL is missing in the repo config (or misnamed) |
2 participants
Still a draft. I've used copilot quite heavily, and there is a certain risk to this contract. We will need at least an internal audit.