Improve chain inventory generating logic

[jwrct]

Rationale

After completing the handshake with the peer, if it is found that the peer's blockchain is longer than the local one, according to the principle of the longest chain, the block synchronization process will be triggered. The message interaction during the synchronization process is shown in the following figure.

After receiving the SyncBlockChainMessage packet from node A, node B calculates the missing blocks of node A and sends the list of missing block IDs to node A through the ChainInventoryMessage packet. The code logic for generating the list is as follows:

private LinkedList<BlockId> getLostBlockIds(List<BlockId> blockIds) throws P2pException {
  BlockId unForkId = null;
  for (int i = blockIds.size() - 1; i >= 0; i--) {
    if (tronNetDelegate.containBlockInMainChain(blockIds.get(i))) {
      unForkId = blockIds.get(i);
      break;
    }
  }
  
  if (unForkId == null) {
    throw new P2pException(TypeEnum.SYNC_FAILED, "unForkId is null");
  }
  
  BlockId headID = tronNetDelegate.getHeadBlockId();
  long headNum = headID.getNum();
  
  long len = Math.min(headNum, unForkId.getNum() + NetConstants.SYNC_FETCH_BATCH_NUM);
  
  LinkedList<BlockId> ids = new LinkedList<>();
  for (long i = unForkId.getNum(); i <= len; i++) {
    if (i == headNum) {
      ids.add(headID);
    } else {
      BlockId id = tronNetDelegate.getBlockIdByNum(i);
      ids.add(id);
    }
  }
  return ids;
}

From the above code, we can see that the maximum common block height is determined and found from the chain summary by node B according to SyncBlockChainMessage sent by node A. And then node B obtains blockIds from the common block heights and adds them to the inventory, with the height increasing by 1 each time, up to 2000. If node B switches the chain during the inventory generation process, it may incur the generated inventory not matching the chain summary since the maximum common block height might change with the chain switching, causing the first blockId in the inventory may not be in the chain summary.

When node A handles inventory message, it includes the following validation logic:

if (!peer.getSyncChainRequested().getKey().contains(blockIds.get(0))) {
      throw new P2pException(TypeEnum.BAD_MESSAGE, "unlinked block, my head: "
          + peer.getSyncChainRequested().getKey().getLast().getString()
          + ", peer: " + blockIds.get(0).getString());
  }

When verifying the inventory message, node A will check if the first blockId in the inventory is included in the sent chain summary. Otherwise, an 'unlinked block' exception will be reported. Although the occurrence of concurrent scenarios of switching forked chains and generating synchronized inventory is very small in the production environment, it is still necessary to optimize the handling of such scenarios.

Implementation

In order not to affect the performance of switching forked chains, it is not possible to solve this problem by adding synchronization control during the chain cutting and inventory generation process. It can be optimized by retrying in the inventory generation process, that is, after generating the inventory, check if the first blockId in the inventory is in the chain summary. If it is not, retry generating the inventory.

Are you willing to implement this feature?
yes

[eodiandie]

hi, @chengtx01
I am a little curious, what are the bad effects this problem leads to? per my understanding,' an 'unlinked block' exception will be reported' is an acceptable case.

[jwrct]

@xq-lu After node A throws an 'unlinked block' exception, it will immediately disconnect from node B. In extreme cases, if there is frequent switching of chains, it will cause instability in the entire blockchain network. Therefore, we should try to avoid this from happening as much as possible.

[xxo1shine]

@chengtx01 How often does this scenario occur?

[jwrct]

@wubin01 So far, the production environment has not really appeared, maybe because the number of switch chains in the production environment is very small, so the probability of concurrent switch chains and generating inventory is even lower.

[yuekun0707]

Will this happens: after switching branch, the blocks after the first one in the generating inventory changed? Does it need to check the following blocks in the inventory?

[jwrct]

@yuekun0707 The first block in the list is the common block with the peer, and the subsequent blocks are considered as missing blocks for the peer, so it is normal for them to be changed and does not need to be checked.

[yuekun0707]

@yuekun0707 The first block in the list is the common block with the peer, and the subsequent blocks are considered as missing blocks for the peer, so it is normal for them to be changed and does not need to be checked.

I see. Another question, if the rest blocks are wrong, and the node requests the wrong block to the target, what will happen? Will they break the connection?

[jwrct]

@yuekun0707 Are you referring to a bad block or a block on a branching chain when you mention a wrong block? If it's a bad block, the connection will be disconnected. If it's a block on a branching chain, it will be processed normally.