Unified BlockchainInterface API; Transaction Handling by KPrasch · Pull Request #1089 · nucypher/nucypher

KPrasch · 2019-06-17T23:27:34Z

Co-Author @vepkenez

Note: Calls to BlockchainPower or TransactingPower can be considered concept-code, for that, see PR #1092

Follow-up for: #1058
Reverts: #1067
Fixes: #1086
Anticipating: #1092

Combines Blockchain and BlockchainInterface, removing chains.py
Provides a single method for all Agent and Deployer transactions on BlockchainInterface.send_transaction.
Handle all transactions using the w3.etn.sendRawTransaction API
Implements local ethereum key signing for EthereumTesterClient(Web3Client)
BlockchainInterface requires a TransactingPower or else it is a READ_ONLY_INTERFACE
Routes all calls to w3 methods through nucypher.eth.clients.Web3Client

Ideas (credit to @jMyles):

Are StakeHolder and Deployer characters that consume TransacingPower?
Does blockchain "connection" happen as part of TransactingPower.consume_power_up?

codecov · 2019-06-18T05:10:06Z

Codecov Report

Merging #1089 into alhambra-verde will decrease coverage by 2.11%.
The diff coverage is 82.34%.

@@                Coverage Diff                 @@
##           alhambra-verde    #1089      +/-   ##
==================================================
- Coverage           82.75%   80.64%   -2.12%     
==================================================
  Files                  68       68              
  Lines                8896     8777     -119     
==================================================
- Hits                 7362     7078     -284     
- Misses               1534     1699     +165

Impacted Files	Coverage Δ
nucypher/policy/models.py	`91.59% <ø> (ø)`	⬆️
nucypher/cli/characters/felix.py	`32.32% <0%> (-35.68%)`	⬇️
nucypher/blockchain/eth/sol/compile.py	`80% <0%> (ø)`	⬆️
nucypher/characters/unlawful.py	`46.42% <0%> (ø)`	⬆️
nucypher/utilities/sandbox/middleware.py	`90.58% <100%> (ø)`	⬆️
nucypher/characters/base.py	`86.97% <100%> (-0.07%)`	⬇️
nucypher/blockchain/eth/decorators.py	`77.14% <100%> (ø)`	⬆️
nucypher/utilities/sandbox/constants.py	`98.14% <100%> (ø)`	⬆️
nucypher/config/characters.py	`98.21% <100%> (-0.9%)`	⬇️
nucypher/blockchain/economics.py	`100% <100%> (ø)`	⬆️
... and 58 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 ddf1d08...cec66a6. Read the comment docs.

vepkenez · 2019-06-20T01:15:29Z

nucypher/cli/deploy.py

-                    click.secho("OK", fg='green', nl=False, bold=True)
-                else:
-                    click.secho("Failed", fg='red', nl=False, bold=True)
+                receipt = deployer.blockchain.client.w3.eth.waitForTransactionReceipt(txhash)


Suggested change

receipt = deployer.blockchain.client.w3.eth.waitForTransactionReceipt(txhash)

receipt = deployer.blockchain.w3.eth.waitForTransactionReceipt(txhash)

Is there a reason that w3 is exposed both in blockchain and blockchain.client?

Is there a reason that w3 is exposed both in blockchain and blockchain.client?

It's a good question, perhaps @vepkenez can help clarify, this has to do with how we are providing compatibility across clients, wrapping thier w3 instances.

tuxxy

Looks great! See my comment on the substantiate_stamp call, though.

tests/cli/test_cli_lifecycle.py

nucypher/blockchain/eth/agents.py

nucypher/blockchain/eth/interfaces.py

nucypher/crypto/powers.py

nucypher/blockchain/eth/actors.py

nucypher/blockchain/eth/interfaces.py

nucypher/network/nodes.py

tests/blockchain/eth/entities/deployers/test_deploy_preallocations.py

