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

Genesis icm #2154

Open
wants to merge 15 commits into
base: acp-77
Choose a base branch
from
Open

Genesis icm #2154

wants to merge 15 commits into from

Conversation

felipemadero
Copy link
Collaborator

@felipemadero felipemadero commented Sep 16, 2024

Closes #2142
Setups icm and registry in subnet-vm genesis.

@felipemadero felipemadero self-assigned this Sep 16, 2024
@felipemadero felipemadero requested a review from a team as a code owner September 16, 2024 22:06
@felipemadero felipemadero requested review from sukantoraymond and arturrez and removed request for a team September 16, 2024 22:07
@felipemadero felipemadero marked this pull request as draft September 16, 2024 22:07
@felipemadero felipemadero marked this pull request as ready for review September 19, 2024 13:58
pkg/vm/create_evm.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@arturrez arturrez left a comment

Choose a reason for hiding this comment

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

commented

@felipemadero felipemadero changed the base branch from main to acp-77 September 19, 2024 20:50
Copy link
Collaborator

@sukantoraymond sukantoraymond left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +280 to +285
if len(allocation.Code) > 0 {
continue
}
if address == common.HexToAddress(icmgenesis.MessengerDeployerAddress) {
continue
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we skip the deployer address and any allocation with code here? Could be helpful to add a comment explaining

@@ -235,3 +235,39 @@ func GetEVMSubnetGenesisNativeMinterAdmin(
}
return getGenesisNativeMinterAdmin(app, network, genesisData)
}

func ContractAddressIsInBlockchainGenesis(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we add unit tests for these two new functions?

return nil
}

func ICMAtBlockchainGenesis(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you add comments documenting what these new functions are and how they are intended to be used?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: In Review 👀
Development

Successfully merging this pull request may close these issues.

setup icm and registry contracts directly into subnet-evm genesis
4 participants