add mainnet examples, readme and configs #97
Conversation
|
there is an issue in post.py file because the networkConfig needs to be passed pointing either to testnet or mainnet again, where it should be done only once in the main python file to be executed |
| "price": 40000, | ||
| "size": 0.01, | ||
| "client_id": randrange(0, MAX_CLIENT_ID), | ||
| "good_till_block": client.get_current_block() + 11, |
There was a problem hiding this comment.
Please change to good_til_block
| price=order["price"], | ||
| size=order["size"], | ||
| client_id=order["client_id"], | ||
| good_til_block=order["good_till_block"], |
There was a problem hiding this comment.
Please do a global search and replace from good_till_block to good_til_block
| """Network configurations.""" | ||
|
|
||
| import warnings | ||
| from dataclasses import dataclass | ||
| from typing import Optional, Union | ||
|
|
||
| from v4_client_py.clients.constants import ( |
There was a problem hiding this comment.
We do not handle main net deployment. Thus, there should not be any reference to mainNet in the code base. Also, consumer of the library should have the freedom to pick the validator and indexer endpoints to connect to.
My suggestion, is to add this convenient function to Network class
@classmethod
def config_network(
grpc_endpoint: str,
rest_endpoint: str,
websocket_endpoint: str,
chain_id: str,
) -> Network:
validator_config=ValidatorConfig(
grpc_endpoint=grpc_endpoint,
chain_id=chain_id,
ssl_enabled=True
)
indexer_config=IndexerConfig(
rest_endpoint=rest_endpoint,
websocket_endpoint=websocket_endpoint,
)
return Network(
env='custom',
validator_config=validator_config,
indexer_config=indexer_config,
)
Then in documentation (or in mainnet example file), replace
network = Network.testnet()
With
network = Network.config_network(
grpc_endpoint = '{get from deployer}',
rest_endpoint= '{get from deployer}',
websocket_endpoint = '{get from deployer}',
chain_id = '{get from deployer}',
)
| @@ -0,0 +1,20 @@ | |||
| # User guide to test mainnet examples | |||
|
|
|||
There was a problem hiding this comment.
We need an #integration section in README.md, probably under #installation, with the simple instructions on how to use the code.
Essentially it should cover two things to get it started
- Set up network configuration
network = Network.config_network(
grpc_endpoint = '{get from deployer}',
rest_endpoint= '{get from deployer}',
websocket_endpoint = '{get from deployer}',
chain_id = '{get from deployer}',
)
- Connect to client
client = CompositeClient(
network,
)
There was a problem hiding this comment.
we still have the problem of the config.py for the NetworkConfig class that needs to be defined
|
hi @johnqh |
| AERIAL_GRPC_OR_REST_PREFIX = '{get from deployer}' | ||
| INDEXER_REST_ENDPOINT = '{get from deployer}' | ||
| INDEXER_WS_ENDPOINT = '{get from deployer}' | ||
| CHAIN_ID = '{get from deployer}' |
There was a problem hiding this comment.
We still have miscommunications.
Those shouldn't be in the library const. A library should be used by installing with pypi. When you have consts which requires modifications, the consumer of this library will have to download the source code of the library, change these consts, and integrate the library source code into their code directly instead of pypi.
dYdX Trading Inc runs a testNet so it is OK to have testnet configurations in the library. However, we are not deployer of mainNet so the library do not have reference to mainNet.
My previous comment is all about providing a clear pathway to use this library through pypi for deployer's mainNet.
Of course, if this library is distributed by dYdX Foundation, it is totally OK to have their mainnet configs here since they are the deployer. But dYdX Trading Inc is not the deployer and we can only support custom configurations.
| ORDER_STATUS_FILLED = 'FILLED' | ||
| ORDER_STATUS_CANCELED = 'CANCELED' | ||
| ORDER_STATUS_UNTRIGGERED = 'UNTRIGGERED' | ||
| ORDER_STATUS_PENDING = "PENDING" |
There was a problem hiding this comment.
It seems that you have a custom lint configurations? Can you have the lint configuration somewhere? Different lint will just cause the source to switch between single quotes and double quotes between commits.
| @@ -148,3 +103,18 @@ def latest_stable_testnet(cls) -> "NetworkConfig": | |||
| DeprecationWarning, | |||
| ) | |||
| return cls.fetch_dydx_stable_testnet() | |||
|
|
|||
| @classmethod | |||
| def fetchai_network_config(cls, chain_id: str, url_prefix: str, url: str) -> "NetworkConfig": | |||
There was a problem hiding this comment.
Let's not use FetchAI in our code. The original code (code in chain/aerial) was a folk from fetchai/cosmpy and their low level code has some references to fetchai (which we should remove), but our high level code should never refer fetchai. This only causes confusion.
|
Is the other PR from your company? #106 That PR handles the network configs well. If you roll this branch back to the commit point of previous PR, incorporate the network config files from the other PR, then it is almost there. I think we only needed change from "good_till_time" to "good_til_time" and some readme comments to be good. |
|
Please address @johnqh's comments and I can approve. |
|
|
||
|
|
||
| # ------------ API URLs ------------ | ||
| INDEXER_API_HOST_MAINNET = None |
There was a problem hiding this comment.
it would be great if there is one example for mainnet, because examples work great on testnet but then all kind of DNS issues come up for mainnet
No description provided.