Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Fixing secretstore TODOs - part 2 #5416

Merged
merged 81 commits into from
Apr 25, 2017
Merged

Fixing secretstore TODOs - part 2 #5416

merged 81 commits into from
Apr 25, 2017

Conversation

svyatonik
Copy link
Collaborator

On top of #5386

Most of still pending TODOs are fixed here, including:

  1. encryption session will now fail if any cluster node is down. It will also fail if some node is misbehaving. So - N in M-of-N is never autodecreased
  2. added 'session timeout' for cases when node is still up (connection is established), but it does not answer to some messages, crucial for the session
  3. sessions are finally removed after completion
  4. decryption session is now able to auto-restart when node goes down even if it is in the middle of decryption process

I want to add some tests for these fixes, so marking as in-progress.

@svyatonik svyatonik added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. M4-core ⛓ Core client code / Rust. labels Apr 6, 2017
@svyatonik
Copy link
Collaborator Author

  1. Added more tests
  2. Fixed 1-of-1
  3. Fixed closing Parity by ^C when secretstore was working

@svyatonik svyatonik added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Apr 10, 2017
@keorn
Copy link

keorn commented Apr 15, 2017

lgtm and seems to work.

impl<T> Drop for KeyServerHttpListener<T> where T: KeyServer + 'static {
fn drop(&mut self) {
// ignore error as we are dropping anyway
let _ = self._http_server.close();
Copy link
Contributor

Choose a reason for hiding this comment

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

does this not close on drop already?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope. That was the rason behind "Fixed closing Parity by ^C when secretstore was working" issue. You must call close() explicitely

/// 1) checks if connected nodes are responding to KeepAlive messages
/// 2) tries to connect to disconnected nodes
/// 3) checks if enc/dec sessions are time-outed
const MAINTAN_INTERVAL: u64 = 10;
Copy link
Contributor

Choose a reason for hiding this comment

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

MAINTAN -> MAINTAIN

@rphmeier rphmeier added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Apr 21, 2017
@gavofyork gavofyork merged commit 1a26204 into master Apr 25, 2017
@gavofyork gavofyork deleted the secretstore_sessionerr branch April 25, 2017 19:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants