Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compute env vars #444

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions frontend/client/types.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { StakingProgramId } from '@/enums/StakingProgram';
import { Address } from '@/types/Address';

import { DeploymentStatus, Ledger, MiddlewareChain, EnvProvisionType } from './enums';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't use EnvProvisionType from enums.ts then can we remove it from there? I'm guessing it's not used anywhere else

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, was using the wrong enum in the file.

import { DeploymentStatus, Ledger, MiddlewareChain } from './enums';

export type ServiceHash = string;

Expand Down Expand Up @@ -50,9 +50,10 @@ export type Service = {
};


export type ServiceEnvVariable = {
export type EnvProvisionType = "fixed" | "user" | "computed";

export type EnvVariableAttributes = {
name: string;
env_variable_name: string;
description: string;
value: string;
provision_type: EnvProvisionType;
Expand All @@ -66,7 +67,7 @@ export type ServiceTemplate = {
service_version: string;
home_chain_id: string;
configurations: { [key: string]: ConfigurationTemplate };
service_env_variables: { [key: string]: ServiceEnvVariable };
env_variables: { [key: string]: EnvVariableAttributes };
deploy?: boolean;
};

Expand Down
13 changes: 1 addition & 12 deletions frontend/constants/serviceTemplates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,80 +29,69 @@ export const SERVICE_TEMPLATES: ServiceTemplate[] = [
},
},
},
service_env_variables: {
env_variables: {
GNOSIS_LEDGER_RPC: {
name: "Gnosis ledger RPC",
env_variable_name: "GNOSIS_LEDGER_RPC",
description: "",
value: "",
provision_type: EnvProvisionType.COMPUTED
},
// ETHEREUM_LEDGER_RPC: {
// name: "Ethereum ledger RPC",
// env_variable_name: "ETHEREUM_LEDGER_RPC",
// description: "",
// value: "",
// provision_type: EnvProvisionType.COMPUTED
// },
// BASE_LEDGER_RPC: {
// name: "Base ledger RPC",
// env_variable_name: "BASE_LEDGER_RPC",
// description: "",
// value: "",
// provision_type: EnvProvisionType.COMPUTED
// },
// OPTIMISM_LEDGER_RPC: {
// name: "Optimism ledger RPC",
// env_variable_name: "OPTIMISM_LEDGER_RPC",
// description: "",
// value: "",
// provision_type: EnvProvisionType.COMPUTED
// },
STAKING_CONTRACT_ADDRESS: {
name: "Staking contract address",
env_variable_name: "STAKING_CONTRACT_ADDRESS",
description: "",
value: "",
provision_type: EnvProvisionType.COMPUTED
},
MECH_ACTIVITY_CHECKER_CONTRACT: {
name: "Mech activity checker contract",
env_variable_name: "MECH_ACTIVITY_CHECKER_CONTRACT",
description: "",
value: "",
provision_type: EnvProvisionType.COMPUTED
},
MECH_CONTRACT_ADDRESS: {
name: "Mech contract address",
env_variable_name: "MECH_CONTRACT_ADDRESS",
description: "",
value: "",
provision_type: EnvProvisionType.COMPUTED
},
MECH_REQUEST_PRICE: {
name: "Mech request price",
env_variable_name: "MECH_REQUEST_PRICE",
description: "",
value: "",
provision_type: EnvProvisionType.COMPUTED
},
USE_MECH_MARKETPLACE: {
name: "Use Mech marketplace",
env_variable_name: "USE_MECH_MARKETPLACE",
description: "",
value: "",
provision_type: EnvProvisionType.COMPUTED
},
REQUESTER_STAKING_INSTANCE_ADDRESS: {
name: "Requester staking instance address",
env_variable_name: "REQUESTER_STAKING_INSTANCE_ADDRESS",
description: "",
value: "",
provision_type: EnvProvisionType.COMPUTED
},
PRIORITY_MECH_ADDRESS: {
name: "Priority Mech address",
env_variable_name: "PRIORITY_MECH_ADDRESS",
description: "",
value: "",
provision_type: EnvProvisionType.COMPUTED
Expand Down
9 changes: 4 additions & 5 deletions operate/operate_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,26 +203,25 @@ class ConfigurationTemplate(TypedDict):
fallback_chain_params: t.Optional[t.Dict]


class ServiceEnvProvisionType(enum.Enum):
class ServiceEnvProvisionType(str, enum.Enum):
"""Service environment variable provision type."""

FIXED = "fixed"
USER = "user"
COMPUTED = "computed"


class ServiceEnvVariable(TypedDict):
class EnvVariableAttributes(TypedDict):
"""Service environment variable template."""

name: str
env_variable_name: str
description: str
value: str
provision_type: ServiceEnvProvisionType


ConfigurationTemplates = t.Dict[str, ConfigurationTemplate]
ServiceEnvVariables = t.Dict[str, ServiceEnvVariable]
EnvVariables = t.Dict[str, EnvVariableAttributes]


class ServiceTemplate(TypedDict):
Expand All @@ -235,7 +234,7 @@ class ServiceTemplate(TypedDict):
service_version: str
home_chain_id: str
configurations: ConfigurationTemplates
service_env_variables: ServiceEnvVariables
env_variables: EnvVariables


@dataclass
Expand Down
6 changes: 1 addition & 5 deletions operate/services/health_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,11 +174,7 @@ def _do_restart() -> None:
service_manager.stop_service_locally(
service_config_id=service_config_id
)
# TODO Optimus patch, chain_id="10"
chain_id = "10"
service_manager.deploy_service_locally(
service_config_id=service_config_id, chain_id=chain_id
)
service_manager.deploy_service_locally(service_config_id=service_config_id)

loop = asyncio.get_event_loop()
with ThreadPoolExecutor() as executor:
Expand Down
67 changes: 48 additions & 19 deletions operate/services/manage.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,12 @@
from operate.keys import Key, KeysManager
from operate.ledger import PUBLIC_RPCS
from operate.ledger.profiles import CONTRACTS, OLAS, STAKING
from operate.operate_types import ChainType, LedgerConfig, ServiceTemplate
from operate.operate_types import (
ChainType,
LedgerConfig,
ServiceEnvProvisionType,
ServiceTemplate,
)
from operate.services.protocol import EthSafeTxBuilder, OnChainManager, StakingState
from operate.services.service import (
ChainConfig,
Expand Down Expand Up @@ -232,6 +237,44 @@ def create(

return service

def _compute_service_env_variables(
self, service: Service, staking_params: t.Dict[str, str]
) -> None:
"""Compute values to override service.yaml variables for the deployment."""

# TODO A customized, arbitrary computation mechanism should be devised.
computed_values = {
"ETHEREUM_LEDGER_RPC": PUBLIC_RPCS[ChainType.ETHEREUM],
Copy link
Collaborator

@truemiller truemiller Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally we can remove these hardcode rpcs from middleware at some point
and instead use valory endpoints from .env, the current PUBLIC_RPCS are public/rate-limited
may be inconsistent with tenderly forks, if they're not overwritten
may be sync issues with rpcs frontend calls & middleware calls

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, there is a refactor task to do in the RPCs. Surprisingly "PUBLIC_RPCs" here are overriden by the environment variables GNOSIS_DEV_RPC, etc.

This is task of another PR, though.

"GNOSIS_LEDGER_RPC": PUBLIC_RPCS[ChainType.GNOSIS],
"BASE_LEDGER_RPC": PUBLIC_RPCS[ChainType.BASE],
"OPTIMISM_LEDGER_RPC": PUBLIC_RPCS[ChainType.OPTIMISM],
"STAKING_CONTRACT_ADDRESS": staking_params.get("staking_contract"),
"MECH_ACTIVITY_CHECKER_CONTRACT": staking_params.get("activity_checker"),
"MECH_CONTRACT_ADDRESS": staking_params.get("agent_mech"),
"MECH_REQUEST_PRICE": "10000000000000000",
"USE_MECH_MARKETPLACE": "mech_marketplace"
in service.chain_configs[
service.home_chain_id
].chain_data.user_params.staking_program_id,
Comment on lines +255 to +258
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@0xArdi Is this approach correct? I mean, if the service is staked in multiple chains, and we could have different combinations of mech marketplace or not, what value should this environment variable have? Currently it is set to true if the home chain staking contract is a mech_marketplace one.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the mech_interact_abci only allows for the agent to work with the mech in a single chain.
This param here might tell us what chain the mech is meant to be used on. Perhaps that needs to be checked instead of assuming its always the home_chain_id.

"REQUESTER_STAKING_INSTANCE_ADDRESS": staking_params.get(
"staking_contract"
),
"PRIORITY_MECH_ADDRESS": staking_params.get("agent_mech"),
}

updated = False
for env_var, attributes in service.env_variables.items():
if (
env_var in computed_values
and attributes["provision_type"] == ServiceEnvProvisionType.COMPUTED
and attributes["value"] != computed_values.get(env_var)
):
attributes["value"] = str(computed_values.get(env_var))
updated = True

if updated:
service.store()

def _get_on_chain_state(self, service: Service, chain_id: str) -> OnChainState:
chain_config = service.chain_configs[chain_id]
chain_data = chain_config.chain_data
Expand Down Expand Up @@ -531,23 +574,7 @@ def _deploy_service_onchain_from_safe( # pylint: disable=too-many-statements,to
agent_mech="0x77af31De935740567Cf4fF1986D04B2c964A786a", # nosec
)

# Override service.yaml variables for the deployment
os.environ["STAKING_CONTRACT_ADDRESS"] = staking_params["staking_contract"]
os.environ["MECH_ACTIVITY_CHECKER_CONTRACT"] = staking_params[
"activity_checker"
]
os.environ["MECH_CONTRACT_ADDRESS"] = staking_params["agent_mech"]
os.environ["MECH_REQUEST_PRICE"] = "10000000000000000"
os.environ["USE_MECH_MARKETPLACE"] = str(
chain_data.user_params.use_mech_marketplace
)
os.environ["REQUESTER_STAKING_INSTANCE_ADDRESS"] = staking_params[
"staking_contract"
]
os.environ["PRIORITY_MECH_ADDRESS"] = staking_params["agent_mech"]
os.environ["ETHEREUM_LEDGER_RPC"] = PUBLIC_RPCS[ChainType.ETHEREUM]
os.environ["BASE_LEDGER_RPC"] = PUBLIC_RPCS[ChainType.BASE]
os.environ["OPTIMISM_LEDGER_RPC"] = PUBLIC_RPCS[ChainType.OPTIMISM]
self._compute_service_env_variables(service, staking_params)

if user_params.use_staking:
self.logger.info("Checking staking compatibility")
Expand Down Expand Up @@ -1572,7 +1599,9 @@ def update_all_matching(
def migrate_service_configs(self) -> None:
"""Migrate old service config formats to new ones, if applies."""

bafybei_count = sum(1 for path in self.path.iterdir() if path.name.startswith("bafybei"))
bafybei_count = sum(
1 for path in self.path.iterdir() if path.name.startswith("bafybei")
)
if bafybei_count > 1:
self.log_directories()
# raise RuntimeError(f"Your services folder contains {bafybei_count} folders starting with 'bafybei'. This is an unintended situation. Please contact support.")
Expand Down
13 changes: 7 additions & 6 deletions operate/services/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
OnChainData,
OnChainState,
OnChainUserParams,
ServiceEnvVariables,
EnvVariables,
ServiceTemplate,
)
from operate.resource import LocalResource
Expand Down Expand Up @@ -428,6 +428,7 @@ def _build_docker(
encoding="utf-8",
)
try:
service.consume_env_variables()
builder = ServiceBuilder.from_dir(
path=service.service_path,
keys_file=keys_file,
Expand Down Expand Up @@ -656,7 +657,7 @@ class Service(LocalResource):
home_chain_id: str
chain_configs: ChainConfigs
description: str
service_env_variables: ServiceEnvVariables
env_variables: EnvVariables

path: Path
service_path: Path
Expand Down Expand Up @@ -761,7 +762,7 @@ def migrate_format(cls, path: Path) -> bool:
service_path = Path(data["service_path"])
if service_path.exists() and service_path.is_dir():
shutil.rmtree(service_path)

path = path.rename(new_path)
service_path = Path(
IPFSTool().download(
Expand All @@ -780,8 +781,8 @@ def migrate_format(cls, path: Path) -> bool:

def consume_env_variables(self) -> None:
"""Consume environment variables."""
for variable in self.service_env_variables.values():
os.environ[variable["env_variable_name"]] = str(variable["value"])
for env_var, attributes in self.env_variables.items():
os.environ[env_var] = str(attributes["value"])

@classmethod
def load(cls, path: Path) -> "Service":
Expand Down Expand Up @@ -858,7 +859,7 @@ def new( # pylint: disable=too-many-locals
chain_configs=chain_configs,
path=service_path.parent,
service_path=service_path,
service_env_variables=service_template["service_env_variables"],
env_variables=service_template["env_variables"],
)
service.store()
return service
Expand Down
11 changes: 8 additions & 3 deletions operate/wallet/master.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
"""Master key implementation"""

import json
import logging
import os
import typing as t
import logging
from dataclasses import dataclass, field
from pathlib import Path

Expand Down Expand Up @@ -426,7 +426,7 @@ def add_or_swap_owner(
def load(cls, path: Path) -> "EthereumMasterWallet":
"""Load master wallet."""
# TODO: This is a complex way to read the 'safes' dictionary.
# The reason for that is that wallet.safes[chain_type] would fail
# The reason for that is that wallet.safes[chain_type] would fail
# (for example in service manager) when passed a ChainType key.

raw_ethereum_wallet = super().load(path) # type: ignore
Expand Down Expand Up @@ -467,7 +467,12 @@ def migrate_format(cls, path: Path) -> bool:
class MasterWalletManager:
"""Master wallet manager."""

def __init__(self, path: Path, password: t.Optional[str] = None, logger: t.Optional[logging.Logger] = None) -> None:
def __init__(
self,
path: Path,
password: t.Optional[str] = None,
logger: t.Optional[logging.Logger] = None,
) -> None:
"""Initialize master wallet manager."""
self.path = path
self._password = password
Expand Down
Loading