Skip to content

CLI staking via UserEscrow#1227

Merged
cygnusv merged 23 commits intonucypher:biznagafrom
cygnusv:userescrow
Oct 2, 2019
Merged

CLI staking via UserEscrow#1227
cygnusv merged 23 commits intonucypher:biznagafrom
cygnusv:userescrow

Conversation

@cygnusv
Copy link
Member

@cygnusv cygnusv commented Aug 16, 2019

The goal of this PR is to integrate UserEscrow with CLI staking (Closes #856)

Instructions:

Create a stake
nucypher stake create --escrow [--beneficiary-address 0xfoobar]

See the staking guide for more information.

Notes

Other:

  • Changes to UserEscrowProxy:
    • Fixes bug in UserEscrowProxy that prevented UserEscrow contracts from staking all their tokens
    • Fixes signature mismatch in method withdrawPolicyReward() wrt to the withdraw() method in PolicyManager
  • Improvement of the @validate_checksum_address decorator: now it validates all arguments ending with _address (e.g, staker_address, worker_address, etc.)

@codecov
Copy link

codecov bot commented Sep 4, 2019

Codecov Report

Merging #1227 into biznaga will increase coverage by 1.37%.
The diff coverage is 86.25%.

Impacted file tree graph

@@             Coverage Diff             @@
##           biznaga    #1227      +/-   ##
===========================================
+ Coverage    81.39%   82.76%   +1.37%     
===========================================
  Files           72       72              
  Lines        10522    10670     +148     
===========================================
+ Hits          8564     8831     +267     
+ Misses        1958     1839     -119
Impacted Files Coverage Δ
nucypher/cli/painting.py 78.54% <ø> (ø) ⬆️
nucypher/crypto/powers.py 94.57% <ø> (+3.61%) ⬆️
nucypher/blockchain/eth/clients.py 63.73% <0%> (+10.94%) ⬆️
nucypher/characters/lawful.py 88% <100%> (-1%) ⬇️
nucypher/blockchain/eth/interfaces.py 71.3% <100%> (+3.14%) ⬆️
nucypher/blockchain/eth/agents.py 90.66% <100%> (+1.91%) ⬆️
nucypher/blockchain/eth/token.py 89.13% <100%> (+3.36%) ⬆️
nucypher/blockchain/eth/deployers.py 88.42% <100%> (+0.02%) ⬆️
nucypher/blockchain/eth/decorators.py 91.07% <100%> (+16.07%) ⬆️
nucypher/blockchain/eth/registry.py 79.5% <100%> (+2.94%) ⬆️
... and 26 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 419c29a...87b5c67. Read the comment docs.

@cygnusv cygnusv changed the base branch from horchata to master September 4, 2019 15:52
@jMyles jMyles added the Staking label Sep 9, 2019
@KPrasch KPrasch added the CLI This effects the nucypher CLI label Sep 9, 2019
@cygnusv cygnusv force-pushed the userescrow branch 2 times, most recently from 73a615b to 0efaef4 Compare September 10, 2019 17:31
KPrasch added a commit to KPrasch/nucypher that referenced this pull request Sep 17, 2019
Co-authored-by: David Núñez <david@nucypher.com>
KPrasch added a commit to KPrasch/nucypher that referenced this pull request Sep 17, 2019
Co-authored-by: David Núñez <david@nucypher.com>
@cygnusv cygnusv changed the base branch from master to biznaga September 17, 2019 09:14
@KPrasch KPrasch mentioned this pull request Sep 19, 2019
@cygnusv cygnusv added ETH Contracts Bug 🐛 Broken functionality labels Sep 19, 2019
@cygnusv cygnusv changed the title [WIP] Allow CLI staking via UserEscrow CLI staking via UserEscrow Sep 24, 2019
@cygnusv cygnusv marked this pull request as ready for review September 24, 2019 16:41
@cygnusv cygnusv requested review from KPrasch and tuxxy September 24, 2019 16:41
Copy link
Member

@mswilkison mswilkison left a comment

Choose a reason for hiding this comment

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

Approving since it technically works :) with the caveat that there are UX/docs difficulties and some bugs as we discussed in Discord.

Copy link
Member

@derekpierre derekpierre left a comment

Choose a reason for hiding this comment

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

@cygnusv Approved but just a friendly reminder about the comments regarding removal of the validate_checksum_address check in 3 places in agents.py - 🙂

@cygnusv
Copy link
Member Author

cygnusv commented Oct 2, 2019

@derekpierre sure, they're in the rebased Branch, not pushed yet

@cygnusv cygnusv merged commit 64f2834 into nucypher:biznaga Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug 🐛 Broken functionality CLI This effects the nucypher CLI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants