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.

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

Remove ALERTS_URL and CHIA_ALERTS_PUBKEY #18027

Merged
merged 1 commit into from
Jul 16, 2024
Merged

Remove ALERTS_URL and CHIA_ALERTS_PUBKEY #18027

merged 1 commit into from
Jul 16, 2024

Conversation

Rigidity
Copy link
Contributor

@Rigidity Rigidity commented May 16, 2024

These config items are confusing and also not used anywhere in either chia-blockchain or chia-blockchain-gui, and the only reference within the org are other config files including them.

It seems after looking at the source code of chia-blockchain==1.0.0, we used to make an API call to this endpoint, parse the JSON, and use this as a source of truth for the GENESIS_CHALLENGE.

However, this is now ConsensusConstants.GENESIS_CHALLENGE, and an API call is no longer used anywhere for this.

@Rigidity Rigidity requested a review from a team as a code owner May 16, 2024 00:02
@Rigidity Rigidity added the Changed Required label for PR that categorizes merge commit message as "Changed" for changelog label May 16, 2024
Copy link
Member

@hoffmang9 hoffmang9 left a comment

Choose a reason for hiding this comment

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

These are vestigial and were part of the fair launch process that occurred on March 19, 2021

@Rigidity
Copy link
Contributor Author

Rigidity commented May 16, 2024

These are vestigial and were part of the fair launch process that occurred on March 19, 2021

I think it would be cool to preserve this message standalone in the CLI and/or GUI:

Operational error causes Fed payment system to crash
We stand on the shoulders of giants, now let's get farming!

But if these config items aren't used in the code (nor do we plan to, since it shouldn't need to phone home for the genesis challenge anymore), imo they should be removed.

Not going to die on this hill though, so if it's worth keeping around to anyone, sure

Copy link
Contributor

This PR has been flagged as stale due to no activity for over 60 days. It will not be automatically closed, but it has been given a stale-pr label and should be manually reviewed by the relevant parties.

@github-actions github-actions bot added the stale-pr Flagged as stale and in need of manual review label Jun 30, 2024
@emlowe
Copy link
Contributor

emlowe commented Jul 16, 2024

close and reopen for CI run

@emlowe emlowe closed this Jul 16, 2024
@emlowe emlowe reopened this Jul 16, 2024
@emlowe emlowe removed the stale-pr Flagged as stale and in need of manual review label Jul 16, 2024
@Starttoaster Starttoaster merged commit eed8f21 into main Jul 16, 2024
722 of 724 checks passed
@Starttoaster Starttoaster deleted the remove-alerts branch July 16, 2024 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changed Required label for PR that categorizes merge commit message as "Changed" for changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants