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

SecretStore: get rid of engine.signer dependency #8173

Merged
merged 11 commits into from
Apr 9, 2018

Conversation

svyatonik
Copy link
Collaborator

Previously it was assumed that in PoA networks KeyServers will only be running on authority nodes && both engine.signer and secretstore.self_secret will be the same. This is PR to get rid of this requirement && that's how KS will be initially deployed on Kovan (not on authority nodes).

@svyatonik svyatonik added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Mar 21, 2018
@5chdn 5chdn added this to the 1.11 milestone Mar 21, 2018
@svyatonik svyatonik added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A0-pleasereview 🤓 Pull request needs code review. labels Mar 23, 2018
@svyatonik
Copy link
Collaborator Author

Also fixed 2 small issues, found while testing:

  1. 1-of-1 generation session was completed only after timeout
  2. illegal (obsolete after new version of contracts) condition in Retry task processing

@svyatonik svyatonik added A0-pleasereview 🤓 Pull request needs code review. A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. A0-pleasereview 🤓 Pull request needs code review. labels Mar 23, 2018
@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 Mar 23, 2018
@svyatonik svyatonik added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A0-pleasereview 🤓 Pull request needs code review. labels Apr 3, 2018
@svyatonik svyatonik added A0-pleasereview 🤓 Pull request needs code review. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Apr 3, 2018
/// Transact contract.
pub fn transact_contract(&self, contract: Address, tx_data: Bytes) -> Result<(), Error> {
let client = self.client.upgrade().ok_or(Error::Internal("cannot submit tx when client is offline".into()))?;
let miner = self.miner.upgrade().ok_or(Error::Internal("cannot submit tx when miner is offline".into()))?;
Copy link
Contributor

Choose a reason for hiding this comment

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

ok_or_else would prevent unconditional allocations of the errors messages.

@5chdn 5chdn added A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. and removed A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. labels Apr 9, 2018
@rphmeier rphmeier added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Apr 9, 2018
@debris debris merged commit 0d75d01 into master Apr 9, 2018
@debris debris deleted the secrestore_ks_on_non_authority_nodes branch April 9, 2018 14:39
svyatonik added a commit that referenced this pull request Apr 18, 2018
* SecretStore: get rid of engine.signer dependency

* SecretStore: fixed self for transact_contract

* SecretStore: fixed pending requests + 1-of-1 sessions completion

* SecretStore: fixed completion signal in 1-of-1 case

* fixed test(s)

* removed obsolete TODO && redundant statement

* ok_or -> ok_or_else
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