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 single note query #4

Merged
merged 2 commits into from
Jul 30, 2024
Merged

Fix single note query #4

merged 2 commits into from
Jul 30, 2024

Conversation

dcadenas
Copy link

@dcadenas dcadenas commented Jul 29, 2024

This fix involves converting the passed ID to binary before using it in the SQL query to ensure proper handling of BLOB data types.

I also removed the unnecessary settings that were added to the connection file, as they are not needed.

Covers #4

This can be tested by running a relay with docker compose up --build, fetching any id and then trying:
nak req -i $ID --force-pre-auth --sec $PERSONAL_SEC_KEY ws://localhost:8000

@dcadenas dcadenas self-assigned this Jul 29, 2024
@dcadenas dcadenas requested a review from mplorentz July 29, 2024 12:37
Copy link
Member

@mplorentz mplorentz left a comment

Choose a reason for hiding this comment

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

This works for me. I think I found another bug on auth_changes though: I tried turning off nip-42 auth in the config.toml but it still requires authentication for publishing new events.

Have you considered contributing this back upstream? At least the changes to sqlite.rs?

@dcadenas
Copy link
Author

I didn't pay much attention about usages not mapped to the olympics relay use case but I'll add what you found to the draft PR that deals with cleanups and tests. Maybe one day we can contribute with that once we are sure it's works nice for a generic solution.

I had added scsibug#201 to contribute with this current fix.

@dcadenas dcadenas merged commit f487fd3 into auth_changes Jul 30, 2024
@dcadenas dcadenas mentioned this pull request Jul 30, 2024
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