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

Add sei #606

Open
wants to merge 24 commits into
base: develop
Choose a base branch
from
Open

Add sei #606

wants to merge 24 commits into from

Conversation

platonfloria
Copy link
Collaborator

No description provided.

@platonfloria platonfloria changed the base branch from main to develop May 3, 2024 09:53
This was referenced May 3, 2024
run_blockchain_terraformer.py Outdated Show resolved Hide resolved
run_blockchain_terraformer.py Outdated Show resolved Hide resolved
@barakman barakman self-requested a review May 3, 2024 11:19
@@ -113,7 +113,7 @@ class MultiCaller(ContextManager):

def __init__(self, contract: web3.contract.Contract,
web3: Web3,
block_identifier: Any = 'latest', multicall_address = "0x5BA1e12693Dc8F9c48aAD8770482f4739bEeD696"):
block_identifier: Any = 'latest', multicall_address = "0xcA11bde05977b3631167028862bE2a173976CA11"):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What is this address and why was it changed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let's replace with MULTICALL_CONTRACT_ADDRESS

Copy link
Collaborator

Choose a reason for hiding this comment

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

The MULTICALL_CONTRACT_ADDRESS constant is currently defined separately per network.
We can move it to global scope, but it will be a drop in the ocean, so no point investing too much effort into this.

@@ -8,8 +8,8 @@
NOTE: this class is not part of the API of the Carbon protocol, and you must expect breaking
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@sklbancor do optimizer changes belong to this PR?

@@ -205,7 +204,16 @@
assert raises(CPC.from_carbon, yint=1, y=1, pa=1800, pb=2200, A=100, pair="ETH/USDC", tkny="ETH", fee=0, cid="1", descr="Carbon", isdydx=False)
assert raises(CPC.from_carbon, yint=1, y=1, pa=1800, pb=2200, B=100, pair="ETH/USDC", tkny="ETH", fee=0, cid="1", descr="Carbon", isdydx=False)
assert raises(CPC.from_carbon, yint=1, y=1, pa=1800, pb=2200, A=100, B=100, pair="ETH/USDC", tkny="ETH", fee=0, cid="1", descr="Carbon", isdydx=False)
assert raises(CPC.from_carbon, yint=1, y=1, pb=1800, pa=2200, pair="ETH/USDC", tkny="ETH", fee=0, cid="1", descr="Carbon", isdydx=False)
#assert raises(CPC.from_carbon, yint=1, y=1, pb=1800, pa=2200, pair="ETH/USDC", tkny="ETH", fee=0, cid="1", descr="Carbon", isdydx=False)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We should avoid committing dead code

@@ -113,7 +113,7 @@ class MultiCaller(ContextManager):

def __init__(self, contract: web3.contract.Contract,
web3: Web3,
block_identifier: Any = 'latest', multicall_address = "0x5BA1e12693Dc8F9c48aAD8770482f4739bEeD696"):
block_identifier: Any = 'latest', multicall_address = "0xcA11bde05977b3631167028862bE2a173976CA11"):
Copy link
Contributor

Choose a reason for hiding this comment

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

@barakman please make sure it's the right move to replace the default

Copy link
Collaborator

@barakman barakman May 8, 2024

Choose a reason for hiding this comment

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

Verified (the only place where this default value is used is in test 899).
Ideally it should be removed, but since there's a default-value argument which appears before this argument, removing the default value of this argument would require removing the default value of the other argument, or replacing the order in which they are passed to the function (which would subsequently require more changes in other files).

"""

NETWORK = S.NETWORK_SEI
NETWORK_ID = "1" # TODO
Copy link
Contributor

@zavelevsky zavelevsky May 8, 2024

Choose a reason for hiding this comment

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

clear TODO

# print(strategy_typed_args)
# calculate the geometric mean
geomean_p_marg = Decimal.sqrt(pmarg0_inv * pmarg1)
self.ConfigObj.logger.debug(f"[poolandtokens.py, _carbon_to_cpc] These cids are identified as overlapping: {[x['cid'] for x in strategy_typed_args]}")
Copy link
Contributor

Choose a reason for hiding this comment

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

reconsider the verbosity

@@ -403,10 +403,25 @@ def _carbon_to_cpc(self) -> ConstantProductCurve:
allow to omit yint (in which case it is set to y, but this does not make
a difference for the result)
"""
def calculate_parameters(y: Decimal, pa: Decimal, pb: Decimal, pm: Decimal, n: Decimal):
Copy link
Contributor

Choose a reason for hiding this comment

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

missing tests for this functionality - check_overlap and "cleanup" of overlapping strategies

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.

5 participants