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

feature command confusingly reports vetoed: true for retired amendments (Version: 1.8.1) #4014

Closed
mDuo13 opened this issue Dec 3, 2021 · 1 comment

Comments

@mDuo13
Copy link
Collaborator

mDuo13 commented Dec 3, 2021

Issue Description

The feature command inaccurately reports a status of "vetoed": true for amendments that have had their pre-amendment code retired.

Steps to Reproduce

Call the feature method with no arguments.

Expected Result

The response should show:

  • How the server actually voted on the amendment 2+ years ago, or;
  • "vetoed": false since that's more likely to be the case for amendments that achieved majority support; or,
  • Omit the "vetoed" field since it's not applicable to retired amendments. (This would be a BREAKING CHANGE since that field is currently guaranteed to be present)

Actual Result

In addition to explicit vetoes, all retired amendments show "vetoed": true which makes it seem like "the validators are revolting against amendments past" (to quote @alloynetworks ).

Environment

rippled 1.8.1 confirmed by several UNL validators

Supporting Files

Partial snippet showing the misleading API result:

        "07D43DCE529B15A10827E5E04943B496762F9A88E3268269D69C44BE49E21104" : {                                                                                                                                    
            "enabled" : true,                                                                                                                                                                                      
            "name" : "Escrow",                                                                                                                                                                                     
            "supported" : true,                                                                                                                                                                                    
            "vetoed" : true                                                                                                                                                                                        
         },                                                                                                                                                                                                        
         "08DE7D96082187F6E6578530258C77FAABABE4C20474BDB82F04B021F1A68647" : {                                                                                                                                    
            "enabled" : true,                                                                                                                                                                                      
            "name" : "PayChan",                                                                                                                                                                                    
            "supported" : true,                                                                                                                                                                                    
            "vetoed" : true                                                                                                                                                                                        
         },                                                                                                                                                                                                        
         "1562511F573A19AE9BD103B5D6B9E01B3B46805AEC5D3C4805C902B514399146" : {                                                                                                                                    
            "enabled" : true,                                                                                                                                                                                      
            "name" : "CryptoConditions",                                                                                                                                                                           
            "supported" : true,                                                                                                                                                                                    
            "vetoed" : true                                                                                                                                                                                        
         },                                                                                                                                                                                                        
         "157D2D480E006395B76F948E3E07A45A05FE10230D88A7993C71F97AE4B1F2D1" : {                                                                                                                                    
            "enabled" : true,                                                                                                                                                                                      
            "name" : "Checks",                                                                                                                                                                                     
            "supported" : true,                                                                                                                                                                                    
            "vetoed" : false                                                                                                                                                                                       
         },                                                                                                                                                                                                        
         "1D3463A5891F9E589C5AE839FFAC4A917CE96197098A1EF22304E1BC5B98A454" : {                                                                                                                                    
            "enabled" : true,                                                                                                                                                                                      
            "name" : "fix1528",                                                                                                                                                                                    
            "supported" : true,                                                                                                                                                                                    
            "vetoed" : true                                                                                                                                                                                        
         },                     
@ximinez
Copy link
Collaborator

ximinez commented Dec 3, 2021

Possibly trivial potential solution: change retireFeature in Feature.cpp to use DefaultVote::yes instead of DefaultVote::no.

I implemented with DefaultVote::no, because I didn't want hypothetical future networks (e.g. sidechains) to unnecessarily vote for amendments that don't have any code associated with them. Once an amendment is retired, adding it to a new ledger is a waste of space.

Edit: The more I think about it, the less I like this solution. I think the most correct solution is to not show the vetoed field if the feature is enabled. For v1, show vetoed = !enabled && vetoed. Since that would be a breaking change, maybe it could go into RPC v2. And we could rename it while we're at it. And maybe even add a DefaultVote field if it's not enabled.

@XRPLF XRPLF deleted a comment Jul 20, 2022
ximinez added a commit to ximinez/rippled that referenced this issue Aug 31, 2022
* Prevents the server from ever voting on the given amendment,
  regardless of operator configuration.
* Also prevents the "feature" command from changing the amendment's
  vote.
* Incidentally, resolves XRPLF#4014.
ximinez added a commit to ximinez/rippled that referenced this issue Aug 31, 2022
* Prevents the server from ever voting on the given amendment,
  regardless of operator configuration.
* Also prevents the "feature" command from changing the amendment's
  vote.
* Incidentally, resolves XRPLF#4014.
ximinez added a commit to ximinez/rippled that referenced this issue Oct 17, 2022
* Prevents the server from ever voting on the given amendment,
  regardless of operator configuration.
* Also prevents the "feature" command from changing the amendment's
  vote.
* Incidentally, resolves XRPLF#4014.
ximinez added a commit to ximinez/rippled that referenced this issue Nov 30, 2022
* Prevents the server from ever voting on the given amendment,
  regardless of operator configuration.
* Also prevents the "feature" command from changing the amendment's
  vote.
* Incidentally, resolves XRPLF#4014.
ximinez added a commit to ximinez/rippled that referenced this issue Dec 14, 2022
* Prevents the server from ever voting on the given amendment,
  regardless of operator configuration.
* Also prevents the "feature" command from changing the amendment's
  vote.
* Incidentally, resolves XRPLF#4014.
ximinez added a commit to ximinez/rippled that referenced this issue Dec 16, 2022
* Prevents the server from ever voting on the given amendment,
  regardless of operator configuration.
* Also prevents the "feature" command from changing the amendment's
  vote.
* Incidentally, resolves XRPLF#4014.
ximinez added a commit to ximinez/rippled that referenced this issue Jan 4, 2023
* Prevents the server from ever voting on the given amendment,
  regardless of operator configuration.
* Also prevents the "feature" command from changing the amendment's
  vote.
* Incidentally, resolves XRPLF#4014.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants
@ximinez @mDuo13 and others