Maintain non-federated CLI #593

KPrasch · 2018-12-03T18:35:38Z

The recent work to build a federated network, left this code on the back-burner for a while. This PR updates reestablishes decentralisation as the primary and default operating mode.

Restore nucypher contract deployment CLI (nucypher-deploy; Implements Deployer actor)
Sets --federated-only False by default (Fixes Set --federated-only back to False by default #600)
Select a (possibly) open network port for test runs (Fixes Pick an available port for test Ursulas #566 )
Ensure deployer uses it's internal blockchain reference to produce entities.
Allow CLI to accept any port number when passed via --rest-port (Fixes Accept any network port number passed via CLI flag #601)
Separates configuration parameter generation and character production
Ensure federating operating mode is determined by configuration, not always by flag.

…nt init; Use pyevm as default provider URI

…d blockchin config log on NodeConfiguration and NodeStorage

…d ursula CLI test module.

jMyles

Looking pretty decent! I think I need to actually run it to see how things look; it's hard to assess how some of the strings are formatted without seeing the result in a terminal.

For now, just a few small changes needed.

nucypher/blockchain/eth/actors.py

jMyles · 2018-12-11T21:01:31Z

nucypher/blockchain/eth/actors.py

+    def __repr__(self):
+        r = '{name}({blockchain}, {deployer_address})'.format(name=self.__class__.__name__,
+                                                              blockchain=self.blockchain,
+                                                              deployer_address=self.deployer_address)


Looks like this will raise if deployer_address is not set - and, unless I'm reading incorrectly - it's not guaranteed to be.

There is a CS constant used as the default value of deployer_address: NO_DEPLOYER_ADDRESS - but it looks like there is a bit of an Issue since there is a reference to deployer_address both here on this object (Deployer) and also on BlockchainInterface, which may be different values.

nucypher/blockchain/eth/interfaces.py

nucypher/cli/deploy.py

nucypher/cli/main.py

nucypher/config/node.py

tests/network/test_node_storage.py

nucypher/cli/deploy.py

michwill

Looks pretty good

nucypher/cli/deploy.py

nucypher/utilities/sandbox/constants.py

Co-Authored-By: KPrasch <kieranprasch@gmail.com>

tuxxy

🐧

cygnusv

Just some minor comments and it's good to go.

cygnusv · 2018-12-13T21:06:02Z

.gitignore

@@ -2,6 +2,7 @@
 __pycache__
 *.pyc
 /.venv
+.eggs


Cool, we have green eggs. We just need some ham now.

nucypher/cli/deploy.py

cygnusv · 2018-12-13T21:32:48Z

nucypher/utilities/sandbox/blockchain.py

@@ -89,7 +89,7 @@ def __generate_insecure_unlocked_accounts(self, quantity: int) -> List[str]:

            umbral_priv_key = UmbralPrivateKey.gen_key()
            address = self.interface.w3.personal.importRawKey(private_key=umbral_priv_key.to_bytes(),
-                                                              password=insecure_password)
+                                                              passphrase=insecure_password)


Not related to this PR, but do we still consider "passphrase" in Sentry scrubbers?

cygnusv · 2018-12-13T21:49:12Z

tests/config/test_firstula_circumstances.py

@@ -19,7 +19,7 @@

 import maya
 import pytest
-import pytest_twisted
+import pytest_twisted as pt


Just curious...why?

https://pypi.org/project/pytest-twisted/

Beware that in situations such as a conftest.py file that the name pytest_twisted may be undesirably detected by pytest as an unknown hook. One alternative is to import pytest_twisted as pt.

Co-Authored-By: KPrasch <kieranprasch@gmail.com>

KPrasch mentioned this pull request Dec 3, 2018

Maintain Gas Estimation Metrics #594

Merged

KPrasch force-pushed the cli-blockchain branch 4 times, most recently from 3e221b3 to b4940ca Compare December 8, 2018 04:28

KPrasch changed the title ~~[WIP] Maintain nucypher-deploy CLI~~ [WIP] Maintain non-federated CLI Dec 8, 2018

KPrasch self-assigned this Dec 8, 2018

KPrasch added Bug 🐛 Broken functionality CLI This effects the nucypher CLI Configuration Node configuration labels Dec 8, 2018

KPrasch changed the title ~~[WIP] Maintain non-federated CLI~~ Maintain non-federated CLI Dec 10, 2018

KPrasch requested review from jMyles, cygnusv, michwill and EvaLovlac3 December 10, 2018 16:47

KPrasch added 11 commits December 11, 2018 10:01

Deployer now track txhashes and agents

19e4420

Restores blockchain interface connection configuration

84a1283

Draft of nucypher-deploy interface

2daf283

Set federated-only to False by default.

81e082e

Bug Fixes: Deployment actor uses self.blockchain for deployer and age…

6f18850

…nt init; Use pyevm as default provider URI

Seperates config.produce from init-parameter generation; DRY up share…

c90026c

…d blockchin config log on NodeConfiguration and NodeStorage

Restore the usage of --federated-only in CLI tests; Dedicate federate…

3b43658

…d ursula CLI test module.

Select a possibly open non-default port for running tests

c235bb4

Allow CLI to accept any port number.

33e1eef

Ensure that operating mode is determined via configuration.

d6ad99a

Set default temp prefix, and provider uri as public attributes

eb04eb4

KPrasch force-pushed the cli-blockchain branch from 2d054cd to eb04eb4 Compare December 11, 2018 18:02

KPrasch mentioned this pull request Dec 11, 2018

Ursula Development Configuration Test #605

Merged

jMyles suggested changes Dec 12, 2018

View reviewed changes

michwill reviewed Dec 12, 2018

View reviewed changes

nucypher/cli/deploy.py Outdated Show resolved Hide resolved

michwill previously approved these changes Dec 12, 2018

View reviewed changes

KPrasch dismissed michwill’s stale review via 50eabd5 December 13, 2018 02:03

KPrasch removed the request for review from EvaLovlac3 December 13, 2018 02:04

Respond to RFCs - Deployer contract names are now public.

58d8c3f

KPrasch force-pushed the cli-blockchain branch from 50eabd5 to 58d8c3f Compare December 13, 2018 02:07

jMyles previously approved these changes Dec 13, 2018

View reviewed changes

tuxxy reviewed Dec 13, 2018

View reviewed changes

nucypher/cli/deploy.py Outdated Show resolved Hide resolved

nucypher/utilities/sandbox/constants.py Show resolved Hide resolved

Update nucypher/cli/deploy.py

854dcf1

Co-Authored-By: KPrasch <kieranprasch@gmail.com>

KPrasch dismissed stale reviews from jMyles via 854dcf1 December 13, 2018 20:53

tuxxy previously approved these changes Dec 13, 2018

View reviewed changes

cygnusv reviewed Dec 13, 2018

View reviewed changes

Update nucypher/cli/deploy.py

c225aab

Co-Authored-By: KPrasch <kieranprasch@gmail.com>

KPrasch dismissed tuxxy’s stale review via c225aab December 13, 2018 23:00

KPrasch mentioned this pull request Dec 13, 2018

Clarify the usage of double-negatives in config and CLI #612

Closed

michwill approved these changes Dec 13, 2018

View reviewed changes

KPrasch merged commit 3a6f474 into nucypher:master Dec 13, 2018

KPrasch deleted the cli-blockchain branch July 21, 2020 02:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Maintain non-federated CLI #593

Maintain non-federated CLI #593

KPrasch commented Dec 3, 2018 •

edited

Loading

jMyles left a comment

jMyles Dec 11, 2018

KPrasch Dec 13, 2018 •

edited

Loading

michwill left a comment

tuxxy left a comment

cygnusv left a comment

cygnusv Dec 13, 2018

KPrasch Dec 13, 2018

cygnusv Dec 13, 2018

KPrasch Dec 13, 2018

cygnusv Dec 13, 2018

KPrasch Dec 13, 2018

Maintain non-federated CLI #593

Maintain non-federated CLI #593

Conversation

KPrasch commented Dec 3, 2018 • edited Loading

jMyles left a comment

Choose a reason for hiding this comment

jMyles Dec 11, 2018

Choose a reason for hiding this comment

KPrasch Dec 13, 2018 • edited Loading

Choose a reason for hiding this comment

michwill left a comment

Choose a reason for hiding this comment

tuxxy left a comment

Choose a reason for hiding this comment

cygnusv left a comment

Choose a reason for hiding this comment

cygnusv Dec 13, 2018

Choose a reason for hiding this comment

KPrasch Dec 13, 2018

Choose a reason for hiding this comment

cygnusv Dec 13, 2018

Choose a reason for hiding this comment

KPrasch Dec 13, 2018

Choose a reason for hiding this comment

cygnusv Dec 13, 2018

Choose a reason for hiding this comment

KPrasch Dec 13, 2018

Choose a reason for hiding this comment

KPrasch commented Dec 3, 2018 •

edited

Loading

KPrasch Dec 13, 2018 •

edited

Loading