Skip to content
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

Claim zaps in batches #1203

Merged
merged 1 commit into from
Jun 6, 2024
Merged

Claim zaps in batches #1203

merged 1 commit into from
Jun 6, 2024

Conversation

benthecarman
Copy link
Collaborator

Helps with #1163

Before we'd claim each zap individually and this could cause problems because relays would send the events with the newest first, so if we stopped claiming the zaps for whatever reason (user closes app), then we'd miss the remaining zaps because we'd set our sync timestamp to the first one we claimed. This fixes this problem by instead accumulating all the zaps until we get the End Of Stored Events message from the relays, and then claiming the zaps, this way we know we got everything from the relay.

@TonyGiorgio
Copy link
Contributor

When does the dm sync timestamp saved?

@benthecarman
Copy link
Collaborator Author

After an individual ecash notification is handled:

storage.set_dm_sync_time(created_at.as_u64(), true)?;

@TonyGiorgio
Copy link
Contributor

After an individual ecash notification is handled:

I don't know what that means. When in the lifetime of an ecash redemption does this even take place?

I guess instead of asking, I should specify that if we're going to move the redemption of the ecash to a batcahable thing in the end, then the timestamp should only be saved once the batched ecash completes fully.

@benthecarman
Copy link
Collaborator Author

I see what you're saying, moved it to set the timestamp after the batch, that should be safer

@TonyGiorgio TonyGiorgio merged commit 542a212 into master Jun 6, 2024
9 checks passed
@TonyGiorgio TonyGiorgio deleted the batched-hermes branch June 6, 2024 22:04
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