Skip to content

Remove sha3, uts46,and graph-node plugins from default client config#1158

Merged
dOrgJelli merged 28 commits into
origin-devfrom
move-sha3-graph-uts46-plugins
Sep 6, 2022
Merged

Remove sha3, uts46,and graph-node plugins from default client config#1158
dOrgJelli merged 28 commits into
origin-devfrom
move-sha3-graph-uts46-plugins

Conversation

@krisbitney
Copy link
Copy Markdown
Contributor

@krisbitney krisbitney commented Aug 18, 2022

This PR removes the sha3, uts46, and graph-node plugins from the default client configuration. It redirects the plugin URIs to the sha3, uts46, and graph-node Wasm wrappers that have been added to the integrations repo.

I updated the ENS Wasm wrapper in the integrations repo to Origin 0.3.0 and replaced the embedded ENS wrapper in the test-env-js package with the updated build. A PR is up to merge the ENS wrapper update in the integrations repo: polywrap/wrap-integrations#112

Also related is a PR to add the sha3, uts46, and graph-node plugins to the integrations repo: polywrap/wrap-integrations#110

Closes #1125.

…-uts46-plugins

# Conflicts:
#	packages/js/plugins/graph-node/package.json
#	packages/js/plugins/graph-node/polywrap.plugin.yaml
#	packages/js/plugins/graph-node/src/index.ts
#	packages/js/plugins/sha3/package.json
#	packages/js/plugins/sha3/polywrap.plugin.yaml
#	packages/js/plugins/sha3/src/index.ts
#	packages/js/plugins/uts46/package.json
#	packages/js/plugins/uts46/polywrap.plugin.yaml
#	packages/js/plugins/uts46/src/index.ts
@krisbitney krisbitney marked this pull request as draft August 18, 2022 19:21
@krisbitney krisbitney changed the title [Draft] remove sha3, uts46,and graph-node plugins from default client config Remove sha3, uts46,and graph-node plugins from default client config Aug 18, 2022
@krisbitney krisbitney marked this pull request as ready for review August 19, 2022 14:37
];

export const defaultWrappers = {
sha3: "wrap://ipfs/QmYFWh4D91sAiYKf8o37EravLwFKAtUAmut4Xnevnt4QZR",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we use an ENS domain here instead? This will ensure it's always kept online by https://wrappers.io

Copy link
Copy Markdown
Contributor Author

@krisbitney krisbitney Aug 20, 2022

Choose a reason for hiding this comment

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

Okay, I added the ENS URIs.

export const defaultWrappers = {
  sha3: "wrap://ens/goerli/sha3.wrappers.eth",
  uts46: "wrap://ens/goerli/uts46.wrappers.eth",
  graphNode: "wrap://ens/goerli/graph-node.wrappers.eth",
};

To improve resolution performance, I added a goerli network using the default infura key we use for mainnet in the etherum plugin config:

networks: {
  mainnet: {
    provider:
      "https://mainnet.infura.io/v3/b00b2c2cc09c487685e9fb061256d6a6",
  },
  goerli: {
    provider:
      "https://goerli.infura.io/v3/b00b2c2cc09c487685e9fb061256d6a6",
  },
},

The resolution performance without it isn't very good. That's why I used the IPFS hashes. Let me know what you think! And thanks.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I went ahead and added the goerli connection into the Ethereum plugin constructor so we guarantee that the user always has the connection to goerli.

The reason for this is that if a user creates a new instance of the client and passes an Ethereum plugin, it will overwrite the default configuration, removing the goerli connection, hence, not being able to achieve the new wrappers deployed to goerli with ENS Uri resolver

I also think that we should stop setting the configuration in client instantiation (i.e: in test env) and just use the config builder, this way things would be cleaner, but this is another topic 😋

Comment thread packages/js/client/src/default-client-config.ts Outdated
…-uts46-plugins

# Conflicts:
#	packages/js/plugins/graph-node/polywrap.plugin.yaml
#	packages/js/plugins/sha3/polywrap.plugin.yaml
#	packages/js/plugins/uts46/polywrap.plugin.yaml
…-uts46-plugins

# Conflicts:
#	packages/js/client/package.json
#	packages/js/plugins/graph-node/package.json
#	packages/js/plugins/sha3/package.json
#	packages/js/plugins/uts46/package.json
@lgtm-com
Copy link
Copy Markdown

lgtm-com Bot commented Aug 20, 2022

This pull request fixes 3 alerts when merging 6d22bdc into c6d1d0b - view on LGTM.com

fixed alerts:

  • 3 for Unused variable, import, function or class

…-uts46-plugins

# Conflicts:
#	packages/js/client/package.json
#	packages/js/plugins/graph-node/package.json
#	packages/js/plugins/graph-node/src/index.ts
#	packages/js/plugins/sha3/package.json
#	packages/js/plugins/sha3/src/index.ts
#	packages/js/plugins/uts46/package.json
#	packages/js/plugins/uts46/src/index.ts
nerfZael
nerfZael previously approved these changes Aug 24, 2022
…-uts46-plugins

# Conflicts:
#	packages/js/client/src/__tests__/core/sanity.spec.ts
#	packages/js/client/src/default-client-config.ts
#	packages/js/plugins/uri-resolvers/ens-resolver/src/__tests__/e2e.spec.ts
cbrzn
cbrzn previously approved these changes Aug 31, 2022
dOrgJelli
dOrgJelli previously approved these changes Sep 6, 2022
@dOrgJelli dOrgJelli dismissed stale reviews from cbrzn and themself via aa80a69 September 6, 2022 22:43
@dOrgJelli dOrgJelli merged commit 9fc870e into origin-dev Sep 6, 2022
@dOrgJelli dOrgJelli deleted the move-sha3-graph-uts46-plugins branch April 10, 2023 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants