Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip advanced block in NodeMessageHandler #505

Merged
merged 4 commits into from
Mar 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions rskj-core/src/main/java/co/rsk/net/BlockProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ public interface BlockProcessor {

boolean hasBetterBlockToSync();

boolean isAdvancedBlock(long number);

// New messages for RSK's sync protocol

void processBlockRequest(MessageChannel sender, long requestId, byte[] hash);
Expand Down
16 changes: 16 additions & 0 deletions rskj-core/src/main/java/co/rsk/net/NodeBlockProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,28 @@ public NodeBlockProcessor(
this.blockSyncService = blockSyncService;
this.syncConfiguration = syncConfiguration;
}

@Override
@Nonnull
public Blockchain getBlockchain() {
return this.blockchain;
}

/**
* Detect a block number that is too advanced
* based on sync chunk size and maximum number of chuncks
*
* @param blockNumber the block number to check
* @return true if the block number is too advanced
*/
@Override
public boolean isAdvancedBlock(long blockNumber) {
int syncMaxDistance = syncConfiguration.getChunkSize() * syncConfiguration.getMaxSkeletonChunks();
long bestBlockNumber = this.getBestBlockNumber();

return blockNumber > bestBlockNumber + syncMaxDistance;
}

/**
* processNewBlockHashesMessage processes a "NewBlockHashes" message. This means that we received hashes
* from new blocks and we should request all the blocks that we don't have.
Expand Down
10 changes: 9 additions & 1 deletion rskj-core/src/main/java/co/rsk/net/NodeMessageHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -282,17 +282,25 @@ private boolean isValidBlock(@Nonnull final Block block) {
*/
private void processBlockMessage(@Nonnull final MessageChannel sender, @Nonnull final BlockMessage message) {
final Block block = message.getBlock();

logger.trace("Process block {} {}", block.getNumber(), block.getShortHash());

if (block.isGenesis()) {
logger.trace("Skip block processing {} {}", block.getNumber(), block.getShortHash());
return;
}

long blockNumber = block.getNumber();

if (this.blockProcessor.isAdvancedBlock(blockNumber)) {
logger.trace("Too advanced block {} {}", blockNumber, block.getShortHash());
return;
}

Metrics.processBlockMessage("start", block, sender.getPeerNodeID());

if (!isValidBlock(block)) {
logger.trace("Invalid block {} {}", block.getNumber(), block.getShortHash());
logger.trace("Invalid block {} {}", blockNumber, block.getShortHash());
recordEvent(sender, EventType.INVALID_BLOCK);
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,11 @@ public BlockCompositeRule(BlockValidationRule... rules) {
@Override
public boolean isValid(Block block) {
String shortHash = block.getShortHash();
logger.debug("Validating block {}", shortHash);
long number = block.getNumber();
logger.debug("Validating block {} {}", shortHash, number);
for(BlockValidationRule rule : this.rules) {
if(!rule.isValid(block)) {
logger.warn("Error Validating block {}", shortHash);
logger.warn("Error Validating block {} {}", shortHash, number);
return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,11 @@ public BlockParentCompositeRule(BlockParentDependantValidationRule... rules) {
@Override
public boolean isValid(Block block, Block parent) {
final String shortHash = block.getShortHash();
logger.debug("Validating block {}", shortHash);
long number = block.getNumber();
logger.debug("Validating block {} {}", shortHash, number);
for(BlockParentDependantValidationRule rule : this.rules) {
if(!rule.isValid(block, parent)) {
logger.warn("Error Validating block {}", shortHash);
logger.warn("Error Validating block {} {}", shortHash, number);
return false;
}
}
Expand Down
18 changes: 18 additions & 0 deletions rskj-core/src/test/java/co/rsk/net/NodeBlockProcessorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,24 @@ public void processBlockWithTooMuchHeight() throws UnknownHostException {
Assert.assertEquals(0, store.size());
}

@Test
public void advancedBlock() throws UnknownHostException {
final BlockStore store = new BlockStore();
final MessageChannel sender = new SimpleMessageChannel();

final BlockNodeInformation nodeInformation = new BlockNodeInformation();
final SyncConfiguration syncConfiguration = SyncConfiguration.IMMEDIATE_FOR_TESTING;

final Blockchain blockchain = BlockChainBuilder.ofSize(0);
final long advancedBlockNumber = syncConfiguration.getChunkSize() * syncConfiguration.getMaxSkeletonChunks() + blockchain.getBestBlock().getNumber() + 1;

BlockSyncService blockSyncService = new BlockSyncService(store, blockchain, nodeInformation, syncConfiguration);
final NodeBlockProcessor processor = new NodeBlockProcessor(store, blockchain, nodeInformation, blockSyncService, syncConfiguration);

Assert.assertTrue(processor.isAdvancedBlock(advancedBlockNumber));
Assert.assertFalse(processor.isAdvancedBlock(advancedBlockNumber - 1));
}

@Test
public void processBlockAddingToBlockchain() {
Blockchain blockchain = BlockChainBuilder.ofSize(10);
Expand Down
22 changes: 22 additions & 0 deletions rskj-core/src/test/java/co/rsk/net/NodeMessageHandlerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,28 @@ public void skipProcessGenesisBlock() throws UnknownHostException {
Assert.assertTrue(pscoring.isEmpty());
}

@Test
public void skipAdvancedBlock() throws UnknownHostException {
SimpleMessageChannel sender = new SimpleMessageChannel();
PeerScoringManager scoring = createPeerScoringManager();
SimpleBlockProcessor sbp = new SimpleBlockProcessor();
sbp.setBlockGap(100000);
NodeMessageHandler processor = new NodeMessageHandler(config, sbp, null, null, null,null, scoring, new DummyBlockValidationRule());
Block block = new BlockGenerator().createBlock(200000, 0);
Message message = new BlockMessage(block);

processor.processMessage(sender, message);

Assert.assertNotNull(sbp.getBlocks());
Assert.assertEquals(0, sbp.getBlocks().size());
Assert.assertTrue(scoring.isEmpty());

PeerScoring pscoring = scoring.getPeerScoring(sender.getPeerNodeID());

Assert.assertNotNull(pscoring);
Assert.assertTrue(pscoring.isEmpty());
}

@Test
public void postBlockMessageTwice() throws InterruptedException, UnknownHostException {
MessageChannel sender = new SimpleMessageChannel();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public class SimpleBlockProcessor implements BlockProcessor {
private long requestId;
private byte[] hash;
private int count;
private long blockGap = 1000000;

@Override
public BlockProcessResult processBlock(MessageChannel sender, Block block) {
Expand All @@ -58,17 +59,26 @@ public void processGetBlock(MessageChannel sender, byte[] hash) {

}

@Override
public boolean isAdvancedBlock(long number) {
return number >= this.blockGap;
}

public void setBlockGap(long gap) {
this.blockGap = gap;
}

@Override
public void processBlockRequest(MessageChannel sender, long requestId, byte[] hash) {
this.requestId = requestId;
this.hash = hash;
this.count = count;
}

@Override
public void processBlockHeadersRequest(MessageChannel sender, long requestId, byte[] hash, int count) {
this.requestId = requestId;
this.hash = hash;
this.count = count;
}

@Override
Expand Down