Extract Stakeholder CLI and Deploy Actions by KPrasch · Pull Request #1056 · nucypher/nucypher

KPrasch · 2019-06-06T23:54:47Z

This PR goes into the Sekanjabin Epic (#1107); Refactors the staking CLI to suit the StakeHolder and makes improvements to the usability of development deployment.

Based on #1064

Fixes #1119
Contributes to fixing #1117

Introduces #1125

Status

Still has failing mixed-configuration tests, possibly related to federated vs. non-federated keyring attachment methods on Ursula. Needs interactive reviews and Issue-opening follow-ups.

Interactive Review in 4 Steps

1- Run Development Blockchain

geth --dev --dev.period 2

2 - Deploy Contracts

nucypher-deploy contracts --provider-uri ipc:///tmp/geth.ipc --poa --dev

3 - Create StakeHolder

nucypher stake new-stakeholder --provider-uri file:///tmp/geth.ipc --registry-filepath  ~/.local/share/nucypher/dev_contract_registry.json  --poa

4 - Initialize Stake

nucypher stake init

codecov · 2019-06-07T00:12:57Z

Codecov Report

Merging #1056 into sekanjabin will increase coverage by 1.9%.
The diff coverage is 78.65%.

@@              Coverage Diff              @@
##           sekanjabin    #1056     +/-   ##
=============================================
+ Coverage       81.55%   83.46%   +1.9%     
=============================================
  Files              70       71      +1     
  Lines            9527     9707    +180     
=============================================
+ Hits             7770     8102    +332     
+ Misses           1757     1605    -152

Impacted Files	Coverage Δ
nucypher/crypto/powers.py	`94.47% <ø> (-0.62%)`	⬇️
nucypher/blockchain/eth/token.py	`87.29% <ø> (+0.33%)`	⬆️
nucypher/config/keyring.py	`89.13% <ø> (+0.63%)`	⬆️
nucypher/config/characters.py	`99.16% <100%> (+0.05%)`	⬆️
nucypher/blockchain/eth/interfaces.py	`80.45% <100%> (-0.16%)`	⬇️
nucypher/cli/types.py	`81.48% <100%> (ø)`	⬆️
nucypher/cli/characters/alice.py	`77.39% <100%> (+1.73%)`	⬆️
nucypher/cli/main.py	`88.7% <100%> (+0.18%)`	⬆️
nucypher/blockchain/eth/agents.py	`91.95% <100%> (+2.09%)`	⬆️
nucypher/config/node.py	`90.53% <100%> (+0.37%)`	⬆️
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 04122fe...2c4b5cf. Read the comment docs.

nucypher/cli/actions.py

…n to/from UrsulaConfiguration; Attempting to fix reward collection CLI integration tests.

… two tests.

…inflation rewards to specified account or funding account. Touch up reward collection assertions.

nucypher/cli/characters/ursula.py

nucypher/cli/stake.py

cygnusv · 2019-07-17T13:12:47Z

Just a placeholder. I want to review this but it will have to be tonight, before I leave. In any case, I've been using this as the base of #1111, so if you don't hear about me after today 23:59 CEST, it's an implicit approval.

cygnusv · 2019-07-17T19:39:26Z

There's a tiny bug when handling the receipts of the deployment of UserEscrowProxy, which is composed of 2 steps, and the only first one is printed, but twice. Just to let you know this is fixed in #1111.
E.g.:

UserEscrowProxy (0xD89a96A5C37b9326Dc332037d6034D89c6d35641)
************************************************************
OK | deployment | 0xf40f3a24c79c1721f671991d68ddd5f59cde26dc7efed3641eeca28b25b95211 (1302579 gas)
Block #114 | 0x4f688ac25eef491830b1031b6d0392d53be267942477a9dcab578986663d3b5b

OK | proxy_deployment | 0xf40f3a24c79c1721f671991d68ddd5f59cde26dc7efed3641eeca28b25b95211 (1302579 gas)
Block #114 | 0x4f688ac25eef491830b1031b6d0392d53be267942477a9dcab578986663d3b5b

tuxxy

A few things here that I uncovered during a UX audit with a hardware wallet:

~~1. Instead of --device, let's do --hw-wallet/--no-hw-wallet.~~
~~2. Don't prompt for stakes in Nunits, let's do a float.~~
3. When I specify --device when creating a new stake-holder, it should specify that that stakeholder is a device so I don't need to do --device when I perform a stake init (maybe? I guess it'll be a default, so it's kinda moot.)
4. It prompts for my passphrase despite using a hardware wallet and then tries to use it when I want to publish the stake on the blockchain; thus, preventing me from staking with a hardware wallet.
5. A better explanation of what a "period" is during the stake initialization process would be ideal.

cygnusv

Apart from the suggested changes, I also had a minor problem with the stakeholder JSON config file. At some moment I had 2 of them, one named stakeholder.json and the other stakeholder-0xfoobar.json, and I had to remove the former and change the name of the latter to stakeholder.json to continue working.

nucypher/cli/stake.py

… staker address.

… namedtuple.

cygnusv

I've just successfully staked (yay!) and set a worker.

One issue I've found is that when using option 'c', it's not creating a new account in the end, but staking with the funding account. I'm sure it's just a minor bug when handling the address in nucypher stake init. Also, as a consequence of the above, it's funding the funding account from the funding account :P

KPrasch · 2019-07-18T06:15:03Z

@cygnusv - Yes I also noticed this. Noted.

nucypher/blockchain/eth/agents.py

Co-Authored-By: Derek Pierre <derek.pierre@gmail.com>

tuxxy

🐧

cygnusv force-pushed the alhambra-verde branch 4 times, most recently from ffdb6f6 to e2a3ec9 Compare June 10, 2019 10:02

KPrasch force-pushed the croquetas branch from 8fc00ca to 1cfa0ab Compare June 10, 2019 18:00

cygnusv force-pushed the alhambra-verde branch 2 times, most recently from 4eca13d to b1572a7 Compare June 11, 2019 08:05

KPrasch force-pushed the croquetas branch from 1cfa0ab to c38d298 Compare June 12, 2019 00:31

cygnusv force-pushed the alhambra-verde branch from b1572a7 to 9738aeb Compare June 13, 2019 18:33

KPrasch added the CLI This effects the nucypher CLI label Jun 13, 2019

KPrasch force-pushed the croquetas branch 2 times, most recently from 72e6633 to 83d7993 Compare June 14, 2019 00:59

KPrasch changed the title ~~Extracts staking CLI to 'nucypher stake' entry point~~ StakeHolder CLI Jun 14, 2019

cygnusv added a commit to cygnusv/nucypher that referenced this pull request Jun 14, 2019

Skip staking ursula tests until nucypher#1056

50a6a50

KPrasch force-pushed the croquetas branch from c4e82fc to 15d5f73 Compare June 15, 2019 05:52

KPrasch force-pushed the alhambra-verde branch from 9720e49 to d3fcff5 Compare June 15, 2019 06:02

KPrasch force-pushed the croquetas branch from 15d5f73 to d867ade Compare June 15, 2019 06:07

KPrasch mentioned this pull request Jun 15, 2019

[EPIC] Alhambra Verde - Introduce Staker and Worker Roles #1029

Merged

8 tasks

mswilkison reviewed Jun 15, 2019

View reviewed changes

nucypher/cli/actions.py Show resolved Hide resolved

cygnusv added a commit to cygnusv/nucypher that referenced this pull request Jun 17, 2019

Skip staking ursula tests until nucypher#1056

00bbf2f

cygnusv force-pushed the alhambra-verde branch from b9aa214 to 0dc5f37 Compare June 18, 2019 11:43

cygnusv added a commit that referenced this pull request Jun 18, 2019

Skip staking ursula tests until #1056

4b81db4

cygnusv force-pushed the alhambra-verde branch from 0dc5f37 to ddf1d08 Compare June 20, 2019 13:58

cygnusv added a commit that referenced this pull request Jun 20, 2019

Skip staking ursula tests until #1056

da04e55

KPrasch force-pushed the croquetas branch from d867ade to 59f0dbf Compare June 21, 2019 16:44

cygnusv force-pushed the alhambra-verde branch from 0cb8025 to 407aaae Compare June 26, 2019 16:19

cygnusv added a commit that referenced this pull request Jun 26, 2019

Skip staking ursula tests until #1056

3d7b06d

KPrasch mentioned this pull request Jun 27, 2019

[WIP] [EPIC] Sekanjabin - StakeHolder & Worker-Ursula Integration #1107

Closed

6 tasks

KPrasch force-pushed the croquetas branch from 59f0dbf to 1a246e6 Compare June 27, 2019 19:55

KPrasch added a commit to KPrasch/nucypher that referenced this pull request Jul 17, 2019

Respond to RFCs in PR nucypher#1056.

4f2d0b2

Respond to RFCs in PR nucypher#1056; Test and CLI touch-ups.

e75c1b6

KPrasch force-pushed the croquetas branch from 4f2d0b2 to e75c1b6 Compare July 17, 2019 00:20

KPrasch added the Ursula 👩‍🚀 Effects the "Ursula" development area label Jul 17, 2019

KPrasch added 3 commits July 16, 2019 19:50

Extracts manual staker and worker fixtures, Fixes incorrect address i…

eca28a1

…n to/from UrsulaConfiguration; Attempting to fix reward collection CLI integration tests.

Use mocked blockchain interface connection within CLI subprocesses in…

1b2016f

… two tests.

Fix keyring checksum address multiple kwarg passing, add withdraw of …

88dd009

…inflation rewards to specified account or funding account. Touch up reward collection assertions.

KPrasch force-pushed the croquetas branch from a54ad60 to 88dd009 Compare July 17, 2019 04:54

michwill reviewed Jul 17, 2019

View reviewed changes

nucypher/cli/characters/ursula.py Show resolved Hide resolved

michwill reviewed Jul 17, 2019

View reviewed changes

nucypher/cli/stake.py Show resolved Hide resolved

KPrasch added 3 commits July 16, 2019 23:26

Skip transfer of rewards back to funding account for now.

a26e82f

Fixes alice persistence test.

05ba65f

Routes Ursula keyring attchment by federated/decentralized mode.

9fded0f

derekpierre reviewed Jul 17, 2019

View reviewed changes

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

derekpierre approved these changes Jul 17, 2019

View reviewed changes

KPrasch changed the title ~~Extract Stakeholder and Common CLI Deploy Actions~~ Extract Stakeholder CLI and Deploy Actions Jul 17, 2019

tuxxy suggested changes Jul 17, 2019

View reviewed changes

Ensure geth --dev accounts are always unlocked

40d0251

cygnusv reviewed Jul 17, 2019

View reviewed changes

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

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

KPrasch added 3 commits July 17, 2019 16:46

Sets Ursula filename modifier to be the worker address instead of the…

7405220

… staker address.

Respond to RFCs in PR nucypher#1056.

3890de2

Updates estimate gas with interactive deployment painting and Secrets…

2e0e4bb

… namedtuple.

cygnusv approved these changes Jul 18, 2019

View reviewed changes

derekpierre reviewed Jul 18, 2019

View reviewed changes

nucypher/blockchain/eth/agents.py Outdated Show resolved Hide resolved

Update nucypher/blockchain/eth/agents.py

2c4b5cf

Co-Authored-By: Derek Pierre <derek.pierre@gmail.com>

tuxxy approved these changes Jul 18, 2019

View reviewed changes

derekpierre mentioned this pull request Jul 18, 2019

Improved deployers tests #1111

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract Stakeholder CLI and Deploy Actions#1056

Extract Stakeholder CLI and Deploy Actions#1056
tuxxy merged 28 commits intonucypher:sekanjabinfrom
KPrasch:croquetas

KPrasch commented Jun 6, 2019 •

edited

Loading

Uh oh!

codecov bot commented Jun 7, 2019 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cygnusv commented Jul 17, 2019

Uh oh!

cygnusv commented Jul 17, 2019

Uh oh!

tuxxy left a comment •

edited by KPrasch

Loading

Uh oh!

cygnusv left a comment

Uh oh!

Uh oh!

Uh oh!

cygnusv left a comment

Uh oh!

KPrasch commented Jul 18, 2019

Uh oh!

Uh oh!

tuxxy left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

KPrasch commented Jun 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Status

Interactive Review in 4 Steps

1- Run Development Blockchain

2 - Deploy Contracts

3 - Create StakeHolder

4 - Initialize Stake

Uh oh!

codecov bot commented Jun 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cygnusv commented Jul 17, 2019

Uh oh!

cygnusv commented Jul 17, 2019

Uh oh!

tuxxy left a comment • edited by KPrasch Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cygnusv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cygnusv left a comment

Choose a reason for hiding this comment

Uh oh!

KPrasch commented Jul 18, 2019

Uh oh!

Uh oh!

tuxxy left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

KPrasch commented Jun 6, 2019 •

edited

Loading

codecov bot commented Jun 7, 2019 •

edited

Loading

tuxxy left a comment •

edited by KPrasch

Loading