-
Notifications
You must be signed in to change notification settings - Fork 123
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 unfreeze feature #3072
Remove unfreeze feature #3072
Conversation
This reverts commit e1972d8.
Let's leave this in to as the template PR to be merged in later, for the next upgrade or once 4,420,000 is reached based on the results. |
Things that affect our core functionalities in particular must not be left in the release without consideration. Especially when there is currently not even a majority for it and the capacities for testing should not be wasted on functions that are not desired by the master nodes and the community. If someone wants to, they can add the functions themselves, but this has no place in the release for the general public and should be removed immediately. |
I agree with Bernd. The potential risk of this being in is massive, with no upside whatsoever. IMHO the only solution is to remove it before release. |
i see it the same, just too critical to have it merged as a "backdoor" as long as there is no real need for it |
We already have other things that are not activated / never activated on chain - this is not new. Let's please not turn this political. This is not untested code. We're testing it (and all the more reason for it to be in changi, and testnet). It can just be later removed for the next release if there if it's never activated, community decides if it doesn't want the code. For now, this is transparently brought about as proposal. And then proposal is not something without insignificant votes. So, the decision is for the MNs. The capability is there either way - that's it. We're also moving governance to GitHub, and setgovheight timelock based mechanisms only (with ahead of time, before any set), so there's no backdoor here. All this sudden conversation about this being new risk is unsubstantiated, as we already have many things like these on code, just not activated. Software engineers shouldn't be playing arbiters of this. That defeats the whole point of governance - that role is assigned to network validators and governance. This PR is purely there as something to go in later, so it's easy to merge after and be removed once the proposal is resolved. |
Can you please provide a full list of all people who will have theoretical access to foundation keys after the fork. So who might be able to activate this? The difference to other features is, that this feature can produce massive damage to the whole system within blocks, if activated accidentally. |
We can defer the testing of this on testnet if that's a concern, if / when in a hypothetical case be needed or voted on to make that easier. |
so you want to put untested code onto mainnet? |
what do you mean untested? we are, and if it wasn't deemed sufficiently safe, we would not have even considered merging in. Just providing another option to avoid a potential testnet rollback if you prefer it to be removed later. This is turning unproductively contentious, again (from Discord). We're trying to make sure all options are available, including the rationale for this PR - so it's available for the community to merge at a later time, and remove. |
I hope that wasn't a serious argument because we used to accept it in the past and we have to continue to do so. In the end, it has never been ok to implement things without a completed DFIP. It was accepted - maybe now is the time to change that. Because who tells me that the current team will even create the next release and then it will stay in there. So my view remains that it has to go out immediately so that we can take care of the important things, successfully voted DFIPs and improvements. Because all of this burns resources unnecessarily and it starts with the fact that someone has released the implementation instead of concentrating on the other things. |
totally agree with @bernd-mack |
Sorry, I disagree with this assessment. Anyone is free to implement any DFIP as long as the proposal is in place and it has non insignificant votes, regardless of where it leans. The only predicament to reject PR would be if it's undeniably spam, has an actual security concern, critical bugs (edit: or performance concerns, indexing data requirements that's detriment to the network, etc that's unresolved). Otherwise, software engineers are playing arbiters and choosing to play additional role of soft governance, making actual governance pointless. It's upto governance to decide.
There's no way to know how things will even work if there's no implementation. Flagged implementation is how both experiments and well modelling of a potential feature will have to work. Not by playing trial and error on economy without implementation. Flagged features are how features move up. Here's it's one potential implementation that's flagged. Without it there's no way to test or visualize. You and I can disagree with it and chose not to be a part of enabling it. But not reject the PR when it's cleanly flagged and left upto the community governance to make the decision. Let's focus on testing the more important things than this discussion that's heated unnecessarily. When the proposal ends, this can be removed for the next fork if so desired. If it get's further contentious, I'd have no choice but to close the PR to avoid this wasting more time on heated unproductive conversations. |
Seems u answered it yourself, it should not go in there cause of security concerns great |
I realize this in unpopular. But it's upto the governance to decide. Please continue to advocate on governance voting. If there's an actual bug or security concern in the implementation, please feel free to point out. That would be a valid reason to consider revert if a fix is not viable. |
The apparent security concern here is completely unsubstantiated. We have many feature flags in the code (that btw has potentially has lot more intrusive abilities than just unfreezing nodes). But it's all based on governance and we're also clamping down on governance execution and slowly deprecating the old governance model by moving everything to timelocked setgovheight only -- which adds safety on anything that's ever activated or deactivated related to these flags. Not only will they have to go through the governance repo, but also sit through transparent timelock, which can be cleared if anything that hasn't seen due process is detected. (these are all the changes that's in the current governance changes). So, this security concern is entirely unsubstantiated, because of disagreement. Please take up disagreements through governance and not here. If there's not an actual security concern, will ask not to waste everyone's time here. Thanks. |
This comment was marked as abuse.
This comment was marked as abuse.
[Hiding last comment as it's unproductive and inciting a revolt. Stated many times that disagreements be dealt with governance and voting. There is an established mechanism to resolve disputes. Inciting revolts here isn't the way.] Conversation locked. Please advocate through governance and votes. |
Summary
Implications
Storage
Consensus