Skip to content

Commit

Permalink
(Near-One#467) Minor CLI improvements. Update README.
Browse files Browse the repository at this point in the history
* CLI: make transfer functions callable directly from CLI (without
  "TESTING" prefix).
* CLI: improve `getErc20Balance` method.
* CLI: add `getBridgeOnNearBalance` method.
* Update README.
* Minor improvements.
  • Loading branch information
sept-en committed Feb 5, 2021
1 parent c3da5d5 commit 5b65516
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 11 deletions.
15 changes: 14 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ To learn the commands that you can use with the rainbow bridge run
Alternatively, clone this repo, `yarn install`, then you can see what commands you can use with:

```
./index.js --help
cli/./index.js --help

This comment has been minimized.

Copy link
@mfornet

mfornet Feb 5, 2021

cli/index.js --help

```

Parameters of each command can be specified through environment variables, command line arguments, entries in the `~/.rainbow/config.json` config file, or the default value will be used -- in that priority.
Expand Down Expand Up @@ -283,12 +283,19 @@ pm2 logs

### Transferring tokens

Let's check the balance of the rainbow bridge prover on Near before transferring the tokens
```bash
rainbow get-bridge-on-near-balance --near-receiver-account rainbow_bridge_eth_on_near_prover

This comment has been minimized.

Copy link
@mfornet

mfornet Feb 5, 2021

If I understand correctly rainbow_bridge_eth_on_near_prover is a placeholder account that is being used to transfer tokens to. It might be confusing, why not using something more explicit like alice

This comment has been minimized.

Copy link
@mfornet

mfornet Feb 5, 2021

I see this was the way it was previously written in the README, not your decision, anyway, I think we should change it to alice (ofc, we need to first create such account)

This comment has been minimized.

Copy link
@sept-en

sept-en Feb 5, 2021

Author Owner

I agree that will be more convenient. I just thought there was some decision regarding such a name placeholder.

```

Finally, let's transfer some tokens

```bash
rainbow transfer-eth-erc20-to-near --amount 1000 --eth-sender-sk 0x2bdd21761a483f71054e14f5b827213567971c676928d9a1808cbfa4b7501200 --near-receiver-account rainbow_bridge_eth_on_near_prover --near-master-account neartokenfactory
```

Now you check the balance of the rainbow bridge prover again. You should notice the balance was changed.

Note, when we deployed ERC20 to the Ethereum blockchain we have minted a large number of tokens to the default master
key of Ganache, so we have transferred ERC20 tokens from it to `alice.test.near`.
Notice that we are using `neartokenfactory` account here to pay for the NEAR gas fees, any account for which we know a secret key would've worked too.
Expand All @@ -302,6 +309,12 @@ rainbow transfer-eth-erc20-from-near --amount 1 --near-sender-account rainbow_br

You should observe the change of the ERC20 balance as reported by the CLI.

You can also manually check the ERC20 balance of the receiver before and after the transfer

This comment has been minimized.

Copy link
@mfornet

mfornet Feb 5, 2021

When user get to this point, is too late to check balance before transfer, we should suggest this earlier.


```bash
rainbow get-erc20-balance 0xEC8bE1A5630364292E56D01129E8ee8A9578d7D8
```

## Contract Development Workflow

Above steps are ways to run a local bridge and development workflows you need if make any changes to rainbow-bridge-cli. If you want to update any of solidity or rust contracts, they're not in this repo now and workflow is as following.
Expand Down
37 changes: 29 additions & 8 deletions cli/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const {
DeployToken,
mintErc20,
getErc20Balance,
getBridgeOnNearBalance,
getClientBlockHeightHash,
getAddressBySecretKey,
ethToNearApprove,
Expand Down Expand Up @@ -640,7 +641,7 @@ const testingCommand = program
)

RainbowConfig.addOptions(
testingCommand
program

This comment has been minimized.

Copy link
@mfornet

mfornet Feb 5, 2021

Let's not make this commands native from this CLI and keep them behind testingCommand for now. Rationale about this, is that we want to separate as much as possible responsibilities between bridge and token-connector. Transfers are on token-connector side. We still need them in this repository for testing purposes, but probably we should refactor so users don't try to use this command. They are not a reliable way to make such transfers since it is possible that a transaction is stopped mid-way and it can't be recovered.

This comment has been minimized.

Copy link
@mfornet

mfornet Feb 5, 2021

Same applies to all commands that are aware about token-connector

This comment has been minimized.

Copy link
@sept-en

sept-en Feb 5, 2021

Author Owner

What do you think if I add it as a subcommand not to the testingCommand (is called with TESTING keyword) but to something more meaningful - e.g. utils, token-connector, etc. I think utils is the most suitable here.

This comment has been minimized.

Copy link
@mfornet

mfornet Feb 5, 2021

I'm not entirely opposed to that idea, but it is intended to be "hard" to use, since we want discourage users from doing this in the future from the cli.

What we should do in the future is move all this commands to token-connector-cli (not existing yet) if at all, otherwise, remove them, and run tests invoking proper functions directly from javascript, rather than calling the cli.

This comment has been minimized.

Copy link
@mfornet

mfornet Feb 5, 2021

The steps in the README regarding transfers, should change soon (probably next week) to point to step-by-step guide and front-end.

.command('transfer-eth-erc20-to-near')
.option('--amount <amount>', 'Amount of ERC20 tokens to transfer')
.option(
Expand Down Expand Up @@ -679,7 +680,7 @@ RainbowConfig.addOptions(
)

RainbowConfig.addOptions(
testingCommand
program
.command('transfer-eth-erc20-from-near')
.option('--amount <amount>', 'Amount of ERC20 tokens to transfer')
.option(
Expand Down Expand Up @@ -754,13 +755,13 @@ RainbowConfig.addOptions(
)

RainbowConfig.addOptions(
testingCommand
.command('get-erc20-balance <eth_account_address> <token_name>')
program
.command('get-erc20-balance <eth_account_address>')
.description('Get ERC20 balance on Ethereum for specific token (e.g. erc20).'),
async (ethAccountAddress, tokenName, args) => {
if (tokenName) {
args.ethErc20Address = RainbowConfig.getParam(`eth-${tokenName}-address`)
}
async (ethAccountAddress, args) => {
console.log(
`(Contract: ${args.ethErc20Address}) ERC20 balance of ${ethAccountAddress} is:`
)
await getErc20Balance({ ethAccountAddress, ...args })
},
[
Expand All @@ -770,6 +771,26 @@ RainbowConfig.addOptions(
]
)

RainbowConfig.addOptions(
program
.command('get-bridge-on-near-balance')
.option(
'--near-receiver-account <near_receiver_account>',
'The account on NEAR blockchain that received the minted token.'
)
.description('Gets balance on Near for Rainbow-Bridge Eth on Near Prover.'),
async (args) => {
await getBridgeOnNearBalance(args)
},
[
'near-master-account',
'near-erc20-account',
'near-network-id',
'near-node-url',
'near-master-sk'
]
)

RainbowConfig.addOptions(
testingCommand
.command('get-client-block-height-hash')
Expand Down
55 changes: 53 additions & 2 deletions testing/adapter/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ const fs = require('fs')
const bs58 = require('bs58')
const { toBuffer } = require('eth-util-lite')
const {
nearAPI,
RobustWeb3,
remove0x,
normalizeEthKey,
Expand Down Expand Up @@ -64,8 +65,8 @@ async function getErc20Balance ({ ethAccountAddress, ethNodeUrl, ethErc20Address
JSON.parse(fs.readFileSync(ethErc20AbiPath)),
remove0x(ethErc20Address)
)
const result = await ethContract.methods.balanceOf(remove0x(ethAccountAddress)).call()
console.log(result)
const balance = await ethContract.methods.balanceOf(remove0x(ethAccountAddress)).call()
console.log(balance)
} catch (error) {
console.log('Failed', error.toString())
}
Expand Down Expand Up @@ -198,10 +199,60 @@ async function nearToEthUnlock ({
exit(0)
}

// View
async function getBridgeOnNearBalance ({
nearReceiverAccount,
nearMasterAccount: nearMasterAccountId,
nearErc20Account,
nearMasterSk,
nearNetworkId,
nearNodeUrl,
}) {
try {
// @ts-ignore
const keyStore = new nearAPI.keyStores.InMemoryKeyStore()
await keyStore.setKey(
nearNetworkId,
nearMasterAccountId,
nearAPI.KeyPair.fromString(nearMasterSk)
)

This comment has been minimized.

Copy link
@mfornet

mfornet Feb 5, 2021

You don't need to pass a secret key if you are only making view calls. It works with an empty keyStore

const near = await nearAPI.connect({
nodeUrl: nearNodeUrl,
networkId: nearNetworkId,
masterAccount: nearMasterAccountId,
deps: { keyStore: keyStore }
})
const nearMasterAccount = new nearAPI.Account(
near.connection,
nearMasterAccountId
)

const nearTokenContract = new nearAPI.Contract(
nearMasterAccount,
nearErc20Account,
{
changeMethods: [],
viewMethods: ['get_balance']
}
)

const balance = await nearTokenContract.get_balance({
owner_id: nearReceiverAccount
})
console.log(
`[Rainbow-Bridge on Near] Balance of ${nearReceiverAccount} is ${balance}`
)
} catch (error) {
console.log('Failed', error.toString())
}
exit(0)
}

exports.ethToNearApprove = ethToNearApprove
exports.ethToNearLock = ethToNearLock
exports.nearToEthUnlock = nearToEthUnlock
exports.mintErc20 = mintErc20
exports.getErc20Balance = getErc20Balance
exports.getAddressBySecretKey = getAddressBySecretKey
exports.getClientBlockHeightHash = getClientBlockHeightHash
exports.getBridgeOnNearBalance = getBridgeOnNearBalance
2 changes: 2 additions & 0 deletions testing/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const {
const {
mintErc20,
getErc20Balance,
getBridgeOnNearBalance,
getClientBlockHeightHash,
getAddressBySecretKey,
ethToNearApprove,
Expand All @@ -18,6 +19,7 @@ exports.TransferEthERC20FromNear = TransferEthERC20FromNear
exports.DeployToken = DeployToken
exports.mintErc20 = mintErc20
exports.getErc20Balance = getErc20Balance
exports.getBridgeOnNearBalance = getBridgeOnNearBalance
exports.getClientBlockHeightHash = getClientBlockHeightHash
exports.getAddressBySecretKey = getAddressBySecretKey
exports.ethToNearApprove = ethToNearApprove
Expand Down

0 comments on commit 5b65516

Please sign in to comment.