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

refactor: allow types to be imported by precompile/contract #1271

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ARR4N
Copy link
Contributor

@ARR4N ARR4N commented Aug 8, 2024

Why this should be merged

Importing core/types from precompile/{modules,contract} results in a circular dependency because types -> params -> precompile/modules -> precompile/contract -> types.

How this works

precompile/modules only depended on precompile/contract for interfaces; by moving the interfaces into the new precompile/interfaces package, there is no longer a transitive dependency params -> precompile/modules -> precompile/contract.

Note that this only allows importing core/types from precompile/contract but not from precompile/modules. This is OK as the latter is merely a registry that has implementations injected into it.

Visualising the dependency graph (raw Graphviz below) with:

$ go install github.com/loov/goda@latest
$ goda graph -cluster -short ./params ./core/types ./precompile/{contract,modules,precompileconfig} | dot -Tpng > depgraph.png

Before

image

After (as at 12febd1 PoC)

image

How this was tested

How is this documented

Raw Graphviz input

Dependency graph at caf34ea master:

digraph G {
    node [penwidth=2 fontsize=10 shape=rectangle target="_graphviz"];
    edge [tailport=e penwidth=2];
    compound=true;
    rankdir=LR;
    newrank=true;
    ranksep="1.5";
    quantum="0.5";
subgraph "cluster_./precompile/interfaces" {
    label="./precompile/interfaces"
    tooltip="./precompile/interfaces"
    href="https://pkg.go.dev/./precompile/interfaces"
    "./precompile/interfaces" [label="" tooltip="./precompile/interfaces" shape=point color="#07918cb2" rank=0];
}
subgraph "cluster_github.com/ava-labs/subnet-evm" {
    label="github.com/ava-labs/subnet-evm (local)"
    tooltip="github.com/ava-labs/subnet-evm (local)"
    href="https://pkg.go.dev/github.com/ava-labs/subnet-evm@"
    "github.com/ava-labs/subnet-evm/core/types" [label="core/types\l3822 / 130.6KB\l" tooltip="github.com/ava-labs/subnet-evm/core/types" href="https://pkg.go.dev/github.com/ava-labs/subnet-evm/core/types" color="#910750b2"];
    "github.com/ava-labs/subnet-evm/params" [label="params\l1669 / 78.0KB\l" tooltip="github.com/ava-labs/subnet-evm/params" href="https://pkg.go.dev/github.com/ava-labs/subnet-evm/params" color="#915107b2"];
    "github.com/ava-labs/subnet-evm/precompile/contract" [label="precompile/contract\l530 / 22.0KB\l" tooltip="github.com/ava-labs/subnet-evm/precompile/contract" href="https://pkg.go.dev/github.com/ava-labs/subnet-evm/precompile/contract" color="#910769b2"];
    "github.com/ava-labs/subnet-evm/precompile/modules" [label="precompile/modules\l113 / 3.5KB\l" tooltip="github.com/ava-labs/subnet-evm/precompile/modules" href="https://pkg.go.dev/github.com/ava-labs/subnet-evm/precompile/modules" color="#075191b2"];
    "github.com/ava-labs/subnet-evm/precompile/precompileconfig" [label="precompile/precompileconfig\l334 / 13.7KB\l" tooltip="github.com/ava-labs/subnet-evm/precompile/precompileconfig" href="https://pkg.go.dev/github.com/ava-labs/subnet-evm/precompile/precompileconfig" color="#07914cb2"];
}
    "github.com/ava-labs/subnet-evm/core/types" -> "github.com/ava-labs/subnet-evm/params" [tooltip="github.com/ava-labs/subnet-evm/core/types -> github.com/ava-labs/subnet-evm/params" color="#915107b2"];
    "github.com/ava-labs/subnet-evm/params" -> "github.com/ava-labs/subnet-evm/precompile/modules" [tooltip="github.com/ava-labs/subnet-evm/params -> github.com/ava-labs/subnet-evm/precompile/modules" color="#075191b2"];
    "github.com/ava-labs/subnet-evm/params" -> "github.com/ava-labs/subnet-evm/precompile/precompileconfig" [tooltip="github.com/ava-labs/subnet-evm/params -> github.com/ava-labs/subnet-evm/precompile/precompileconfig" color="#07914cb2"];
    "github.com/ava-labs/subnet-evm/precompile/contract" -> "github.com/ava-labs/subnet-evm/precompile/precompileconfig" [tooltip="github.com/ava-labs/subnet-evm/precompile/contract -> github.com/ava-labs/subnet-evm/precompile/precompileconfig" color="#07914cb2"];
    "github.com/ava-labs/subnet-evm/precompile/modules" -> "github.com/ava-labs/subnet-evm/precompile/contract" [tooltip="github.com/ava-labs/subnet-evm/precompile/modules -> github.com/ava-labs/subnet-evm/precompile/contract" color="#910769b2"];
}

Dependency graph at 12febd1 proof-of-concept:

digraph G {
    node [penwidth=2 fontsize=10 shape=rectangle target="_graphviz"];
    edge [tailport=e penwidth=2];
    compound=true;
    rankdir=LR;
    newrank=true;
    ranksep="1.5";
    quantum="0.5";
subgraph "cluster_github.com/ava-labs/subnet-evm" {
    label="github.com/ava-labs/subnet-evm (local)"
    tooltip="github.com/ava-labs/subnet-evm (local)"
    href="https://pkg.go.dev/github.com/ava-labs/subnet-evm@"
    "github.com/ava-labs/subnet-evm/core/types" [label="core/types\l3822 / 130.6KB\l" tooltip="github.com/ava-labs/subnet-evm/core/types" href="https://pkg.go.dev/github.com/ava-labs/subnet-evm/core/types" color="#910750b2"];
    "github.com/ava-labs/subnet-evm/params" [label="params\l1669 / 78.0KB\l" tooltip="github.com/ava-labs/subnet-evm/params" href="https://pkg.go.dev/github.com/ava-labs/subnet-evm/params" color="#915107b2"];
    "github.com/ava-labs/subnet-evm/precompile/contract" [label="precompile/contract\l486 / 20.5KB\l" tooltip="github.com/ava-labs/subnet-evm/precompile/contract" href="https://pkg.go.dev/github.com/ava-labs/subnet-evm/precompile/contract" color="#910769b2"];
    "github.com/ava-labs/subnet-evm/precompile/interfaces" [label="precompile/interfaces\l61 / 2.4KB\l" tooltip="github.com/ava-labs/subnet-evm/precompile/interfaces" href="https://pkg.go.dev/github.com/ava-labs/subnet-evm/precompile/interfaces" color="#075a91b2"];
    "github.com/ava-labs/subnet-evm/precompile/modules" [label="precompile/modules\l113 / 3.5KB\l" tooltip="github.com/ava-labs/subnet-evm/precompile/modules" href="https://pkg.go.dev/github.com/ava-labs/subnet-evm/precompile/modules" color="#075191b2"];
    "github.com/ava-labs/subnet-evm/precompile/precompileconfig" [label="precompile/precompileconfig\l334 / 13.7KB\l" tooltip="github.com/ava-labs/subnet-evm/precompile/precompileconfig" href="https://pkg.go.dev/github.com/ava-labs/subnet-evm/precompile/precompileconfig" color="#07914cb2"];
}
    "github.com/ava-labs/subnet-evm/core/types" -> "github.com/ava-labs/subnet-evm/params" [tooltip="github.com/ava-labs/subnet-evm/core/types -> github.com/ava-labs/subnet-evm/params" color="#915107b2"];
    "github.com/ava-labs/subnet-evm/params" -> "github.com/ava-labs/subnet-evm/precompile/modules" [tooltip="github.com/ava-labs/subnet-evm/params -> github.com/ava-labs/subnet-evm/precompile/modules" color="#075191b2"];
    "github.com/ava-labs/subnet-evm/params" -> "github.com/ava-labs/subnet-evm/precompile/precompileconfig" [tooltip="github.com/ava-labs/subnet-evm/params -> github.com/ava-labs/subnet-evm/precompile/precompileconfig" color="#07914cb2"];
    "github.com/ava-labs/subnet-evm/precompile/contract" -> "github.com/ava-labs/subnet-evm/core/types" [tooltip="github.com/ava-labs/subnet-evm/precompile/contract -> github.com/ava-labs/subnet-evm/core/types" color="#910750b2"];
    "github.com/ava-labs/subnet-evm/precompile/contract" -> "github.com/ava-labs/subnet-evm/precompile/interfaces" [tooltip="github.com/ava-labs/subnet-evm/precompile/contract -> github.com/ava-labs/subnet-evm/precompile/interfaces" color="#075a91b2"];
    "github.com/ava-labs/subnet-evm/precompile/contract" -> "github.com/ava-labs/subnet-evm/precompile/precompileconfig" [tooltip="github.com/ava-labs/subnet-evm/precompile/contract -> github.com/ava-labs/subnet-evm/precompile/precompileconfig" color="#07914cb2"];
    "github.com/ava-labs/subnet-evm/precompile/interfaces" -> "github.com/ava-labs/subnet-evm/precompile/precompileconfig" [tooltip="github.com/ava-labs/subnet-evm/precompile/interfaces -> github.com/ava-labs/subnet-evm/precompile/precompileconfig" color="#07914cb2"];
    "github.com/ava-labs/subnet-evm/precompile/modules" -> "github.com/ava-labs/subnet-evm/precompile/interfaces" [tooltip="github.com/ava-labs/subnet-evm/precompile/modules -> github.com/ava-labs/subnet-evm/precompile/interfaces" color="#075a91b2"];
}

There are tests of specific precompile configs being unmarshalled, which reintroduces the circular dependency. The `_test` idiom overcomes this instead of an otherwise major refactor.
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.

1 participant