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

Change the contributor suggested (default) voting for three amendments #4282

Closed

Conversation

RichardAH
Copy link
Collaborator

Change the contributor suggested (default) voting to yes for three amendments:

  • NonFungibleTokensV1_1 (XLS20)
  • ExpandedSignerList
  • CheckCashMakesTrustLine

Following discussions with other contributors about how default voting should work, I am submitting my suggestion, as a contributor, that validators who have opted not to cast a vote should now vote yes by default on these amendments.

…s: NonFungibleTokensV1_1 (XLS20), ExpandedSignerList, and CheckCashMakesTrustLine from no to yes.
@MarkusTeufelberger
Copy link
Collaborator

Why only those 3 and not the other 5?

@RichardAH
Copy link
Collaborator Author

Why only those 3 and not the other 5?

I assume you're referring to these?

REGISTER_FEATURE(OwnerPaysFee,                  Supported::no,  DefaultVote::no);
REGISTER_FEATURE(CryptoConditionsSuite,         Supported::yes, DefaultVote::no);
REGISTER_FEATURE(NonFungibleTokensV1,           Supported::yes, DefaultVote::no);
REGISTER_FIX    (fixNFTokenDirV1,               Supported::yes, DefaultVote::no);
REGISTER_FIX    (fixNFTokenNegOffer,            Supported::yes, DefaultVote::no);
  • OwnerPaysFee is not supported so can't be voted on anyway.
  • CryptoConditionsSuite is not for production / deprecated
  • NonFungibleTokensV1, fixNFTTokenDirV1 and fixNFTokenNegOffer are deprecated in favour of a roll up amendment NonFungibleTokensV1_1.

@wojake
Copy link
Collaborator

wojake commented Aug 24, 2022

To back this PR, most validators are supporting 3 of these amendments with proper consideration/governance in mind that correlates with network health & stability. There have been issues with amendments passing as most validators on the dUNL are inactive and are not participating in amendment voting, which leads to amendments not being able to pass the 80% network approval threshold. +1 from me.

@MarkusTeufelberger
Copy link
Collaborator

MarkusTeufelberger commented Aug 24, 2022

most validators are supporting 3 of these amendments with proper consideration/governance in mind that correlates with network health & stability.

https://xrpscan.com/amendment/32A122F1352A4C7B3A6D790362CC34749C5E57FCE896377BFDC6CCD14F6CD627 only shows the votes of a fraction of validating nodes (the ones on the "dUNL"), not globally.

It is probably possible to also get a global view on amendment support (but then one needs to consider that it isn't clear which of those votes are actually unique entities, so this can only be taken as an indication, not as ground truth), which likely looks different.

According to https://xrpscan.com/validators for example there are more validators voting for 20/5 fees than 10/2. If those just follow the default votes, also most validators globally would oppose these changes here at the moment.

All 3 amendments are optional new features, they are not related to increasing stability but increasing complexity and risk. Still of course it might be nice to have new things, this is why the voting process exists after all. I'm all for setting bugfix amendments to default yes to push them through as soon as enough people have upgraded, but pure feature amendments should be voted in, not out.

most validators on the dUNL are inactive

The dUNL can be changed freely. Why are there inactive operators on there or at least ones that don't care about the XRPL enough to make a choice regarding the protocol every few months? Also the dUNL still does contain duplicate entries for Ripple to this day, its entries can hardly be called "unique"...

Also the "no" (=default or explicit) votes on these amendments are already in the minority on the dUNL, which would mean most operators there actually did actively vote for these changes. Just not enough to safely go through yet.

Edit:
This is something you suggested to be added to this process as a comment in #4263 just 3 weeks ago:

// Generally, amendments which introduce new features should be set as
// "DefaultVote::no" whereas in rare cases, amendments that fix critical
// bugs should be set as "DefaultVote::yes", if off-chain consensus is
// reached amongst reviewers, validator operators, and other participants.

What made you change your mind so quickly?

@RichardAH
Copy link
Collaborator Author

@MarkusTeufelberger in my analysis there are at least 10 passive (non-voting) validators on the dUNL. These are easily identified because they are the validators who do not vote for 10/2. There's only two possible reasons you would vote this way: either you want to increase the reserve after it has already been decreased, or you didn't change your fee voting to begin with. Voting the reserve higher is highly inadvisable once it has already been lowered, thus I take that to mean 10 validators are passive validators with respect to amendment and fee voting.

Amendments require 80% yes on the dUNL for two weeks to pass. If 10 of the 35 dUNL validators are passive then even if all active validators vote yes we will achieve only 71%, which is not sufficient to pass any amendment.

After discussions with other community members on the XRPLF mattermost it was concluded that passive validators effectively delegate their voting to the contributors... which along with the two ways of actively voting is also their right to do. Currently without the contributor's votes no amendments can pass. So as a contributor with these factors in mind I submit a vote of yes for these amendments.

@cjcobb23
Copy link
Contributor

I think the presence of passive, non-voting validators on the dUNL is a significant issue that warrants larger discussion. This discussion should occur here on GitHub, where everyone can see, as opposed to on the Mattermost, which is invite only. It's definitely not good to have passive, non-voting validators on the dUNL.

@RichardAH
Copy link
Collaborator Author

I think the presence of passive, non-voting validators on the dUNL is a significant issue that warrants larger discussion. This discussion should occur here on GitHub, where everyone can see, as opposed to on the Mattermost, which is invite only. It's definitely not good to have passive, non-voting validators on the dUNL.

I agree the discussion should occur here on github. @ximinez made a very compelling case that passive validators are taking the legitimate action of delegating their votes to the contributors, which is effectively a third way of voting along with yes and no. I changed my mind from what was effectively your position to his after this. Maybe he can restate it briefly here :)

