-
Notifications
You must be signed in to change notification settings - Fork 21
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
infinite_looping_in_forward_transaction_#931 #937
infinite_looping_in_forward_transaction_#931 #937
Conversation
0947d41
to
440b8ed
Compare
440b8ed
to
44e8d37
Compare
44e8d37
to
0a032dc
Compare
cf66c73
to
317602d
Compare
527d370
to
cc2ffbd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, can your rebase to solve conflict and not override new changes ?
6b3285f
to
2be71ef
Compare
5c47598
to
47d2597
Compare
47d2597
to
7baa028
Compare
7baa028
to
08082c6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make the test work I had to hardcode a network patch, because there is a randomness in local development that breaks this feature.
I tested this scenario:
✔️ 3 nodes, I modified the code of 2 nodes to mark them as invalid for processing (to force the forward). I then sent a transaction to A
that forward it to B
that forward it to C
.
- When you remove the task.async, we now have
A
that wait until timeout, and when the timeout occur,A
will try to send toC
(in the same scenario as above), which I believe is not good.
ps: please keep commit history
Hi, we are relying on timeout to notifiy user. I updated geo patch for null patch for localhost |
Indeed, I wasn't against to spawn a process it's just the Task.async expected to await something, maybe a start_child would be sufficient. (But QUID of the error network_issue then ?) |
…//github.com/apoorv-2204/aenode into infinite_looping_in_forward_transaction_#931
Maybe I got it wrong, but I believe my last comment is still relevant. We talked about an ACK msg no? |
Replay back a message? |
The ack message is returned in the |
Description
Fixes
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist: