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 WETC Bridge plugin #1

Closed
wants to merge 4 commits into from
Closed

Add WETC Bridge plugin #1

wants to merge 4 commits into from

Conversation

patitonar
Copy link
Collaborator

Relates to omni/tokenbridge-contracts#365

Follow instructions in README file to test the plugin.

The plugin adds the following definitions to be used by the existing Exchange plugin:

Assets:

  • ETC
  • ERC677
  • WETC

Gateway:

  • EtcGateway

Pair:

  • Bridge
  • WETCBridge

Project structure

  • tokenbridge-plugin plugin definition
  • local-wallet Burner wallet using the plugin with a Native - ERC20 bridge deployed in Sokol - Kovan on top of AMB.
  • basic-wallet Burner wallet using the plugin with the existing WETC Bridge in mainnet and Classic.

I tested the plugin in Sokol - Kovan and also with the WETC Bridge in mainnet and Classic and it worked correctly.

Here are some notes from my testing related more to the Burner Wallet user experience rather than the plugin itself:

  • When importing the PK into the wallet I found no issues using the exchange.
  • When instead of using the PK, Metamask or Nifty Wallet is used, the popup to sign the transaction is not triggered when using different networks than the required for the exchange transaction and no feedback is provided. I guess this behavior is Ok but giving some feedback to the user that it is in an incorrect network would help. This is related to the official exchange plugin.
  • When using Nifty wallet, the popup to sign the transaction is not triggered for the exchange transaction even when it is set to the correct network. It works correctly when using Metamask. I think it is probably related to the official metamask-plugin.
  • For all cases, after the exchange transaction is performed and mined there is no feedback to the user. You have just to wait until the balances are updated. In this case of bridges, you can see one balance reduced and after a couple of seconds, the other balance is updated with the bridged tokens. This is related to the official exchange plugin, I guess a possible improvement to propose would be to allow Exchange Pairs to set a feedback message after the execution so the user can be informed on the delay due to the bridge operations.

@patitonar patitonar self-assigned this Mar 26, 2020
@patitonar patitonar requested a review from akolotov March 26, 2020 19:30
Copy link
Collaborator

@akolotov akolotov left a comment

Choose a reason for hiding this comment

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

General questions:

  1. What difficulties would we face to integrate the plugin to the tokenbridge monorepo? In this case we will not need to copy the token ABI and could integrate it in ultimate e2e testing.
  2. My assumption is that we need to have separate plugin for every pair of mediators: ETC-WETC (native-to-erc677), STAKE (erc677-to-erc677), POA-POA20 (erc20-to-erc677). Is this statement correct? So, what would be a strategy to host these plugins? Will they be in a separate directories in the repo?
  3. May you suggest a Dockerfile to use the setup locally without a need to install the node.js environment?

@patitonar
Copy link
Collaborator Author

  1. What difficulties would we face to integrate the plugin to the tokenbridge monorepo? In this case we will not need to copy the token ABI and could integrate it in ultimate e2e testing.

Looks like a good idea, I don't see any difficulties to integrate this project into the monorepo.

  1. My assumption is that we need to have separate plugin for every pair of mediators: ETC-WETC (native-to-erc677), STAKE (erc677-to-erc677), POA-POA20 (erc20-to-erc677). Is this statement correct? So, what would be a strategy to host these plugins? Will they be in a separate directories in the repo?

Actually, the plugin only exports a set of assets, gateways and exchange pairs. We can use only one plugin to define the needed assets/gateways/pairs for all the bridges. That's why I thought the name of the plugin should be tokenbridge-plugin (as it is now or similar) instead <specific-bridge>-plugin

  1. May you suggest a Dockerfile to use the setup locally without a need to install the node.js environment?

Sure! make sense

@patitonar
Copy link
Collaborator Author

I'm closing this because we are integrating the plugin in the monorepo in omni/tokenbridge#306

@patitonar patitonar closed this Apr 23, 2020
@patitonar patitonar deleted the add-wetc-bridge branch April 23, 2020 16:13
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.

2 participants