RLP encode subcommand #944
Conversation
…is file This is intentionally not a generic command as anyway RPL encoding is specific to each type of data. It was coded with efficiency in mind and as there's only one encodable type for the moment, the --type option is not provided. We'll add the option later if needed and the current type will be the default one. However a RLPEncodable interface is added to handle the RLP encode behaviour. JSON annotations were added for type mapping. made the file output optional and write to standard output by default, same for standard input. Some mnore tests are required, this is a preview version.
NicolasMassart
added
work in progress
NicolasMassart
requested review from
ajsutton,
mbaxter,
shemnon and
smatthewenglish
consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftExtraData.java
Outdated
Show resolved
Hide resolved
consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftExtraData.java
Outdated
Show resolved
Hide resolved
pantheon/src/main/java/tech/pegasys/pantheon/cli/RLPSubCommand.java
Outdated
Show resolved
Hide resolved
pantheon/src/main/java/tech/pegasys/pantheon/cli/RLPSubCommand.java
Outdated
Show resolved
Hide resolved
pantheon/src/main/java/tech/pegasys/pantheon/cli/RLPSubCommand.java
Outdated
Show resolved
Hide resolved
pantheon/src/main/java/tech/pegasys/pantheon/cli/RLPSubCommand.java
Outdated
Show resolved
Hide resolved
…with JSON @ Also found a way to make the enum able to return a type in a generic way using an interface so the `--type` option can be used. We still have only one type but at least it's ready for two. Still some unit tests to add in the next commit.
moved the provider interface into the enum as only used by it for impl purpose
update doc accordingly
| * Adapter to convert a typed JSON to an IbftExtraData object This adapter handles the JSON to RLP | ||
| * encoding | ||
| */ | ||
| public class IbftExtraDataCLIAdapter implements JSONToRLP { |
There was a problem hiding this comment.
Instead of an extra interface with an adapter, I think it would be more concise and direct to move the method body into the enum type and add an "encode" method on that enum:
There was a problem hiding this comment.
The thing is that for now we only have one type in the enum but we may end up with more and I wanted to keep the enum as simple as possible by just referencing an adapter and have one adapter for each data type, well isolated into each class file. But why not. What would you think @mbaxter and @rain-on ?
There was a problem hiding this comment.
You can use the same brace-initialized-enum-value pattern for future encoders, so this doesn't preclude more values in the enum.
There was a problem hiding this comment.
No it doesn't but the more values we'll have in the enum the more code we'll have and I'm happier with small files when possible. But I admit your proposal removes a class and an interface so why not. Waiting for a second advice.
There was a problem hiding this comment.
keeping the enum simple makes sense to me
There was a problem hiding this comment.
Indeed I would like to try to start splitting things to prevent mega classes like PantheonCommand. CLI is usually the place where you enter Pantheon so having it very easy to navigate seems important to me. So yes it requires a bit more classes and interfaces but it also makes it easier for someone new to understand and add a feature in my opinion.
Add RLP encoding subcommand for at least ibftExtraData to be inserted in genesis file. There's only one encodable type for the moment, however the `--type` option is already there with a default value on this type. We'll add more possible types later if needed and the current type will remain the default one. The file output is optional and it writes to standard output by default, same for standard input or a file. Tests and doc change.
RLP encode subcommand backport (#944) Add RLP encoding subcommand for at least ibftExtraData to be inserted in genesis file. There's only one encodable type for the moment, however the `--type` option is already there with a default value on this type. We'll add more possible types later if needed and the current type will remain the default one. The file output is optional and it writes to standard output by default, same for standard input or a file. Tests and doc change.
PR description
Add RLP encoding subcommand for at least ibftExtraData to be inserted in genesis file.
There's only one encodable type for the moment, however the
--typeoption is already there with a default value on this type. We'll add the more possible types later if needed and the current type will remain the default one.The file output is optional and it writes to standard output by default, same for
standard input or a file.
Tests and doc changes are added.
Fixed Issue(s)
fixes PAN-2318