nucypher/crypto/powers.py

cygnusv

I like some of the simplifications that come with this PR, but I strongly reject the new version of BlockchainPower. This requires more discussion.

nucypher/crypto/powers.py

nucypher/network/nodes.py

nucypher/blockchain/eth/actors.py

nucypher/blockchain/eth/clients.py

nucypher/blockchain/eth/agents.py

nucypher/blockchain/eth/interfaces.py

derekpierre · 2019-06-20T13:37:45Z

nucypher/cli/deploy.py

-                    click.secho("OK", fg='green', nl=False, bold=True)
-                else:
-                    click.secho("Failed", fg='red', nl=False, bold=True)
+                receipt = deployer.blockchain.client.w3.eth.waitForTransactionReceipt(txhash)


Is there a reason that w3 is exposed both in blockchain and blockchain.client?

nucypher/cli/characters/felix.py

nucypher/cli/deploy.py

…ockchainPower.

cygnusv

Awesome! I was very tired of the interface.w3.eth everywhere 🙄

cygnusv · 2019-06-21T07:34:51Z

nucypher/network/nodes.py

    __DEFAULT_MIN_SEED_STAKE = 0

    def __init__(self,
+                 worker_address: str,


It seems this parameter is not necessary anymore.

cygnusv · 2019-06-21T07:54:21Z

nucypher/blockchain/eth/actors.py

 from decimal import Decimal
 from json import JSONDecodeError
 from typing import Tuple, List, Dict, Union
+from eth_utils import keccak


What should we use, this or ours (nucypher.crypto.api.keccak_digest)?

Oh - What's the difference?

cygnusv · 2019-06-21T08:01:08Z

nucypher/characters/chaotic.py

    """

-    _default_crypto_powerups = [SigningPower, BlockchainPower]
+    _default_crypto_powerups = [SigningPower]


Why does Felix lose his BlockchainPower?

At the moment, I cannot determine a way for BlockchainPower to be consumed as a default powerup, it's constructed and consumed in the __init__ of the character.

derekpierre

💯

KPrasch added the API label Jun 17, 2019

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

KPrasch force-pushed the circumflex branch from 80490c0 to 774f62c Compare June 18, 2019 17:13

KPrasch assigned vepkenez and KPrasch Jun 18, 2019

KPrasch requested a review from vepkenez June 18, 2019 17:18

KPrasch force-pushed the circumflex branch 4 times, most recently from 98658df to fcf629a Compare June 19, 2019 22:24

KPrasch requested review from cygnusv, derekpierre, michwill and tuxxy June 19, 2019 23:55

KPrasch force-pushed the circumflex branch 2 times, most recently from 93a97ad to 430cb50 Compare June 20, 2019 00:02

KPrasch changed the title ~~Condensed Blockchain API~~ Unified BlockchainInterface API; Transaction Handling Jun 20, 2019

KPrasch force-pushed the circumflex branch from fc56cc0 to f3825b6 Compare June 20, 2019 00:40

KPrasch marked this pull request as ready for review June 20, 2019 01:07

vepkenez reviewed Jun 20, 2019

View reviewed changes

KPrasch mentioned this pull request Jun 20, 2019

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

Merged

8 tasks

KPrasch added a commit to KPrasch/nucypher that referenced this pull request Jun 20, 2019

Respond to RFCs in PR nucypher#1089

027a36d

tuxxy approved these changes Jun 20, 2019

View reviewed changes

cygnusv requested changes Jun 20, 2019

View reviewed changes

nucypher/crypto/powers.py Show resolved Hide resolved

nucypher/crypto/powers.py Outdated Show resolved Hide resolved

nucypher/network/nodes.py Outdated Show resolved Hide resolved

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

derekpierre reviewed Jun 20, 2019

View reviewed changes

KPrasch added 3 commits June 20, 2019 10:23

Combines Blockchain and BlockchainInterface.

701206b

Use condensed Blockchain API in config

ef07719

Use condensed Blockchain API nucypher.blockchian

7f6e1b5

Ensuring Alice has a writable BlockchainInterface, minimally.

3696387

KPrasch force-pushed the circumflex branch from 027a36d to 225213c Compare June 20, 2019 17:23

KPrasch added a commit to KPrasch/nucypher that referenced this pull request Jun 20, 2019

Respond to RFCs in PR nucypher#1089

225213c

Returning to a single-account BlockchainPower;TODO annotations for Bl…

9fe8b72

…ockchainPower.

KPrasch force-pushed the circumflex branch from 225213c to 9fe8b72 Compare June 20, 2019 18:35

KPrasch added a commit to KPrasch/nucypher that referenced this pull request Jun 20, 2019

Respond to RFCs in PR nucypher#1089

32a4800

KPrasch changed the title ~~Unified BlockchainInterface API; Transaction Handling~~ [WIP] Unified BlockchainInterface API; Transaction Handling Jun 20, 2019

KPrasch force-pushed the circumflex branch from 32a4800 to abc1841 Compare June 20, 2019 19:30

KPrasch added a commit to KPrasch/nucypher that referenced this pull request Jun 20, 2019

Respond to RFCs in PR nucypher#1089

abc1841

Pretending to consume BlockchainPower in actor and agent tests.

11e79d0

KPrasch force-pushed the circumflex branch from 7b10bd0 to 44bcd92 Compare June 20, 2019 20:35

KPrasch added a commit to KPrasch/nucypher that referenced this pull request Jun 20, 2019

Respond to RFCs in PR nucypher#1089

44bcd92

KPrasch changed the title ~~[WIP] Unified BlockchainInterface API; Transaction Handling~~ Unified BlockchainInterface API; Transaction Handling Jun 20, 2019

KPrasch force-pushed the circumflex branch from 44bcd92 to a963353 Compare June 20, 2019 21:06

KPrasch added a commit to KPrasch/nucypher that referenced this pull request Jun 20, 2019

Respond to RFCs in PR nucypher#1089

a963353

KPrasch force-pushed the circumflex branch from a963353 to 8535572 Compare June 20, 2019 21:07

KPrasch added a commit to KPrasch/nucypher that referenced this pull request Jun 20, 2019

Respond to RFCs in PR nucypher#1089

8535572

Respond to RFCs in PR nucypher#1089

cec66a6

KPrasch force-pushed the circumflex branch from 8535572 to cec66a6 Compare June 20, 2019 21:15

KPrasch changed the base branch from alhambra-verde to master June 21, 2019 05:20

KPrasch changed the base branch from master to alhambra-verde June 21, 2019 05:21

cygnusv approved these changes Jun 21, 2019

View reviewed changes

derekpierre approved these changes Jun 21, 2019

View reviewed changes

KPrasch merged commit 0cb8025 into nucypher:alhambra-verde Jun 21, 2019

This was referenced Jun 21, 2019

Confirm activity loop cannot transact during pytests #1100

Closed

Extract Transacting from Agency onto Blockchain #1086

Closed

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

Respond to RFCs in PR #1089

407aaae

cygnusv pushed a commit that referenced this pull request Jun 28, 2019

Respond to RFCs in PR #1089

93e9cf3

cygnusv pushed a commit that referenced this pull request Jul 4, 2019

Respond to RFCs in PR #1089

420c30f

KPrasch deleted the circumflex branch March 22, 2021 18:41

	receipt = deployer.blockchain.client.w3.eth.waitForTransactionReceipt(txhash)
	receipt = deployer.blockchain.w3.eth.waitForTransactionReceipt(txhash)

Conversation

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

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tuxxy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cygnusv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cygnusv left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

derekpierre left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

KPrasch commented Jun 17, 2019 •

edited

Loading

codecov bot commented Jun 18, 2019 •

edited

Loading