Add support ERC20 to Native #138

patitonar · 2018-10-02T19:44:14Z

This PR includes the following changes:

Get Bridge mode from contract method
Add check for fromBlock parameter to be > 0 on getPastEvents method
Add support ERC20 to Native mode
Add e2e tests for ERC20 to Native mode

Pending:

After Add ERC20 to Native bridge mode tokenbridge-contracts#82 is merged, the poa-bridge-contract submodule and contract Dockerfile has to be updated to use the correct branch.
After Add support ERC20 to Native token-bridge#85 is merged, bridge-nodejs Dockerfile has to be updated to use the correct branch.

igorbarinov · 2018-10-02T19:44:18Z

Deploy preview for kind-kilby-95344f processing.

Built with commit 0ed18c7

https://app.netlify.com/sites/kind-kilby-95344f/deploys/5bcf108a67610c7a9cf926c6

akolotov

Could you clarify the process what gas price is set for transactions in both networks? Currently it is possible to configure the speed chosen for the response of the gas price oracle. Will it affect the value on the window presented to the user? What value is used for the network if the gas price oracle is not configured or what will happen if the gas price oracle is not configured. Does it makes sense to introduce parameters to configure a fallback gas price?
This question is due to the fact that gas price in the shard we are creating could be 1000 gwei. I am curious if the legacy code still use the value in 1 gwei for Home network.

akolotov · 2018-10-05T18:09:20Z

src/stores/TxStore.js

