-
Notifications
You must be signed in to change notification settings - Fork 32
Add Support for Cosmos SDK 0.50.1 #383
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
base: master
Are you sure you want to change the base?
Conversation
…v set to DEV_MODE
…CTIVATED substrate container
…IVATED substrate api
COSMOS SDK v0.50.1
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.
Some changes requested but nothing major.
Good job overall 🚀
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.
Can you please explain why values in .env were changed?
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.
I'm fine with all values changing except for DEV_MODE=true. In my opinion it should be kept as false because users are expected to be running this in production.
| FROM node:16 | ||
| FROM node:23-alpine | ||
|
|
||
| RUN apk add bash |
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.
Build does not work for me without this
| RUN apk add bash | |
| RUN apk add --no-cache bash python3 make g++ \ | |
| && ln -sf python3 /usr/bin/python | |
| FROM node:14 | ||
| FROM node:20-alpine | ||
|
|
||
| RUN apk add bash |
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.
Build does not work for me without this:
| RUN apk add bash | |
| RUN apk add --no-cache bash python3 make g++ \ | |
| && ln -sf python3 /usr/bin/python |
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.
Update inline documentation due to the fact that we are now supporting v50
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.
mostly between lines 14 and 24
| def get_syncing(self, cosmos_rest_url: str) -> Dict: | ||
| """ | ||
| This function retrieves data from the cosmos_rest_url/syncing endpoint, | ||
| and is compatible with both v0.39.2 and v0.42.6 of the Cosmos SDK | ||
| and is *NOT* compatible with both v0.39.2 and v0.42.6, | ||
| but *ONLY* compatible with v0.50.1 of the Cosmos SDK | ||
| :param cosmos_rest_url: The Cosmos REST url of the data source | ||
| :return: Retrieves data from the cosmos_rest_url/syncing endpoint | ||
| """ | ||
| endpoint = cosmos_rest_url + '/syncing' | ||
| endpoint = cosmos_rest_url + '/cosmos/base/tendermint/v1beta1/syncing' | ||
| return get_cosmos_json(endpoint=endpoint, logger=self.logger, | ||
| verify=self.verify, timeout=self.timeout) |
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 always tried to keep PANIC backwards compatible. I suggest that we follow a similar implementation to get_staking_validators where multiple versions are supported.
| """ | ||
| This function parses the proposal retrieved from the source node and | ||
| returns the corresponding value to be used by the PANIC components. | ||
| Note that this function is compatible with both v0.39.2, v0.42.6 and v0.50.1 of |
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.
| Note that this function is compatible with both v0.39.2, v0.42.6 and v0.50.1 of | |
| Note that this function is compatible with v0.39.2, v0.42.6 and v0.50.1 of |
| @@ -1,4 +1,4 @@ | |||
| version: '3.7' | |||
| version: '3.8' | |||
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.
can be removed as this is deprecated in newer versions of docker.
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.
can do the same in tests compose file.
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.
Pull Request Overview
This PR updates the application to support Cosmos SDK v0.50.1. The changes encompass updates to Mongoose model builders, revised REST endpoints in the Cosmos API wrappers and monitors, and relevant test adjustments as well as alerts message reformats to align with the new version.
Reviewed Changes
Copilot reviewed 53 out of 57 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| api/src/v1/builder/* | Updated imports and type usage (switching to double quotes and proper generic types) for consistency. |
| alerter/src/api_wrappers/cosmos.py | Modified REST endpoint paths and added new methods for v0.50.1 support. |
| alerter/src/monitors/* | Extended Cosmos monitor functionality with v0.50.1 retrieval functions and updated logic in supported retrievals. |
| Test files under alerter/test/* | Updated SDK version constants, metric key names, and test cases to reflect v0.50.1 requirements. |
| alerter/src/alerter/alerts/network/cosmos.py | Adjusted alert message formatting to use updated key names from the new SDK version. |
Files not reviewed (4)
- .env: Language not supported
- api/Dockerfile: Language not supported
- api/package.json: Language not supported
- api/run_server.sh: Language not supported
Comments suppressed due to low confidence (3)
alerter/src/api_wrappers/cosmos.py:88
- The updated docstring specifies that get_syncing is only compatible with Cosmos SDK v0.50.1 and uses the endpoint '/cosmos/base/tendermint/v1beta1/syncing'. Please verify this endpoint against the latest Cosmos SDK documentation to ensure accuracy.
def get_syncing(self, cosmos_rest_url: str) -> Dict:
alerter/src/monitors/cosmos.py:52
- [nitpick] Changing the default REST retrieval version to v0.50.1 is a significant update; please ensure that all related data retrieval logic and compatibility checks are comprehensively tested with this new version.
self._last_rest_retrieval_version = _REST_VERSION_COSMOS_SDK_0_50_1
alerter/src/alerter/alerts/network/cosmos.py:45
- [nitpick] The alert message now references updated key names (e.g. 'yes_count' instead of 'yes'). Confirm that these changes are consistently applied in all components and that they accurately reflect the new Cosmos SDK response format.
final_tally_result['yes_count'], final_tally_result['abstain_count'],
|
Also please make sure that all tests pass: Running the PANIC test suiteTo run the tests for the alerter component within PANIC, do the following: docker-compose kill # To stop any running containers (to avoid conflicts)
docker-compose -p panic-tests -f docker-compose-tests.yml up --build -d # To build the tests container
docker-compose -p panic-tests -f docker-compose-tests.yml logs test-suite # To see the result of the tests
docker-compose -p panic-tests -f docker-compose-tests.yml kill # To remove test environmentTo run the tests for the API component within PANIC, navigate to the npm install # Install API project dependencies
npm test # Run API unit testsTo run the tests for the Substrate API component within PANIC, navigate to the npm install # Install API project dependencies
npm test # Run API unit testsTo run the tests for the UI component within PANIC, navigate to the npm install # Install UI project dependencies
npm test # Run UI E2E and unit tests |
added tendermint namespace
Description:
This PR updates the Panic Monitoring Dashboard to support the latest Cosmos SDK, ensuring compatibility with modern validator setups across the Cosmos ecosystem.
Changes:
Motivation:
This update is part of a collaborative effort between Node.Monster’s Quick Grant Program and Secret Network to enhance monitoring capabilities for Cosmos validators. By upgrading Panic, we contribute to a more robust and decentralized validator ecosystem.
Developer:
👨💻 VogonAnthology (https://github.com/VogonAnthology)