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

Modify Ethereum plugin config to use Connections store #1143

Merged
merged 21 commits into from
Aug 24, 2022

Conversation

krisbitney
Copy link
Contributor

@krisbitney krisbitney commented Aug 16, 2022

This PR modifies the Ethereum plugin's config to use a Connections store. The Connections class stores Connection instances. App developers can keep a reference to the Connections store and use it to update networks and providers without reconstructing the plugin or client.

Original spec: https://hackmd.io/@polywrap-dao/BkQdG7NAq

Implemented UX:

const connections = new Connections({
  networks: {
    mainnet: new Connection({ provider }),
    rinkeby: new Connection(...)
  },
  defaultNetowrk: "mainnet"
})

ethereumPlugin({
  connections
});

// assign a Connection to "ropsten" network, or replace existing connection if ropsten exists in store
connections.set("ropsten", new Connection({ provider: newProvider })); // pass a Connection
connections.set("ropsten", newProvider); // or pass a provider directly

// get a Connection and call setProvider to avoid constructing a new Connection instance
connections.get("rinkeby").setProvider(newProvider);

Feedback is welcome!

defaultNetwork?: string;
}

// https://www.typescriptlang.org/play?pretty=true&ts=4.7.4#code/LAKAxgNghgzjAEBJAdgEwKYA92oAoCcB7ABwGUoBbYideeAb1DoG1lL0AueGAF3wEtkAc3gAfbgE8KAI0IQAulyjIJAblBN4xAQDcoPWr338w8QRmyoAEstQ18XAoUwSbaewB4UFnE7KVqdAA+eABeBk06IXQeAAoeKHxongAaLSJidHweCQBKCJA6Ivh8GIBXfGR4BKSY5m0SLJz5dUKigF8UyO4Y+MTktIbM7Ik0vQgy9FyuWTl0ZQLi4prk+oymiXkw+HHJ1qWi0p4Kqr497vbNdtbNMEJkXnwysB5CfFj8xjbio5P4ZHQAHd4E4XPEABb8GBpbxYXwZchUGgAOnMcOstnsuX28EuIE0pSgqFhlj821iQy4j0EInEMCksgg03gygkYRCXwOv0q1UhMDWJBaVw0IDxIr0+Gq6F42wBwJJ8JIiMCHxuIAMvGYACIhgBhMq8QgULVbcJa3bofWG41qu4PObIiCEITxaU8bV6g2vY3ybHwAD0-vg5qgE0tXqNWtAdpgDqdLo1PGRhOJaHRfliOoyVu9WtyfsDwYtOcj0fusZR8ddMsERmQYHQhAAZkg06SEQEaAWg2d0GX7ZXnbEAPLSABW6BeyIA1ugJDBqzx86oA0HtSmFXgMlq0lmSCWffBQEA
Copy link
Contributor

Choose a reason for hiding this comment

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

can this be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, of course. That was just a note for myself that I forgot to delete. It was related to something I was thinking of doing originally, before I realized it wouldn't work out.

}

/** sets Connection to index of network name */
set(network: string, connection: Connection): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit concerned with how we don't have much testing that exhausts these new functions. Could we add a test file for connections + connection?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, of course. I'll update with tests asap.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added tests for Connections and Connection

@krisbitney
Copy link
Contributor Author

krisbitney commented Aug 17, 2022

Added support for passing providers directly into connections.set, to match the original spec:

connections.set("ropsten", newProvider);

@krisbitney krisbitney requested a review from dOrgJelli August 19, 2022 07:19
krisbitney and others added 4 commits August 23, 2022 14:19
…ections-store

# Conflicts:
#	packages/js/plugins/ethereum/src/Connection.ts
#	packages/js/plugins/ethereum/src/index.ts
Copy link
Contributor

@dOrgJelli dOrgJelli left a comment

Choose a reason for hiding this comment

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

🔥 🔥 🔥 🔥 🔥 🔥 🔥 🔥

@dOrgJelli dOrgJelli merged commit c6fbcfc into origin-dev Aug 24, 2022
@dOrgJelli dOrgJelli deleted the eth-plugin-connections-store branch April 10, 2023 17:03
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.

3 participants