"Resume" and single-contract deployments via CLI.#1131
"Resume" and single-contract deployments via CLI.#1131KPrasch merged 3 commits intonucypher:sekanjabinfrom
Conversation
|
|
||
| @staticmethod | ||
| def __collect_deployment_secret(deployer) -> str: | ||
| def collect_deployment_secret(deployer) -> str: |
There was a problem hiding this comment.
Yeah, I needed to use this function outside of the" automated series of deployments" (deploy_network_contracts) method below this one in the CLI when deploying an individual upgradeable contract, and collecting it's secret.
…single contract deployments. Increase Deployer timeout to 600 seconds.
nucypher/blockchain/eth/clients.py
Outdated
| duration = (last_update - start_time).seconds | ||
| if duration > t: | ||
| raise self.SyncTimeout | ||
| return True # Just continue on. # TODO: does this make sense? |
There was a problem hiding this comment.
Eh? I don't understand why.
Neither do I understand sleep(0) below
There was a problem hiding this comment.
Okay - I restored the exception here.
There was a problem hiding this comment.
With regard to time.sleep(0) - Take a look at this stack overflow: https://stackoverflow.com/questions/7273474/behavior-of-pythons-time-sleep0-under-linux-does-it-cause-a-context-switch.
A comment may be helpful here.
| if not scheme: | ||
| scheme = self.IPC_PROTOCOL | ||
| if scheme == 'file': | ||
| if scheme in ('file', 'ipc'): |
| secret = deployer.collect_deployment_secret(deployer=contract_deployer) | ||
| receipts, agent = deployer.deploy_contract(contract_name=contract_name, plaintext_secret=secret) | ||
| else: | ||
| receipts, agent = deployer.deploy_contract(contract_name=contract_name, gas_limit=gas) |
… TImeout. Mock ChainId RPC Endpoint in some client integration tests.
Codecov Report
@@ Coverage Diff @@
## sekanjabin #1131 +/- ##
=============================================
- Coverage 83.62% 83.6% -0.02%
=============================================
Files 72 72
Lines 9642 9643 +1
=============================================
- Hits 8063 8062 -1
- Misses 1579 1581 +2
Continue to review full report at Codecov.
|
|
🤠 |
cygnusv
left a comment
There was a problem hiding this comment.
Looking good!
Just some minor comments:
- When deploying contracts individually, it's not printing the registry path at the end.
- The gas limit is ignored sometimes: for instance, when deploying all contracts (i.e., no contract name is given), or when a deploying a named contract but it's upgradeable. See my comments above.
3)Something perhaps not specific to this PR but that I've just realized:nucypher stake set-workerasks you to select the stake, but in reality the stake itself is irrelevant here, just the staker. So it may be the case that you have 5 stakes with the same staker, and once you set a worker, all 5 stakes will show the same worker (which is the correct behavior, btw). Question: Is is possible thatnucypher stake listshows stakes for different stakers? I guess that was the point of Stakeholder, but I don't know how to try it.
| click.secho(f"Deploying {contract_name}") | ||
| if contract_deployer._upgradeable: | ||
| secret = deployer.collect_deployment_secret(deployer=contract_deployer) | ||
| receipts, agent = deployer.deploy_contract(contract_name=contract_name, plaintext_secret=secret) |
There was a problem hiding this comment.
| receipts, agent = deployer.deploy_contract(contract_name=contract_name, plaintext_secret=secret) | |
| receipts, agent = deployer.deploy_contract(contract_name=contract_name, plaintext_secret=secret, gas_limit=gas) |
It also needs to be passed to deployer.deploy_network_contracts(secrets=secrets) in line 243:
https://github.com/nucypher/nucypher/pull/1131/files#diff-4809c009687ec0119c014880f31f9111R243
| # Support older and newer versions of web3 py in-test | ||
| version = 5 | ||
| chainID = 5 | ||
| chainId = hex(5) |
There was a problem hiding this comment.
Just out of curiosity: why hex here?
There was a problem hiding this comment.
Just following what geth nodes return as a type on this endpoint for mocking
Yeah, I am aware of this, I think gas management is generally outside the scope of this PR. If you prefer a higher degree of precision here I can remove these lines for now. (#842 )
Yes - you can run |
Based on #1130
Uses Agency and Deployer methods to deploy a single contract by it's registry name form the CLI. This may act as a good secondary means of managing failed deployments as part of an automated series.