-
Notifications
You must be signed in to change notification settings - Fork 34
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
fix: set numPeers correctly #386
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK. Looks good, but I wish it was just a super small change that only includes the changes in SetupContext.tsx
. We can append the other changes as extra opportunistic commits to clean up the code.
@@ -31,7 +31,6 @@ export const ConnectGuardians: React.FC<Props> = ({ next }) => { | |||
const { | |||
state: { role, peers, numPeers, configGenParams, ourCurrentId }, | |||
} = useSetupContext(); | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: unnecessary change in file
@@ -23,19 +23,7 @@ const LOCAL_STORAGE_SETUP_KEY = 'setup-guardian-ui-state'; | |||
* Creates the initial state using loaded state from local storage. | |||
*/ | |||
function makeInitialState(loadFromStorage = true): SetupState { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Kodylow , this is the actual fix to the bug! Nice fix. Could we just scope the PR to changes in this file and we land it?
bb68f2b
to
763424d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅
fixes #385
We weren't properly loading the state from localStorage, and we were setting numPeers from connected Peers not from stateNumPeers. This lets the setup leader refresh and maintain state and connections to peers, and doesn't kick to DKG immediately on refresh.