Skip to content
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.

Sign up for GitHub

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

Jump to bottom

add admin functionality to disable module incase of failure. #411

Merged
merged 3 commits into from
Feb 15, 2023
Merged

add admin functionality to disable module incase of failure. #411

merged 3 commits into from
Feb 15, 2023

Conversation

puneet2019
Copy link
Member

1. Overview

The messages defined in x/lscosmos/handler.go:21-35 can only be executed when the module state is enabled. If the module is disabled, the epoch logic and main messages, such as liquid staking, are prohibited. However, the current implementation only allows the module to be disabled from InitGenesis, as seen in x/lscosmos/genesis.go:16. This implies that all validators would need to coordinate to halt and upgrade the chain in order to set the module to disabled.
Coordinating an upgrade with validators is a manual and time-intensive progress, which prevents swift reactions to security incidents. For example, suppose the ATOM chain halts, and the module needs to be temporarily disabled.
We recommend implementing the following suggestions:

2. Implementation details

  • adds admin functionality to disable/reenable the module state.

3. How to test/use

manual

@github-actions
Copy link

Coverage after merging puneet/changemodulestate into main

8.87%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
ante
   ante.go100%100%0%..., 51, 52, 53, 54
app
   genesis_account.go100%100%0%..., 41, 42, 43, 46
   encoding.go100%100%0%..., 14, 15, 16, 17
   export.go100%100%0%..., 96, 97, 98, 99
   genesis.go100%100%0%17, 18, 19, 20
   constants.go100%100%60%..., 65, 66, 67, 69
   app.go100%100%0.68%..., 939, 940, 941, 942
cmd/pstaked/cmd
   genaccounts.go100%100%14.39%..., 96, 97, 98, 99
   testnet.go100%100%8.30%..., 81, 82, 83, 84
   root.go100%100%62.89%..., 64, 65, 68, 69
tests/e2e
   io.go100%100%64.29%..., 28, 29, 39, 40
   keys.go100%100%17.14%..., 51, 52, 53, 55
   genesis.go100%100%68.35%..., 87, 88, 98, 99
   util.go100%100%66.67%..., 33, 34, 37, 38
   validator.go100%100%78.95%..., 81, 82, 97, 98
   chain.go100%100%58.44%..., 96, 97, 98, 99
x/lscosmos
   module_ibc.go100%100%0%..., 84, 92, 93, 94
   proposal_handler.go100%100%38.46%..., 22, 23, 25, 26
   module.go100%100%50%..., 84, 87, 88, 89
   handler.go100%100%15.63%..., 41, 42, 43, 44
   genesis.go100%100%84.44%..., 29, 30, 32, 33
   module_simulation.go100%100%4%..., 59, 60, 61, 62
x/lscosmos/client/utils
   utils.go100%100%49.54%..., 74, 75, 76, 78
x/lscosmos/keeper
   module_state.go100%100%100%
   hooks.go100%100%27.87%..., 70, 71, 72, 73
   governance_proposal.go100%100%0%..., 96, 97, 98, 99
   grpc_query.go100%100%20.71%..., 96, 97, 98, 99
   keeper.go100%100%52.81%..., 93, 94, 95, 99
   msg_server.go100%100%0.69%..., 94, 95, 96, 97
   transient_store.go100%100%68.75%..., 74, 75, 76, 78
   icq_callbacks.go100%100%78.18%..., 57, 58, 65, 66
   invariants.go100%100%42.11%..., 30, 31, 32, 33
   host_accounts.go100%100%100%
   delegator_unbonding_epoch_entry.go100%100%72.88%..., 72, 73, 74, 76
   allowlisted_validators.go100%100%100%
   params.go100%100%100%
   host_chain_reward_address.go100%100%100%
   grpc_query_params.go100%100%100%
   delegation_state.go100%100%44.62%..., 199, 201, 85, 86
   handshake.go100%100%0%..., 94, 95, 98, 99
   ica_txs.go100%100%24.14%..., 45, 46, 47, 48
   delegation_strategy.go100%100%57.78%..., 87, 89, 96, 97
   abci.go100%100%5.48%..., 96, 97, 98, 99
   c_value.go100%100%95%30, 31, 43
   host_chain_params.go100%100%100%
   unbonding_epoch_c_value.go100%100%0%..., 55, 56, 57, 9
x/lscosmos/types
   msgs.go100%100%13.20%..., 93, 94, 95, 96
   msgs.pb.go100%100%0.73%..., 996, 997, 998, 999
   lscosmos.go100%100%32.19%..., 94, 97, 98, 99
   query.pb.go100%100%0.78%..., 995, 996, 997, 998
   genesis.pb.go100%100%0.91%..., 96, 97, 98, 99
   packet.pb.go100%100%1.87%..., 92, 93, 94, 95
   governance_proposal.go100%100%87.54%..., 314, 374, 96, 97
   genesis.go100%100%87.50%30, 31, 32, 33
   query.pb.gw.go100%100%0%..., 991, 992, 993, 997
   governance_proposal.pb.go100%100%0.60%..., 996, 997, 998, 999
   params.go100%100%47.37%..., 36, 37, 38, 39
   msgs.pb.gw.go100%100%0%..., 96, 97, 98, 99
   delegation_strategy.go100%100%80.72%..., 124, 125, 126, 65
   lscosmos.pb.go100%100%0.87%..., 995, 996, 998, 999
   keys.go100%100%0%..., 125, 129, 130, 131
   params.pb.go100%100%3.19%..., 96, 97, 98, 99
   codec.go100%100%46.15%..., 46, 47, 48, 49

@github-actions
Copy link

Coverage after merging puneet/changemodulestate into main

8.87%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
ante
   ante.go100%100%0%..., 51, 52, 53, 54
app
   genesis.go100%100%0%17, 18, 19, 20
   export.go100%100%0%..., 96, 97, 98, 99
   app.go100%100%0.68%..., 939, 940, 941, 942
   encoding.go100%100%0%..., 14, 15, 16, 17
   genesis_account.go100%100%0%..., 41, 42, 43, 46
   constants.go100%100%60%..., 65, 66, 67, 69
cmd/pstaked/cmd
   testnet.go100%100%8.30%..., 81, 82, 83, 84
   genaccounts.go100%100%14.39%..., 96, 97, 98, 99
   root.go100%100%62.89%..., 64, 65, 68, 69
tests/e2e
   validator.go100%100%78.95%..., 81, 82, 97, 98
   genesis.go100%100%68.35%..., 87, 88, 98, 99
   chain.go100%100%58.44%..., 96, 97, 98, 99
   io.go100%100%64.29%..., 28, 29, 39, 40
   util.go100%100%66.67%..., 33, 34, 37, 38
   keys.go100%100%17.14%..., 51, 52, 53, 55
x/lscosmos
   genesis.go100%100%84.44%..., 29, 30, 32, 33
   proposal_handler.go100%100%38.46%..., 22, 23, 25, 26
   module_ibc.go100%100%0%..., 84, 92, 93, 94
   module_simulation.go100%100%4%..., 59, 60, 61, 62
   module.go100%100%50%..., 84, 87, 88, 89
   handler.go100%100%15.63%..., 41, 42, 43, 44
x/lscosmos/client/utils
   utils.go100%100%49.54%..., 74, 75, 76, 78