@nbougalis
Copy link
Contributor

I'm opposed to this. We (collectively) decided on a "default no" policy for new features and I think we ought to stick with it.

@rippleitinnz
Copy link

rippleitinnz commented Aug 25, 2022

Valid points. A big concern I have had regarding amendments, that has been mentioned previously by me, and others, is that a passive vote, being cast by default, is not a vote and should not be reflected as such. It fails to give an accurate view of the actual position of a validator as there is no current way of determining whether a vote to reject was a default or whether it was configured by the validator.

Given the current default status, some validators may feel there is no need to interact if their intent was to reject, for whatever reason, once again, not giving a clear indication.

The remedy, a neutral position on updating with a requirement to actually cast a vote. There has been some talk of making it a requirement to vote in order to activate validation. A great idea, however network wide it may cause the network to deplete especially with auto-updating.

Community involvement in the voting process could/should be considered. If, say >50% of (total of non UNL) validators in the wider network have voted to reject or enable an amendment then a proportion of voting rights is afforded to them in the allocation to get to the (current) 80% threshold. It could be apportioned by say ~10% of UNL or similar which could make 38 combined UNL and they would have 3 votes.

UNL members need to be diligent, timely and contactable by list providers and vacated from lists if they are not being a good reliable validator.

@wojake
Copy link
Collaborator

wojake commented Aug 25, 2022

What made you change your mind so quickly?

I proposed that policy as a standard, that anyone can refer to if they wish to do so: In this PR and In my mind, I don't see why we should be following it right now when the problem is, there are inactive validator operators that aren't voting-in or voting-out amendments.

@RichardAH
Copy link
Collaborator Author

I'm opposed to this. We (collectively) decided on a "default no" policy for new features and I think we ought to stick with it.

Then no amendments can pass until the dUNL changes. Since that requires fairly exact coordination between the three VL publishers who knows when that will be.

@MarkusTeufelberger
Copy link
Collaborator

Community involvement in the voting process could/should be considered. If, say >50% of (total of non UNL) validators in the wider network have voted to reject or enable an amendment then a proportion of voting rights is afforded to them in the allocation to get to the (current) 80% threshold. It could be apportioned by say ~10% of UNL or similar which could make 38 combined UNL and they would have 3 votes.

This just means that sybil attacks can and will happen. Of course dUNL owners should take a look at the community at large ideally and react to that, but in general there should be no way for someone outside my UNL to influence my vote or the aggregated outcome. I don't even see a feasible way to implement your suggestion in practice by the way and am wary of magic numbers (why 3 votes and not 2 or 4? Why 10%?).

Then no amendments can pass until the dUNL changes.

No amendment that has a "no" default vote, not no amendment at all. Also publishing a strict subset of a current list is safe and requires no coordination as long as over time the same subset is published. Lastly it might be worth a discussion to think about how entities end up on these lists that don't demonstrate the capability of configuring amendment and/or fee votes and do something about that instead of "tricking" these passive entities to vote for/against certain amendments or fee settings.

This seems like a way to address shortcomings from dUNL providers in their policies and transparency about member selection instead of going further (e.g. requiring explicit votes when validating) or removing the "bugfix yes, feature no" policy and thus risking pushing through feature amendments that weren't really well understood or tested (or even desired). Who gets to decide if my hypotherical feature amendment of "freeze all XRP that Ripple owns" should be a default yes vote or even included in the code after all? If it were default no and the code was sensible, it might get added for a while to have this as an option, if the policy gets weakened then it is up to a handful of people with commit rights here to decide which amendments have a chance of being voted upon - not the actual validators.

In conclusion:
It is not the job of people contributing to this software to decide on which feature amendments have a chance to pass. It is already bad enough that the dUNL publishers apparently have a large amount of power in this regard (because they add(ed) a too large portion of apparently passive participants to their list). Ideally this decision should only be in the hands and votes of responsible validator operators.
If the current way of how voting is structured, communicated and configured does not support this model, this should be changed instead of keeping the current model of "just follow the defaults - that WE, the code owners set for your own good".

@RichardAH
Copy link
Collaborator Author

RichardAH commented Aug 25, 2022

It is not the job of people contributing to this software to decide on which feature amendments have a chance to pass.

This was previously also my position, however in practical terms whomsoever is in charge of publishing the binary packages that everyone runs ultimately has 100% control over what code, including which amendments, run on the XRPL. In fact there is nothing to prevent those binaries being compiled from code other than this repository.
There's plenty of problems with the current distribution of trust and power. Let's either fix it completely (which requires accepting some hard truths I won't enumerate here yet) or continue pretending it doesn't exist in which case default::yes so we can actually get some work done.

@ximinez
Copy link
Collaborator

ximinez commented Aug 26, 2022

Invoked by @RichardAH , I'll restate / summarize my opinion about default votes:

  • One of the main reasons that DefaultVote was added was to allow an amendment to be supported without being voted yes by default, which allows a more gradual update path for operators.
    • For example, an amendment introduced in 1.7 would have been voted on as soon as a node was upgraded unless the operator took the time / action to vote against it. That could easily lead to nodes still running 1.6 to get amendment blocked if enough UNL validators upgraded without changing their vote, which would be particularly bad if they were UNL nodes that hadn't upgraded, but could also cause problems for exchanges and such. This sometimes discouraged UNL operators from upgrading.
    • Now, support for amendment X can be added in, say, 1.9 with a default vote of no. Nodes running 1.8 should still upgrade, but they aren't forced to rush to do it within 2 weeks for fear of getting amendment blocked. There could be additional releases after that - 1.9.1, 1.10.0, etc. Once a sufficient portion of the network has upgraded to a version that supports X, the next version of rippled (which may be much later, e.g. 1.13) can safely change the default vote for that amendment to yes, which should motivate the stragglers to update, seeing as they've presumably now had plenty of time to test.
  • Default votes are merely recommendations from the contributors. Operators can always choose to vote differently. If they decide to accept the default, that is also a choice (a less active choice, but still a choice). That allows validators who can keep a reliable connection, on reliable hardware, and reliably upgrading to stay functional without necessarily being "active" about voting. To the best of my knowledge, actively voting on amendments has not been considered a requirement to be on a UNL.
  • The contributors have only changed the default vote from no to yes once so far, because the default vote functionality is still pretty new. Still, 8266d9d changed the default for NegativeUNL from no to yes.

@MarkusTeufelberger
Copy link
Collaborator

Still, 8266d9d changed the default for NegativeUNL from no to yes.

The reason given in #4215 was:

Well, I figured it was particularly safe since the NegativeUNL amendment is already live on the main net. So this specific change should be very safe.

I am not happy about this drive-by change of unrelated default votes, nUNL does to my knowledge have nothing to do with the rest of the contents or the topic of #4215. Then again the nUNL amendment was enabled in late 2021 (https://xrpscan.com/tx/1500FADB73E7148191216C53040990E829C7110788B26E7F3246CB3660769EBA) already and the PR is from June of 2022, so apparently "default vote: no" feature amendments could pass less than a year ago... what changed?

The documentation states it is enabled by default vote "no" by the way: https://xrpl.org/known-amendments.html#negativeunl

@RichardAH
Copy link
Collaborator Author

I think the developer community is at an impasse. If more than 20% of dUNL validators are inattentive, inactive or passive in their voting then nothing will pass. Over time the chain will become dated technology that people do not want to work with.

@cjcobb23
Copy link
Contributor

cjcobb23 commented Aug 29, 2022

Why don't we just remove the passive validators from the dUNL?

@RichardAH
Copy link
Collaborator Author

RichardAH commented Aug 29, 2022

Why don't we just remove the passive validators from the dUNL?

Rippled unions the validator lists you supply. So if you remove from just one of the VLs listed in validators.txt then it will still potentially be in the other. There are three VL publishers that dUNL validators (probably) listen to: ripple, coil and xrplf. If they don't coordinate the removal then you could produce a fork condition, because you do not know which dUNL listen to which combination of VLs.

@cjcobb23
Copy link
Contributor

Let's coordinate it then. I think if a validator operator is not actually even paying attention to the community and development, then they probably should not be a validator on the dUNL.

@RichardAH
Copy link
Collaborator Author

Let's coordinate it then. I think if a validator operator is not actually even paying attention to the community and development, then they probably should not be a validator on the dUNL.

100%. But tell them that 🤣 I can sway XRPLF I'm an advisor. Can you sway ripple? Then we just need to find someone who can get coil in.

@MarkusTeufelberger
Copy link
Collaborator

There are only 2 publishers in the default file (XRPLF and Ripple Labs Inc.), not 3. You're on the team of one of those 2 entities... The guidelines there state the following: https://foundation.xrpl.org/unl/

Active participation in the amendment voting process

As long as the one published by the XRPLF is a subset of the one published by Ripple Labs, there is no need to delay or coordinate anything.

@RichardAH
Copy link
Collaborator Author

RichardAH commented Aug 29, 2022

There are only 2 publishers in the default file (XRPLF and Ripple Labs Inc.), not 3. You're on the team of one of those 2 entities... The guidelines there state the following: https://foundation.xrpl.org/unl/

Active participation in the amendment voting process

As long as the one published by the XRPLF is a subset of the one published by Ripple Labs, there is no need to delay or coordinate anything.

There's no guarantee the dUNL validators are listening to the default VLs in the default configuration. Notably the configuration wasn't always this way, and if they have been on the dUNL for some time they might listen to some other list. It is prudent to coordinate between the three well known VL publishers.

I am tagging @alloynetworks to advise removal of any validators not voting 10/2 reserve from the XRPLF's VL now.

@MarkusTeufelberger
Copy link
Collaborator

I don't think coil was ever part of this file: https://github.com/XRPLF/rippled/commits/develop/cfg/validators-example.txt

@RichardAH
Copy link
Collaborator Author

I don't think coil was ever part of this file: https://github.com/XRPLF/rippled/commits/develop/cfg/validators-example.txt

a4a46a4

@RichardAH
Copy link
Collaborator Author

If it forks it forks. I am sick of the impasse. Everyone change their VLs please.

@cjcobb23
Copy link
Contributor

cjcobb23 commented Aug 29, 2022

Let's coordinate it then. I think if a validator operator is not actually even paying attention to the community and development, then they probably should not be a validator on the dUNL.

100%. But tell them that 🤣 I can sway XRPLF I'm an advisor. Can you sway ripple? Then we just need to find someone who can get coil in.

Yeah I'll do my best to sway

@MarkusTeufelberger
Copy link
Collaborator

I don't think coil was ever part of this file: https://github.com/XRPLF/rippled/commits/develop/cfg/validators-example.txt

a4a46a4

This is commented out, just like http://127.0.0.1:8000...

@RichardAH
Copy link
Collaborator Author

I don't think coil was ever part of this file: https://github.com/XRPLF/rippled/commits/develop/cfg/validators-example.txt

a4a46a4

This is commented out, just like http://127.0.0.1:8000...

It's an official example for which a key is also provided. You have no way of knowing who is using this VL in their validators.txt

@ximinez
Copy link
Collaborator

ximinez commented Aug 31, 2022

Well, I figured it was particularly safe since the NegativeUNL amendment is already live on the main net. So this specific change should be very safe.

I only looked at the code, not the PRs, so it's a fair point that changing the default vote didn't really do anything on mainnet.

@ximinez
Copy link
Collaborator

ximinez commented Aug 31, 2022

If it forks it forks. I am sick of the impasse. Everyone change their VLs please.

IMHO, a fork would be much more detrimental to the ecosystem than slow uptake on amendments.

@ximinez
Copy link
Collaborator

ximinez commented Aug 31, 2022

One more thing I'd like to point out is that in PR #3877, which implemented the option to not vote for supported amendments by default, I wrote:

This will allow completed features to be released and supported for one or more release cycles before they are voted on by default. Operators will still have full control over voting for or against amendments on their own nodes.

I know that developer notes are not canonical or binding in any way, and that best practices can be changed in ways that aren't directly reflected in code, but at least at that time, my intention was that default votes would be changed from "no" to "yes" at some point in an amendment's life cycle.

@wojake
Copy link
Collaborator

wojake commented Aug 31, 2022

IMHO, a fork would be much more detrimental to the ecosystem than slow uptake on amendments.

Of course it is, we shouldn't risk a fork from happening.

Please, let's just cooperate and change your VLs: remove passive operators that have been put for political motives, that do not care about the network's advancement. The very least that all of us could do is to remove all passive validators that:

  • are not voting for 10/2 fee reserve
  • operators have not participated in any recent off-chain discussions

from the dUNL, before this PR was issued. This is the bare minimum, scrape off the obvious bad apples from the dUNL.

@RichardAH
Copy link
Collaborator Author

Closing for lack of support

@RichardAH RichardAH closed this Sep 5, 2022
@intelliot
Copy link
Collaborator

To those who participated in this discussion: please review #4291

Thanks! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants