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

fix: update pre-haves as they change #594

Merged
merged 1 commit into from
May 2, 2024
Merged

fix: update pre-haves as they change #594

merged 1 commit into from
May 2, 2024

Conversation

EvanHahn
Copy link
Contributor

In the following scenario:

  1. Alice and Bob connect but do not start sync.
  2. Alice adds an observation.

Bob should see that there is something new to sync.

Previously, this wouldn't happen. Now, when a writer core is appended to or cleared, we send new pre-have messages.

Closes #582.

In the following scenario:

1. Alice and Bob connect but do not start sync.
2. Alice adds an observation.

Bob should see that there is something new to sync.

Previously, this wouldn't happen. Now, when a writer core is appended to
or cleared, we send new pre-have messages.

Closes [#582].

[#582]: #582
Comment on lines -307 to -311
// A non-writer core will emit 'append' when its length is updated from the
// initial sync with a peer, and we will not have sent a "maybe want" for
// this range, so we need to do it now. Subsequent appends are propogated
// (more efficiently) via range broadcasts, so we only need to listen to the
// first append.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved this comment down and re-formatted it so no line was longer than 80 characters; I didn't change it other than fixing a typo.

Comment on lines +318 to +323
const originalClear = core.clear
core.clear = function clear() {
const result = originalClear.apply(this, /** @type {any} */ (arguments))
result.then(sendHaves)
return result
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I considered patching Hypercore itself, which doesn't feel meaningfully different. I don't think Hypercore offers a way to address this in a better way, even on newer versions.

@EvanHahn
Copy link
Contributor Author

EvanHahn commented May 2, 2024

Merging without review. I'll ask @gmaclennan to review soon.

@EvanHahn EvanHahn merged commit 6ad3bea into main May 2, 2024
4 checks passed
@EvanHahn EvanHahn deleted the evanhahn/582 branch May 2, 2024 14:36
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.

Adding data after a peer connects doesn't show up in sync state
1 participant