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

TIP-621: Add code version column to HelloMessage #621

Closed
317787106 opened this issue Nov 1, 2023 · 6 comments · Fixed by tronprotocol/java-tron#5584
Closed

TIP-621: Add code version column to HelloMessage #621

317787106 opened this issue Nov 1, 2023 · 6 comments · Fixed by tronprotocol/java-tron#5584

Comments

@317787106
Copy link
Contributor

317787106 commented Nov 1, 2023

TIP: 621
Title: Add code version column to HelloMessage
Author: 317787106@qq.com
Status: Draft
Type: Standards Track
Category: Networking
Created: 2023-11-01

Simple Summary

This TIP describes why the code version column needs to be added to HelloMessage when node handshakes with peers.

Abstract

A new column code version is added to the proto message HelloMessage so that the local node can learn more information about peers when handshaking.

Motivation

There have been 73 release versions of Java-tron until 2023-11-01. In the mainnet or Nile network, nodes running different versions may exist at the same time. Generally, each new version fixes some bugs from the previous one. How to verify that we have fixed this bug? Suppose one peer sends a message to the local node, but this local node with the latest code fails to process it. If the peer runs an older version, this failure could be expected.

The key point is that the peer's code version can not be acquired easily. One possible way is to get it through the /wallet/getnodeinfo API, but in most cases, we cannot access HTTP services. Another solution is to add a codeVersion field in the HelloMessage when handshaking with peers. This does not rely on HTTP and can be easily implemented. Please find the details below.

Specification

In the proto file protos/core/Tron.proto, add one column codeVersion:

message HelloMessage {
  message BlockId {
    bytes hash = 1;
    int64 number = 2;
  }

  Endpoint from = 1;
  int32 version = 2;
  int64 timestamp = 3;
  BlockId genesisBlockId = 4;
  BlockId solidBlockId = 5;
  BlockId headBlockId = 6;
  bytes address = 7;
  bytes signature = 8;
  int32 nodeType = 9;
  int64 lowestBlockNum = 10;
  bytes codeVersion = 11; //new added
}

The column codeVersion comes from Version of org.tron.program.Version, latest value is "4.7.3".

Compatibility

If the node parses the message without columncodeVersion, it uses default null. This column will only be logged.

@317787106 317787106 changed the title Add code version column to HelloMessage TIP-621: Add code version column to HelloMessage Nov 1, 2023
@lurais
Copy link

lurais commented Nov 9, 2023

Suppose one peer sends a message to the local node, but this local node with the latest code fails to process it. If the peer runs an older version, this failure could be expected. So, we can close the connection directly when we received a HelloMessage which contains an older version, is this right?

@317787106
Copy link
Contributor Author

Suppose one peer sends a message to the local node, but this local node with the latest code fails to process it. If the peer runs an older version, this failure could be expected. So, we can close the connection directly when we received a HelloMessage which contains an older version, is this right?

Not exactly, there is an prerequisites, namely one bug exist in code with old version. Connection will be closed if node fails to process to message.

@lurais
Copy link

lurais commented Nov 10, 2023

Suppose one peer sends a message to the local node, but this local node with the latest code fails to process it. If the peer runs an older version, this failure could be expected. So, we can close the connection directly when we received a HelloMessage which contains an older version, is this right?

Not exactly, there is an prerequisites, namely one bug exist in code with old version. Connection will be closed if node fails to process to message.

Is that reasonable to close the connection when it fails to process the message? So, will it be easier to recognize this situation?

@jwrct
Copy link
Contributor

jwrct commented Nov 10, 2023

Suppose one peer sends a message to the local node, but this local node with the latest code fails to process it. If the peer runs an older version, this failure could be expected. So, we can close the connection directly when we received a HelloMessage which contains an older version, is this right?

@lurais The current design is that the newly added field is only used to carry additional information and is not used for logical judgment. In addition, protobuf is compatible with newly added fields and will not cause message parsing issues between new and old code. I hope my answer can resolve your doubts.

@lurais
Copy link

lurais commented Nov 10, 2023

Suppose one peer sends a message to the local node, but this local node with the latest code fails to process it. If the peer runs an older version, this failure could be expected. So, we can close the connection directly when we received a HelloMessage which contains an older version, is this right?

@lurais The current design is that the newly added field is only used to carry additional information and is not used for logical judgment. In addition, protobuf is compatible with newly added fields and will not cause message parsing issues between new and old code. I hope my answer can resolve your doubts.

So, why should add this additional information? what will it be used for ?

@317787106
Copy link
Contributor Author

Suppose one peer sends a message to the local node, but this local node with the latest code fails to process it. If the peer runs an older version, this failure could be expected. So, we can close the connection directly when we received a HelloMessage which contains an older version, is this right?

@lurais The current design is that the newly added field is only used to carry additional information and is not used for logical judgment. In addition, protobuf is compatible with newly added fields and will not cause message parsing issues between new and old code. I hope my answer can resolve your doubts.

So, why should add this additional information? what will it be used for ?

@lurais This information helps to resolve problem, some issue only exist when interaction with different version of java-tron

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants