search

fedimint / fedimint

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

feat: meta caching #4768

Closed maan2003 closed 1 month ago

maan2003 commented 1 month ago

implement meta caching and updating

left for follow-up:

on top of #4783

Summary by CodeRabbit

coderabbitai[bot] commented 1 month ago

Walkthrough

The update introduces a comprehensive system for managing metadata within the fedimint-client, including the ability to store, update, and manage metadata fields. A new dependency, reqwest, is added to facilitate external metadata fetching. The changes span across multiple files, establishing structures and mechanisms for handling metadata, including a MetaManager for orchestrating metadata sources and updates, and a Waiter module in fedimint-core for asynchronous task management. This enhancement enriches the client's functionality with dynamic metadata management capabilities.

Changes

File(s) Summary
fedimint-client/Cargo.toml Added reqwest dependency with specific features and default features disabled.
fedimint-client/src/db.rs Introduced ClientMetaField enum variant and related functionality for database meta fields.
fedimint-client/src/lib.rs, .../init.rs Added MetaManager to Client and ClientBuilder; updated method implementations.
fedimint-client/src/meta.rs Added functionality for managing metadata sources and values.
fedimint-core/src/task/waiter.rs Introduced a Waiter struct for asynchronous task waiting.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)
Tips ### Chat There are 3 ways to chat with CodeRabbit: - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit .` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai generate interesting stats about this repository and render them as a table.` - `@coderabbitai show all the console.log statements in this repository.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. ### CodeRabbit Commands (invoked as PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger a review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai help` to get help. Additionally, you can add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. ### CodeRabbit Configration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - The JSON schema for the configuration file is available [here](https://coderabbit.ai/integrations/coderabbit-overrides.v2.json). - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json` ### CodeRabbit Discord Community Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback.
maan2003 commented 1 month ago

in 0.5 maybe we will fire the manager

dpc commented 1 month ago

in 0.5 maybe we will fire the manager

In my experience it's very hard to fire a manager. They tend to fire you instead, and then.

The moment you introduce a first manager, you set in motion a fault-tolerant system that is impossible to stop, and you get Manager Directory and VPs really quick.

Just saying...

maan2003 commented 1 month ago

what is worse meta managers or Meta services?

fedimint now has Meta services integrated for telemetry :smile:

maan2003 commented 1 month ago

changes: priority is not for source, but for values now

maan2003 commented 1 month ago

found a bug, sources can never remove values from meta

edit: now fixed

dpc commented 1 month ago

In my mind if the Federation config has a module with meta kind, the client can just ignore other metas. Merging anything should be avoided.

I bet the reason for all complexity here is because people are really set on reusing meta module for this gateway vetting, which doesn't really fit here. LN module should keep track of it's vetted gateway on its own.

It creates dependencies between modules, will have a different lifecycle, etc.

Eg. LN module can just have an admin endpoint for guardians to vet/ban gateways, and return such info on a dedicated LN-module endpoint. Clients can fetch and cache such list when they need it. So if you don't make LN payements, you don't need to check anything every 10 minutes. All sorts of smarter and more agile things can be done then - e.g. count number of guardians vetting things, require different amount of thresholds, etc. While meta module is specifically for slow-moving meta information. All without any inter-module dependencies.

@elsirion @maan2003

maan2003 commented 1 month ago

LN module should keep track of it's vetted gateway on its own.

too late :/ we already cut a 0.3 with vetted gateways in meta

maan2003 commented 1 month ago

wasn't meta module meant as storage for other modules

we even thought of storing registered gateways in meta module

maan2003 commented 1 month ago

Lots of changes. there is only one meta source instead of multiple. MetaSource is now responsible for merge and retrying

elsirion commented 1 month ago

In my mind if the Federation config has a module with meta kind, the client can just ignore other metas. Merging anything should be avoided.

That's a good point actually. Initially my idea was that we want a smooth transition, but that's not actually the case before 0.4 the earliest: new federations will use the meta module while old ones have to continue using the override file.

If we were to automatically add the meta module to existing federations having the merge behavior would be important imo to not accidentally remove all the meta fields from a user's pov.

I bet the reason for all complexity here is because people are really set on reusing meta module for this gateway vetting

I don't think that has much to do with it, just the thought of requiring two client restarts to get the latest meta values is unappealing (first: fetch values on start, but still return old ones to not block, second: actually return new ones). Then the natural conclusion is we need an update in the background that can communicate to the frontend once new values have arrived. At that point, why not just let the updater run continuously for slightly more timely changes on the client side (it's basically free compared to the other shenanigans we do in state machines).

elsirion commented 1 month ago

That observation that vetted gateways don't need consensus is correct imo, could just be a per-guardian endpoint and then the client counts which GW comes out on top.

dpc commented 1 month ago

we already cut a 0.3 with vetted gateways in meta

Aaaagh! :D

wasn't meta module meant as storage for other modules

No... As a name of the module suggest, it is supposed to be a replacement for "metadata" about Federation that guardians can update over time, without using centralized place. It supports max 256 key-value pairs, and these were added for potential stuff like ... uploading an logo or some other static assets, not to compete with redis.

We probably have put LN vetting gateways in external meta, as a hack "because it can be easily updated". Now hacks get married, have kids, the kids get ID, voting rights...

we even thought of storing registered gateways in meta module

I don't even...

new federations will use the meta module while old ones have to continue using the override file.

Note: even if existing Federations could add meta module, they could just put existing meta into meta module and thus switch right into it atomically. Then after some time (Clients get to see it) just delete the whole external module (can't delete the one in config, I guess, but otherwise also could be done).

If we were to automatically add the meta module to existing federations having the merge behavior would be important imo to not accidentally remove all the meta fields from a user's pov.

Meta module keys start with uninitialized value, so even right after being addded (if it was possible), the client can tell that nothing was set yet and fallaback to previous sources.

I don't think that has much to do with it, just the thought of requiring two client restarts to get the latest meta values is unappealing (first: fetch values on start, but still return old ones to not block, second: actually return new ones).

I was just thinking yesterday, that we could add to ClientModuleInit an ability to flag that they don't ever need any recover and thus can be initialized immediately, avoiding the restart after the recovery.

Either a fn ClientModuleInit::needs_recovery() -> bool or just fn ClientModuleINit::recover(...) -> Option<impl Future<...>> where None means, I'm done, no need to wait.

it's basically free compared to the other shenanigans we do in state machines

Most state machines should be chatty for a couple of minutes of activity and wrap it up, or go to a long, long sleep. Otherwise people will complain about batter life, and that might matter a lot in rural areas on second hand old phones.

That observation that vetted gateways don't need consensus is correct imo, could just be a per-guardian endpoint and then the client counts which GW comes out on top.

If e.g. a gateway missbehaves it might take a really inconvenient time to gather threshold amount of votes to change the meta keys, while

maan2003 commented 1 month ago

significantly changed revision see: https://github.com/fedimint/fedimint/compare/c2079950dba1f86145e1b5554b4bf421bb0b3818..d61300fb2b05c49be84fa9b9e0ac093e85d90b25

fedimint-backports commented 1 month ago

Backport failed for releases/v0.3, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin releases/v0.3
git worktree add -d .worktree/backport-4768-to-releases/v0.3 origin/releases/v0.3
cd .worktree/backport-4768-to-releases/v0.3
git switch --create backport-4768-to-releases/v0.3
git cherry-pick -x 6697b722b063fd9a25664c55f9e2dead6774a9a7