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

Update precompile setup for warp #390

Merged
merged 19 commits into from
Nov 28, 2023
Merged

Conversation

aaronbuchwald
Copy link
Collaborator

This PR migrates Coreth to use the latest precompile package structure from Subnet-EVM in preparation for migrating the Warp precompile.

@aaronbuchwald aaronbuchwald marked this pull request as ready for review November 16, 2023 16:19
core/evm.go Outdated Show resolved Hide resolved
core/vm/contracts.go Outdated Show resolved Hide resolved
core/vm/contracts.go Outdated Show resolved Hide resolved
params/avalanche_params.go Show resolved Hide resolved
// - Enabling or disabling precompiles as network upgrades.
type UpgradeConfig struct {
// Config for enabling and disabling precompiles as network upgrades.
PrecompileUpgrades []PrecompileUpgrade `json:"precompileUpgrades,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need this? Isn't warp going to be activated with DUpgrade itself?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes it's activated with the DUpgrade in vm.go. I thought it was easier to keep Coreth and Subnet-EVM closer in line rather than having a different mechanism.

Additionally, we can't import the Warp precompile in the params/ package, so it needs to be done externally anyways. Open to suggestion, but after playing around a bit, I thought the best path was to simply share the same code with Subnet-EVM and set the PrecompileUpgrades field in vm.go here: https://github.com/ava-labs/coreth/blob/migrate-warp-precompile/plugin/evm/vm.go#L470

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was just in favor of having less code to reduce risk. But I think that's fine.

Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO it's probably better to skip unused code, and this should not be needed by coreth since precompiles should be activated by direct network upgrades.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

@ceyonur ceyonur left a comment

Choose a reason for hiding this comment

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

just few nits

core/vm/contracts_stateful.go Outdated Show resolved Hide resolved
core/txpool/txpool.go Outdated Show resolved Hide resolved
ceyonur
ceyonur previously approved these changes Nov 28, 2023
darioush
darioush previously approved these changes Nov 28, 2023
Base automatically changed from warp-backend to master November 28, 2023 17:14
@aaronbuchwald aaronbuchwald dismissed stale reviews from darioush and ceyonur November 28, 2023 17:14

The base branch was changed.

@aaronbuchwald aaronbuchwald merged commit cbc0d3a into master Nov 28, 2023
4 of 7 checks passed
@aaronbuchwald aaronbuchwald deleted the warp-precompile-setup branch November 28, 2023 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants