Bqeth testing and usage script #289

theref · 2024-07-09T14:11:24Z

Closes https://github.com/orgs/nucypher/projects/55/views/1?pane=issue&itemId=69683595

wip: change of plans was a mistakerino

`ManagedAllowList`

Co-authored-by: Manuel Montenegro <manuel@nucypher.com>

Co-authored-by: David Núñez <david@nucypher.com>

manumonti

LGTM! 👌

Final params for BqETHSubscription deployment on mainnet

Add mainnet deployment artifacts for BqETH

scripts/bqeth_subscription.py

KPrasch · 2024-08-05T09:48:42Z

scripts/bqeth_subscription.py

+
+# Load environment variables
+load_dotenv()
+PROVIDER_URL = os.environ.get("PROVIDER_URL")


This can be a CLI parameter instead of an env-var. This may alleviate some potential issues when changing providers between mainnet and testnet while testing. Also, if you want to keep the env var functionality, click provides some out-of-the-box functionality for this:

https://click.palletsprojects.com/en/8.1.x/options/#values-from-environment-variables
https://click.palletsprojects.com/en/8.1.x/options/#dynamic-defaults-for-prompts

scripts/bqeth_subscription.py

KPrasch · 2024-08-05T09:57:29Z

scripts/bqeth_subscription.py

+    tx_hash = context.subscription_contract.functions.payForEncryptorSlots(extra_slots).transact(
+        {"from": context.account.address}
+    )


Does this function also imply the transfer of ERC20 fees to the subscription contract?

If so - we may need to include ERC20 approval here also?

Perhaps this is related to how you handle MAX_NODES above as a means of "pre-approving" an ERC20 amount?

KPrasch · 2024-08-05T09:58:18Z

scripts/bqeth_subscription.py

+def initiate_ritual(context, num_nodes, duration, random_seed, num_rituals):
+    """Initiate a ritual."""
+    if NUCYPHER_DOMAIN == "mainnet":
+        porter_request = f"https://porter.nucypher.community/bucket_sampling?quantity={num_nodes}&random_seed={random_seed}"


Instead of URL-encoding in a string template, requests does have the ability to accept a dictionary of params.

KPrasch · 2024-08-05T09:58:34Z

scripts/bqeth_subscription.py

+    click.echo(f"Account address that will sign the transaction: {context.account.address}")
+    click.echo(f"Initiating ritual with {num_nodes} providers for {duration} seconds.")
+    click.echo(f"Endpoint used: {PROVIDER_URL}")


These can be combined into a single expression.

KPrasch · 2024-08-05T09:59:21Z

scripts/bqeth_subscription.py

+        tx_hash = context.coordinator_contract.functions.initiateRitual(
+            context.subscription_contract.address,
+            nodes,
+            context.account.address, # TODO: parametrize this


This is the authority address of the ritual. For production use-cases this will not be the same as the initiator so we need to parameterize this.

KPrasch · 2024-08-05T10:04:08Z

scripts/bqeth_subscription.py

+@click.option("--random-seed", help="Random seed integer for sampling.")
+@click.option("--num-rituals", default=1, help="Number of rituals to initiate.", type=int)
+@setup_context
+def initiate_ritual(context, num_nodes, duration, random_seed, num_rituals):


This function has a sufficiently different scope than subscription payment/management and will be executed by a different permissioned role. IMO this command can be removed from this PR entirely. In fact, there is already a ritual initiation script in this same directory.

- Use typed syntax instead of named tuple - CHAIN is now derived from domain - domain is a cli argument - dotenv overrides each time the script is used - MAX NODES is removed

KPrasch · 2024-08-07T07:52:19Z

scripts/bqeth_subscription.py

+def build_constants(domain, provider_url, private_key):
+    DOMAIN_TO_CHAIN = {"lynx": "80002", "tapir": "80002", "mainnet": "137"}
+    chain = DOMAIN_TO_CHAIN[domain]
+    PORTER_ENDPOINT = f"https://porter-{domain}.nucypher.community"


Will this work on mainnet given the url is porter.nucypher.io (without hyphen)?

Let's use the .io domain instead of the .community domain (community is planed for deprecation) https://porter.nucypher.io

Consider using the PORTER_SAMPLING_ENDPOINTS constant defined in `deployments/constants.py

KPrasch · 2024-08-07T07:56:47Z

scripts/bqeth_subscription.py

+
+
+@cli.command()
+@click.option("--num-nodes", default=2, help="Number of nodes to use for the ritual.")


IMO this can be required without a default since each ritual is intentionally created for a specific purpose.

Suggested change

@click.option("--num-nodes", default=2, help="Number of nodes to use for the ritual.")

@click.option("--num-nodes", required=True, help="Number of nodes to use for the ritual.")

KPrasch · 2024-08-07T07:58:15Z

scripts/bqeth_subscription.py

+        [
+            u["checksum_address"]
+            for u in requests.get(
+                f"{context.constants['PORTER_ENDPOINT']}/get_ursulas?quantity={num_nodes}"


Do you still intend to use the bucket sampling endpoint for mainnet - or is this script only meant for testnet now?

KPrasch · 2024-08-07T07:58:59Z

scripts/bqeth_subscription.py

+    nodes = list(sorted(
+        [
+            u["checksum_address"]
+            for u in requests.get(
+                f"{context.constants['PORTER_ENDPOINT']}/get_ursulas?quantity={num_nodes}"
+            ).json()["result"]["ursulas"]
+        ]
+    ))


This comprehension has grown beyond readability and will be more maintainable if it's broken up into a few variables instead.

KPrasch · 2024-08-07T08:01:03Z

scripts/bqeth_subscription.py

+    start_of_subscription = context.subscription_contract.functions.startOfSubscription().call()
+    duration = (
+        context.subscription_contract.functions.subscriptionPeriodDuration().call()
+        + context.subscription_contract.functions.yellowPeriodDuration().call()
+        + context.subscription_contract.functions.redPeriodDuration().call()
+    )


Ah - This is very interesting functionality! Just making sure I understand correctly it looks like you are correlating a ritual's duration with the specific temporal rules of a subscription?

@theref Can you make a commit into PR #297 to include these calculations?

#297 is merged now but it will still be good to include these changes in initiate_ritual.py @theref Do you want to take care of that?

scripts/bqeth_subscription.py

KPrasch · 2024-08-07T08:08:27Z

scripts/bqeth_subscription.py

+
+
+def build_constants(domain, provider_url, private_key):
+    DOMAIN_TO_CHAIN = {"lynx": "80002", "tapir": "80002", "mainnet": "137"}


There are string constants for these domain strings in deployments/constants.py

Suggested change

DOMAIN_TO_CHAIN = {"lynx": "80002", "tapir": "80002", "mainnet": "137"}

DOMAIN_TO_CHAIN = {LYNX: "80002", TAPIR: "80002", MAINNET: "137"}

KPrasch · 2024-08-07T08:10:17Z

scripts/bqeth_subscription.py

+        "ERC20_CONTRACT_ADDRESS": registry[chain][f"{domain.title()}RitualToken"]["address"],
+        "ERC20_CONTRACT_ABI": registry[chain][f"{domain.title()}RitualToken"]["abi"],


Is this script intended only for testnets (absolutely fine if it is given the name of the PR is "testing")?

theref · 2024-08-07T12:36:32Z

@KPrasch do we still need this with #299 ?

KPrasch · 2024-08-07T12:49:09Z

@KPrasch do we still need this with #299 ?

Do you mean this entire PR? This script broke the ice for testnet fee model deployments but I'm in favor of closing this PR as long as you feel that all of the useful bits have been included elsewhere. Also, I want to acknowledge that #299 is based off your initial work here (I made sure to include you as a commit author).

That being said (almost) all of the functionality implemented in this PR has been implemented in #299 and #297. The main difference is that this PR has fewer dependencies (#299 heavily depends on eth-ape abstractions). Personally, I do not view that as a major issue. There are a few interesting bits of logic here, particularly with regard to ritual duration calculation that I'd like to understand better so we may possibly include the same changes in #297. (see #289 (comment))

Additional thoughts?

KPrasch · 2024-08-15T13:30:58Z

This branch was closed by automation after deleting the epic-subscription branch. I suggest making a new PR into main for any additional follow-ups.

cygnusv and others added 30 commits April 15, 2024 21:35

Add simple hooks to GlobalAllowList to allow simple extensibility

5526814

wip: managed allow list

8693901

wip: add some tests

0205594

wip: change of plans

f9ba98d

wip: change of plans was a mistakerino

Generalize modifier to set authorizations in GlobalAllowList

8130996

ManagedAllowList now inherits from GlobalAllowList

6eca6cc

Rename allowances in ManagedAllowList

720e83c

Fix tests for ManagedAllowList

9e42114

Draft for Subscriptions and how can be used with AllowLists

ee85f5e

wip: change of plans was a mistakerino

ab6db0a

wip: subscription manager draft

e3d996f

wip: add way too much comments

d82db48

wip: more tests, yes yes

c5d981f

wip: test test test

070ca81

wip: fix solhint

d684c6c

wip: add subscription tests draft

8cef8bb

wip: more subscription tests

e387159

apply pr suggestions

08fe8a4

apply pr suggestions

dbe1ea2

apply pr suggestions

98dc648

fix solhint failing silently on ci

138d716

remove solidity oopsies

7da09ad

Initial commit

f9298c4

Merge pull request nucypher#255 from piotr-roslaniec/managed-allow-list

df671df

`ManagedAllowList`

Extracting fee model from Coordinator

c99e06b

Moves global allow list test to the separate file

e6cb43e

FlatRateFeeModel: restrict payment method

ee824b5

Minor renames

8ca37ec

Apply suggestions from code review

2a08260

Co-authored-by: Manuel Montenegro <manuel@nucypher.com>

Return back storing accessControl in Coordinator

fa27729

Co-authored-by: David Núñez <david@nucypher.com>

cygnusv and others added 6 commits July 16, 2024 15:52

On mainnet, adopter setter will be NuCo

a7de9b0

BqETH subscription period is 183 days

ecdac75

Actually use bqeth deployment parameters!

ab75978

Remove beta program initiator parameters

27d762c

Add max nodes param to bqeth deployment file

2ed370f

Add mainnet deployment artifacts for BqETH

8966bd6

manumonti approved these changes Jul 17, 2024

View reviewed changes

manumonti and others added 8 commits July 17, 2024 17:05

Add Free Fee Model contract deployment script

edd31ed

Add mainnet deployment artifact of FreeFeeModel

8616609

Merge pull request nucypher#294 from cygnusv/final-params

262a3f6

Final params for BqETHSubscription deployment on mainnet

Merge pull request nucypher#295 from manumonti/bqeth-mainnet-deployments

7bb50e8

Add mainnet deployment artifacts for BqETH

Sort list of nodes after fetching from porter

690f2a3

Decrease duration of ritual when subscription has already started

fdd7aee

Run black formatter on bqeth screen

8bcf5ad

Merge branch 'epic-subscription' into bqeth_script

35c599d

derekpierre reviewed Jul 24, 2024

View reviewed changes

scripts/bqeth_subscription.py Outdated Show resolved Hide resolved

KPrasch mentioned this pull request Aug 5, 2024

Ape-based generic subscription and authorization management CLI #299

Merged

KPrasch reviewed Aug 5, 2024

View reviewed changes

KPrasch requested a review from vzotova August 5, 2024 10:00

KPrasch reviewed Aug 5, 2024

View reviewed changes

theref force-pushed the bqeth_script branch from 7d3612c to 35c599d Compare August 6, 2024 13:32

Address PR review requests

aef0976

- Use typed syntax instead of named tuple - CHAIN is now derived from domain - domain is a cli argument - dotenv overrides each time the script is used - MAX NODES is removed

KPrasch reviewed Aug 7, 2024

View reviewed changes

derekpierre force-pushed the epic-subscription branch from 0dc8055 to 05b960e Compare August 9, 2024 17:42

KPrasch deleted the branch nucypher:epic-subscription August 15, 2024 13:28

KPrasch closed this Aug 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bqeth testing and usage script #289

Bqeth testing and usage script #289

theref commented Jul 9, 2024 •

edited

Loading

manumonti left a comment

KPrasch Aug 5, 2024

KPrasch Aug 5, 2024 •

edited

Loading

KPrasch Aug 5, 2024

KPrasch Aug 5, 2024

KPrasch Aug 5, 2024

KPrasch Aug 5, 2024

KPrasch Aug 7, 2024 •

edited

Loading

KPrasch Aug 7, 2024

KPrasch Aug 7, 2024

KPrasch Aug 7, 2024

KPrasch Aug 7, 2024

KPrasch Aug 12, 2024

KPrasch Aug 15, 2024

KPrasch Aug 7, 2024

KPrasch Aug 7, 2024 •

edited

Loading

theref commented Aug 7, 2024

KPrasch commented Aug 7, 2024 •

edited

Loading

KPrasch commented Aug 15, 2024



		@cli.command()
		@click.option("--num-nodes", default=2, help="Number of nodes to use for the ritual.")

	@click.option("--num-nodes", default=2, help="Number of nodes to use for the ritual.")
	@click.option("--num-nodes", required=True, help="Number of nodes to use for the ritual.")



		def build_constants(domain, provider_url, private_key):
		DOMAIN_TO_CHAIN = {"lynx": "80002", "tapir": "80002", "mainnet": "137"}

	DOMAIN_TO_CHAIN = {"lynx": "80002", "tapir": "80002", "mainnet": "137"}
	DOMAIN_TO_CHAIN = {LYNX: "80002", TAPIR: "80002", MAINNET: "137"}

		"ERC20_CONTRACT_ADDRESS": registry[chain][f"{domain.title()}RitualToken"]["address"],
		"ERC20_CONTRACT_ABI": registry[chain][f"{domain.title()}RitualToken"]["abi"],

Bqeth testing and usage script #289

Bqeth testing and usage script #289

Conversation

theref commented Jul 9, 2024 • edited Loading

manumonti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KPrasch Aug 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KPrasch Aug 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KPrasch Aug 7, 2024 • edited Loading

Choose a reason for hiding this comment

theref commented Aug 7, 2024

KPrasch commented Aug 7, 2024 • edited Loading

KPrasch commented Aug 15, 2024

theref commented Jul 9, 2024 •

edited

Loading

KPrasch Aug 5, 2024 •

edited

Loading

KPrasch Aug 7, 2024 •

edited

Loading

KPrasch Aug 7, 2024 •

edited

Loading

KPrasch commented Aug 7, 2024 •

edited

Loading