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

generate keyfile as part of network key tx #423

Merged
merged 1 commit into from
Mar 24, 2020
Merged

generate keyfile as part of network key tx #423

merged 1 commit into from
Mar 24, 2020

Conversation

liam-fitzgerald
Copy link
Member

We generate the keyfile as part of the network key transaction to
hopefully prevent downloading stale.

Hopefully mitigates #346 and #137 although I am unable to reproduce
it.

We generate the keyfile as part of the network key transaction to
hopefully prevent downloading stale.

Hopefully mitigates #346 and #137 although I am unable to reproduce
it.
@liam-fitzgerald liam-fitzgerald requested a review from Fang- March 24, 2020 07:41
@Fang-
Copy link
Member

Fang- commented Mar 24, 2020

Can you explain the reasoning behind this fix? I'm having trouble seeing what problematic case this solves, and how exactly it solves that.

@liam-fitzgerald
Copy link
Member Author

If the new keyfile takes a while to generate, then the downloadkeyfilebutton will serve a stale keyfile while the new keyfile is generating. Hence we don't show the downloadkeyfile button until we've finished generating a new one

Copy link
Member

@Fang- Fang- left a comment

Choose a reason for hiding this comment

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

Okay. I couldn't quite conclude whether that actually holds looking at useKeyfileGenerator, but looking again now, I'm reasonably confident it does. Good fix, let's get it in!

@Fang- Fang- merged commit 6121696 into master Mar 24, 2020
@Fang- Fang- deleted the stale-net-keys branch May 5, 2020 14:49
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.

2 participants