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

dev(scripts): deploy default DualVM tokens #1499

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

enitrat
Copy link
Collaborator

@enitrat enitrat commented Oct 11, 2024

Deploys DualVM tokens for all Starkgate addresses, mirroring the underlying starknet token.


This change is Reviewable

Comment on lines +33 to +35
# %%


Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# %%
# %%

evm_deployments: Dict[str, Any],
) -> None:
"""
Deploys a new DualVMToken for a corresponding Starknet ERC20 token.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Deploys a new DualVMToken for a corresponding Starknet ERC20 token.
Deploy a new DualVMToken for a corresponding Starknet ERC20 token.

Comment on lines +94 to +97
async def load_tokens(network_name: str) -> List[Dict[str, Any]]:
file_name = network_name.replace("starknet-", "")
file_path = TOKEN_ADDRESSES_DIR / f"{file_name}.json"
return json.loads(file_path.read_text())
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
async def load_tokens(network_name: str) -> List[Dict[str, Any]]:
file_name = network_name.replace("starknet-", "")
file_path = TOKEN_ADDRESSES_DIR / f"{file_name}.json"
return json.loads(file_path.read_text())
def load_tokens(network_name: str) -> List[Dict[str, Any]]:
file_name = network_name.replace("starknet-", "")
file_path = TOKEN_ADDRESSES_DIR / f"{file_name}.json"
return json.loads(file_path.read_text())



async def load_tokens(network_name: str) -> List[Dict[str, Any]]:
file_name = network_name.replace("starknet-", "")
Copy link
Member

Choose a reason for hiding this comment

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

useless given the fact that you renamed the networks

Comment on lines +82 to +91
async def get_tokens(network) -> List[Dict[str, Any]]:
"""
Get the list of tokens for a given network.
If in dev mode, will return the sepolia token list.
"""
if network["type"] == NetworkType.DEV:
return await load_tokens("sepolia")
if network["name"] not in ("mainnet", "sepolia"):
raise ValueError(f"No known token addresses for network: {network['name']}")
return await load_tokens(network["name"])
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
async def get_tokens(network) -> List[Dict[str, Any]]:
"""
Get the list of tokens for a given network.
If in dev mode, will return the sepolia token list.
"""
if network["type"] == NetworkType.DEV:
return await load_tokens("sepolia")
if network["name"] not in ("mainnet", "sepolia"):
raise ValueError(f"No known token addresses for network: {network['name']}")
return await load_tokens(network["name"])
def get_tokens(network) -> List[Dict[str, Any]]:
"""
Get the list of tokens for a given network.
If in dev mode, will return the sepolia token list.
"""
if network["type"] == NetworkType.DEV:
return load_tokens("sepolia")
return load_tokens(network["name"])

Copy link
Member

Choose a reason for hiding this comment

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

the check on the existence of the list belongs to the load_tokens functions, which is responsible for loading or raising.


async def load_tokens(network_name: str) -> List[Dict[str, Any]]:
file_name = network_name.replace("starknet-", "")
file_path = TOKEN_ADDRESSES_DIR / f"{file_name}.json"
Copy link
Member

Choose a reason for hiding this comment

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

you can add a here

if not file_path.exists():
    # raise ValueError
    # logger.error

but you can also just let the open crashes



async def deploy_starknet_token(owner_address: str) -> Any:
address = await deploy_starknet("StarknetToken", int(1e18), owner_address)
Copy link
Member

Choose a reason for hiding this comment

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

decimals should come from the token list

Copy link
Member

Choose a reason for hiding this comment

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

and we need to update the StarknetToken to accept name and symbol. Or just use the cairo zero ERC20 already in the repo as well

Comment on lines +67 to +74
for token in tokens:
token_name = token["name"]
if token_name not in evm_deployments:
await deploy_new_token(token_name, token, kakarot, evm_deployments)
else:
await verify_and_update_existing_token(
token_name, token, kakarot, evm_deployments
)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for token in tokens:
token_name = token["name"]
if token_name not in evm_deployments:
await deploy_new_token(token_name, token, kakarot, evm_deployments)
else:
await verify_and_update_existing_token(
token_name, token, kakarot, evm_deployments
)
for token in tokens:
token_name = token["name"]
if token_name not in evm_deployments:
await deploy_new_token(token_name, token, kakarot, evm_deployments)
else:
await verify_and_update_existing_token(
token_name, token, kakarot, evm_deployments
)

why would you need to extract token_name from token to eventually give them both to the functions?

# %%


async def get_tokens(network) -> List[Dict[str, Any]]:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
async def get_tokens(network) -> List[Dict[str, Any]]:
async def get_tokens_list(network) -> List[Dict[str, Any]]:

"StarknetToken", int(1e18), owner.starknet_contract.address
)
return get_contract_starknet("StarknetToken", address=address)
async def starknet_token(dual_vm_token):
Copy link
Member

Choose a reason for hiding this comment

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

scope="session"

True,
)
return dual_vm_token
async def dual_vm_token():
Copy link
Member

Choose a reason for hiding this comment

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

scope="sessions

Copy link
Member

Choose a reason for hiding this comment

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

please define fixture in order

Copy link

codecov bot commented Oct 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.8%. Comparing base (9c83960) to head (f6e4fe7).

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #1499   +/-   ##
=====================================
  Coverage   61.7%   61.8%           
=====================================
  Files         44      44           
  Lines       8197    8197           
=====================================
+ Hits        5060    5066    +6     
+ Misses      3137    3131    -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@enitrat enitrat marked this pull request as draft October 11, 2024 18:02
@@ -86,6 +86,7 @@ async def _factory(amount=0):
if balance < tx_cost:
continue

breakpoint()
Copy link
Collaborator

Choose a reason for hiding this comment

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

should not be here ?

@@ -41,6 +39,13 @@ func totalSupply{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr
return (totalSupply=totalSupply);
}

@view
func total_supply{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}() -> (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why modifying this file ?
I suppose this is to support the dualVMToken ?
It seems too me the end-to-end tests are using the starknet_token in Cairo folder.
Moreover it corresponds to a specific OZ version v0.6.0 hence any token in this version not having those modification will not be supported by the dualVMToken
Unless I miss something

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.

3 participants