Skip to content
This repository has been archived by the owner on Jun 25, 2021. It is now read-only.

Fix incorrect section key assigned to non-elders #2177

Merged
merged 1 commit into from Aug 20, 2020

Conversation

madadam
Copy link
Contributor

@madadam madadam commented Aug 19, 2020

When creating the new SharedState (on EldersUpdate or NodeApproval), we incorrectly used the previous section key instead of the latest one for the section chain. This made every EldersUpdate message bounce back as untrusted.

@madadam madadam requested a review from maqi August 19, 2020 09:12
Self {
our_history: SectionProofChain::new(elders_info.proof.public_key),
Copy link
Member

Choose a reason for hiding this comment

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

just want to confirm that elders_info.proof.public_key is the DKG key with participants of elders_info.value (i.e. the elders)? i.e. the section_key of the elders of the elders_info, right ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, that's the issue this PR is trying to fix. The key the elders_info is signed with is not the latest section key - it's the one before that. The reason for this is that we vote for the elders_info and the new section key at the same time, so we don't yet have the new key in the section chain. We could probably change it so that the elders_info is signed with the latest key (because we do have the new DKG result at that time), and maybe we should do it, but that's not how things currently work.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I got it.
Thx for the clarification

Copy link
Member

Choose a reason for hiding this comment

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

maybe worth to put your above wording as a comment in the code ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 will do.

When creating the new `SharedState` on `EldersUpdate` or `NodeApproval`, we incorrectly used the previous section key instead of the latest one for the section chain. This made every `EldersUpdate` message bounce back as untrusted.
@madadam madadam merged commit 91fe3b1 into maidsafe:fleming Aug 20, 2020
@madadam madadam deleted the fix-elders-update branch August 20, 2020 09:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants