fedimint / fedimint

Federated E-Cash Mint
https://fedimint.org/
MIT License
536 stars 209 forks source link

Dynamic meta fields #2791

Closed elsirion closed 1 month ago

elsirion commented 10 months ago

Fedimint has the ability to communicate additional config, a.k.a. meta fields to clients. Meta fields are not related to consensus inside the federation but clients validate that they get the same value from all guardian servers, so these fields follow the same trust model as the rest of Fedimint.

These meta fields being part of the config and agreed upon poses a problem though: editing them means changing consensus config on the server side and regenerating the client config signature. This process is very brittle.

There should be a way via the admin API to propose changes to meta fields and vote on these. This could be the precursor to more general config updates, but is much simpler since the server does not care about the value of meta fields at all, so no live consensus changes are necessary.

wbobeirne commented 8 months ago

Meta fields are not related to consensus inside the federation

Oh that's interesting. Should this be implemented separately from the ability to modify consensus-required fields? Or should we consider what that looks like as well, and make it possible to generate any arbitrary proposal change? Or should we make the meta fields part of consensus?

I've been thinking through how this would work, but without having config changes be part of epochs, I can't think of a good way to ensure that all guardians are returning the same client config at the same time. There would always be the risk of one or more guardians failing to receive communication that they've all approved the change, and to switch to the new config.

elsirion commented 8 months ago

Should this be implemented separately from the ability to modify consensus-required fields?

It's certainly simpler because no migration of running consensus rules is necessary, the field simply changes. It might be seen as a precursor to more general dynamic consensus parameters, but that too far away to worry about it.

The basic idea would be to have guardians propose changes via consensus and if a quorum is reached they return the updated value. If there is temporary inconsistency (at most a few seconds) that's ok as long as we use the correct query strategy that will re-try fetching the config/meta fields till it gets the right answer from a quorum of guardian servers.

To make them dynamic we should likely take them out of the client config for now and make them a new API endpoint since we are not prepared to deal with client config changing in arbitrary ways.

dpc commented 3 months ago

I accidentally created https://github.com/fedimint/fedimint/issues/4087 which is a dup of this one. Might be useful to read through because I described it in a very "how should it work" way.

My main motivation was that implementing this should be relatively easy, while problems due to lack of it keep adding up.

On the call @elsirion said that he would prefer to make it a part a core consensus because it doesn't generate inputs and outputs, but I have some arguments pro-module:

The fact that doesn't produce inputs and outputs is not a problem, IMO. Generally if something can be a module, I think it's better to make it a module.

elsirion commented 3 months ago

easier to rotate into a different thing if we ever want to make significant changes.

It would also avoid us having to do a consensus upgrade right now, which I'd really like to avoid for 0.3, we have enough on our plate already.

The main problem we'd be facing is that there is no way you can add a module to a running federation right now without totally understanding what you do (you'd be manually editing all the configs). Changing this sounds like even a bigger project than attempting a consensus upgrade.

dpc commented 3 months ago

The main problem we'd be facing is that there is no way you can add a module to a running federation right now without totally understanding what you do (you'd be manually editing all the configs). Changing this sounds like even a bigger project than attempting a consensus upgrade.

I don't understand. Old Federations will just ignore the new module (as it's not instantiated), old clients will ignore it too. The existing external metadata can keep being utilized until phased out completely.

In other words - I am not aiming at retrofitting old federations with dynamic meta fields. New Fedierations can start using the new module, existing can keep doing what they were doing. Compatibility-wise everything should mostly work, unless I'm missing something.

dpc commented 3 months ago

Notes from internal discussion we had at Fedi: Mostly the design outlined in https://github.com/fedimint/fedimint/issues/4087 makes sense, thought the consensus seems to be that having all metadata as one key of a "dynamic consensus meta module" where the value is just a string containing a Json seems preferable, as it guarantees that approved consensus switches atomically and "makes sense", while splitting it into multiple key-values can lead to tricky situations. The module could still implement keeping track of arbitrary keys where only one would be currently used for metadata-json: "just in case" but it's unclear if it's "over-engineering" or "future-proofing". If we decide to do mutli key-values, we could e.g. ignore/reject other keys than the one we plan to use, and just make the APIs accept/return "key" to leave a possiblity in the future, without bothering with implementation details like limits on a number of pending proposals (keys).

