Skip to content

Conversation

l-monninger
Copy link
Contributor

@l-monninger l-monninger commented Jun 16, 2025

Summary

The previous AccountsIterator relied on checking senders of transactions. This does not, in fact, account for accounts which have been created but have not yet sent a transaction--as is often the case with drops.

Warning

The errors in multiprocessing have been requested for revisit under #158, i.e., in a follow-on PR.

@l-monninger l-monninger marked this pull request as ready for review June 17, 2025 05:33
@l-monninger
Copy link
Contributor Author

The CI is suffering from the same issue as #155 which is exacerbated by #158 and #98.

Please ignore this CI failure in your initial review.

// reset the states of the movement migrator
info!("Resetting movement migrator states");
movement_migrator
.reset_states()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to reset the MovementMigrator? Each check calls checked_migration() with a fresh instance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, because the state can be filled during the first run. Then, another clone can be dropped causing the application to be shut down. Finally, the states represented in the struct do not actually represent the state of the application.

The better thing to do would probably be to place reset_states in the drop implementation itself. However, I might have to be a little clever to get around async in sync issues.

info!("Getting next transaction from block iterator");
if let Some(iter) = &mut self.current_block_iter {
if let Some(tx) = iter.next() {
info!("Got next transaction from block iterator {:?}", tx);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps use debug!() since there will be a large number of transactions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, good call.

Copy link
Collaborator

@sebtomba sebtomba left a comment

Choose a reason for hiding this comment

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

LGTM

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.

3 participants