x/lscosmos/keeper
   params.go100%100%100%
   hooks.go100%100%27.87%..., 70, 71, 72, 73
   host_chain_reward_address.go100%100%100%
   delegator_unbonding_epoch_entry.go100%100%72.88%..., 72, 73, 74, 76
   delegation_strategy.go100%100%57.78%..., 87, 89, 96, 97
   icq_callbacks.go100%100%78.18%..., 57, 58, 65, 66
   unbonding_epoch_c_value.go100%100%0%..., 55, 56, 57, 9
   grpc_query.go100%100%20.71%..., 96, 97, 98, 99
   invariants.go100%100%42.11%..., 30, 31, 32, 33
   keeper.go100%100%52.81%..., 93, 94, 95, 99
   grpc_query_params.go100%100%100%
   handshake.go100%100%0%..., 94, 95, 98, 99
   c_value.go100%100%95%30, 31, 43
   delegation_state.go100%100%44.62%..., 199, 201, 85, 86
   ica_txs.go100%100%24.14%..., 45, 46, 47, 48
   transient_store.go100%100%68.75%..., 74, 75, 76, 78
   host_accounts.go100%100%100%
   governance_proposal.go100%100%0%..., 96, 97, 98, 99
   module_state.go100%100%100%
   msg_server.go100%100%0.69%..., 94, 95, 96, 97
   allowlisted_validators.go100%100%100%
   host_chain_params.go100%100%100%
   abci.go100%100%5.48%..., 96, 97, 98, 99
x/lscosmos/types
   codec.go100%100%46.15%..., 46, 47, 48, 49
   packet.pb.go100%100%1.87%..., 92, 93, 94, 95
   query.pb.gw.go100%100%0%..., 991, 992, 993, 997
   query.pb.go100%100%0.78%..., 995, 996, 997, 998
   genesis.go100%100%87.50%30, 31, 32, 33
   delegation_strategy.go100%100%80.72%..., 124, 125, 126, 65
   governance_proposal.go100%100%87.54%..., 314, 374, 96, 97
   params.go100%100%47.37%..., 36, 37, 38, 39
   msgs.go100%100%13.20%..., 93, 94, 95, 96
   genesis.pb.go100%100%0.91%..., 96, 97, 98, 99
   lscosmos.go100%100%32.19%..., 94, 97, 98, 99
   governance_proposal.pb.go100%100%0.60%..., 996, 997, 998, 999
   msgs.pb.go100%100%0.73%..., 996, 997, 998, 999
   params.pb.go100%100%3.19%..., 96, 97, 98, 99
   msgs.pb.gw.go100%100%0%..., 96, 97, 98, 99
   lscosmos.pb.go100%100%0.87%..., 995, 996, 998, 999
   keys.go100%100%0%..., 125, 129, 130, 131

@github-actions
Copy link

Coverage after merging puneet/changemodulestate into main

8.87%

Coverage Report
FileBranchesFuncsLinesUncovered Lines
ante
   ante.go100%100%0%..., 51, 52, 53, 54
app
   export.go100%100%0%..., 96, 97, 98, 99
   genesis_account.go100%100%0%..., 41, 42, 43, 46
   encoding.go100%100%0%..., 14, 15, 16, 17
   genesis.go100%100%0%17, 18, 19, 20
   app.go100%100%0.68%..., 939, 940, 941, 942
   constants.go100%100%60%..., 65, 66, 67, 69
cmd/pstaked/cmd
   root.go100%100%62.89%..., 64, 65, 68, 69
   testnet.go100%100%8.30%..., 81, 82, 83, 84
   genaccounts.go100%100%14.39%..., 96, 97, 98, 99
tests/e2e
   util.go100%100%66.67%..., 33, 34, 37, 38
   keys.go100%100%17.14%..., 51, 52, 53, 55
   chain.go100%100%58.44%..., 96, 97, 98, 99
   io.go100%100%64.29%..., 28, 29, 39, 40
   genesis.go100%100%68.35%..., 87, 88, 98, 99
   validator.go100%100%78.95%..., 81, 82, 97, 98
x/lscosmos
   module_simulation.go100%100%4%..., 59, 60, 61, 62
   module_ibc.go100%100%0%..., 84, 92, 93, 94
   handler.go100%100%15.63%..., 41, 42, 43, 44
   module.go100%100%50%..., 84, 87, 88, 89
   proposal_handler.go100%100%38.46%..., 22, 23, 25, 26
   genesis.go100%100%84.44%..., 29, 30, 32, 33
