-
Notifications
You must be signed in to change notification settings - Fork 138
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
Unbonding ops are put on hold even w/o consumer chains #118
Conversation
x/ccv/provider/keeper/keeper.go
Outdated
// Call back into staking to tell it to stop this op from unbonding when the unbonding period is complete | ||
h.k.stakingKeeper.PutUnbondingOnHold(ctx, ID) | ||
// Call back into staking to tell it to stop this op from unbonding when the unbonding period is complete | ||
h.k.stakingKeeper.PutUnbondingOnHold(ctx, ID) |
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 happens theoretically if the unbonding is put on hold while child chains exist, but then child chain gets removed with gov proposal before hold is lifted?
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.
That's where the lockUnbondingOnTimeout
parameter is used (see technical spec). The consumer chain is removed using the function specified here, i.e.,
function StopConsumerChain(chainId: string, lockUnbonding: Bool)
However, now that I look at it, the code is not covering the removal of a consumer chain when there is only one consumer chain, i.e.,
if !lockUnbonding {
// remove chainId form all outstanding unbonding operations
foreach id IN vscToUnbondingOps[(chainId, _)] {
unbondingOps[id].unbondingChainIds.Remove(chainId)
if unbondingOps[id].unbondingChainIds.IsEmpty() {
// notify the Staking module that the unbonding can complete
stakingKeeper.UnbondingCanComplete(id)
// remove unbonding operation
unbondingOps.Remove(id)
}
}
// clean up vscToUnbondingOps mapping
vscToUnbondingOps.Remove((chainId, _))
}
Basically, the code in onRecvVSCMaturedPacket
should be called (see 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.
Nice catch @AdityaSripal
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.
Nice catch @mpoke ! LGTM
commit b6c7a12ea8bf8e9d7b96401f95d16292730e6f9f Author: Daniel <[email protected]> Date: Wed Jun 8 13:20:44 2022 +0100 Removes extra param in diff_test commit 2eeca10dd639ced65a4d32bebae81e5dd71cdd7e Author: Daniel <[email protected]> Date: Wed Jun 8 13:18:19 2022 +0100 Squashed commit of the following: commit d0245c7 Merge: 2b965ff 9e19dd1 Author: Daniel T <[email protected]> Date: Wed Jun 8 10:37:43 2022 +0100 Merge pull request #129 from cosmos/marko/112_unused_code chore: remove unused code commit 9e19dd1 Author: Marko Baricevic <[email protected]> Date: Tue Jun 7 23:58:37 2022 +0200 remove unused code commit 2b965ff Author: Marko <[email protected]> Date: Tue Jun 7 20:46:00 2022 +0200 separate consumer from provider in app.go (#120) * remove consumer from provider app.go * add staking keeper to evidence keeper * remove provider from consumer/app.go * remove genesis test * remove ibc scoped keeper for provider * remove ibc scoped keeper for consumer Authored-by: Marko Baricevic <[email protected]> commit 579db08 Author: Marko <[email protected]> Date: Mon Jun 6 12:54:15 2022 +0200 rename paramSpace and accountKeeper (#119) Authored-by: Marko Baricevic <[email protected]> commit f0de0c5 Author: Marius Poke <[email protected]> Date: Fri Jun 3 15:00:10 2022 +0200 Unbonding ops are put on hold even w/o consumer chains (#118) * conditional PutUnbondingOnHold and test * change if condition commit 5acf1ee Author: frog power 4000 <[email protected]> Date: Mon May 30 11:19:09 2022 -0700 ConsumerRedistributeFrac now hardcoded (#102) * ConsumerRedistributeFrac now hardcoded * Update x/ccv/consumer/keeper/distribution.go * fix test to use 75% redistribution fraction Co-authored-by: Marius Poke <[email protected]> commit dc5367e Author: Marius Poke <[email protected]> Date: Mon May 30 20:10:59 2022 +0200 Fix proto-gen (#116) * fix proto-gen * go mod tidy commit 3542da1 Author: Marius Poke <[email protected]> Date: Mon May 30 17:15:59 2022 +0200 Update CODEOWNERS
Closes #117