Coordinator Alpha 12 Support by KPrasch · Pull Request #3213 · nucypher/nucypher

KPrasch · 2023-08-28T11:10:10Z

Type of PR:
Feature

Required reviews:
3

What this does:

Assorted updates based on Coordinator Alpha 12 nucypher-contracts#92
Utilizes nucypher-contracts version from Tweaks for Alpha 12 nucypher-contracts#116 (includes fixes/functionality needed for this PR)
Completes Enrico authorization
- Fixes Hookup allow logic contract call from stub #3206
- Fixes Onchain enrico allow list #3147
- Fixes DKG Authorization Models #3050
Fixes Should DKG Threshold be specified for Ritual #3095 .

Follow-up PR will be made for coordinator alpha-13 work i.e TACoRoot/Child Application work - #3227

nucypher/blockchain/eth/agents.py

This commit is aimed to update test to the following changes on nucypher-contracts repo: - Update SimplePREApplication with TACoApplication - Replace ThresholdStakingForPREApplicationMock contract with ThresholdStakingForTACoApplicationMock - Use the new Coordinator contract - Add the deployment of StakeInfo contract The use of these new contracts made necessary the following changes: - Add the deployment of a new Token for ritual fees: RitualToken. This contract has been added as local deployment. - Add the deployment of local contract ConditionNFT Additionally, this commit updates the key character for arguments in ape-config.yaml. From '::variable::' to <variable>, which results in a more legible code. Finally, this commit solves some linting and formatting issues. Co-authored-by: LunarBytes <kieran@nucypher.com>

…al and dependency contract ABI entries.

…llowList in tests

…la; Supporting G2 point class

… can be made while testing.

…public keys. Move G2Point out of Ritual in CoordinatorAgent.

…bject. MockCoordinatorAgent simply uses a class method to determine threshold (allows for both simple/precomputed) to still be tested in integration tests.

…values stored by the Ritual struct.

…based on test mnemonic.

KPrasch

🤠 approved

derekpierre

🎸 - co-authorship on this PR though 😄

cygnusv

Initial pass

cygnusv · 2023-09-13T14:24:14Z

nucypher/blockchain/eth/agents.py

    contract_name: str = "Coordinator"
    _proxy_name = None

+    class G2Point(NamedTuple):


Why is this class necessary?

Getting/setting the provider public key.

cygnusv · 2023-09-13T14:28:55Z

nucypher/characters/lawful.py

+        conditions: Lingo,
    ) -> ThresholdMessageKit:
+        if not self.signer:
+            raise TypeError("This Enrico doesn't have a signer.")


There's missing commit authorship in this line 🤖

I think he means that Github Copilot probably helped here 🤖🤖🤖

cygnusv · 2023-09-13T14:30:04Z

nucypher/characters/lawful.py

