shield: implement reimbursement for shield claim proposals #132

dshe-certik · 2020-10-30T03:28:45Z

Closes: #XXX
Related: #120

Description

Implemented reimbursement for shield claim proposals.

For contributor use:

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards.
Wrote unit and integration tests
Updated relevant documentation (docs/) or specification (x/<module>/spec/)
Added relevant godoc comments.
Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer

For admin use:

Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
Reviewers assigned
Squashed all commits, uses message "Merge pull request #XYZ: [title]"

codecov-io · 2020-10-30T03:40:58Z

Codecov Report

Merging #132 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #132      +/-   ##
==========================================
+ Coverage   53.76%   53.78%   +0.01%     
==========================================
  Files          49       49              
  Lines        3383     3384       +1     
==========================================
+ Hits         1819     1820       +1     
  Misses       1432     1432              
  Partials      132      132

Impacted Files	Coverage Δ
app/app.go	`97.36% <100.00%> (+<0.01%)`	⬆️
x/gov/test_common.go	`98.00% <100.00%> (ø)`

x/shield/keeper/proposal.go

hacheigriega · 2020-11-04T11:48:55Z

x/shield/keeper/proposal.go

+	// Update provider's collateral and total withdraw.
+	provider.Collateral = provider.Collateral.Sub(payout)
+	provider.Withdrawing = provider.Withdrawing.Sub(payoutFromWithdraw)
+


We could early return here if payoutFromWithdraw is not positive.

The check will be called once the for loop starts, so I think it's acceptable?

x/shield/keeper/proposal.go

yoongbok-lee · 2020-11-04T18:41:47Z

x/shield/keeper/proposal.go

