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 template #1

Merged
merged 35 commits into from
Jun 29, 2023
Merged

Add template #1

merged 35 commits into from
Jun 29, 2023

Conversation

ceyonur
Copy link
Collaborator

@ceyonur ceyonur commented Feb 23, 2023

Adds template repository

Example branch and PR based on this template can be found at: #2

- package-ecosystem: "gomod" # See documentation for possible values
directory: "/" # Location of package manifests
schedule:
interval: "daily"

Choose a reason for hiding this comment

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

I know there's a precedent already here, but in my experience, weekly PRs from dependabot are actually merged quicker.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My main aim was to put a specific daily check for subnet-evm repo. But I feel like this is not possible.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed to weekly, if you know anything regarding how we can differentiate this for per-repo, LMK so we can put a specific rule for subnet-evm

README.md Outdated

## How to use

There is an example branch [hello-world-example](https://github.com/ava-labs/precompilevm/tree/hello-world-example) in this repository. You can check the example branch to see how to register precompiles and test them.

Choose a reason for hiding this comment

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

Can we get CI to automatically keep the hello-world-example branch up to date?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good idea

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It certainly possible to keep it up-to-date with precompilevm/master but we also need to keep it up-to-date with subnet-evm + precompile generate tool. So it is mostly a manual process to regenerate those files and fix any conflicts. I will add an action with https://github.com/marketplace/actions/sync-branches to create PRs, but since the branch will be mentioned in docs, we have to keep this a manual process. but we can use CI as a reminder.

Choose a reason for hiding this comment

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

@ceyonur, you want to create an issue and reference it here, then resolve this conversation?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@@ -0,0 +1,50 @@
import "@nomiclabs/hardhat-waffle"

Choose a reason for hiding this comment

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

Do we even use waffle in the tests?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this was generated from hardhat by default. Probably to be used for task.

contract-examples/hardhat.config.ts Outdated Show resolved Hide resolved
contract-examples/package.json Outdated Show resolved Hide resolved
contract-examples/tasks.ts Outdated Show resolved Hide resolved
plugin/main.go Show resolved Hide resolved
ceyonur and others added 2 commits May 19, 2023 23:03
Co-authored-by: Richard Pringle <rpring9@gmail.com>
Signed-off-by: Ceyhun Onur <ceyhunonur54@gmail.com>
Comment on lines 25 to 52
func TestE2E(t *testing.T) {
gomega.RegisterFailHandler(ginkgo.Fail)
ginkgo.RunSpecs(t, "subnet-evm precompile ginkgo test suite")
}

// BeforeSuite starts an AvalancheGo process to use for the e2e tests
var _ = ginkgo.BeforeSuite(func() {
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()

wd, err := os.Getwd()
gomega.Expect(err).Should(gomega.BeNil())
log.Info("Starting AvalancheGo node", "wd", wd)
startCmd, err = utils.RunCommand("./scripts/run.sh")
gomega.Expect(err).Should(gomega.BeNil())

// Assumes that startCmd will launch a node with HTTP Port at [utils.DefaultLocalNodeURI]
healthClient := health.NewClient(utils.DefaultLocalNodeURI)
healthy, err := health.AwaitReady(ctx, healthClient, 5*time.Second, nil)
gomega.Expect(err).Should(gomega.BeNil())
gomega.Expect(healthy).Should(gomega.BeTrue())
log.Info("AvalancheGo node is healthy")
})

var _ = ginkgo.AfterSuite(func() {
gomega.Expect(startCmd).ShouldNot(gomega.BeNil())
gomega.Expect(startCmd.Stop()).Should(gomega.BeNil())
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we reduce further code duplication by exporting this from Subnet-EVM and importing it here?

Should make our lives easier if we change this in the future

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we currently run the same process for every js/ts test file paired with a genesis file, we could also create a utility in Subnet-EVM that automatically collects all of the tests that need to be run and runs all of them, which we could then simply import and run here as well to get rid of a lot of the template code. Just spitballing, don't mean to feature creep for this PR

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

imported

I think we have a issue for the second part right? ava-labs/subnet-evm#458

2: "Admin",
}

const getRole = async (allowList, address) => {
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 these here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not currently, removed

//SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

interface IAllowList {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we import this from Subnet-EVM instead of copy pasting it here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, seems like we could get rid of this completely if we did not need AllowList functionality in the next PR. Can we just make it so that HelloWorld does not extend AllowList ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd like to keep HelloWorld same as the one in https://docs.avax.network/subnets/hello-world-precompile-tutorial#tutorial
I will try to import it from subnet-evm

@aaronbuchwald
Copy link
Collaborator

I think the primary goal for this repo should be to create the simplest and smallest possible example of how to create a Subnet-EVM with custom precompiles.

It seems like there are a few easy ways we can make this repo a bit smaller:

  • Remove the allow list from HelloWorld so that we don't need to copy paste it (could we also import it?)
  • Refactor the ginkgo tests in Subnet-EVM to automatically scrape a given genesis/test directory for the tests that it should run and import that here to run the relevant e2e tests

@@ -0,0 +1,167 @@
# Subnet EVM Contracts
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 to have a README both here and at the root of the repo? Since the only thing in the repo is a precompile, it seems like one README should be all we need

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 consolidate these into one README and either include explicit step by step instructions how to use this repo and create your own precompile or create a separate md file that includes instructions, which we can link in the README?

richardpringle
richardpringle previously approved these changes Jun 15, 2023
richardpringle
richardpringle previously approved these changes Jun 26, 2023
@ceyonur ceyonur merged commit 6255416 into main Jun 29, 2023
@ceyonur ceyonur deleted the add-template branch June 29, 2023 21:11
@ceyonur ceyonur linked an issue Jun 29, 2023 that may be closed by this pull request
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.

Initialize template repo and guide
4 participants