Fix/hard reset recovery #1608
Fix/hard reset recovery #1608
Conversation
Codecov ReportBase: 94.89% // Head: 94.89% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## main #1608 +/- ##
=======================================
Coverage 94.89% 94.89%
=======================================
Files 68 68
Lines 2411 2411
=======================================
Hits 2288 2288
Misses 123 123
Flags with carried forward coverage won't be shown. Click here to find out more. Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| # in case of successful reset we store the reset params in the shared state, | ||
| # so that in the future if the communication with tendermint breaks, and we need to | ||
| # perform a hard reset to restore it, we can use these as the right ones | ||
| self.context.shared_state["reset_params"] = reset_params |
There was a problem hiding this comment.
shared_state is not conceptually the right place. There we store information which is relevant cross skills. I'd save it in a model on the skill.
There was a problem hiding this comment.
@angrybayblade could rename shared_state for v2 and to call it skills_stared_state or similar. It gets often misinterpreted.
There was a problem hiding this comment.
| @property | ||
| def sleep_before_hard_reset(self) -> float: | ||
| """ | ||
| Amount of time to sleep before performing a hard reset. | ||
|
|
||
| We sleep for half the observation interval as there are no immediate transactions on either side of the reset. | ||
|
|
||
| :returns: the amount of time to sleep in seconds | ||
| """ | ||
| return self.params.observation_interval / 2 | ||
|
|
||
| @property | ||
| def sleep_after_hard_reset(self) -> float: | ||
| """ | ||
| Amount of time to sleep after performing a hard reset. | ||
|
|
||
| We sleep for half the observation interval as there are no immediate transactions on either side of the reset. | ||
|
|
||
| :returns: the amount of time to sleep in seconds | ||
| """ | ||
| return self.params.observation_interval / 2 |
There was a problem hiding this comment.
Why not just make this as a one variable since they both return the same value
There was a problem hiding this comment.
I suppose I did it like this to make it more readable. It seemed weird to have a property for half the observation interval.
| # in case of successful reset we store the reset params in the shared state, | ||
| # so that in the future if the communication with tendermint breaks, and we need to | ||
| # perform a hard reset to restore it, we can use these as the right ones | ||
| self.context.shared_state["reset_params"] = reset_params |
There was a problem hiding this comment.
…ith `hard_reset_sleep`
# Conflicts: # autonomy/constants.py # docs/guides/quick_start.md # docs/package_list.md # packages/packages.json # packages/valory/agents/hello_world/aea-config.yaml # packages/valory/agents/register_reset/aea-config.yaml # packages/valory/agents/register_termination/aea-config.yaml # packages/valory/agents/registration_start_up/aea-config.yaml # packages/valory/agents/test_abci/aea-config.yaml # packages/valory/services/hello_world/service.yaml # packages/valory/services/register_reset/service.yaml # packages/valory/skills/hello_world_abci/skill.yaml # packages/valory/skills/register_reset_abci/skill.yaml # packages/valory/skills/register_termination_abci/skill.yaml # packages/valory/skills/registration_abci/skill.yaml # packages/valory/skills/reset_pause_abci/skill.yaml # packages/valory/skills/safe_deployment_abci/skill.yaml # packages/valory/skills/termination_abci/skill.yaml # packages/valory/skills/test_abci/skill.yaml # packages/valory/skills/transaction_settlement_abci/skill.yaml
Fixes
This PR fixes 2 issues with hard reset when it is used as a recovery mechanism for broken agent <-> tendermint communication. Namely:
Types of changes
What types of changes does your code introduce? (A breaking change is a fix or feature that would cause existing functionality and APIs to not work as expected.)
Put an
xin the box that appliesChecklist
Put an
xin the boxes that apply.mainbranch (left side). Also you should start your branch off ourmain.Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...