-
Notifications
You must be signed in to change notification settings - Fork 12
Bond minter interface #1
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
Conversation
c757fa7 to
db3fbef
Compare
8047686 to
ee9d822
Compare
contracts/ACash.sol
Outdated
| int256 fee = mintFeePct * int256(mintAmt) / (10 ** int256(PCT_DECIMALS)); | ||
| if (fee >= 0) { | ||
| IERC20(feeToken).transferFrom(msg.sender, this, fee); // todo: safe versions | ||
| // using SPOT as the fee token |
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've stubbed both implementations (native and foreign fee tokens) and so we can choose either one later ..
94d83c5 to
89faf4d
Compare
89faf4d to
b0663a5
Compare
4f0f8ce to
494df50
Compare
brandoniles
left a comment
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.
Here's my first round of comments, but still need to dig deeper in ACash and YieldStrategy
26ab5d5 to
02a06c2
Compare
contracts/ACash.sol
Outdated
| // This is very scary! | ||
| IERC20(feeToken).transfer(msg.sender, fee); | ||
| // get bond price, ie amount of SPOT for trancheAmts[i] amount of t tranches | ||
| mintAmt += pricingStrategy.getTranchePrice(t, trancheAmts[i]); |
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.
could this value be 0? (e.g. for a Z tranche). If so, should the pricingStrategy price be checked before the transfer on line 88 even if we still wait to increment mintAmt until after the transfer?
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 the 'yield' parameter on the pricing strategy indicates if a tranche is to be used or not. Post Brandon's suggestion to move that parameter to the ACash contract, we'll be able to do what you suggest.
|
|
||
| struct TrancheConfig { | ||
| IBondController bond; | ||
| uint256[] trancheRatios; |
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.
Shouldn't the Bond be able to provide the trancheRatios?
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.
Not directly. We have to query each tranche to get the ratios ..
https://github.com/buttonwood-protocol/tranche/blob/main/contracts/interfaces/IBondController.sol#L25
contracts/ACash.sol
Outdated
| AddressQueue.Queue public bondQueue; | ||
|
|
||
| // system only keeps bonds which further than the `tolarableBondMaturiy` in the queue | ||
| uint256 private _tolarableBondMaturiy; |
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.
What about minBondTimeLeftSec?
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.
it's essentially a lower limit on the maturity dates of the bonds in the bond queue.
minMaturityDate & maxMaturityDate ?
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'd like to make clear this is a timespan (in seconds), rather than a timestamp/date.
minMaturityTimeSec and maxMaturityTimeSec ?
contracts/ACash.sol
Outdated
| function advanceMintBond(IBondController newBond) public { | ||
| require(address(newBond) != bondQueue.head(), "New bond already in queue"); | ||
| require(bondIssuer.isInstance(address(newBond)), "Expect new bond to be minted by the minter"); | ||
| require(newBond.maturityDate() > tolarableBondMaturiyDate(), "New bond matures too soon"); |
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 was wondering about the case where a bond is mistakenly added to the queue that matures 1000yrs from now.
We might need a way for governance to kick a bond to the icebox, or have a safety check here.
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.
The safety check does make sense. tolarableBondMaturiyDate effectively acts as a lower limit to the bond maturity date. Maybe we could add a upper limit as well?
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.
Yeah I like that approach since it keeps governance out of the workings more.
| for (uint256 i = 0; i < latestBond.trancheCount(); i++) { | ||
| (ITranche t, ) = latestBond.tranches(i); | ||
| if (t.balanceOf(address(this)) > 0) { | ||
| trancheIcebox[t] = true; |
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 also need to emit a addedToIcebox(t) event so that the app can display the icebox elements to the rotators later.
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 have a todo (at the top) to emit events. Will get to that in a later PR ..
Co-authored-by: Brandon Iles <brandon@fragments.org>
71dabcf to
0fafbe6
Compare
0fafbe6 to
9c57c18
Compare
brandoniles
left a comment
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.
LGTM for now. Once you wrap up the current comments let's get the code in so we can keep moving 👍
* fee policy restructure * updated perp fee and rebalance handling, audit cleanup * updated vault fee and rebalance handling, audit cleanup * interface and lib updates * updated unit tests * code review fixes #1 * code review fixes #2 * code review fix #3 * Removed subscription ratio for a global vault:perp tvl (or system ratio) (#254) * Using a system ratio target * using system state to calculate fees * using system state for fees and removed subscription liquidity check * interface updates * unit test updates * rename
* Bump coverallsapp/github-action from 2.3.0 to 2.3.3 (#228) Bumps [coverallsapp/github-action](https://github.com/coverallsapp/github-action) from 2.3.0 to 2.3.3. - [Release notes](https://github.com/coverallsapp/github-action/releases) - [Commits](coverallsapp/github-action@v2.3.0...v2.3.3) --- updated-dependencies: - dependency-name: coverallsapp/github-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * updated cdr pricer (#229) * Deployed v4.1.0 implementations * Bump coverallsapp/github-action from 2.3.3 to 2.3.4 (#230) Bumps [coverallsapp/github-action](https://github.com/coverallsapp/github-action) from 2.3.3 to 2.3.4. - [Release notes](https://github.com/coverallsapp/github-action/releases) - [Commits](coverallsapp/github-action@v2.3.3...v2.3.4) --- updated-dependencies: - dependency-name: coverallsapp/github-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * updated spot subgraph to keep track of vault swap volume * Vault Refactor & Meta Oracle (#224) * removed pricing strategies in favor of meta oracle * Bill broker refactor, using new meta oracle * Charm vault refactor, using new meta oracle * Updated Interfaces and Errors * updated unit tests * code review changes * Scalable usdc spot strategy (#231) Apply suggestions from code review Co-authored-by: Brandon Iles <brandon@fragments.org> review fix caching bool instead of prev deviation * billy subgraph (#234) * Deployed new oracle contract and managers (#233) * deployed new oracle and managers * updated readme * Staking subgraph (#235) * staking subgraph, refreshing store on rebase * charm wampl vault subgraph * adds timestamp to swap events subgraph (#236) * adds timestamp to swap events subgraph Signed-off-by: nms-7 <nixon.ubud@gmail.com> * linting Signed-off-by: nms-7 <nixon.ubud@gmail.com> * fixed null swap creation with 0 timestamp Signed-off-by: nms-7 <nixon.ubud@gmail.com> --------- Signed-off-by: nms-7 <nixon.ubud@gmail.com> * Bump coverallsapp/github-action from 2.3.4 to 2.3.6 (#238) Bumps [coverallsapp/github-action](https://github.com/coverallsapp/github-action) from 2.3.4 to 2.3.6. - [Release notes](https://github.com/coverallsapp/github-action/releases) - [Commits](coverallsapp/github-action@v2.3.4...v2.3.6) --- updated-dependencies: - dependency-name: coverallsapp/github-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bill broker fee curve change (#237) * using fee factor instead of percentages to support -ve fees * unit tests * code review * convergent fee curve * updated charm fee yield logic (#239) * Fee curve update (#232) * fee curve update * code review fixes * Rollover fee curve update (#240) * rollover fee handles undersubscribed vault * bumped up hardhat * code review comments * DR based flash swap fees * comment updates * updated unit tests to use ethers v6, using custom mocking library * updated tasks and deps * updated perp mint fees to be paid to the vault * removed perp share of fees from flash swaps, all fees go to the vault * added configurable dr soft bounds (curve cutoff point) to flash swap fees * updated unit tests * removed rollover fees from perp * Daily Rebalance * updated unit tests * constant rate enrichment/debasement * mint2, redeem2 * added linked library to limit contract size * unit tests * melding perps immediately after rebalance * Apply suggestions from code review Suggestions Co-authored-by: Brandon Iles <brandon@fragments.org> * Update spot-contracts/contracts/FeePolicy.sol Co-authored-by: Brandon Iles <brandon@fragments.org> * code review fixes * code review fixes v2 * code review fixes v3 * code review v4 * during rebalance, vault pays perps by transferring tranches into perp's reserve * rebalance eql * configurable rebalance freq * code review v5 * Apply suggestions from code review Co-authored-by: Brandon Iles <brandon@fragments.org> * code review fixes * Code review fixes Co-authored-by: Brandon Iles <brandon@fragments.org> * bill broker single sided deposit fix (#253) * Dev Audit Changes (#252) * fee policy restructure * updated perp fee and rebalance handling, audit cleanup * updated vault fee and rebalance handling, audit cleanup * interface and lib updates * updated unit tests * code review fixes #1 * code review fixes #2 * code review fix #3 * Removed subscription ratio for a global vault:perp tvl (or system ratio) (#254) * Using a system ratio target * using system state to calculate fees * using system state for fees and removed subscription liquidity check * interface updates * unit test updates * rename * updated deps --------- Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: nms-7 <nixon.ubud@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: nms-7 <57442379+nms-7@users.noreply.github.com> Co-authored-by: Brandon Iles <brandon@fragments.org>
Features:
Minor changes:
usableTrancheCountin the mint function totrancheCountinstead oftrancheCount - 1, to keep the contract generic so we could even create a perpetual Z contract if we wanted