+
+	// Update unbonding delegations between the delegator and the validator.
+	for i := range unbonding.Entries {
+		if unbonding.Entries[i].Balance.Equal(ubd.Entries[0].Balance) && unbonding.Entries[i].CompletionTime.Equal(ubd.Entries[0].CompletionTime) {


what is this doing? why not if i == 0?

ubd comes from GetSortedUnbondingDelegations() so it only has one entry, while unbonding comes from GetUnbondingDelegation and has multiple entries. Their indices are thus different.

yoongbok-lee · 2020-11-04T18:49:14Z

great! Thanks for the work. Considering the number of lines we added in for update 1, we'll need thorough testing until the release to ensure the numbers and logics are correct. 👍

* start reimplementation of ClaimLock * ClaimUnlock and RestoreShield * update purchase in ClaimLock * fix obvious errors * update collateral check logic * implement locking * restore claim proposal handler * clean code and fix obvious mistakes * add unbonding info field under withdraw * add unbonding info to withdrawals * add tests and add LockedCollateral type under Provider * delay only necessary amounts * error fix * add claim proposal sim * simulation restored and small bug fixes * implement loss portion-based claim process * fix obvious mistake * update total claimed correctly * fix init genesis * remove unnecessary comment * remove simapp * separate withdraw delay & unbonding delay * error fixes * fix index error * delete purchase queue entry 4 days after protection end time * fix init genesis error * fix purchase dequeue * revise dequeue purchase logic * revise dequeue pool purchaser pair logic * revert name change * comments addressed * additional changes not included in the last commit by mistake * revert securing purchased ratio and address comments * improve secure collateral logic * shield: implement reimbursement for shield claim proposals (#132) * Init reimbursement for claims. * Implemented basic workflow for reimbursement. * Added comments. * Enabled withdraws for reimbursements. * Updated according to latest doc. * Temp commit. Implementing payouts. * Finished skeleton of the new strategy. * Implemented paying by collateral. * Implemented paying from withdraws. * Implemented paying from unbondings. * Fixed an error caused by truncation errors. * Reorganized code to make logics clear. * Fixed PayFromUnbonding. * Fixed PayFromDelegation. * Update unbonding queue when an entry is removed. * Update delegated vesting for vesting accounts. * Fixed UndelegateShares. * Addressed comments. * Reverted some modifications to minimize changes. * A minor change. * Update the module account invariant. * Fixed an incorrect key for reimbursement. * Fixed incorrect collateral updates. * Consider total claimed in simulations. * gov: increase voters and yes votes in simulation (#141) * add debug prints * Update operations.go * remove debug prints * remove debug log * fix certifier voting * Made lint. Co-authored-by: Dan <dan.she@certik.io> * Fixed an incorrect index. * add new max shield consideration to all relevant operations * Fixed total withdraw update and temp disable param change sim in gov. * fix PayFromDelegation and staking source pool * Re-enabled param change sims in gov. To be fixed in another PR. * Fixed incorrect unbonding updates. * Added reimbursements in genesis. * Fixed incorrect payout update in UpdateProviderCollateralForPayout. * Updated descriptions in invariants. * Fixed account update, with debugging lines unremoved. * Update vested coins for manual vesting account in shield. * address most of the comments * Fixed manual vesting account update in shield. * Removed debugging lines. * Fixed simulations of creating pools. * remove vesting types other than manual in sim * shield: add query endpoints for reimbursements (#147) * delay unbonding only if necessary * disable staking param change * reverse iteration of timeslices * additional small fixes and addressing rest of the review comments * add withdraw reimbursement operation to sim * remove debug lines * Fixed simulations for withdrawing reimbursements. * Fixed some naming issues. Co-authored-by: Dan <57596823+dshe-certik@users.noreply.github.com> Co-authored-by: Dan <dan.she@certik.io> Co-authored-by: Ziyi Zhang <66639536+ziyi-zhang-1130@users.noreply.github.com> Co-authored-by: yoongbok-lee <52583590+yoongbok-lee@users.noreply.github.com>

Update master with public latest

dshe-certik added 10 commits October 20, 2020 23:47

Init reimbursement for claims.

2784d72

Implemented basic workflow for reimbursement.

591c768

Added comments.

161e545

Enabled withdraws for reimbursements.

785f790

Updated according to latest doc.

8e82f15

Temp commit. Implementing payouts.

529cf5e

Finished skeleton of the new strategy.

1933b3a

Implemented paying by collateral.

70b064a

Implemented paying from withdraws.

4beec5f

Implemented paying from unbondings.

4e72fc5

hacheigriega reviewed Oct 30, 2020

View reviewed changes

x/shield/keeper/proposal.go Outdated Show resolved Hide resolved

hacheigriega reviewed Oct 30, 2020

View reviewed changes

x/shield/keeper/proposal.go Outdated Show resolved Hide resolved

hacheigriega reviewed Oct 31, 2020

View reviewed changes

x/shield/keeper/proposal.go Show resolved Hide resolved

dshe-certik added 7 commits October 31, 2020 22:51

Fixed an error caused by truncation errors.

285f9f2

Reorganized code to make logics clear.

fdc4abe

Fixed PayFromUnbonding.

48d30ef

Fixed PayFromDelegation.

b425cc0

Update unbonding queue when an entry is removed.

d5d7ea0

Update delegated vesting for vesting accounts.

a22ea67

Merge branch 'master' into ds/shield-claim-handler

7ce89f6

dshe-certik marked this pull request as ready for review November 4, 2020 06:32

Fixed UndelegateShares.

c5874b2

dshe-certik requested review from yoongbok-lee, hacheigriega and zhaozhongni November 4, 2020 06:49

hacheigriega approved these changes Nov 4, 2020

View reviewed changes

yoongbok-lee approved these changes Nov 4, 2020

View reviewed changes

Addressed comments.

a3c3ff5

dshe-certik changed the base branch from master to hy/shield-claim November 4, 2020 19:10

Merge branch 'hy/shield-claim' into ds/shield-claim-handler

d545ad4

dshe-certik merged commit 6f9938d into hy/shield-claim Nov 4, 2020

dshe-certik deleted the ds/shield-claim-handler branch November 4, 2020 19:24

yoongbok-lee added a commit that referenced this pull request May 20, 2022

Merge pull request #132 from ShentuChain/public-latest

5e6fb20

Update master with public latest

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

shield: implement reimbursement for shield claim proposals #132

shield: implement reimbursement for shield claim proposals #132

dshe-certik commented Oct 30, 2020 •

edited

codecov-io commented Oct 30, 2020 •

edited

hacheigriega Nov 4, 2020

dshe-certik Nov 4, 2020

yoongbok-lee Nov 4, 2020

dshe-certik Nov 4, 2020

yoongbok-lee commented Nov 4, 2020

shield: implement reimbursement for shield claim proposals #132

shield: implement reimbursement for shield claim proposals #132

Conversation

dshe-certik commented Oct 30, 2020 • edited

Description

codecov-io commented Oct 30, 2020 • edited

Codecov Report

hacheigriega Nov 4, 2020

Choose a reason for hiding this comment

dshe-certik Nov 4, 2020

Choose a reason for hiding this comment

yoongbok-lee Nov 4, 2020

Choose a reason for hiding this comment

dshe-certik Nov 4, 2020

Choose a reason for hiding this comment

yoongbok-lee commented Nov 4, 2020

dshe-certik commented Oct 30, 2020 •

edited

codecov-io commented Oct 30, 2020 •

edited