@@ -105,8 +100,6 @@ class TxStore {
    web3.eth.getTransactionReceipt(hash, (error, res) => {
      if(res && res.blockNumber){
        if(res.status === '0x1'){


consider to compare the number 1 instead of the string in hexadecimal 0x1. The status code is not standardized well that is why some clients could return 0x01 (rskj for example).

patitonar · 2018-10-05T20:16:13Z

Currently gas price is fetched from oracle, the same value is used for both networks. You can only configure the speed using REACT_APP_GAS_PRICE_SPEED_TYPE. There are no fallback values if oracle fails. So I think we should:

Add ability to use different oracle urls and speed for each network.
Get gas price from contracts for each network.
Add fallback gas price for each network on .env.

So same as it works on bridge-nodejs, we can use oracle's gas price, if cannot be obtained, use gas price from contract, if it also fails, use fallback gas price from .env.

Does it makes sense @akolotov ?

akolotov · 2018-10-05T20:18:01Z

Thank you for the investigation. This answer is exactly what I looked for. Let's implement this in this version of the Bridge UI.

Also: check gas price in contract if oracle fails, and use fallback value if that fails too.

fvictorio · 2018-10-08T21:02:32Z

@akolotov the last commit allows using a different gas price for home and foreign, calls the contract if the oracle fails, and uses a fallback value if that also fails.

The setHomeWeb3Promise is ugly, but it was the only way I found to avoid a circular dependency between stores.

fvictorio · 2018-10-08T21:30:50Z

@akolotov Also, notice that much of the logic here duplicates the one in bridge-nodejs. I think this is going to keep happening, and it's problematic. Maybe we should consider how to deal with that. Some options are moving everything (bridge, ui, monitor, contracts) to a monorepo, or creating a bridge-common package. If that sounds like a good idea to you, we can discuss it in an issue.

patitonar · 2018-10-09T11:52:29Z

src/stores/Web3Store.js

@@ -1,4 +1,4 @@
-import { action, observable } from "mobx";
+import { action, computed, observable } from "mobx";


I think computed is not used here, so we can avoid importing it

patitonar · 2018-10-10T20:58:53Z

@akolotov I'm having the same issues as you with that chain on validators on status page. The way how validators are retrieved is by listening all ValidatorAdded and ValidatorRemoved events and then filtering only the ones that were added but not removed. In this case, in both cases doesn't find any event. Is validator contract properly set?

patitonar · 2018-10-10T21:04:14Z

@akolotov for now I added 3 digits after the point on network details you mentioned, I need to investigate best way to implement the logic you suggested after I finish with all of the issues

src/stores/utils/gas.js

akolotov · 2018-10-11T13:22:07Z

src/components/StatisticsPage.js

                foreignSymbol={foreignStore.symbol}
                foreignSupply={foreignStore.totalSupply} />
          </div>
          <div className='statistics-transaction-container'>
            <div className='statistics-deposit-container'>
-              <span className='statistics-deposit-title statistics-title'>Network Deposits</span>
+              <span className='statistics-deposit-title statistics-title'>Tokens Deposits</span>


The logic when we use erc-to-erc and erc-to-native modes are reversed.

native-to-erc: tokens are deposited by sending coins from Home and withdrawn by sending tokens from Foreign

erc-to-erc: tokens are deposited by sending tokens from Foreign and withdrawn by sending tokens from Home

erc-to-native: tokens are deposited by sending tokens from Foreign and withdrawn by sending coins from Home

The current layout of lables "Tokens Deposits" and "Tokens Withdrawals" confuses.

Since the amount of withdrawn tokens cannot be more than the amount of deposited tokens.

src/stores/TxStore.js

# Conflicts: # README.md # src/stores/utils/web3.js

akolotov · 2018-10-25T19:35:23Z

I think we can merge the changes to develop branch.

patitonar added 13 commits September 27, 2018 12:10

Add .nvmrc

975e534

Update submodule to erc20-to-native-#79 branch

f6bba80

Use stable docker image for parity

847d1dd

Use explicit tag for redis docker image

8827f4a

Remove unused variables on TxStore

bf0a2ef

Get bridge mode from contract method

7a0265b

Add erc20 to native support

79a68dd

Update e2e contracts branch

81b1fbb

Fix homeStore erc-to-erc initialization

d932e2f

Update e2e contracts address

c4ec770

Update e2e bridge-nodejs Dockerfile

bdfd6ea

Check fromBlock value on getEvents

459f423

Add erc20-native e2e tests

627e5db

patitonar requested review from fvictorio and akolotov October 2, 2018 19:44

ghost assigned patitonar Oct 2, 2018

ghost added the in progress label Oct 2, 2018

ghost added the review label Oct 2, 2018

patitonar removed the in progress label Oct 2, 2018

akolotov requested changes Oct 5, 2018

View reviewed changes

fvictorio mentioned this pull request Oct 5, 2018

Support RSK #141

Merged

Use different gas price in home and foreign

af91746

Also: check gas price in contract if oracle fails, and use fallback value if that fails too.

ghost assigned fvictorio Oct 8, 2018

ghost added the in progress label Oct 8, 2018

patitonar commented Oct 9, 2018

View reviewed changes

Display 3 digits after point on network details

29e27ac

ghost added the in progress label Oct 10, 2018

Add support for Bytes32 symbol and name

0189e27

ghost removed the in progress label Oct 10, 2018

akolotov reviewed Oct 10, 2018

View reviewed changes

src/stores/utils/gas.js Outdated Show resolved Hide resolved

patitonar added 2 commits October 11, 2018 09:14

Use fallback gas price if oracle not available

f130527

Add default gas price update Interval

786e058

ghost added the in progress label Oct 11, 2018

akolotov reviewed Oct 11, 2018

View reviewed changes

patitonar added 2 commits October 11, 2018 11:36

Fix statistics title names

c0320e1

Fix docker parity version to v1.11.11

4b44111

ghost removed the in progress label Oct 11, 2018

Update networks names

bfb7771

ghost added the in progress label Oct 11, 2018

fvictorio mentioned this pull request Oct 11, 2018

Consider moving everything to a monorepo or creating a bridge-common repo omni/token-bridge#101

Closed

Use explorer url from .env on event page

6e05c82

ghost removed the in progress label Oct 11, 2018

Update network name on event page

c619608

ghost added the in progress label Oct 11, 2018

patitonar added 2 commits October 23, 2018 08:38

Merge branch 'develop' into add-support-erc20-native

754875d

# Conflicts: # README.md # src/stores/utils/web3.js

Update poa-bridge-contracts branch on Dockerfile

0ed18c7

ghost removed the in progress label Oct 23, 2018

akolotov approved these changes Oct 25, 2018

View reviewed changes

akolotov merged commit c4ff4b6 into develop Oct 26, 2018

ghost removed the review label Oct 26, 2018

akolotov mentioned this pull request Oct 26, 2018

UI update required #133

Closed

akolotov deleted the add-support-erc20-native branch May 24, 2019 06:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support ERC20 to Native #138

Add support ERC20 to Native #138

patitonar commented Oct 2, 2018

igorbarinov commented Oct 2, 2018 •

edited by netlify bot

Loading

akolotov left a comment

akolotov Oct 5, 2018

patitonar Oct 9, 2018

patitonar commented Oct 5, 2018

akolotov commented Oct 5, 2018

fvictorio commented Oct 8, 2018

fvictorio commented Oct 8, 2018

patitonar Oct 9, 2018

patitonar commented Oct 10, 2018

patitonar commented Oct 10, 2018

akolotov Oct 11, 2018

patitonar Oct 11, 2018

akolotov commented Oct 25, 2018

		@@ -1,4 +1,4 @@
		import { action, observable } from "mobx";
		import { action, computed, observable } from "mobx";

Add support ERC20 to Native #138

Add support ERC20 to Native #138

Conversation

patitonar commented Oct 2, 2018

igorbarinov commented Oct 2, 2018 • edited by netlify bot Loading

akolotov left a comment

Choose a reason for hiding this comment

akolotov Oct 5, 2018

Choose a reason for hiding this comment

patitonar Oct 9, 2018

Choose a reason for hiding this comment

patitonar commented Oct 5, 2018

akolotov commented Oct 5, 2018

fvictorio commented Oct 8, 2018

fvictorio commented Oct 8, 2018

patitonar Oct 9, 2018

Choose a reason for hiding this comment

patitonar commented Oct 10, 2018

patitonar commented Oct 10, 2018

akolotov Oct 11, 2018

Choose a reason for hiding this comment

patitonar Oct 11, 2018

Choose a reason for hiding this comment

akolotov commented Oct 25, 2018

igorbarinov commented Oct 2, 2018 •

edited by netlify bot

Loading