Feature/kovan parity #57

jcortejoso · 2018-11-23T14:32:31Z

Description

Added support to spin up a parity client which connects to kovan network and Ocean testnet POA.

Is this PR related with an open issue?

Related to Issue #51

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

Follows the code style of this project.
Tests Cover Changes
Documentation

Funny gif

ttmc

It would be nice to have some documentation about how to use this option as well.

start_ocean.sh

aaitor

looks good to me. Because adding this change, to deploy in kovan/test net require to unlock the address, can you add some info about how the users should unlock their accounts?

start_ocean.sh

README.md

Co-Authored-By: jcortejoso <[email protected]>

README.md

H34D · 2018-11-26T12:13:20Z

README.md

+then `docker-compose-only-parity.yml` Docker Compose file will be used. It will start only one container (`oceanprotocol/parity-ethereum:master`) that will connect to the Kovan testnet network. The parity client is configure to allow connections with the RPC interface. With this option, you must place in `./parity/kovan/account.json` your account json and in `./parity/kovan/password` a file with the account passsword (default locations). Also you must export the variable `UNLOCK_ADDRESS` with the account address of this account. For example:
+
+```bash
+export UNLOCK_ADDRESS="0x00bd138abd70e2f00903268f3db08f2d25677c9e"


we should use an nmemoric for this instead of private key / address / password combination

Sure, any recommendation?

H34D · 2018-11-26T12:18:19Z

./start_ocean.sh --no-pleuston --latest --kovan-parity-node gives me Option --no-pleuston is not compatible with option --local-secret-store

Co-Authored-By: jcortejoso <[email protected]>

jcortejoso · 2018-11-26T12:22:52Z

@H34D not sure if we should include logic about compatibility of option, but in that case --kovan-parity-node and --no-pleuston are kind of incompatible. I mean, it would have different results running ./start_ocean.sh --no-pleuston --kovan-parity-node and ./start_ocean.sh --kovan-parity-node --no-pleuston, which is not a good deal. I would say it is better to error when the script is run with two option that force to use different compose files. Additionally, --latest when running --kovan-parity-node or --testnet-parity-node has no effect.

But the message is not correct. Thanks!

H34D · 2018-11-26T12:30:34Z

We should never run nodes (to persistent blockchains) with unlocked account. I also don't see the need to have unlocked accounts anyway.

There are a few stages:

Development / Local: Usually a docker or ganache-cli or parity --dev instance. It's ok to have unlocked accounts because you use them for development of smart contracts or unit tests aka development of (L1 and L2).
Integration: in our case this will be our POA AWS network, we must make sure that there is never a node with unlocked accounts. Thats an antipattern. Deployments to this network could be done from keeper-contracts like they are done now to kovan (with a nmemoric phrase that is private). Why should this docker deploy anything anyway? There is a release process of contracts already.
Test: Kovan, there is a deployment process in place. No need for unlocked accounts at all.
Live: Mainnet, never unlock accounts here.

Prosal:

remove unlocked accounts
remove migration process from docker
add flags for using different networks underneath, local poa, aws poa, kovan, mainnet

H34D · 2018-11-26T12:33:24Z

@H34D not sure if we should include logic about compatibility of option, but in that case --kovan-parity-node and --no-pleuston are kind of incompatible. I mean, it would have different results running ./start_ocean.sh --no-pleuston --kovan-parity-node and ./start_ocean.sh --kovan-parity-node --no-pleuston, which is not a good deal. I would say it is better to error when the script is run with two option that force to use different compose files.

@jcortejoso but why? there is no need for this dependency other than they are in two different compose files.

I can start the whole infrastructure without pleuston and use kovan for the rest, Spin up my own pleuston and connect to kovan with ease.

jcortejoso · 2018-11-26T12:34:57Z

@H34D the idea about having unlock (locally) an account was to avoid the way we were deploying currently the contracts. Using keeper-contracts locally to connect directly to the container so we do not need to connect to a remote node with an account unlocked. But if there is a better approach to this we can change it.

H34D · 2018-11-26T12:36:55Z

@H34D the idea about having unlock (locally) an account was to avoid the way we were deploying currently the contracts. Using keeper-contracts locally to connect directly to the container so we do not need to connect to a remote node with an account unlocked. But if there is a better approach to this we can change it.

yes but this is only needed for local deployments (ganache-cli or parity --dev) and is a huge risk in the other cases.

jcortejoso · 2018-11-26T12:37:16Z

@H34D about he compatibility of the compose files, the result of running start-ocean.sh is running only one compose files. To run simultaneously multiple compose files, you need to run multiple times start-ocean.sh. So any option which implies different compose-files are not very intuitive.

H34D · 2018-11-26T12:37:53Z

we should declare poa network or kovan to the default and remove the migration part completely from docker. Development stage is an edge case, not the default.

jcortejoso · 2018-11-26T12:39:36Z

@H34D good, I change the unlock part.

H34D · 2018-11-26T12:42:14Z

can we do something like a plugin system where we keep a base and plug the nodes to it like docker-compose -f docker-compose.yml -f docker-compose.kovan.yml up -d

What i can define so far is:

base components: mongo-db, brizo, aquarius
plugin secret store: secret-store, local parity node to sign, cors proxy
plugin node: ganache-cli OR local poa OR poa test OR kovan (represented by different configured parity nodes or ganache)
plugin frontend: pleuston OR nothing

H34D · 2018-11-26T13:18:57Z

can we do something like a plugin system where we keep a base and plug the nodes to it like docker-compose -f docker-compose.yml -f docker-compose.kovan.yml up -d

What i can define so far is:

base components: mongo-db, brizo, aquarius

plugin secret store: secret-store, local parity node to sign, cors proxy

plugin node: ganache-cli OR local poa OR poa test OR kovan (represented by different configured parity nodes or ganache)

plugin frontend: pleuston OR nothing

i tried that out and it works, giving me the following diagram. All of the stacks can be started individually:

this will give the flexibility to start in whatever combination we want combined to full flexibility of node setup, blockchain node could be ganache or mainnet, doesn't matter as long it is exposing port 8545

H34D · 2018-11-26T13:43:41Z

maybe lets describe all components i am starting with all permutations of the blockchain nodes that i can imagine:

Local Ganache node (ganache-cli forming a local private network)
Local Parity node (parity --chain dev forming a local private network)
Local POA Parity node (parity forming a local private network that behaves like POA AWS (but with different Network ID))
AWS POA Parity node (parity node connected to POA network in AWS)
Kovan node (parity node connected to kovan network)
Mainnet node (parity node connected to mainnet network)

| Docker          | Location | Type     | Net  | node.yml |
|=================|==========|==========|======|==========|
| docker-compose- | aws-poa- | parity-  |      | node.yml |
| docker-compose- | kovan-   | parity-  |      | node.yml |
| docker-compose- | local-   | ganache- |      | node.yml |
| docker-compose- | local-   | parity-  | dev- | node.yml |
| docker-compose- | local-   | parity-  | poa- | node.yml |

…r-images into feature/kovan_parity

jcortejoso · 2018-11-26T14:32:11Z

maybe lets describe all components i am starting with all permutations of the blockchain nodes that i can imagine:

* Local Ganache node (`ganache-cli` forming a local private network)

* Local Parity node (`parity --chain dev` forming a local private network)

* Local POA Parity node (`parity` forming a local private network that behaves like POA AWS (but with different Network ID))

* AWS POA Parity node (`parity` node connected to POA network in AWS)

* Kovan node (`parity` node connected to `kovan` network)

* Mainnet node (`parity` node connected to `mainnet` network)

| Docker          | Location | Type     | Net  | node.yml |
|=================|==========|==========|======|==========|
| docker-compose- | aws-poa- | parity-  |      | node.yml |
| docker-compose- | kovan-   | parity-  |      | node.yml |
| docker-compose- | local-   | ganache- |      | node.yml |
| docker-compose- | local-   | parity-  | dev- | node.yml |
| docker-compose- | local-   | parity-  | poa- | node.yml |

👍 Sounds very good. I will open a new PR for facing that refactor.

I have updated this PR removing the possibility to unlock accounts. @H34D does this make sense?

ttmc · 2018-11-26T14:35:27Z

I didn't know that Docker Compose could take multiple docker-compose files in one command-line command, but it can. See:

https://docs.docker.com/compose/reference/overview/#specifying-multiple-compose-files

If we kill off ganache completely, then that would simplify things: the only node "Type" would be parity, so it wouldn't have to be specified at all.

H34D · 2018-11-26T14:52:00Z

I didn't know that Docker Compose could take multiple docker-compose files in one command-line command, but it can. See:

it can an we can build fancy mix and match layer system on top

H34D · 2018-11-26T14:54:07Z

If we kill off ganache completely, then that would simplify things: the only node "Type" would be parity, so it wouldn't have to be specified at all.

but why limiting? i renamed the container to blockchain holding what ever your plugin specifies as blockchain, could be ganache, parity or geth. Or even somthing complelty different, as long it exposes the ethereum protocol on port 8545.

ttmc · 2018-11-26T14:59:58Z

but why limiting? i renamed the container to blockchain holding what ever your plugin specifies as blockchain, could be ganache, parity or geth. Or even somthing complelty different, as long it exposes the ethereum protocol on port 8545.

I'm okay with keeping ganache as an option. I was just looking for opportunities to simplify.

H34D

lgtm

H34D · 2018-11-26T15:39:36Z

.gitignore

+parity/ocean-network/account.json
+parity/ocean-network/password
+parity/kovan/account.json


can be removed

H34D · 2018-11-27T10:27:08Z

./start_ocean.sh --local-secret-store will no give me Option --no-pleuston is not compatible with option --local-secret-store all the time

H34D · 2018-11-27T13:51:25Z

relates to #64

…r-images into feature/kovan_parity

H34D · 2018-11-30T09:46:51Z

README.md

@@ -64,8 +64,10 @@ Option | Description
 ---    | ---
 `--latest` | Get the `latest` versions of all components, referring to their `develop` branches.
 `--no-pleuston` | Start up Ocean without an instance of `pleuston`. Helpful for development on `pleuston`.
-`--local-parity-node` | Runs a local parity POA node and Secret Store instead of ganache-cli.
+`--local-secret-store` | Runs a local parity POA node and Secret Store instead of ganache-cli.
 `--reuse-database` | Start up Ocean and reuse the Database from ganache. Helpful for development.


should be something like --reuse-ganache-database because it triggers a reuse of the blockchain db not the mongodb. This is also a super special case.

H34D · 2018-11-30T09:47:25Z

README.md

 `--reuse-database` | Start up Ocean and reuse the Database from ganache. Helpful for development.
+`--testnet-parity-node` | Start up a parity client connected to Ocean testnet.


does this really connect to the testnet in aws? or is ist setting up an own testnet?

H34D · 2018-11-30T09:47:59Z

README.md

 `--reuse-database` | Start up Ocean and reuse the Database from ganache. Helpful for development.
+`--testnet-parity-node` | Start up a parity client connected to Ocean testnet.
+`--kovan-parity-node` | Start up a parity client connected to Kovan testnet.


parity can be removed for a user it does not matter if it spins up a parity or geth node to connect to kovan

H34D · 2018-11-30T09:48:49Z

compose-files/keeper_contracts.yml

+    environment:
+      LOCAL_CONTRACTS: "true"
+      DEPLOY_CONTRACTS: ${DEPLOY_CONTRACTS}
+      DATABASE_PATH: "/ganache-db"


GANACHE_DATABASE_PATH: "/ganache-db"

H34D · 2018-11-30T09:49:07Z

compose-files/keeper_contracts.yml

+      LOCAL_CONTRACTS: "true"
+      DEPLOY_CONTRACTS: ${DEPLOY_CONTRACTS}
+      DATABASE_PATH: "/ganache-db"
+      REUSE_DATABASE: ${REUSE_DATABASE}


GANCHE_REUSE_DATABASE

H34D · 2018-11-30T09:49:45Z

compose-files/keeper_contracts.yml

+      DATABASE_PATH: "/ganache-db"
+      REUSE_DATABASE: ${REUSE_DATABASE}
+      NETWORK_NAME: ${KEEPER_NETWORK_NAME}
+      POA_HOST: "parity-node"


BLOCKCHAIN_NODE_HOST: "parity-node"

… feature/kovan_parity

jcortejoso · 2018-12-05T16:13:43Z

I think we can close in favor of #71 . Will open a new PR when that is merged to work on Ocean testnet compatibility.

jcortejoso added 2 commits November 23, 2018 15:26

Do not include kovan account

3a594a7

Added option to run parity client in kocan network

d001b43

jcortejoso requested review from ttmc, aaitor and H34D November 23, 2018 14:32

ttmc reviewed Nov 23, 2018

View reviewed changes

start_ocean.sh Outdated Show resolved Hide resolved

jcortejoso added 2 commits November 25, 2018 21:38

Added parity client connected to ocean testnet

1c7e986

Added documentation. Refactor

cac983b

aaitor reviewed Nov 26, 2018

View reviewed changes

ttmc suggested changes Nov 26, 2018

View reviewed changes

start_ocean.sh Outdated Show resolved Hide resolved

start_ocean.sh Outdated Show resolved Hide resolved

README.md Show resolved Hide resolved

jcortejoso and others added 3 commits November 26, 2018 12:23

Adding note about how to connect to local client

8fb67e4

Added missing compose file

368f927

Update start_ocean.sh

fc288cc

Co-Authored-By: jcortejoso <[email protected]>

ttmc reviewed Nov 26, 2018

View reviewed changes

README.md Show resolved Hide resolved

H34D reviewed Nov 26, 2018

View reviewed changes

Update start_ocean.sh

744b9ec

Co-Authored-By: jcortejoso <[email protected]>

jcortejoso added the Status: InProgress Work in progress, don't merge label Nov 26, 2018

ttmc approved these changes Nov 26, 2018

View reviewed changes

jcortejoso added 2 commits November 26, 2018 15:28

Removed unlock account

8646e25

Merge branch 'feature/kovan_parity' of github.com:oceanprotocol/docke…

d41efc8

…r-images into feature/kovan_parity

H34D approved these changes Nov 27, 2018

View reviewed changes

.gitignore

parity/ocean-network/account.json

parity/ocean-network/password

parity/kovan/account.json

Copy link

Contributor

H34D Nov 26, 2018

Choose a reason for hiding this comment

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

can be removed

H34D and others added 3 commits November 27, 2018 11:34

tried to fix --no-pleuston bug

70c3820

Merge remote-tracking branch 'origin/develop' into feature/kovan_parity

9b1a4fa

Updated testnet files

0f86990

jcortejoso added 2 commits November 29, 2018 15:25

Merge branch 'feature/kovan_parity' of github.com:oceanprotocol/docke…

9736b9d

…r-images into feature/kovan_parity

Refactor compose files in individual files

369d221

H34D reviewed Nov 30, 2018

View reviewed changes

jcortejoso added 2 commits December 5, 2018 12:09

Merge branch 'develop' of github.com:oceanprotocol/docker-images into…

a7c285d

… feature/kovan_parity

typo

5954b92

jcortejoso added the freeze label Dec 5, 2018

jcortejoso closed this Dec 5, 2018

jcortejoso removed the Status: InProgress Work in progress, don't merge label Dec 5, 2018

H34D deleted the feature/kovan_parity branch December 10, 2018 12:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/kovan parity #57

Feature/kovan parity #57

jcortejoso commented Nov 23, 2018 •

edited

Loading

ttmc left a comment

aaitor left a comment

H34D Nov 26, 2018

jcortejoso Nov 26, 2018

H34D commented Nov 26, 2018

jcortejoso commented Nov 26, 2018 •

edited

Loading

H34D commented Nov 26, 2018 •

edited

Loading

H34D commented Nov 26, 2018 •

edited

Loading

jcortejoso commented Nov 26, 2018

H34D commented Nov 26, 2018

jcortejoso commented Nov 26, 2018

H34D commented Nov 26, 2018 •

edited

Loading

jcortejoso commented Nov 26, 2018

H34D commented Nov 26, 2018 •

edited

Loading

H34D commented Nov 26, 2018 •

edited

Loading

H34D commented Nov 26, 2018 •

edited

Loading

jcortejoso commented Nov 26, 2018

ttmc commented Nov 26, 2018

H34D commented Nov 26, 2018

H34D commented Nov 26, 2018

ttmc commented Nov 26, 2018

H34D left a comment

H34D Nov 26, 2018

H34D commented Nov 27, 2018

H34D commented Nov 27, 2018

H34D Nov 30, 2018

H34D Nov 30, 2018

H34D Nov 30, 2018

H34D Nov 30, 2018

H34D Nov 30, 2018

H34D Nov 30, 2018

jcortejoso commented Dec 5, 2018

		`--reuse-database` \| Start up Ocean and reuse the Database from ganache. Helpful for development.
		`--testnet-parity-node` \| Start up a parity client connected to Ocean testnet.

Feature/kovan parity #57

Feature/kovan parity #57

Conversation

jcortejoso commented Nov 23, 2018 • edited Loading

Description

Is this PR related with an open issue?

Types of changes

Checklist:

Funny gif

ttmc left a comment

Choose a reason for hiding this comment

aaitor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

H34D commented Nov 26, 2018

jcortejoso commented Nov 26, 2018 • edited Loading

H34D commented Nov 26, 2018 • edited Loading

Prosal:

H34D commented Nov 26, 2018 • edited Loading

jcortejoso commented Nov 26, 2018

H34D commented Nov 26, 2018

jcortejoso commented Nov 26, 2018

H34D commented Nov 26, 2018 • edited Loading

jcortejoso commented Nov 26, 2018

H34D commented Nov 26, 2018 • edited Loading

H34D commented Nov 26, 2018 • edited Loading

H34D commented Nov 26, 2018 • edited Loading

jcortejoso commented Nov 26, 2018

ttmc commented Nov 26, 2018

H34D commented Nov 26, 2018

H34D commented Nov 26, 2018

ttmc commented Nov 26, 2018

H34D left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

H34D commented Nov 27, 2018

H34D commented Nov 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcortejoso commented Dec 5, 2018

jcortejoso commented Nov 23, 2018 •

edited

Loading

jcortejoso commented Nov 26, 2018 •

edited

Loading

H34D commented Nov 26, 2018 •

edited

Loading

H34D commented Nov 26, 2018 •

edited

Loading

H34D commented Nov 26, 2018 •

edited

Loading

H34D commented Nov 26, 2018 •

edited

Loading

H34D commented Nov 26, 2018 •

edited

Loading

H34D commented Nov 26, 2018 •

edited

Loading