Skip to content

Conversation

@aarmoa
Copy link
Collaborator

@aarmoa aarmoa commented May 25, 2023

  • Modeled the Markets (SpotMarket, DerivativeMarket and BinaryOptionMarket) and Token.
  • AsynClient initializes list of all markets and tokens, and uses them to create Composer instances.
  • Composer uses the markets and/or tokens instances to translate human-readable values into chain accepted format.
  • Composer is still able to use the information from the configuration files with Denom instances if they are created without passing the list of markets and tokens. That will still be the default behavior for any current SDK user until they migrate their code to start getting the Composer instances from the AsyncClient.
  • All example files have been updated to reflect the new way of creating Composer instances

@aarmoa aarmoa requested a review from achilleas-kal May 25, 2023 22:57

def Trades(self, request, context):
"""Trades gets the trades of a Derivative Market.
"""Trades gets the trades of a Derivative SpotMarket.
Copy link
Member

Choose a reason for hiding this comment

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

Derivative SpotMarket doesnt make sense @aarmoa

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

)
self.Market = channel.unary_unary(
'/injective_derivative_exchange_rpc.InjectiveDerivativeExchangeRPC/Market',
'/injective_derivative_exchange_rpc.InjectiveDerivativeExchangeRPC/SpotMarket',
Copy link
Member

Choose a reason for hiding this comment

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

Did you manually edit these files? These should be auto-generated afaik @aarmoa

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

@aarmoa
Copy link
Collaborator Author

aarmoa commented May 29, 2023

I have analyzed the differences between the current production code (using the Denoms loaded from the .ini files) and the logic using the new Markets and Tokens in this branch in mainnet. The script code is the following:

import asyncio
from decimal import Decimal

import pyinjective.constant as constant
from pyinjective.constant import Denom
from pyinjective.async_client import AsyncClient
from pyinjective.core.market import SpotMarket, DerivativeMarket, BinaryOptionMarket

network = constant.Network.mainnet()
client = AsyncClient(network, insecure=False)

spot_markets = asyncio.get_event_loop().run_until_complete(client.all_spot_markets())
derivative_markets = asyncio.get_event_loop().run_until_complete(client.all_derivative_markets())
binary_option_markets = asyncio.get_event_loop().run_until_complete(client.all_binary_option_markets())
all_tokens = asyncio.get_event_loop().run_until_complete(client.all_tokens())

markets_not_found = []
markets_with_diffs = []
peggy_denoms_not_found = []
peggy_denoms_with_diffs = []

for config_key in constant.mainnet_config:
    if config_key.startswith("0x"):
        denom = Denom.load_market(network=network.string(), market_id=config_key)
        if config_key in spot_markets:
            market: SpotMarket = spot_markets[config_key]
            if (market.base_token.decimals != denom.base
                    or market.quote_token.decimals != denom.quote
                    or market.min_price_tick_size != Decimal(str(denom.min_price_tick_size))
                    or market.min_quantity_tick_size != Decimal(str(denom.min_quantity_tick_size))):
                markets_with_diffs.append([
                    {"denom-market": config_key, "base_decimals": denom.base, "quote_decimals": denom.quote, "min_quantity_tick_size": denom.min_quantity_tick_size, "min_price_tick_size": denom.min_price_tick_size},
                    {"newer-market": market.id, "base_decimals": market.base_token.decimals, "quote_decimals": market.quote_token.decimals, "min_quantity_tick_size": float(market.min_quantity_tick_size), "min_price_tick_size": float(market.min_price_tick_size), "ticker": market.ticker},
                ])
        elif config_key in derivative_markets:
            market: DerivativeMarket = derivative_markets[config_key]
            if (market.quote_token.decimals != denom.quote
                    or market.min_price_tick_size != Decimal(str(denom.min_price_tick_size))
                    or market.min_quantity_tick_size != Decimal(str(denom.min_quantity_tick_size))):
                markets_with_diffs.append([
                    {"denom-market": config_key, "quote_decimals": denom.quote,
                     "min_quantity_tick_size": denom.min_quantity_tick_size,
                     "min_price_tick_size": denom.min_price_tick_size},
                    {"newer-market": market.id, "quote_decimals": market.quote_token.decimals, "min_quantity_tick_size": float(market.min_quantity_tick_size), "min_price_tick_size": float(market.min_price_tick_size), "ticker": market.ticker},
                ])
        elif config_key in binary_option_markets:
            market: BinaryOptionMarket = binary_option_markets[config_key]
            if (market.quote_token.decimals != denom.quote
                    or market.min_price_tick_size != Decimal(str(denom.min_price_tick_size))
                    or market.min_quantity_tick_size != Decimal(str(denom.min_quantity_tick_size))):
                markets_with_diffs.append([
                    {"denom-market": config_key, "quote_decimals": denom.quote,
                     "min_quantity_tick_size": denom.min_quantity_tick_size,
                     "min_price_tick_size": denom.min_price_tick_size},
                    {"newer-market": market.id, "quote_decimals": market.quote_token.decimals, "min_quantity_tick_size": float(market.min_quantity_tick_size), "min_price_tick_size": float(market.min_price_tick_size), "ticker": market.ticker},
                ])
        else:
            markets_not_found.append({"denom-market": config_key, "description": denom.description})

    elif config_key == "DEFAULT":
        continue
    else:
        # the configuration is a token
        peggy_denom, decimals = Denom.load_peggy_denom(network=network.string(), symbol=config_key)
        if config_key in all_tokens:
            token = all_tokens[config_key]
            if (token.denom != peggy_denom
                    or token.decimals != decimals):
                peggy_denoms_with_diffs.append([
                    {"denom_token": config_key, "peggy_denom": peggy_denom, "decimals": decimals},
                    {"newer_token": token.symbol, "peggy_denom": token.denom, "decimals": token.decimals}
                ])
        else:
            peggy_denoms_not_found.append({"denom_token": config_key, "peggy_denom": peggy_denom, "decimals": decimals})

for diff_pair in markets_with_diffs:
    print(f"{diff_pair[0]}\n{diff_pair[1]}")
print(f"\n\n")
for missing_market in markets_not_found:
    print(f"{missing_market}")
print(f"\n\n")
for diff_token in peggy_denoms_with_diffs:
    print(f"{diff_token[0]}\n{diff_token[1]}")
print(f"\n\n")
for missing_peggy in peggy_denoms_not_found:
    print(f"{missing_peggy}")

print("Done!")

The differences founds are the following:

  • Markets in INI file and in the new version, but with differences:
    -- Market GF/USDT
    --- min_price_tick_size is 1e-15 in INI file, 1e-16 in the info loaded from the indexer (current branch logic).
    -- Market STRD/USDT
    --- min_price_tick_size is 0.001 in INI file, 0.0001 in the info loaded from the indexer (current branch logic).
    -- Market SOMM/USDT
    --- min_quantity_tick_size is 10000000 in INI file, 1000000 in the info loaded from the indexer (current branch logic).
    -- Market LDO/USDC
    --- base_decimals is 8 in INI file, 6 in the info loaded from the indexer (current branch logic).
    -- Market STX/USDT PERP
    --- min_price_tick_size is 1000 in INI file, 100 in the info loaded from the indexer (current branch logic).

  • Markets in INI file not found in the new version:
    -- ETH/USDT 19SEP22 and 1000PEPE/USDT PERP. They seem to be markets that no longer exist and they should not be present in the INI file (again here the info in this branch code is more accurate)

  • Tokens in INI file and in the new version, but with differences:
    -- LDO
    --- decimals is 8 in INI file, 6 in the information loaded from the indexer (current branch logic). The denom also differs, but I trust the correct one is the one provided by the indexer

  • Tokens in INI file not found in the new version:
    -- wMATIC
    --- Not found in the new version because the token symbol according to the indexer is WMATIC instead of wMATIC. The denom and decimals of both tokens are identical. The markets using the token have ticker names using WMATIC and not wMATIC, confirming that the correct information is the one provided by the indexer (current branch logic)

After this validation, I think that it is safe to merge this branch into the dev branch and then release it to master. Also the change is safe because the current users will keep using the INI files to load the markets and tokens, until they change their logic to create the Composer using the AsyncClient instead of creating it manually.
What is your opinion about this validation @albertchon @achilleas-kal ?

@aarmoa aarmoa requested a review from albertchon May 29, 2023 16:11
@achilleas-kal
Copy link
Collaborator

be_amount = token.chain_formatted_value(human_readable_value=Decimal(str(amount)))
TypeError: Token.chain_formatted_value() missing 1 required positional argument: 'self'

@aarmoa please run the examples to make sure there's backwards compatibility. We used this script: https://github.com/InjectiveLabs/sdk-python/blob/master/run-examples.sh a while back to run all examples - we can modify and use it moving forward.

Abel Armoa added 2 commits May 30, 2023 01:45
@aarmoa
Copy link
Collaborator Author

aarmoa commented May 30, 2023

be_amount = token.chain_formatted_value(human_readable_value=Decimal(str(amount)))
TypeError: Token.chain_formatted_value() missing 1 required positional argument: 'self'

@aarmoa please run the examples to make sure there's backwards compatibility. We used this script: https://github.com/InjectiveLabs/sdk-python/blob/master/run-examples.sh a while back to run all examples - we can modify and use it moving forward.

Thank you @achilleas-kal for spotting this. I was able to reproduce the issue and fix the problem, and added a unit tests for the scenario to avoid the same error happening again in the future.
I have also added some changes in the script to run all examples because it was outdated. It can be used now to run all examples again.

@aarmoa aarmoa merged commit 485a27b into dev Jul 12, 2023
@aarmoa aarmoa deleted the feat/refactor_markets_and_tokens branch July 12, 2023 17:11
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