Personally, I only feel strongly that this is worth prioritizing, the core vs module I have a slight module preference, and "just one key-value" vs "multiple key-values" I'm entirely not sure, but I would leave the "multi-key" open as it seems trivial and doesn't cost much.

joschisan commented 3 months ago

Just wanted to mention here that the client config is not signed anymore and can be modified as long as the guardian api endpoint set remains unchanged.

So a crude solution that does not require any consensus synchronisation is to enable the guardian UI to change meta fields and tell guardians to agree on the changes beforehand and that a threshold of 2f+1 has to do it somewhat simultaneously as the client config cannot be downloaded as if less then 2f+1 endpoints return different values (the call to download it will not fail though it will hang).

Otherwise, if we want to remove the requirement for doing this simultaneously, I would suggest a new consensus item instead of a new module, where peers vote on the new metafields as a BTreeMap<String,String> and we update the previous metafields when a quorum is reached. To bring this to a new federation would require a synchronised shutdown after a given session, so guardian UX is to enter a certain session_index, wait for the system to stop and the check if a. guardians stopped correctly. Then we just switch to the new version and everything should work.

This two options are not exclusive but we could do the first for 0.3 without any synchronised update as a quick fix. This work would not be wasted since its mostly work on the guardian UI which would be necessary for the second option two. We should then be able to remove the requirement for coordination of the guardians via a new consensus item in 0.4.

Later on we can also use the same voting mechanism for consensus critical variables like dynamic feerates for example.

elsirion commented 3 months ago

I am not aiming at retrofitting old federations

That would be rather annoying for existing federations I guess, although they likely are used to the override meta, so not a big deal sort term. But long term we should not disadvantage old federations, that would send a signal to wait with setting one up.

dpc commented 3 months ago

But long term we should not disadvantage old federations, that would send a signal to wait with setting one up.

I don't even know if changing behavior of existing Federations that already use external meta would be a good idea.

BTW. Isn't introducing a new core consensus item technically a consensus breaking change (hard-fork, not a soft-fork)? It's probably possible to weasel-in such a change, under the condition that no peer will actually use it until the whole Federation is up to date, but technically...

And I think I would rather have this feature implemented and working on new Federations than delay the whole thing until everything is figured out how to backport it and enable existing Federations to use it.

joschisan commented 3 months ago

BTW. Isn't introducing a new core consensus item technically a consensus breaking change (hard-fork, not a soft-fork)? It's probably possible to weasel-in such a change, under the condition that no peer will actually use it until the whole Federation is up to date, but technically...

All guardians need to be upgraded in synchrony as described above.

The hard fork soft fork dichotomy does not work for us. Softforks occur when we tighten the consensus rules, however in our case that means that an old guardian would accept a transaction that the new version does not accept - hence the created session outcomes and db states would be different. The old guardian would stop working at the end of the session when it produces a session outcome header that is different to everybody else.

Therefore, we only have hardforks since relaxing and tightening of the consensus rules would cause old guardians to diverge.

elsirion commented 3 months ago

And I think I would rather have this feature implemented and working on new Federations than delay the whole thing until everything is figured out how to backport it and enable existing Federations to use it.

I then let's go the module route and figure out how to add modules after the fact in 0.4. Since we need to keep around the old meta infra for a bit I'd propose to have an order of precedence:

  1. New module if present
  2. Meta override file if present
  3. Meta fields from client config
dpc commented 2 months ago

WIP: https://github.com/fedimint/fedimint/pull/4513

elsirion commented 2 months ago

After https://github.com/fedimint/fedimint/pull/4513 is merged we should still impl a default fetch function with fallbacks to the legacy ways of fetching meta fields.

justinmoon commented 1 month ago

Should we close this now?