Skip to content

fix: wait to set socket to active after chain_sync transitions to the correct state#48

Merged
daveminer merged 4 commits intomainfrom
chainsync-message-actions
Jan 13, 2026
Merged

fix: wait to set socket to active after chain_sync transitions to the correct state#48
daveminer merged 4 commits intomainfrom
chainsync-message-actions

Conversation

@daveminer
Copy link
Collaborator

@daveminer daveminer commented Sep 13, 2025

Fixes the bug where a received message could be processed by the socket while in a state not meant for message processing.

@daveminer daveminer changed the title wait to set socket to active after chain_sync transitions to the correct state fix: wait to set socket to active after chain_sync transitions to the correct state Sep 13, 2025
@daveminer
Copy link
Collaborator Author

We might want to see if there is a yaci config that can help test this before we merge it.


case CSResponse.decode(combined_payload) do
{:ok, %AwaitReply{}} ->
:ok = setopts_lib(client).setopts(socket, active: :once)
Copy link
Member

Choose a reason for hiding this comment

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

The reason for setting the socket to active here and then below is that we need it to be after invoking the read_remaining_payload function. Notice that line 277 manually calls client.recv(socket, recv_payload_length, @recv_timeout)

I'm running this branch and don't see new blocks displayed so I suspect the changes break the current functionality.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Interesting, it was testing out for me. Am I missing something?

Screenshot from 2025-09-26 11-07-27

Even so, what you say makes sense and I realizing this probably isn't what this comment is talking about:
https://github.com/wowica/xander/pull/48/files#diff-61848f9c738b8acca1018367012bcf6f97ca7ac8de8e8a6efbcbf2df4765d5f5L397

@caike
Copy link
Member

caike commented Sep 17, 2025

Fixes the bug where a received message could be processed by the socket while in a state not meant for message processing.

Not sure I understand the scenario. Left a comment on the code but am curious to hear more about the issue you had in mind.

@daveminer
Copy link
Collaborator Author

daveminer commented Sep 26, 2025

Fixes the bug where a received message could be processed by the socket while in a state not meant for message processing.

Not sure I understand the scenario. Left a comment on the code but am curious to hear more about the issue you had in mind.

I was looking to resolve the bug mentioned here:

https://github.com/wowica/xander/pull/48/files#diff-61848f9c738b8acca1018367012bcf6f97ca7ac8de8e8a6efbcbf2df4765d5f5L397

Regardless of the comment above, I think I misinterpreted what the TODO was saying. The issue isn't that there's a period of time in the new_blocks/3 where the socket is in the wrong state, it's that we could receive socket messages (:info) while we're in catching_up, and we don't have a method signature to handle it.

I've made a commit that should be closer to an actual solution for the problem for discussion purposes. The code is redundant and would need to be DRYed up, but right now I'm just trying to make sure it's addressing the actual issue.

@daveminer daveminer merged commit 397da52 into main Jan 13, 2026
7 checks passed
@daveminer daveminer deleted the chainsync-message-actions branch January 13, 2026 01:58
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 this pull request may close these issues.

2 participants