x/lscosmos/client/utils
   utils.go100%100%49.54%..., 74, 75, 76, 78
x/lscosmos/keeper
   governance_proposal.go100%100%0%..., 96, 97, 98, 99
   host_chain_reward_address.go100%100%100%
   delegation_strategy.go100%100%57.78%..., 87, 89, 96, 97
   handshake.go100%100%0%..., 94, 95, 98, 99
   ica_txs.go100%100%24.14%..., 45, 46, 47, 48
   grpc_query.go100%100%20.71%..., 96, 97, 98, 99
   host_chain_params.go100%100%100%
   allowlisted_validators.go100%100%100%
   hooks.go100%100%27.87%..., 70, 71, 72, 73
   msg_server.go100%100%0.69%..., 94, 95, 96, 97
   unbonding_epoch_c_value.go100%100%0%..., 55, 56, 57, 9
   invariants.go100%100%42.11%..., 30, 31, 32, 33
   keeper.go100%100%52.81%..., 93, 94, 95, 99
   grpc_query_params.go100%100%100%
   transient_store.go100%100%68.75%..., 74, 75, 76, 78
   icq_callbacks.go100%100%78.18%..., 57, 58, 65, 66
   module_state.go100%100%100%
   c_value.go100%100%95%30, 31, 43
   host_accounts.go100%100%100%
   abci.go100%100%5.48%..., 96, 97, 98, 99
   params.go100%100%100%
   delegation_state.go100%100%44.62%..., 199, 201, 85, 86
   delegator_unbonding_epoch_entry.go100%100%72.88%..., 72, 73, 74, 76
x/lscosmos/types
   packet.pb.go100%100%1.87%..., 92, 93, 94, 95
   lscosmos.pb.go100%100%0.87%..., 995, 996, 998, 999
   codec.go100%100%46.15%..., 46, 47, 48, 49
   genesis.pb.go100%100%0.91%..., 96, 97, 98, 99
   governance_proposal.go100%100%87.54%..., 314, 374, 96, 97
   msgs.pb.go100%100%0.73%..., 996, 997, 998, 999
   delegation_strategy.go100%100%80.72%..., 124, 125, 126, 65
   query.pb.go100%100%0.78%..., 995, 996, 997, 998
   governance_proposal.pb.go100%100%0.60%..., 996, 997, 998, 999
   params.go100%100%47.37%..., 36, 37, 38, 39
   params.pb.go100%100%3.19%..., 96, 97, 98, 99
   msgs.go100%100%13.20%..., 93, 94, 95, 96
   query.pb.gw.go100%100%0%..., 991, 992, 993, 997
   keys.go100%100%0%..., 125, 129, 130, 131
   genesis.go100%100%87.50%30, 31, 32, 33
   msgs.pb.gw.go100%100%0%..., 96, 97, 98, 99
   lscosmos.go100%100%32.19%..., 94, 97, 98, 99

RohitAudit
RohitAudit reviewed Feb 14, 2023
View reviewed changes
Copy link

@RohitAudit RohitAudit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Who will be the admin and how can we set the admin?

@puneet2019
Copy link
Member Author

Who will be the admin and how can we set the admin?

We had set it in genesis. And we can change/ set it with a setFeeAddress proposal which has to pass on persistence chain

RohitAudit
RohitAudit approved these changes Feb 15, 2023
View reviewed changes
Copy link

@RohitAudit RohitAudit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks good to go. The setting and checking of address is added.

@puneet2019 puneet2019 merged commit a1d6641 into main Feb 15, 2023
@puneet2019 puneet2019 deleted the puneet/changemodulestate branch February 15, 2023 06:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RohitAudit RohitAudit RohitAudit approved these changes

@gmakestyle gmakestyle Awaiting requested review from gmakestyle

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@puneet2019 @RohitAudit