-        if not self._policy_pubkey:
+        if not self._encrypting_key:
            raise TypeError(
                "This Enrico doesn't know which policy encrypting key he used.  Oh well."


manumonti

This is huge! Great work

I have some suggestions, but overall it looks good

nucypher/cli/commands/status.py

nucypher/characters/lawful.py

manumonti · 2023-09-14T10:36:10Z

nucypher/characters/lawful.py

-        if not self._policy_pubkey:
+        if not self._encrypting_key:
            raise TypeError(
                "This Enrico doesn't know which policy encrypting key he used.  Oh well."


manumonti · 2023-09-14T10:37:25Z

nucypher/characters/lawful.py

+        conditions: Lingo,
    ) -> ThresholdMessageKit:
+        if not self.signer:
+            raise TypeError("This Enrico doesn't have a signer.")


I think he means that Github Copilot probably helped here 🤖🤖🤖

manumonti · 2023-09-14T11:39:21Z

tests/utils/ape.py

-def deploy_contracts(nucypher_contracts: DependencyAPI, accounts):
+def deploy_contracts(
+    nucypher_contracts: DependencyAPI,
+    test_contracts: DependencyAPI,


test_contracts are not been deployed using this function.

AFAIK, these test contracts are deployed by APE since they are under the contracts folder. So we can delete this line.

Suggested change

test_contracts: DependencyAPI,

^ This whole setup is changed in #3227 (follow-up PR) - so this is/will be addressed there.

manumonti · 2023-09-14T12:24:16Z

tests/acceptance/ape-config.yaml

  - name: nucypher-contracts
-    github: nucypher/nucypher-contracts
-    ref: main
+    github: derekpierre/nucypher-contracts


this is temporal, right? should we add a TODO comment?

Yes. It can be merged with that reference since the PR is going into an EPIC (#3226), but it will be changed to nucypher-contracts/main as part of merging the epic.

tests/acceptance/conftest.py

894dc5a addresses the respective suggestions.

manumonti

Great! LGTM ✌️

piotr-roslaniec · 2023-09-14T16:24:46Z

tests/acceptance/characters/test_operator.py

+        "value": Web3.to_wei("1", "ether"),
+    }
+    txhash = testerchain.w3.eth.send_transaction(tx)
+    _ = testerchain.wait_for_receipt(txhash)


I noticed that tx hashes are often assigned to a variable and then ignored (like _hash in a different file). Is this some sort of Pythonic idiom?

The txhash is used to wait for an tx execution receipt, but the receipt is not needed hence the _- it just indicates that the tx was mined/completed.

Some/most of these are removed/replaced in #3227 by using ape functionality for tx execution which does not requiring waiting for a tx receipt - other than the txs for transferring eth to other accounts (haven't gotten around to figuring out the ape equivalent for that as yet, but something I'm looking into for #3227).

vzotova · 2023-09-14T20:01:39Z

tests/acceptance/conftest.py

    )
-    blockchain = pre_application_agent.blockchain
+    blockchain = taco_application_agent.blockchain
+    minimum_stake = (


In some places it called min_authorization, would be great to have same names everywhere

👍 - we can address in #3227 .

Update newsfragment for nucypher#3213.

Update newsfragment for #3213.

KPrasch mentioned this pull request Aug 30, 2023

[WIP] Fix tests updating them to new nucypher contracts #3219

Closed

7 tasks

derekpierre mentioned this pull request Sep 7, 2023

Tweaks for Alpha 12 nucypher/nucypher-contracts#116

Merged

derekpierre reviewed Sep 7, 2023

View reviewed changes

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

KPrasch and others added 26 commits September 7, 2023 18:27

newsfragment for PR nucypher#3213

5bdb6a8

modifies ape -> nucypher registry adapter function to handle both loc…

0e96f5c

…al and dependency contract ABI entries.

re-establish staking providers authorization

c0c77b3

initial steps to establish on-chain ritual access control via GlobalA…

2342f7f

…llowList in tests

Pathfinding an entry point for ferveo publick key commitments in Ursu…

d121b1c

…la; Supporting G2 point class

Fix mispelled constant.

2e47626

Update CoordinatorAgent.G2Point code to create FerveoPublicKey.

beb3b40

Modify nucypher-contracts repos being used so that concurrent changes…

1cd23c2

… can be made while testing.

Update StakeInfo contract with operator address and authorized amount.

2e255a7

Fix parameters to Coordinator contract in ape configuration file.

f0908d5

Modifications to make some coordinator agent acceptance tests pass.

4534a5a

Use random global allow list value for ritualist unit test.

dc68949

Fix MockCoordinatorAgent to properly handle setting/getting provider …

f0e64f2

…public keys. Move G2Point out of Ritual in CoordinatorAgent.

Accomodate obtaining dkg threshold from Coordinator contract/Ritual o…

81121bc

…bject. MockCoordinatorAgent simply uses a class method to determine threshold (allows for both simple/precomputed) to still be tested in integration tests.

initiator -> authority for round 1

1fa4641

use RitualToken for ritual init

c7b1990

g2 point varible style fix

949558a

commentary about checking for on-chain ferveo keys on node launch.

f67822b

update the Ritual stuct to sync with Corrdinator alpha 12

83f0594

deployer account fixture

c005ee8

assorted 'initiator' -> 'authority'

56486f7

setStakes -> authorizationIncreased

9227835

handle fees, authority, and access controller in several tests

0c69c14

Add required arguments for tests using MockCoordinator given the new …

3a98c38

…values stored by the Ritual struct.

Substitute address index for actual deployer test account at index 0 …

9aa1e31

…based on test mnemonic.

derekpierre force-pushed the alpha-12 branch from cb9c262 to 9aa1e31 Compare September 7, 2023 22:44

derekpierre changed the base branch from development to contracts-alpha-13 September 12, 2023 17:00

derekpierre requested review from arjunhassard, cygnusv, jMyles, manumonti, piotr-roslaniec, theref and vzotova September 12, 2023 17:00

derekpierre marked this pull request as ready for review September 12, 2023 17:00

This was referenced Sep 12, 2023

[EPIC] Coordinator Alpha 13 Support #3226

Merged

Coordinator Alpha 13 Support #3227

Merged

KPrasch commented Sep 13, 2023

View reviewed changes

derekpierre approved these changes Sep 13, 2023

View reviewed changes

cygnusv reviewed Sep 13, 2023

View reviewed changes

manumonti previously requested changes Sep 14, 2023

View reviewed changes

Apply suggestions from review for nucypher#3213.

894dc5a

derekpierre requested a review from manumonti September 14, 2023 14:14

manumonti approved these changes Sep 14, 2023

View reviewed changes

piotr-roslaniec reviewed Sep 14, 2023

View reviewed changes

piotr-roslaniec approved these changes Sep 14, 2023

View reviewed changes

vzotova reviewed Sep 14, 2023

View reviewed changes

vzotova approved these changes Sep 14, 2023

View reviewed changes

derekpierre merged commit 84d8344 into nucypher:contracts-alpha-13 Sep 14, 2023

derekpierre added a commit to derekpierre/nucypher that referenced this pull request Sep 20, 2023

Add newsfragment for nucypher#3227.

83348ec

Update newsfragment for nucypher#3213.

derekpierre mentioned this pull request Sep 20, 2023

v7.0.0 Major feature freeze #3200

Closed

KPrasch added a commit that referenced this pull request Nov 30, 2023

newsfragment for PR #3213

7d54cc0

KPrasch pushed a commit that referenced this pull request Nov 30, 2023

Apply suggestions from review for #3213.

10af99b

KPrasch pushed a commit that referenced this pull request Nov 30, 2023

Add newsfragment for #3227.

fc7bb3d

Update newsfragment for #3213.

derekpierre mentioned this pull request Feb 14, 2024

Source ritual validator public keys from Coordinator contract nucypher/taco-web#234

Closed

Conversation

KPrasch commented Aug 28, 2023 • edited by derekpierre Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

KPrasch left a comment

Choose a reason for hiding this comment

Uh oh!

derekpierre left a comment

Choose a reason for hiding this comment

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!

Choose a reason for hiding this comment

Uh oh!

manumonti left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

manumonti left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

derekpierre Sep 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

KPrasch commented Aug 28, 2023 •

edited by derekpierre

Loading

derekpierre Sep 14, 2023 •

edited

Loading