-
Notifications
You must be signed in to change notification settings - Fork 170
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
feat: add MsgMaxBorrow #1690
feat: add MsgMaxBorrow #1690
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1690 +/- ##
==========================================
- Coverage 57.97% 57.57% -0.40%
==========================================
Files 73 73
Lines 7821 7894 +73
==========================================
+ Hits 4534 4545 +11
- Misses 2934 2996 +62
Partials 353 353
|
// create an additional supplier (DUMP, PUMP tokens) | ||
surplus := s.newAccount(coin(dumpDenom, 100_000000), coin(pumpDenom, 100_000000)) | ||
s.supply(surplus, coin(pumpDenom, 100_000000)) | ||
s.supply(surplus, coin(dumpDenom, 100_000000)) |
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.
This code should be reused, rather than copied over.
@@ -37,6 +37,7 @@ var ( | |||
ErrLiquidationIneligible = sdkerrors.Register(ModuleName, 403, "borrower not eligible for liquidation") | |||
ErrMaxWithdrawZero = sdkerrors.Register(ModuleName, 404, "max withdraw amount was zero") | |||
ErrNoHistoricMedians = sdkerrors.Register(ModuleName, 405, "insufficient historic medians available") | |||
ErrMaxBorrowZero = sdkerrors.Register(ModuleName, 406, "max borrow amount was zero") |
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.
Why this is an error? If there is nothing more to borrow, we just return zero, rather error. Imaging smart contract just want to automatically set max borrow. It should handle zero value rather than dealing with failed transaction.
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.
Any supply / withdraw / borrow / etc that is a No-Op currently fails rather then succeeding with consumed gas. This follows suit
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.
If transaction fails, then the whole provided fee for gas is consumed anyway. So, it only makes doing composability more difficult.
… no more to borrow (#1694) ## Description ref: #1690 (comment) --- ### Author Checklist _All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues._ I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] added appropriate labels to the PR - [ ] targeted the correct branch (see [PR Targeting](https://github.com/umee-network/umee/blob/main/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist _All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items._ I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable)
If this is ready before Jan 6, we can add it to
v4.0
.Mimics
MsgMaxWithdraw
changes from #1642 and #1680Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...