Skip to content
This repository has been archived by the owner on Aug 23, 2020. It is now read-only.

IRI can be now deployed as testnet or mainnet compatible #610

Merged
merged 9 commits into from
Apr 4, 2018

Conversation

GalRogozinski
Copy link
Contributor

@GalRogozinski GalRogozinski commented Apr 3, 2018

Note
Since the object of this PR is to help getting PRs get merged quickly, without creating too many conflicts with the PRs that are now waiting in queue. This fix is quick and dirty.
When Configuration will be refactored we will no longer have to pass values via constructors and methods. In the meanwhile when you review the code try to concentrate on things that break functionality.

Description

A prerequisite for merging a PR, no matter how small and benign it seems, is to let it mature on the testnet. This hinders the mergers of PRs to this branch.

We are now creating a new process that will allow PRs to be merged quickly after they passed a review and basic sanity tests. In order to make this process work IRI code should be able to take the roll of a testnet deployment. Even though there is a --testnet flag, manual changes were required.

This change involves 3 commits:

  1. Added an API call that is only functional on the testnet. This is actually not the correct way to do this and should be part of an ixi module. See Create an ixi for testnet store messages api call #609.
  2. Give the --testnet flag the ability to deploy iri as testnet. New flags were also introduced that will enable to run IRI in a customized private testnet.
  3. Integrated a global snapshot of the public testnet (calm down mainnet is unaffected)
  4. Version is updated because of testnet global snapshot.

List of new flags:
--testnet: [bool] flag to trigger further non-mainnet config
--testnet-coordinator: [string] coo address (no checksum)
--testnet-no-coo-validation: [bool] signature validation on/off
--snapshot: [file path] [default snapshotMainnet.txt/snapshotTestnet.txt]
--snapshot-sig: [file path] [default snapshotMainnet.sig/snapshotTestnet.sig] (although on testnet the signature is not verified)
--mwm: [int] [default 14/9] [min 13/0] [max ~/9]
--milestone-start [int] (subtangle first milestone, usually it's the latest snapshot milestone and/or most recent bootstrap+1 milestone) [default mainnet/testnet]
--milestone-keys: (advanced option: defines the depth of the merkle tree used to sign milestones by the coo) [int] [defaults 20/22]
--snapshot-timestamp: [epoch] [defaults epoch-mainnet/epoch-testnet]

Type of change

  • Enhancement (non-breaking change which adds functionality)

How Has This Been Tested?

Community testing is required

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation - TBD
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@@ -143,7 +143,6 @@ private static void validateParams(final Configuration configuration, final Stri
configuration.put(DefaultConfSettings.COORDINATOR, Configuration.TESTNET_COORDINATOR_ADDRESS);
configuration.put(DefaultConfSettings.SNAPSHOT_FILE, Configuration.TESTNET_SNAPSHOT_FILE);
configuration.put(DefaultConfSettings.MILESTONE_START_INDEX, Configuration.TESTNET_MILESTONE_START_INDEX);
configuration.put(DefaultConfSettings.SNAPSHOT_SIGNATURE_FILE, Configuration.TESTNET_SNAPSHOT_SIG_FILE);
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand the reasoning here - that snapshotTestnet.sig is not validated so there is no need to point to it,
However, his means that snapshotMainnet.sig will be read also for testnet, even if the signature is not validated it still mixes file names.

better to set as null / ""

Copy link
Contributor Author

Choose a reason for hiding this comment

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

set to "".
I think null will raise an exception when inserted as a value to ConcurrentHashMap<>

try {
InputStream inputStream = SignedFiles.class.getResourceAsStream(filename);
if (inputStream == null) {
inputStream = new FileInputStream(filename);
Copy link
Contributor

Choose a reason for hiding this comment

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

add a comment to explicitly state that if a path is not in resources, then reading the file from OS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added similar comment in Snapshot.java as well

private static InputStream getSnapshotStream(String snapshotPath) throws FileNotFoundException {
InputStream inputStream = Snapshot.class.getResourceAsStream(snapshotPath);
if (inputStream == null) {
inputStream = new FileInputStream(snapshotPath);
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment as in signedFiles

@GalRogozinski
Copy link
Contributor Author

Sanity tests were performed

@alon-e alon-e merged commit d59de9f into iotaledger:dev Apr 4, 2018
@footloosejava
Copy link
Contributor

footloosejava commented Apr 5, 2018

Nice work! Let me know if you need help with Configuration. I might have a few ideas - see https://github.com/footloosejava/annoconf. I'm pretty handy with Guice too ;-) I'm also a big fan of thread-safe classes and passing in configurable values via the constructor - and that is where Guice makes things really easy ... and fast to design, test and reconfigure.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants