Always send TransactionUpdate to the sender #1111
Conversation
drogus
added
release-any
| @@ -241,7 +246,21 @@ impl SubscriptionManager { | |||
| ); | |||
| drop(span); | |||
|
|
|||
| let _span = tracing::info_span!("eval_send").entered(); | |||
| if !eval.contains_key(&event.caller_identity) { | |||
There was a problem hiding this comment.
Do we want to send it to all clients with the same identity? I think probably we only want to send it to the (identity, address) pair (aka client) that made the call, right?
There was a problem hiding this comment.
Good question. This will only send to the original caller, cause I pass the caller connection client back from the reducer call, so we won't send to all of the clients with the same identity, but there is likely a bug in the if statement itself. If there are two clients connected with the same identity and if only the client that is not making the call is subscribed, this will return false and the caller will not get the update.
There was a problem hiding this comment.
The code actually has a bug related to the same issue. The list of subscriptions is indexed with identity, so if we there are two clients connected with the same identity, with different queries subscribed, one of the clients will get ignored.
There was a problem hiding this comment.
In order to fix the bug in this PR I have to fix the existing bug, so I'll submit the bugfix
There was a problem hiding this comment.
So the bug was already fixed yesterday in #1121, I updated this PR to reflect the changes
There was a problem hiding this comment.
Quite a saga! I'm glad it uncovered other issues though 🙏
drogus
force-pushed
the
drogus/always-send-transaction-update-to-sender
branch
from
9fb125e to
c4144e2
Compare
This commit changes transaction update broadcast to always send an update to the sender, even if the sender is not subscribed to any data that was committed. In such case the transaction update is sent with an empty database update.
drogus
force-pushed
the
drogus/always-send-transaction-update-to-sender
branch
from
c4144e2 to
ca7729b
Compare
drogus
force-pushed
the
drogus/always-send-transaction-update-to-sender
branch
from
7ae1e96 to
a0f932b
Compare
Co-authored-by: Mazdak Farrokhzad <[email protected]> Signed-off-by: Piotr Sarnacki <[email protected]>
|
@Centril thanks for a review, I fixed the stuff you pointed out |
…ender Signed-off-by: Piotr Sarnacki <[email protected]>
drogus
force-pushed
the
drogus/always-send-transaction-update-to-sender
branch
from
3ffeddb to
b31073a
Compare
Description of Changes
This commit changes transaction update broadcast to always send an update to the sender, even if the sender is not subscribed to any data that was committed. In such a case, the transaction update is sent with an empty database update.
API and ABI breaking changes
None
Expected complexity level and risk
2
Testing