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

Remove ping pong from reply stage #1745

Merged
merged 3 commits into from
Feb 6, 2020
Merged

Remove ping pong from reply stage #1745

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6925e2d
Remove ping pong
DyrellC Feb 6, 2020
fb07882
Remove ping pong code from reply stage
DyrellC Feb 6, 2020
428b58d
Merge branch 'dev' into remove-ping-pong
Feb 6, 2020
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
10 changes: 0 additions & 10 deletions src/main/java/com/iota/iri/network/pipeline/ReplyStage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,16 +146,6 @@ public ProcessingContext process(ProcessingContext ctx) {
return ctx;
}

// we didn't have the requested transaction (random or explicit) from the neighbor but we will immediately reply
// with the latest known milestone and a needed transaction hash, to keep up the ping-pong
try {
final TransactionViewModel msTVM = TransactionViewModel.fromHash(tangle,
latestMilestoneTracker.getLatestMilestoneHash());
neighborRouter.gossipTransactionTo(neighbor, msTVM, false);
} catch (Exception e) {
e.printStackTrace();
}

Comment on lines -149 to -158
Copy link
Contributor

Choose a reason for hiding this comment

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

I am trying to think...
Intuitively I would think that there is an else is missing above the try...
This is because this is what the comment says. But let's think critically about this.

If we delete this code it means that if we request a non-existing tx the ping-pong would stop.
If I am a non-synced node and I am getting requests for transactions that I don't have then I won't request transactions that are missing from those nodes, and the ping pong would stop.

However if a neighbor is requesting from me transactions that I don't have then the neighbor is:

  1. Also out of sync
  2. Malicious

In case of (1) it means that I am sending the same transactions over and over again, thus slowing down an not helping with the sync. This will stop the ping pong between them but newly broadcasted txs should liven it up

In case of (2) we obviously shouldn't send a message

As long as there is 1 synced node in the network it should be able to sync the rest of the nodes eventually
So I support deleting and not adding else

ctx.setNextStage(TransactionProcessingPipeline.Stage.FINISH);
return ctx;
}
Expand Down