Persist API-configured voting settings

[mDuo13]

Summary

The server should remember voting settings that were configured through the API.

Motivation

The feature method lets a server admin configure it to vote in favor or against certain amendments, but those settings don't persist after a server restart.

If you want to configure voting settings that stick, you have to use the config file, which is more error prone and requires a server restart. It also limits the ability to build a nicer interface on top of the

It would be a more useful feature if the server saved any voting settings configured via the API and remembered them on future restarts.

Among other things, this would make a theoretical web interface for managing the server more useful.

Solution

The server should persist the voting settings, probably in wallet.db or someplace along those lines. (wallet.db contains a bunch of random persistent server settings, like advisory deletion and cached peer addresses, I think. It's a poorly named config file, but eh.)

It could get confusing if someone uses a mix of config file edits and the API to configure their voting settings. For example, imagine an operator vetoes an amendment in the config, the uses the API to vote in favor of that amendment, then edits their config to change some other setting (not touching the amendments stanza) and restarts the server. Should the server prefer the API setting (where the admin actually expressed their preference regarding that specific amendment), or the config setting (the file is technically newer, but if the admin didn't touch that section of the config, it could represent an older preference)?

I propose resolving votes using a strict hierarchy, as follows:

1. Explicit voting settings from the config file always take precedence. This includes votes in favor ([amendments] stanza) and votes against ([veto_amendments] stanza).

2. Voting settings configured via the API apply only for amendments that are not configured in the config file.

   I would change the current behavior, which allows the API to (temporarily) override voting settings that are explicitly defined in the config file. Instead, if you try to configure voting for an amendment that has an explicit setting in the config file, the API should reject the request with an rpc error.

3. Default voting settings (in favor of known amendments, against unknown amendments) apply only for amendments where the admin has never explicitly configured a voting preference.

After an amendment becomes enabled, any persisted vote settings for that amendment should be erased from wallet.db (or wherever) to prevent them from building up—or from applying if you "hop networks" from a devnet to testnet/mainnet.

Paths Not Taken

There are a couple alternative ways one could resolve conflicting vote settings between amendments API.

• My recommended approach (described above) is to reject API settings that conflict with config file settings.

• A "lazier" way would be to allow the API to override voting settings from the config file temporarily, but to prefer explicit config file options over persisted API settings when loading from a restart. So it would be as if settings were only persisted for amendments that the config file doesn't have votes for/against. This would be a strict extension of the current functionality, but harder to explain IMO.

• A third option would be to prefer persisted API settings over what's in the config file. This would have the confusing consequence of ignoring amendment vote settings from the config file if an admin has ever used the API to configure a vote for that amendment. Optionally you could extend the API to "clear" the API-defined voting settings for a given amendment (or all amendments) so that config file settings could take precedence again. I don't know why you'd take this path.

Stretch Goals

Provide an API for fee voting that also persists settings.

[MarkusTeufelberger]

Alternatively: Remove the "set" functionality form this API. It doesn't take long for a server to restart to read in a new config and outside of testing I don't see much use for it.

[movitto]

What is the reason why amendment voting is supported via the API? Supporting solutions that yield more continuous uptime is desirable but this does somewhat increase the complexity and result in potential discrepancies between wallet.db and rippled.cfg

An alternate solution would be to detect these discrepancies on startup and fail to start unless they are resolved. In this case the administrator would not be presented with any surprises as to which settings are in effect. In this case it would be trivial to modify the rippled.cfg file to make the change there but modifying wallet.db manually would be harder and should be discouraged (not ideal if the 'correct' config setting is in rippled.cfg and wallet.db has the incorrect one)

At the very least any discrepancies should be logged, but that is easy to miss given the extended server output on startup.

[mDuo13]

As noted above, the advantage of being able to vote via the API is that one could build a more friendly user interface on top of it. A UI for editing the config file, by contrast, would more likely be fragile and error-prone.

And yes, also, the short time it takes to restart and sync after updating the config file is downtime for a validator, which we'd like to minimize. I think it's generally just a couple minutes to be fully synced again, but even so the network is more robust if validators don't have to do that.

Personally, I think one should choose either to vote via API or via config file and be consistent from there. But rather than try and impose that rule with an iron fist, I'd rather just insist that people don't set mutually-exclusive settings in the two.

And yes, as @movitto noted, if there is a discrepancy, the config file is easy to change and wallet.db is hard to change, so it makes sense to me that the config file would take precedence.

[MarkusTeufelberger]

There are also friendly user interfaces for editing text files or predefined portions thereof. Validators should be made redundant (so one can go down while the other is still validating and vice versa) if minimal downtime is a requirement. I wrote a proposal about this a while back (#2601) that would allow for this, if network robustness is at stake by someone taking their server offline for a few minutes a year (their ISP/datacenter likely guarantees far less uptime than that by the way).

Having 2 ways to do one thing (that can collide and need to be reconciled) also adds complexity, (maintenance) cost and risk. All that for avoiding a few minutes of potential(!) downtime and a potentially easier way to administrate a server through a web browser (which is also possible by just editing a ConfigMap or - if you want to get fancy - a CRD in a Kubernetes dashboard in a browser, just sayin'...)?

As this has already moved to "To Do" however, I guess this comment is in vain anyways. :-(

[mDuo13]

If the rippled.cfg file used a well-defined format that there were nice editors for, that would be a point in your favor, @MarkusTeufelberger. But, sadly, it doesn't, so editing it with a GUI is probably full of pitfalls. (And much as several of us would like to migrate it to something better-defined, like TOML, I think that's a big effort that's not in the works just yet.)

I disagree with your other point, though. Voting through the API is a relatively easy way to avoid downtime. Even if there are other sources of downtime that are unavoidable, it's still good to avoid more. Given how many other settings are persisted in a similar way in wallet.db already, adding one more for this seems like a worthwhile tradeoff.

[nbougalis]

My preference would be to deprecate the configuration section entirely and migrate to only using the command-line API. Specifically what I propose is:

1. On startup, parse the configuration file.
2. If the [veto_amendments] section is present, check if the AMENDMENTS table is present in wallet.db.
3. If it is not, create the AMENDMENTS table and transfer the settings from the config file.
4. Proceed normally but only reference the AMENDMENTS table instead of the config file.

We can add a warning that is displayed if the config contains any amendments and the AMENDMENTS table exists.

The rationale here is to:

1. Make it easier for server operators to adjust their voting without having to do any more work than necessary. Currently you can dynamically switch your vote using the API, but the option isn't persisted.
2. Reduce the probability that a validator will "accidentally" switch its vote after a restart.

I believe that this proposal reduces confusion but it does have one downside: it's not as easy to transfer the "voting state" to a different server; with the existing scheme, you just copy-paste a blob of text.

[ximinez]

I believe that this proposal reduces confusion but it does have one downside: it's not as easy to transfer the "voting state" to a different server; with the existing scheme, you just copy-paste a blob of text.

How feasible would it be to add an option to the features RPC that outputs the commands that could be used to reproduce the local config?

[Digifin]

That makes sense Nik. Is there any reason to have 2 methods of voting anyway, as you would need to resart using the rippled.cfg anyway, its more straightforward to have just the feature request method. Is there not anyway to make that a more permanent persistant method, in case of an unplanned restart.

[MarkusTeufelberger]

If this area is to be refactored, I'd rather have a "vote for" and a "vote against" section in the validators_file that is the only source for amendment and fee votes - no built in lists of amendments to automatically vote for/against, recommended default fee settings or API calls to change votes at runtime.

Creating/updating such a file (if REALLY necessary, even with values that Ripple would recommend) could be done with the validator key tool.

Currently for example veto_amendments is undocumented in the example file anyways and only mentioned a single time on xrpl.org (at https://xrpl.org/amendments.html#configuring-amendment-voting). Moving this config section to the validator.txt (non-validators don't vote anyways...) might be a logical step and making these choices explicit would remove some hard-coded values and settings from the code base that are really hard to argue about anyways (why is the recommended default reserve for an account exactly 20 XRP, even if the price in USD changes such a lot and even if the amount of accounts created varied a lot as well over the years?).

If you then need a nice UI (for the extremely niche use case of administrating a validator through a UI!) you can write a UI for the validator key tool.

[ximinez]

no built in lists of amendments to automatically vote for/against

The problem with any approach that attempts to remove defaults is that the way amendments currently work, there is no such thing as a vote to abstain. If you're not voting, you're voting no.

It's possible in principal to add such functionality, and that would be fine to start a discussion about, but I think that discussion is beyond the scope of this proposed change, though if you disagree with me, I'm certainly willing to listen.

[MarkusTeufelberger]

Well, the server simply wouldn't start if there's no vote configured for all possible amendments, so there's no way to not vote if you're validating. The default vote recommended by the Ripple team would be visible in the validators.txt file, similar to the current default validator list from vl.ripple.com.

[WietseWind]

Another idea: what if the API would be replaced by an API that reloads certain sections from the config? Allowing one to update the config file and then issue a config section reload (where only certain sections are eligible for reload through API)

[MarkusTeufelberger]

Restarting the program and getting back to sync should be fast enough, but a config-reload API would also be a nice thing to have even for other reasons (cluster settings for example).

[scottschurr]

FWIW, I also think that an API that does a partial reload of the config makes a lot of sense for this problem. It may be applicable to other problems as well.