Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Split Block Validation from Importing #579

Merged
merged 2 commits into from
Jan 16, 2019

Conversation

rain-on
Copy link
Contributor

@rain-on rain-on commented Jan 16, 2019

Ibft is required to validate a block upon reception, but not import
it until a later time.

As such, IBFT will require validation and importing to be separated.

The validator has been exposed as part of the ProtocolSpecification.

Ibft is required to validate a block upon reception, but not import
it until a later time.

As such, IBFT will require validation and importing to be separated.

The validator has been exposed as part of the ProtocolSpecification.
Copy link
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

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

LGTM, just a few quite minor nits.


public interface BlockValidator<C> {

class BlockValidationOutputs {
Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned on #565 before it got chopped up, this is maybe more BlockProcessingOutputs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

if (!result.isSuccessful()) {
return false;
}
outputs.ifPresent(o -> persistState(o, block, context));
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be great to avoid using single letter variable names. Always takes me ages to work out what the heck o actually is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

BlockHeaderValidator<T> blockHeaderValidator,
BlockBodyValidator<T> blockBodyValidator,
BlockProcessor blockProcessor);
}

public interface BlockImporterBuilder<T> {
BlockImporter<T> apply(BlockValidator<T> blockValidator);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: How come all the existing interfaces got an extra blank line added and the new one didn't? (I like the no blank line version not that it really matters)

Copy link
Contributor Author

@rain-on rain-on Jan 16, 2019

Choose a reason for hiding this comment

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

(no idea) Attempted to maintain consistency with existing code (i.e no new lines).

@rain-on rain-on merged commit f643dc9 into PegaSysEng:master Jan 16, 2019
@rain-on rain-on deleted the import_split branch January 16, 2019 21:34
iikirilov pushed a commit to vinistevam/pantheon that referenced this pull request Jan 24, 2019
Ibft is required to validate a block upon reception, but not import
it until a later time.

As such, IBFT will require validation and importing to be separated.

The validator has been exposed as part of the ProtocolSpecification.
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.

2 participants