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

Conversation

jmoreira-valory
Copy link
Collaborator

Proposed changes

Compute env vars

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Comment on lines +255 to +258
"USE_MECH_MARKETPLACE": "mech_marketplace"
in service.chain_configs[
service.home_chain_id
].chain_data.user_params.staking_program_id,
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.

@@ -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.


# 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.

@jmoreira-valory jmoreira-valory merged commit c4a6253 into feat/allow-multiple-services-middleware Nov 13, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants