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

feat: save invite code for each guardian #4318

Merged
merged 1 commit into from
Feb 20, 2024
Merged

Conversation

kernelkind
Copy link
Contributor

Closes: #4225

The code compiles, but I didn't write any tests for it because I'm not sure where to put them. All of the tests in tests.rs seem like big integration tests and testing the small Federation::invite_code_for method along side the other big integration tests doesn't seem good.

justinmoon
justinmoon previously approved these changes Feb 15, 2024
devimint/src/federation.rs Outdated Show resolved Hide resolved
devimint/src/federation.rs Outdated Show resolved Hide resolved
@@ -203,6 +218,13 @@ impl Federation {
Ok(invite_code)
}

pub fn invite_code_for(&self, peer_id: u16) -> Result<String> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self not used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should Federation::invite_code(&self) -> Result<String> be changed to Federation::invite_code() -> Result<String> as well? This method is already being used and all of the places it is used would need to be changed since it would become an associated function instead of a method without self

@kernelkind
Copy link
Contributor Author

I've been amending the commit & force pushing, is that the desired workflow? Or should I make additional commits when there are comments to address?

@kernelkind kernelkind requested a review from dpc February 18, 2024 20:35
m1sterc001guy
m1sterc001guy previously approved these changes Feb 19, 2024
let peer_data_dir = utf8(&peer_to_env_vars_map[&0].FM_DATA_DIR);
tokio::fs::copy(
format!("{peer_data_dir}/{invite_code_filename_original}"),
format!("{client_dir}/{invite_code_filename_original}"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one will be identical to the …-0 version, right? Do we even still need it? Or did you leave it to avoid having to rewrite a lot of testing code (which is probably a good enough reason)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the invite-code file is identical to invite-code-0, and yeah, it was left so it didn't break code elsewhere which relies on the presence of invite-code

@elsirion elsirion added this pull request to the merge queue Feb 20, 2024
Merged via the queue into fedimint:master with commit f486b57 Feb 20, 2024
20 checks passed
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.

Devimint should save one invite code per guardian
5 participants