Deprecates "funding account" and account creation options by KPrasch · Pull Request #1130 · nucypher/nucypher

KPrasch · 2019-07-22T23:12:19Z

The name says it all.

…ke CLI.

tuxxy · 2019-07-22T23:40:41Z

nucypher/blockchain/eth/actors.py

        if not self.token_balance >= amount:
-            raise self.StakerError(f"Insufficient token balance ({self.token_agent}) "
-                                   f"for new stake initialization of {amount}")
+            raise self.InsufficientTokens(f"Insufficient token balance ({self.token_agent}) "


tuxxy · 2019-07-22T23:41:48Z

nucypher/blockchain/eth/actors.py

-        if checksum_address:
-            if not is_checksum_address(checksum_address):
-                raise ValueError(f"{checksum_address} is an invalid EIP-55 checksum address.")
+        if not is_checksum_address(checksum_address):


Don't we have a decorator for this, IIRC?

tuxxy · 2019-07-22T23:42:13Z

nucypher/cli/stake.py

 @click.option('--offline', help="Operate in offline mode", is_flag=True)
 @click.option('--provider-uri', help="Blockchain provider's URI i.e. 'file:///path/to/geth.ipc'", type=click.STRING)
-@click.option('--funding-address', help="Address to stake NU ERC20 tokens", type=EIP55_CHECKSUM_ADDRESS)
-@click.option('--pre-funded', help="Do not fund new stake's accounts", is_flag=True, default=False)


tuxxy · 2019-07-22T23:42:29Z

tests/blockchain/eth/entities/actors/test_stakeholder.py

-
-
-def test_initialize_stake_with_existing_staking_account(software_stakeholder, stake_value, token_economics):
+def test_initialize_stake_with_existing_account(software_stakeholder, stake_value, token_economics):


tests/blockchain/eth/entities/actors/test_stakeholder.py

tuxxy · 2019-07-22T23:43:00Z

tests/fixtures.py

    _receipt = testerchain.wait_for_receipt(txhash)
+
+    token_agent = Agency.get_agent(NucypherTokenAgent)
+    token_agent.transfer(amount=NU(200_000, 'NU').to_nunits(),


tests/blockchain/eth/entities/actors/test_stakeholder.py

michwill · 2019-07-23T08:09:55Z

Looks good, but somehow I have the following problem.
If I run all tests with py.test, it fails with eth.exceptions.OutOfGas: Out of gas: Needed 4424000 - Remaining 4375218 - Reason: Write contract code for CREATE. But somehow, if I run the failing test with py.test -x tests/crypto/test_powers.py, it works fine.

michwill · 2019-07-23T10:48:58Z

Despite having the registry set, nucypher stake still tries to use memory registry in this PR:

nucypher stake --registry-filepath ~/.local/share/nucypher/dev_contract_registry.json list

raises an exception with the following message:

nucypher.blockchain.eth.registry.NoRegistry: No registry at filepath: ::memory-registry::

Should all the commands work after merging this PR?

KPrasch · 2019-07-23T16:05:40Z

@michwill - I suspect that you were running the tests at the same time you were running this command? I think there is an outstanding bug, wherein your config file is overwritten by some test. Looking into this (This is Issue is not introduced in this PR).

Yes - I think all previously functional commands will still work here.

derekpierre

🤠

michwill · 2019-07-23T19:29:42Z

@KPrasch not at the same time: before. But I see what you say - maybe can remove all configs after running tests

codecov · 2019-07-23T23:10:14Z

Codecov Report

Merging #1130 into sekanjabin will decrease coverage by 23.54%.
The diff coverage is 10.52%.

@@               Coverage Diff               @@
##           sekanjabin    #1130       +/-   ##
===============================================
- Coverage          82%   58.46%   -23.55%     
===============================================
  Files              72       71        -1     
  Lines            9733     9616      -117     
===============================================
- Hits             7982     5622     -2360     
- Misses           1751     3994     +2243

Impacted Files	Coverage Δ
nucypher/cli/stake.py	`26.19% <0%> (-45.33%)`	⬇️
nucypher/blockchain/eth/actors.py	`40.14% <14.28%> (-36.92%)`	⬇️
nucypher/blockchain/eth/policies.py	`0% <0%> (-77.62%)`	⬇️
nucypher/utilities/controllers.py	`28.2% <0%> (-66.67%)`	⬇️
nucypher/keystore/keystore.py	`32.53% <0%> (-63.86%)`	⬇️
nucypher/characters/control/interfaces.py	`35.51% <0%> (-60.75%)`	⬇️
nucypher/characters/lawful.py	`36.81% <0%> (-53.28%)`	⬇️
nucypher/network/server.py	`37.21% <0%> (-48.44%)`	⬇️
nucypher/characters/unlawful.py	`0% <0%> (-46.43%)`	⬇️
nucypher/policy/models.py	`45.64% <0%> (-46.08%)`	⬇️
... and 42 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 df10a43...301f684. Read the comment docs.

KPrasch · 2019-07-23T23:11:25Z

@michwill - I added an additional mock to help ensure that the default configuration root is not being written to the disk during tests. I do not know for certain that it is fixed.

michwill

It worked!

Deprecates funding account and account creation options. Touch up sta…

eeded76

…ke CLI.

KPrasch force-pushed the drama-school branch from da00366 to eeded76 Compare July 22, 2019 23:21

KPrasch requested review from derekpierre, jMyles, michwill and tuxxy July 22, 2019 23:23

KPrasch added CLI This effects the nucypher CLI ux design User experience enhancements Ursula 👩‍🚀 Effects the "Ursula" development area labels Jul 22, 2019

KPrasch marked this pull request as ready for review July 22, 2019 23:35

KPrasch changed the title ~~[WIP] Deprecates "funding account" and account creation options~~ Deprecates "funding account" and account creation options Jul 22, 2019

KPrasch mentioned this pull request Jul 22, 2019

[EPIC] Sekanjabin - StakeHolder & Worker-Ursula Integration, Deployer Coverage, UX Baseline. #1124

Merged

13 tasks

tuxxy approved these changes Jul 22, 2019

View reviewed changes

KPrasch requested review from fjarri and vepkenez July 22, 2019 23:45

KPrasch mentioned this pull request Jul 23, 2019

"Resume" and single-contract deployments via CLI. #1131

Merged

michwill reviewed Jul 23, 2019

View reviewed changes

tests/blockchain/eth/entities/actors/test_stakeholder.py Show resolved Hide resolved

derekpierre approved these changes Jul 23, 2019

View reviewed changes

Mock to_config_file briefly to avoid writing to the default config root.

301f684

KPrasch force-pushed the drama-school branch from d9c0fa4 to 301f684 Compare July 23, 2019 23:14

michwill approved these changes Jul 24, 2019

View reviewed changes

KPrasch merged commit 54a5684 into nucypher:sekanjabin Jul 24, 2019

KPrasch deleted the drama-school branch March 22, 2021 18:41



		def test_initialize_stake_with_existing_staking_account(software_stakeholder, stake_value, token_economics):
		def test_initialize_stake_with_existing_account(software_stakeholder, stake_value, token_economics):

Conversation

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

Uh oh!

Uh oh!

tuxxy Jul 22, 2019

Choose a reason for hiding this comment

Uh oh!

tuxxy Jul 22, 2019

Choose a reason for hiding this comment

Uh oh!

tuxxy Jul 22, 2019

Choose a reason for hiding this comment

Uh oh!

tuxxy Jul 22, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tuxxy Jul 22, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

michwill commented Jul 23, 2019

Uh oh!

michwill commented Jul 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

derekpierre left a comment

Choose a reason for hiding this comment

Uh oh!

michwill commented Jul 23, 2019

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

KPrasch commented Jul 23, 2019

Uh oh!

michwill left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

KPrasch commented Jul 22, 2019 •

edited

Loading

michwill commented Jul 23, 2019 •

edited

Loading

KPrasch commented Jul 23, 2019 •

edited

Loading

codecov bot commented Jul 23, 2019 •

edited

Loading