[PIE-1809] Add import chain json utility #1832

mbaxter · 2019-08-07T23:16:13Z

PR description

Add a CLI utility for importing blocks from a json file containing minimally specified block and transaction data.

Example command:

pantheon --network=DEV blocks import --format=JSON --from="/path/to/file.json"

Example import file:

{
  "blocks": [
    {
      "number": 1,
      "transactions": [
        {
          "fromPrivateKey": "8f2a55949038a9610f50fb23b5883af3b4ecb3c3bb792cbcefbd1542c692be63",
          "gasLimit": "0xFFFFF1",
          "gasPrice": "0xFF",
          "value": "0x01",
          "to": "627306090abaB3A6e1400e9345bC60c78a8BEf57"
        },
        {
          "fromPrivateKey": "8f2a55949038a9610f50fb23b5883af3b4ecb3c3bb792cbcefbd1542c692be63",
          "gasLimit": "0xFFFFF2",
          "gasPrice": "0xEF",
          "to": "f17f52151EbEF6C7334FAD080c5704D77216b732"
        }
      ]
    }
  ]
}

Example output:

2019-08-07 19:07:58.453-04:00 | main | INFO  | BlocksSubCommand | Import JSON block data from /path/to/file.json 
2019-08-07 19:07:59.334-04:00 | main | INFO  | KeyPairUtil | Loaded key 0x5285b91de23f6e519836e0d0fe8413eef43f453a2e0af2d154b6bbae92175bdbfca37d0eec7095bf4b8ddfbf813aa669b26640dbeaf218ed3846744cd869bc01 from /data/key 
2019-08-07 19:07:59.594-04:00 | main | INFO  | ProtocolScheduleBuilder | Protocol schedule created with milestones: [ConstantinopleFix: 0] 
2019-08-07 19:07:59.775-04:00 | main | WARN  | ChainImporter | Importing to a non-empty database with chain height 2.  This may cause imported blocks to be considered non-canonical. 
2019-08-07 19:07:59.828-04:00 | main | INFO  | ChainImporter | Preparing to import block at height 1 (parent: 0xa08d1edb37ba1c62db764ef7c2566cbe368b850f5b3762c6c24114a3fd97b87f) 
2019-08-07 19:08:02.961-04:00 | main | INFO  | ChainImporter | Successfully created and imported block at height 1 (0xb062d54fbcdbe150f2ee38d10f2393c796f66048d5fc36ef8cee02c0bf807e3e) 
2019-08-07 19:08:02.967-04:00 | main | WARN  | ChainImporter | 1 / 1 imported blocks are not on the canonical chain: #1 (0xb062d54fbcdbe150f2ee38d10f2393c796f66048d5fc36ef8cee02c0bf807e3e)

RatanRSur · 2019-08-08T21:05:21Z

pantheon/src/main/java/tech/pegasys/pantheon/cli/subcommands/blocks/BlocksSubCommand.java

@@ -256,4 +322,9 @@ private void outputBlock(final List<Block> blocks) {
    }
    return metricsService;
  }
+
+  @FunctionalInterface
+  public interface ChainImporterFactory {


Why does this need to be an explicit functional interface? Could we not use a Supplier?

We could use Function<PantheonController<T>, ChainImporter<T>> instead. But, I prefer making explicit interfaces for reasons of readability and discoverability. It's a pattern we use pretty extensively in the codebase.

Interesting, I find the explicit interface less readable because the parameter looks like it could be a full blown object with more methods. Not a blocking issue though

RatanRSur · 2019-08-08T21:14:45Z

pantheon/src/main/java/tech/pegasys/pantheon/chainimport/ChainImporter.java

+          "Unable to create block using current consensus engine: "
+              + genesisConfigOptions.getConsensusEngine());
+    }
+    return maybeBlock.get();


(optional) use orElseThrow and inline maybeBlock

RatanRSur · 2019-08-08T21:25:43Z

pantheon/src/main/java/tech/pegasys/pantheon/chainimport/ChainImporter.java

+  }
+
+  public void importChain(final String chainJson) throws IOException {
+    checkDatabaseIsEmpty();


(optional) this sounds stricter than it is, how about warnIfDatabaseNotEmpty?

RatanRSur · 2019-08-08T21:28:33Z

pantheon/src/main/java/tech/pegasys/pantheon/chainimport/ChainData.java

@@ -0,0 +1,33 @@
+/*


Is there a way to parse the blocks directly into a sequence without the need of a separate class?

Maybe? But I think the structure is pretty clear as is and it provides a place to add additional data if we need it in the future.

RatanRSur · 2019-08-08T21:29:53Z

pantheon/src/main/java/tech/pegasys/pantheon/chainimport/ChainImporter.java

+  public void importChain(final String chainJson) throws IOException {
+    checkDatabaseIsEmpty();
+
+    ChainData chainData = mapper.readValue(chainJson, ChainData.class);


Regarding the comment on the ChainData file. This reads like there's probably more to ChainData than just a sequence of blocks

RatanRSur · 2019-08-08T21:30:40Z

pantheon/src/main/java/tech/pegasys/pantheon/chainimport/ChainImporter.java

+  public void importChain(final String chainJson) throws IOException {
+    checkDatabaseIsEmpty();
+
+    ChainData chainData = mapper.readValue(chainJson, ChainData.class);


can this be final?

yep - I always miss the finals 😄

RatanRSur · 2019-08-08T21:37:33Z

pantheon/src/main/java/tech/pegasys/pantheon/chainimport/ChainImporter.java

+    }
+
+    // Check that imported blocks form the canonical chain, if not print a warning
+    this.checkImportedBlocksAreOnCanonicalChain(importedBlocks);


(optional) The comment could probably be removed (and thus not at risk of getting outdated) if the method was something like warnIfImportedBlocksNotOnCanonicalChain

pantheon/src/main/java/tech/pegasys/pantheon/chainimport/ChainImporter.java

RatanRSur · 2019-08-09T01:31:57Z

pantheon/src/test/java/tech/pegasys/pantheon/cli/subcommands/blocks/BlockSubCommandTest.java

          + System.lineSeparator()
          + "This command imports blocks from a file into the database."
          + System.lineSeparator()
-          + "      --from=<FILE>   File containing blocks to import"
+          + "      --format=<format>   The type of data to be imported (default: RLP)."


Is there somewhere the user can see that JSON is a valid option to this flag?

Good call - will update!

shemnon · 2019-08-12T15:17:12Z

pantheon/src/main/java/tech/pegasys/pantheon/cli/subcommands/blocks/BlocksSubCommand.java

-        description = "The type of data to be imported (default: ${DEFAULT-VALUE}).",
+        hidden = true,
+        description =
+            "The type of data to be imported, possible values are: ${COMPLETION-CANDIDATES} (default: ${DEFAULT-VALUE}).",


I didn't know ${COMPLETION-CANDIDATES} existed. Looks like I should open another PR to add this to other options as well.

mbaxter added 10 commits August 7, 2019 11:49

Add json variant of blocks import command

7afe197

Add tests

8b910ce

Undo gradle change

2dcd89b

Rework new API

dc4b0a6

Fix bug with new getConsensusEngine method, add tests

dfab38f

Modify ChainImporter to work with clique, add parameterized tests

ce269c3

Split up singleton tests from parameterized tests

f0a5a02

Merge branch 'master' into PIE-1809/add-import-chain-json-utility

2254e46

Calculate transaction nonces so the user doesn't have to

474379c

Add default value to --format description

c7d98c8

mbaxter marked this pull request as ready for review August 8, 2019 20:44

RatanRSur reviewed Aug 8, 2019

View reviewed changes

RatanRSur reviewed Aug 9, 2019

View reviewed changes

pantheon/src/main/java/tech/pegasys/pantheon/chainimport/ChainImporter.java Outdated Show resolved Hide resolved

RatanRSur reviewed Aug 9, 2019

View reviewed changes

mbaxter added 5 commits August 9, 2019 15:32

Address review comments

db20a0b

Merge branch 'master' into PIE-1809/add-import-chain-json-utility

25e34bc

Merge branch 'master' into PIE-1809/add-import-chain-json-utility

70c8408

Rename test to match class name

3b18ef5

Hide format option for now

284dd39

shemnon approved these changes Aug 12, 2019

View reviewed changes

mbaxter added 2 commits August 12, 2019 11:48

Run formatter

d3e3ff0

Merge branch 'master' into PIE-1809/add-import-chain-json-utility

029e2b4

mbaxter merged commit 6998e65 into PegaSysEng:master Aug 12, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PIE-1809] Add import chain json utility #1832

[PIE-1809] Add import chain json utility #1832

mbaxter commented Aug 7, 2019 •

edited

Loading

RatanRSur Aug 8, 2019

mbaxter Aug 9, 2019

RatanRSur Aug 9, 2019

RatanRSur Aug 8, 2019 •

edited

Loading

RatanRSur Aug 8, 2019 •

edited

Loading

RatanRSur Aug 8, 2019

mbaxter Aug 9, 2019

RatanRSur Aug 8, 2019 •

edited

Loading

RatanRSur Aug 8, 2019

mbaxter Aug 9, 2019

RatanRSur Aug 8, 2019

RatanRSur Aug 9, 2019

mbaxter Aug 9, 2019

shemnon Aug 12, 2019

[PIE-1809] Add import chain json utility #1832

[PIE-1809] Add import chain json utility #1832

Conversation

mbaxter commented Aug 7, 2019 • edited Loading

PR description

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RatanRSur Aug 8, 2019 • edited Loading

Choose a reason for hiding this comment

RatanRSur Aug 8, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RatanRSur Aug 8, 2019 • edited Loading

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

Choose a reason for hiding this comment

mbaxter commented Aug 7, 2019 •

edited

Loading

RatanRSur Aug 8, 2019 •

edited

Loading

RatanRSur Aug 8, 2019 •

edited

Loading

RatanRSur Aug 8, 2019 •

edited

Loading