fix: add missing is_latest filter to network account query#1578
fix: add missing is_latest filter to network account query#1578
Conversation
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! I left one comment inline to be tackled in a separate PR.
| QueryDsl::select( | ||
| schema::accounts::table | ||
| .filter(schema::accounts::network_account_id_prefix.is_not_null()), | ||
| .filter(schema::accounts::network_account_id_prefix.is_not_null()) |
There was a problem hiding this comment.
Not for this PR, but we should probably replace network_account_id_prefix with something like network_account_type.
|
I'll check tomorrow, but the ntx does need to handle duplicate accounts in its event loop. This is because it's possible for the account info to arrive from the store and also as part of the mempool event. However the store should only be sending unique network accounts. |
I think we should merge this regardless as it fixes the store-side issue. We may need to follow up with improved handling on the NTX builder side though. |
|
I will continue with this two other PRs then, one for the prefix removal and another for the duplication handling |
|
Thank you! |
Fixes ntx-builder crash on node restart after a network transaction has been processed. After a network transaction updates a network account, restarting the node causes the ntx-builder to fail with:
The
select_all_network_account_idsquery was missing anis_latest = truefilter. Without filtering foris_latest, both historical and current rows were returned, causing duplicate account IDs to be sent to the ntx-builder, which then attempted to spawn multiple actors for the same account.Added
.filter(schema::accounts::is_latest.eq(true))to ensure only the current state of each account is returned. I saw that this filter was removed in #1481 so I want to confirm with @drahnr if we should fix it in another way.