Binary port request/response can get out of sync #312
Assignees
Labels
Comments
This was referenced May 27, 2024
casperlabs-bors-ng bot
added a commit
to casper-network/casper-node
that referenced
this issue
Jun 18, 2024
4726: Introduce Request Id for binary port communication r=rafal-ch a=rafal-ch Heads-up: There are several functional changes in this single PR just because each breaking change to the sidecar requires some coordinated merging effort between node and sidecar. Hence, it's more convenient to deliver such changes in one go. This PR introduces: 1. the concept of "request id" to the binary port. This is used to dodge an edge case where it may happen that a response which has not been consumed is incorrectly served as a request to the next request. The corresponding PR in casper-sidecar is [here](casper-network/casper-sidecar#315). 2. Binary request header version - this will prevent binary port from trying to parse binary message headers of incompatible types 3. Promote binary port error code from `u8` to `u16`, because we already have nearly 60 error codes 4. Additional unit tests verifying whether the binary message codec can properly handle large quantities of messages. 5. Additional tests to see if protocol version check is respected Fixes: casper-network/casper-sidecar#312 Co-authored-by: Rafał Chabowski <rafal@casperlabs.io>
casperlabs-bors-ng bot
added a commit
to casper-network/casper-node
that referenced
this issue
Jun 18, 2024
4726: Introduce Request Id for binary port communication r=rafal-ch a=rafal-ch Heads-up: There are several functional changes in this single PR just because each breaking change to the sidecar requires some coordinated merging effort between node and sidecar. Hence, it's more convenient to deliver such changes in one go. This PR introduces: 1. the concept of "request id" to the binary port. This is used to dodge an edge case where it may happen that a response which has not been consumed is incorrectly served as a request to the next request. The corresponding PR in casper-sidecar is [here](casper-network/casper-sidecar#315). 2. Binary request header version - this will prevent binary port from trying to parse binary message headers of incompatible types 3. Promote binary port error code from `u8` to `u16`, because we already have nearly 60 error codes 4. Additional unit tests verifying whether the binary message codec can properly handle large quantities of messages. 5. Additional tests to see if protocol version check is respected Fixes: casper-network/casper-sidecar#312 Co-authored-by: Rafał Chabowski <rafal@casperlabs.io>
casperlabs-bors-ng bot
added a commit
to casper-network/casper-node
that referenced
this issue
Jun 18, 2024
4726: Introduce Request Id for binary port communication r=rafal-ch a=rafal-ch Heads-up: There are several functional changes in this single PR just because each breaking change to the sidecar requires some coordinated merging effort between node and sidecar. Hence, it's more convenient to deliver such changes in one go. This PR introduces: 1. the concept of "request id" to the binary port. This is used to dodge an edge case where it may happen that a response which has not been consumed is incorrectly served as a request to the next request. The corresponding PR in casper-sidecar is [here](casper-network/casper-sidecar#315). 2. Binary request header version - this will prevent binary port from trying to parse binary message headers of incompatible types 3. Promote binary port error code from `u8` to `u16`, because we already have nearly 60 error codes 4. Additional unit tests verifying whether the binary message codec can properly handle large quantities of messages. 5. Additional tests to see if protocol version check is respected Fixes: casper-network/casper-sidecar#312 Co-authored-by: Rafał Chabowski <rafal@casperlabs.io>
casperlabs-bors-ng bot
added a commit
to casper-network/casper-node
that referenced
this issue
Jun 18, 2024
4726: Introduce Request Id for binary port communication r=rafal-ch a=rafal-ch Heads-up: There are several functional changes in this single PR just because each breaking change to the sidecar requires some coordinated merging effort between node and sidecar. Hence, it's more convenient to deliver such changes in one go. This PR introduces: 1. the concept of "request id" to the binary port. This is used to dodge an edge case where it may happen that a response which has not been consumed is incorrectly served as a request to the next request. The corresponding PR in casper-sidecar is [here](casper-network/casper-sidecar#315). 2. Binary request header version - this will prevent binary port from trying to parse binary message headers of incompatible types 3. Promote binary port error code from `u8` to `u16`, because we already have nearly 60 error codes 4. Additional unit tests verifying whether the binary message codec can properly handle large quantities of messages. 5. Additional tests to see if protocol version check is respected Fixes: casper-network/casper-sidecar#312 Co-authored-by: Rafał Chabowski <rafal@casperlabs.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
There is an edge case in which the request/responses send to and from the binary port can get out of sync. For example: sidecar sends request #1, the response is not consumed but it stays in the buffer, then sidecar sends request #2 but gets the response for request #1. This issue will not sort itself out and requires restart of the sidecar. Usually this results in unexpected payload variant received error message, but if the subsequent requests are of the same type this will go unnoticed and could lead to serious misinformation. The root cause is being investigated, but regardless of it, we'll extend the binary port communication protocol to use the request ID.
The text was updated successfully, but these errors were encountered: