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 dexie cache filter #239

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DanConwayDev
Copy link

by correcting the event format passed to nostr-tool's matchFilter

the dexie cache adaptor applies some simple filtering for efficiency reasons before using nostr-tool's matchFilter to apply the full NIP-01 ruleset

basic root cause analysis:

it is likely that nostr-tools changed the event format expected by matchFilter and this was missed during a dependancy upgrade for a number of reasons:

  1. the use of as any remove type checking
  2. nostr-tools doesn't use semantic versioning to highlight breaking changes
  3. a cursory test using simple filters would have returned correct results

by correcting the event format passed to nostr-tool's matchFilter

the dexie cache adaptor applies some simple filtering for efficiency
reasons before using nostr-tool's matchFilter to apply the full
NIP-01 ruleset

basic root cause analysis:

it is likely that nostr-tools changed the event format expected by
matchFilter and this was missed during a dependancy upgrade for
a number of reasons:

1. the use of `as any` remove type checking
2. nostr-tools doesn't use semantic versioning to highlight
   breaking changes
3. a cursory test using simple filters would have returned
   correct results
@erskingardner
Copy link
Collaborator

I'm not sure I understand the issue here? Is the cache adapter not returning the right results? Or returning no results?

@DanConwayDev
Copy link
Author

in some cases the cache adatper returns too many results
take a filter that includes #a and #t
the correct behavior is to return events that match both #a AND #t
the cache adapter returns events for #a OR #t

it does this because it fails to call nostr-tool's matchFilter function correctly.

@erskingardner
Copy link
Collaborator

Gotcha, thanks. I'll respond on the other thread now.

@pablof7z
Copy link
Collaborator

hey @DanConwayDev -- finally had a chance to dig into this -- I think this PR is incorrect, the event you are using in this PR is the dexie representation of the event and not a nostr-tool compatible version.

I added a test to check and it seems the filter is being matched properly.

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