-
Notifications
You must be signed in to change notification settings - Fork 4
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
fix: fix some logics and sync spec to latest source code #18
Conversation
DoCancelProvideInsurance when cancel provide insurance, we should return all of its spendable coins from both derived address and fee pool address. DoWithdrawInsurance we can accept request only paired or unpaired insurances, not unpairing insurance. it because unpairing insurance is already in state transition situation at epoch, so its weird to queue the request for that insurance.
fix core logics before: * there can be bug because chunk's status is changed to unpairing but, current paired chunk's status is still Paired and chunk have paired insurance id even if it is unpairing chunk. after: if paired insurance of paired chunk have invalid insurance, then unpairing it and add it to out insurances to hande just like other unpairing chunks. add missing invariant checks * newly added RedelegationInfosInvariant was not included chore * refactor variables name in invariants.go * use lsm's own event key types, not other module's. * add module name to each event
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 5.75% (double_sign_friction + down_time_friction) notation on the spec is guaranteed only when param change is limited through the antihandler, so the antihandler content on spec needs to be updated
chore: since pb.go has been modified, re-generation of swagger and statik is needed
also updated swagger docs and statik
* if there were slashing (token value of chunk's del < chunk size), then we consider insurance balance now. * remove NativeTokenToLiquidStakeToken, just use MintRate. it is more easy to understand.
* add description about utilization ratio * definition of NetAmount is changed. we consider insurance coverage now. * add missing item to a formula of NetAmount: sum of all unbonding balance of chunks (insurance coverage included) * MaximumDiscountRate is defined as governance parameter
x/liquidstaking/spec/01_concepts.md
Outdated
@@ -76,4 +77,4 @@ The rewards accumulated on the **reward module account** can be withdrawn by any | |||
|
|||
The discount rate is calculated as follows: `discount rate = reward module account's balance / (num paired chunks * chunk size)` |
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 discussed the discount rate need to fix to use NetAmount instead of num-paired chunks, like the utilization ratio
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.
Oh, I'll update it soon. sorry for miss
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.
Updated! thanks. 2fd453d
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.
@dongsam
Any additional comments for this?
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, now we can proceed with the code update based on the updated spec
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.
chore: Transition Diagram png files seem to be missing
forum post also should be updated
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.
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.
Added missing diagrams and contents for security cap in d1733cc
Now I'll proceed the code updates.
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 update LGTM as well
TODO: fix broken tests
I updated source codes based on updated spec. Please review file changes from several commits 🙏 |
those functions are moved to type level functions
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!
Description
Fixed
I found some logical errors during syncing spec to latest source codes.
DoCancelProvideInsurance
DoWithdrawInsurance
GetAllRePairableChunksAndOutInsurances
add missing invariant checks
RedelegationInfosInvariant
was not includedUpdated
calculation of NetAmount
calculation of TotalRemainingRewards
before:
del_reward - insurance_commission
after:
Chore
Docs
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...
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...