eosnetworkfoundation / eos-evm-node

Other
3 stars 5 forks source link

misuse of state history's `get_blocks_ack_request_v0` can lead to "stuck" state history connection #302

Open spoonincode opened 3 weeks ago

spoonincode commented 3 weeks ago

eos-evm-node's state history request sets max_messages_in_flight to 4096, https://github.com/eosnetworkfoundation/eos-evm-node/blob/3a1eaaa85e2aa43435d2e31598fc6f8b43452314/src/ship_receiver_plugin.cpp#L117-L121 But when it sends get_blocks_ack_request_v0 it seems to send the total number of messages it has received so far (I don't see num_messages reset to 0, except on connection failure), https://github.com/eosnetworkfoundation/eos-evm-node/blob/3a1eaaa85e2aa43435d2e31598fc6f8b43452314/src/ship_receiver_plugin.cpp#L348-L350 This is not proper use of the get_blocks_ack_request_v0 interface. max_messages_in_flight+get_blocks_ack_request_v0 is a simple credit system where on the initial request the number of credits are established, nodeos decreases its credit count on each block it sends, and get_blocks_ack_request_v0 credits back the given number.

If I did the math right, after about 3 million blocks the current code will roll over nodeos' credit counter. When this occurs nodeos might think it has 0 credits and stop sending blocks.

For a c++ reader not sure it's worth bothering with non-UINT32_MAX max_messages_in_flight since socket back pressure works properly. The primary reason this exists as part of the ship protocol is that websockets in nodejs lacked (lacks?) proper socket back pressure. So IMO should just set max_messages_in_flight = UINT32_MAX and get rid of the ack stuff.

stephenpdeos commented 3 days ago

Aligned on approach to set max_messages_in_flight = UINT32_MAX and get rid of the ack stuff