Signing Cohort Formation #393
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new CLI command for forming a signing cohort and updates the constants to support testnet providers.
- Added a new CLI command "form-signing-cohort" in scripts/form_signing_cohort.py.
- Introduced the TESTNET_PROVIDERS mapping in deployment/constants.py to support different domains.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/form_signing_cohort.py | Added command line options and transaction initiation logic |
| deployment/constants.py | Added TESTNET_PROVIDERS constant mapping |
Comments suppressed due to low confidence (1)
scripts/form_signing_cohort.py:70
- The domain parameter is validated using click.Choice(SUPPORTED_TACO_DOMAINS) but is later restricted to LYNX and TAPIR. Ensure that the allowed domain values remain consistent across the code.
if domain not in (LYNX, TAPIR):
scripts/form_signing_cohort.py
Outdated
| threshold, | ||
| chain_id, | ||
| auto, | ||
| condition_file, |
There was a problem hiding this comment.
The 'condition_file' parameter is defined but not utilized within the function body. Consider either removing it or adding logic to handle the condition configuration if intended.
There was a problem hiding this comment.
^ Guess we can remove condition_file.
Adding the condition can be done in a separate script - I don't think you can add the conditions until the cohort is deemed active i.e. the ritual is completed.
scripts/form_signing_cohort.py
Outdated
| "--duration", | ||
| "-t", | ||
| help="Duration of the cohort in seconds. Must be at least 24h.", | ||
| type=MinInt(1), |
There was a problem hiding this comment.
This is in seconds i.e.
| type=MinInt(1), | |
| type=MinInt(60*60*24), |
scripts/form_signing_cohort.py
Outdated
| if domain not in (LYNX, TAPIR): | ||
| raise click.ClickException(f"Unsupported domain: {domain}. Supported domains are: {SUPPORTED_TACO_DOMAINS}") | ||
| providers = TESTNET_PROVIDERS[domain] |
There was a problem hiding this comment.
(LYNX, TAPIR) is different from SUPPORTED_TACO_DOMAINS (which includes mainnet), right? So a bit of a contradiction here.
scripts/form_signing_cohort.py
Outdated
| threshold, | ||
| chain_id, | ||
| auto, | ||
| condition_file, |
There was a problem hiding this comment.
^ Guess we can remove condition_file.
Adding the condition can be done in a separate script - I don't think you can add the conditions until the cohort is deemed active i.e. the ritual is completed.
| transactor = Transactor(account=account, autosign=auto) | ||
| signing_coordinator = registry.get_contract(domain=domain, contract_name="SigningCoordinator") | ||
|
|
||
| result = transactor.transact( |
| raise click.ClickException("Condition file is empty or not provided.") | ||
|
|
||
| if domain not in (LYNX, TAPIR): | ||
| raise click.ClickException(f"Unsupported domain: {domain}. Supported domains are: {SUPPORTED_TACO_DOMAINS}") |
There was a problem hiding this comment.
Same comment here about mismatch between SUPPORTED_TACO_DOMAINS (which includes mainnet) and (LYNX, TAPIR).
| print(f"Setting conditions for cohort {cohort_id} on {domain}:{network} with chain ID {chain_id}") | ||
|
|
||
| transactor = Transactor(account=account, autosign=auto) | ||
| signing_coordinator = registry.get_contract(domain=domain, contract_name="SigningCoordinator") |
2 participants
Type of PR:
Feature
Required reviews:
1