Liq 2.0 #228
Merged
Liq 2.0 #228
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cmooney-20200725: various bug fixes
* SC-4072: limit amount of Dai out for liquidation * remove changes for another PR * added BLN * fix comment * change MILLION to MIl * fixed up the formula * fixed bug in MIL va. MLN * forgot rdiv() * missing closing ) * add comments on types * typo, needed param to be data * fix: update dog to fix stack to deep, compiles * feat: update dog to use LIQ-1.2 logic * fix: update comments, remove dunk, change bone to digs * fix: gas optimize dink > 0 require * fix: update comments, add spot to brace block, add cost var * fix: change cost to due * fix: formatting Co-authored-by: Lucas Manuel <lucas@makerdao.com>
fix dust check
* feat: add auction reset functionality * fix: add storage vars, fix compilation issues * feat: add require to warm, change to external * fix: update comments * fix: use safe sub, add dead auction check in warm and take
* define price decrease function interface and some trial implementations * commit sample price decrease functions * dapp install https://github.com/makerdao/dss.git * feat: add testing for linear/exp price dec functions * feat: add unit tests to stairstep, linear price decrease functions * fix: add comments about precision * fix: update exp test tolerance to 1e-22 * feat: tests working consistently with precision tolerance * fix: fix wrong comments * dapp uninstall dss * fix: update variables, comments Co-authored-by: Lucas Manuel <lucas@makerdao.com>
* Add on-chain tracking of live auctions * rename last to move for clarity * Add splice and count funtions for array management * Fix index shifting * Change _stopBaking to _remove * Clean up and fixes * remove list(uint256,uint256)(uint256[]) and replace with getId(uint256)(uint256) * Replace .length-- with .pop() for forward compatibility * Remove comment * fix: update memes, comments Co-authored-by: Lucas <lucas@makerdao.com>
* feat: start bake test * dapp install ds-value * feat: add take tests for over/under/at tab * feat: add take test for multiple bids, tests pass * fix: delete notes file * fix: remove empty lib/dss * dapp install dss * feat: add more testing, setup * feat: all tests pass except exp decrease * fix: fix exp decrease step and cut values * fix: remove rpow, change max/pay to pay/val, update dirt require * Fix import file and rename uint to uin256 * fix: update room require * fix: update val to price * fix: remove vat.flux(), grab straight to oven * fix: remove rely/deny/hope/nope from oven file in dog * fix: update imports * fix: update dog.digs() comment * fix: update vat helpers, hevm.warping, address(this) * fix: use defined CHEAT_CODE * feat: add pos to testing * fix: remove unused MLN, BLN vars in test * uint to uint256 * Compile with solc 0.5 and 0.6 Co-authored-by: Gonzalo Balabasquer <gbalabasquer@gmail.com>
Added logic to `stop` and `start` the clipper. All major functions are frozen when `stopped == true`.
* add continuous exponential decrease * Update src/test/abaci.t.sol Co-authored-by: Gonzalo Balabasquer <gbalabasquer@gmail.com> * harmonize comments Co-authored-by: Gonzalo Balabasquer <gbalabasquer@gmail.com>
#213) * More efficient lock in case tx reverts or if in future there are not refunds * Fix spacing Co-authored-by: Brian McMichael <brian@brianmcmichael.com>
* Return lot size with getStatus * Add tab to getStatus
* add a breaker level to disable kick and redo but not take * test re-enabling taking Co-authored-by: Gonzalo Balabasquer <gbalabasquer@gmail.com> * fix tests * fix tests post-rebase Co-authored-by: Gonzalo Balabasquer <gbalabasquer@gmail.com>
* update Dog comment * typos and phrasing * improve kick param comments * change kpr comments for kick and redo
* rename getPrice to getFeedPrice Note that this is inconsistent with daiwanese practices * rename the return value of getFeedPrice()
* gas tests and don't use optimizations * update shell.nix to not compile w/optimizations
* properly account for chop in dust checks in the Clipper * cached chost, uniform checks, tests * update take comment * Cache chost * Remove unused variable * Change _ position * don't set upchost in constructor * cache ilk, fix comment * actually don't cache, ilk is immutable * remove underscore * remove extra comma Co-authored-by: Gonzalo Balabasquer <gbalabasquer@gmail.com>
gbalabasquer
requested review from
godsflaw,
brianmcmichael,
kmbarry1,
e18r and
WilfredTA
hexonaut
reviewed
Apr 2, 2021
Comment on lines
+152
to
+154
| function chop(bytes32 ilk) external view returns (uint256) { | ||
| return ilks[ilk].chop; | ||
| } |
There was a problem hiding this comment.
Instead of attempting to guess what specific parameters we will need out a struct in the future for SLOAD-optimized lookups we should be providing a general bytes32 storage lookup:
function _peek(bytes32 addr) external pure returns (bytes32 b) {
assembly {
b := mload(addr)
}
}
This should be attached to all contracts that do not have private information.
There was a problem hiding this comment.
This is a good one to discuss for liquidation 2.1
There was a problem hiding this comment.
There are pros and cons to this approach, we should discuss for the next revision.
There was a problem hiding this comment.
Yeah I've considered this, might be worth doing. Here, chop() was specifically added by request for B.Protocol, so it predates this realization of the need to gas optimize storage reads.
There was a problem hiding this comment.
(and, I should add, it's much friendlier for integrators this way as they don't need to rely on a storage slot which could have its use changed in an update, or even the parameter could be redefined or expressed differently)
| vat.flux(ilk, address(this), msg.sender, sales[id].lot); | ||
| _remove(id); | ||
| emit Yank(id); | ||
| } |
There was a problem hiding this comment.
I think we should start including this on every contract to reduce the need for this upchost-style caching with big structs:
function _peek(bytes32 addr) external pure returns (bytes32 b) {
assembly {
b := mload(addr)
}
}
There was a problem hiding this comment.
As I said in a prev comment, we should discuss this for liquidations 2.1. Maybe it is just easier to add all the methods manually when we have a struct, but this is also a cool idea that as you mentioned works in the meanwhile we do not want to protect an onchain read for a value.
kmbarry1
reviewed
Apr 2, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Final PR for merging liquidations 2.0 to
master.