Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Enable forging on Genesis Delegates while starting devnet - Closes #3734 #3740

Merged
merged 5 commits into from
Jun 5, 2019

Conversation

MaciejBaj
Copy link
Contributor

What was the problem?

Each time I start an application created using Alpha SDK I need to specify the full set of delegates in the entry file of a project.

How did I fix it?

  • Create a devnet genesis block containing all possible defaults
  • Assign chain default configuration using devnet sample
  • Assign http_api default configuration using devnet sample
  • Assign network default configuration using devnet sample
  • Assign components defaults configuration using devnet sample

How to test it?

Start a sample app and verify whether the "Forgin enabled" logs are there.

Review checklist

Copy link
Contributor

@nazarhussain nazarhussain left a comment

Choose a reason for hiding this comment

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

You are setting default for all components and all modules from the sample. But not using it for app controller, that may lead to confusion in future.

https://github.com/LiskHQ/lisk-sdk/blob/ac1f56536f934188cb610990066c3f6a5bc3b966/framework/src/controller/schema/application_config_schema.js#L207-L240

Do you really think linking sample files to the framework is a good idea?

I would suggest to keep these two completely separate, you can still export the devnet json file. And the user should explicitly import and use it in the app. So he must be aware what he is doing or what sensitive information like delegates, forging he is using.

const {Application, sampleGenesisBlock, forgingEnabledConfig} = require('lisk-framework');

const app = new Application(sampleGenesisBlock, forgingEnabledConfig);

So for end user you can export different configurations files with different useable cases, and they will required that file and know why the app is behaving like that.

@MaciejBaj
Copy link
Contributor Author

@nazarhussain I will export the config filled with all of the possible defaults from lisk-sdk and use it in sample app by passing as a second parameter after GenesisBlock.
I will remove the reference to this sample from the defaults.

@MaciejBaj MaciejBaj force-pushed the 3734-enable_forging_by_default branch from e85045a to b644e13 Compare May 28, 2019 16:17
@shuse2 shuse2 requested a review from nazarhussain June 3, 2019 08:25
"port": 5432,
"database": "lisk_dev",
"user": "lisk",
"password": "password",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest not to set username/password here. So while running the sample, user can use their own PostgreSQL user, rather than creating a new user.

If usage of this file is intended for internal demo, then ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That password was "always" present here - the default lisk user set up is described in the Postgres installation manual.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer to leave it this way, so there is a default setting.

@shuse2 shuse2 merged commit 78490f1 into release/2.1.0 Jun 5, 2019
@shuse2 shuse2 deleted the 3734-enable_forging_by_default branch June 5, 2019 22:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants