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

Branded token setup #112

Merged
merged 16 commits into from
Mar 7, 2019
Merged

Branded token setup #112

merged 16 commits into from
Mar 7, 2019

Conversation

0xsarvesh
Copy link
Contributor

@0xsarvesh 0xsarvesh commented Mar 1, 2019

  • code
  • documentation
  • tests

Fixes #94

This PR is about exposing setup which makes deployment on branded token and Utility branded token easy for the consumer.

Readme is updated which demonstrates how to use setup.
Setup exposes three functions:

  1. organization: This deploys an organization contract.
  2. brandedToken: This deploys Branded token contract.
  3. utilityToken: This deploys auxiliary organization, utility branded token, EIP20Gateway and EIP20CoGateway.

@0xsarvesh 0xsarvesh self-assigned this Mar 1, 2019
Copy link
Contributor

@schemar schemar left a comment

Choose a reason for hiding this comment

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

* [x]  code

* [x]  documentation

* [ ]  tests

Fixes #94

This PR is about exposing setup which makes deployment on branded token and Utility branded token easy for the consumer.

Readme is updated which demonstrates how to use setup.
Setup exposes three functions:

1. organization: This deploys an organization contract.

2. brandedToken: This deploys Branded token contract.

3. utilityToken: This deploys auxiliary organization, utility branded token, EIP20Gateway and EIP20CoGateway.

Nice 👍
Is it correct that 3. also deploys the organization? Or should it accept a deployed organization from an earlier step? In the same vein: Should "setup utility token" deploy a CoGateway? Maybe there should be a "setup economy" instead, which deploys all?

Should we fix it here and remove deployer from the deployment configs (at least for brandedtoken.js, mosaic.js is out-of-scope)? Should be set as txOptions.from, I think.

README.MD Outdated Show resolved Hide resolved
README.MD Outdated Show resolved Hide resolved
lib/Setup/brandedtoken.js Show resolved Hide resolved
lib/Setup/utilitybrandedtoken.js Outdated Show resolved Hide resolved
benjaminbollen
benjaminbollen previously approved these changes Mar 1, 2019
Copy link
Contributor

@benjaminbollen benjaminbollen left a comment

Choose a reason for hiding this comment

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

LGTM

@0xsarvesh
Copy link
Contributor Author

* [x]  code

* [x]  documentation

* [ ]  tests

Fixes #94
This PR is about exposing setup which makes deployment on branded token and Utility branded token easy for the consumer.
Readme is updated which demonstrates how to use setup.
Setup exposes three functions:

1. organization: This deploys an organization contract.

2. brandedToken: This deploys Branded token contract.

3. utilityToken: This deploys auxiliary organization, utility branded token, EIP20Gateway and EIP20CoGateway.

Nice 👍
Is it correct that 3. also deploys the organization? Or should it accept a deployed organization from an earlier step? In the same vein: Should "setup utility token" deploy a CoGateway? Maybe there should be a "setup economy" instead, which deploys all?

Should we fix it here and remove deployer from the deployment configs (at least for brandedtoken.js, mosaic.js is out-of-scope)? Should be set as txOptions.from, I think.

The first step only deploys origin organization, it doesn't deploy pair.

Gateways deployment config has deployer key, it delegates config to mosaic.js setup module. I think mosaic.js needs to be updated first.

@schemar
Copy link
Contributor

schemar commented Mar 4, 2019

* [x]  code

* [x]  documentation

* [ ]  tests

Fixes #94
This PR is about exposing setup which makes deployment on branded token and Utility branded token easy for the consumer.
Readme is updated which demonstrates how to use setup.
Setup exposes three functions:

1. organization: This deploys an organization contract.

2. brandedToken: This deploys Branded token contract.

3. utilityToken: This deploys auxiliary organization, utility branded token, EIP20Gateway and EIP20CoGateway.

Nice 👍
Is it correct that 3. also deploys the organization? Or should it accept a deployed organization from an earlier step? In the same vein: Should "setup utility token" deploy a CoGateway? Maybe there should be a "setup economy" instead, which deploys all?
Should we fix it here and remove deployer from the deployment configs (at least for brandedtoken.js, mosaic.js is out-of-scope)? Should be set as txOptions.from, I think.

The first step only deploys origin organization, it doesn't deploy pair.

Gateways deployment config has deployer key, it delegates config to mosaic.js setup module. I think mosaic.js needs to be updated first.

What I meant is: should step 3 deploy an organization or should it actually get a deployed organization as parameter?

@0xsarvesh
Copy link
Contributor Author

* [x]  code

* [x]  documentation

* [ ]  tests

Fixes #94
This PR is about exposing setup which makes deployment on branded token and Utility branded token easy for the consumer.
Readme is updated which demonstrates how to use setup.
Setup exposes three functions:

1. organization: This deploys an organization contract.

2. brandedToken: This deploys Branded token contract.

3. utilityToken: This deploys auxiliary organization, utility branded token, EIP20Gateway and EIP20CoGateway.

Nice 👍
Is it correct that 3. also deploys the organization? Or should it accept a deployed organization from an earlier step? In the same vein: Should "setup utility token" deploy a CoGateway? Maybe there should be a "setup economy" instead, which deploys all?
Should we fix it here and remove deployer from the deployment configs (at least for brandedtoken.js, mosaic.js is out-of-scope)? Should be set as txOptions.from, I think.

The first step only deploys origin organization, it doesn't deploy pair.
Gateways deployment config has deployer key, it delegates config to mosaic.js setup module. I think mosaic.js needs to be updated first.

What I meant is: should step 3 deploy an organization or should it actually get a deployed organization as parameter?

Why do you think an organization will exist if the economy(UBT and Co-gateway) doesn't exist? 🤔

@schemar
Copy link
Contributor

schemar commented Mar 7, 2019

* [x]  code

* [x]  documentation

* [ ]  tests

Fixes #94
This PR is about exposing setup which makes deployment on branded token and Utility branded token easy for the consumer.
Readme is updated which demonstrates how to use setup.
Setup exposes three functions:

1. organization: This deploys an organization contract.

2. brandedToken: This deploys Branded token contract.

3. utilityToken: This deploys auxiliary organization, utility branded token, EIP20Gateway and EIP20CoGateway.

Nice 👍
Is it correct that 3. also deploys the organization? Or should it accept a deployed organization from an earlier step? In the same vein: Should "setup utility token" deploy a CoGateway? Maybe there should be a "setup economy" instead, which deploys all?
Should we fix it here and remove deployer from the deployment configs (at least for brandedtoken.js, mosaic.js is out-of-scope)? Should be set as txOptions.from, I think.

The first step only deploys origin organization, it doesn't deploy pair.
Gateways deployment config has deployer key, it delegates config to mosaic.js setup module. I think mosaic.js needs to be updated first.

What I meant is: should step 3 deploy an organization or should it actually get a deployed organization as parameter?

Why do you think an organization will exist if the economy(UBT and Co-gateway) doesn't exist? 🤔

Just thinking it could exist, e.g. if I already have another BT economy on an auxiliary chain. It's probably not that important right now.

Copy link
Contributor

@schemar schemar left a comment

Choose a reason for hiding this comment

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

Nice :shipit:

@schemar schemar merged commit 1ad51bb into OpenST:develop Mar 7, 2019
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.

3 participants