Update to latest nostrdb #2121
Merged
Update to latest nostrdb #2121
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
Author
|
This broke a bunch of tests which I'm fixing now |
Collaborator
Author
|
One thing I did here is add nostrdb's patch history to our local copy of nostrdb. This ensure we don't lose bisectability when updating nostrdb. I've also removed a lot of overlap between damus' C code and nostrdb, so most of the C code should be in nostrdb now. |
This was referenced Apr 17, 2024
jb55
force-pushed
the
nostrdb-update
branch
3 times, most recently
from
10bc4f4 to
c476272
Compare
This was
linked to
issues
May 6, 2024
Closed
jb55
force-pushed
the
nostrdb-update
branch
3 times, most recently
from
1f3d846 to
f8931a6
Compare
Closed
jb55
force-pushed
the
nostrdb-update
branch
3 times, most recently
from
8bc95f2 to
e56f0a5
Compare
Collaborator
Author
|
@danieldaquino this is ready for testing! |
jb55
changed the title
Collaborator
Author
|
looks like there are a fix issues with block parsing... |
Collaborator
Author
|
Fixed an iOS crash in the latest version of nostrdb. is a bus error (unaligned memory access) when trying to parse an empty json string when processing inner kind6 reposts (this is a new feature in this version of nostrdb) |
10 tasks
Collaborator
Author
|
The last issue seems to be DM rendering |
Closes: #2885 Changelog-Changed: Use NostrDB for rendering note contents Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
NostrDB relies on manual memory management, so it is a good idea to enable the address sanitizer on debug configurations, as it helps find memory-related issues on the app, which will allow us to identify memory issues and potential crashes earlier in the development process. Changelog-None Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
This makes it easier to work from the command line when needed Changelog-None Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Some tests have been broken at some point during the nostrdb migration. Disable them for now and address them later (#3112) Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Currently NostrDB does not seem to handle encryption/decryption of DMs. Since NostrDB now controls the block parsing process and fetches note contents directly from the database, we have to add a specific condition that injects decrypted content directly to the ndb content parser. This is done in conjunction with some minor refactoring to `NdbBlocks` and associated structs, as in C those are separated between the content string and the offsets for each block, but in Swift this is more ergonomically represented as a standalone/self-containing object. No changelog entry is added because the previously broken version was never released to the public, and therefore this fix produces no user-facing changes compared to the last released version. Changelog-None Closes: #3106 Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Changelog-None Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Previously two addresses from different memory regions were being subtracted, which will lead to the incorrect number. This commit improves the calculation. Changelog-None Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Changelog-None Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Changelog-None Closes: #3127 Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
This commit fixes a logical error in the blocks rendering function. Changelog-None Closes: #3133 Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
This commit fixes a stack corruption issue caused by an off-by-one error in one of the functions responsible for parsing bech32 entities. Changelog-None Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Changelog-None Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
… blocks iteration indexing Changelog-None Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Changelog-None Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
It was decided on a standup meeting that this feature is not important and failing tests can be disabled. Changelog-None Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
To be fixed on #3154 Changelog-None Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Changelog-None Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Changelog-None Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Closes: #3129 Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Note: This brings us closer to feature parity with the master branch, so there is no changelog item to be added Closes: #3156 Changelog-None Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
This commit fixes a crash that occurred when swapping between Damus and other apps. When Damus enters background mode, NostrDB is closed and its resources released. When Damus re-enters foreground mode, NostrDB is reopened. However, an issue with the transaction inheritance logic caused a race condition where a side menu profile lookup would get an obsolete transaction containing pointers that have been freedwhen NostrDB was closed, causing a "use-after-free" memory error. The issue was fixed by improving the transaction inheritance logic to double-check if the "generation" counter (which auto increments when Damus closes and re-opens) matches the generation marked on the thread-specific transaction. This effectively prevents lookups from inheriting an obsolete transaction from a previous NostrDB generation. Closes: #3167 Changelog-Fixed: Fixed an issue where the app would crash when swapping between apps Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
This fixes a regression that caused quoted notes not to appear. Changelog-None Closes: #3163 Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
This commit introduces a verification step at the relay connection
level, to help ensure notes get validated at the source and prevent
security issues associated with untrusted relays.
`RelayConnection.swift` — the source that initially handles WebSocket
messages — was analyzed, and measures were put in place to prevent
(or at least minimize) unverified nostr event data being spread
throughout the app.
The following measures were taken:
1. A note verification step was added prior to the `self.handleEvent(.nostr_event(ev))` call (which sends a Nostr response to the rest of the app for logical handling).
a. From code analysis, there is only one such call in `RelayConnection.swift`.
2. `NostrConnectionEvent`, the object that gets passed to event handlers, had its interface modified to remove the "message" case, since:
a. that could be a source of unverified nostr events.
b. it is redundant an unneeded due to the `.nostr_event` case.
c. there were no usages of it around the codebase
3. The raw websocket event handler had its label renamed to "handleUnverifiedWSEvent", to make it clear to the caller about the verification status of the data.
a. Usages of this were inspected and no significant risk was detected.
4. A new `verify` method in NdbNote was created to verify Nostr notes, and unit tests were added to confirm tampering detections around all the major fields in a Nostr note.
5. Care was taken to ensure the performance regression is as little as
possible.
It is worth noting that we will not need this once the local relay model
architecture is introduced, since that architecture ensures note
validation before it reaches the rest of the application and the user.
In other words, this is a temporary fix.
However, since the migration to that new architecture is a major
undertaking that will take some time to be completed, this fix was written
in order to address security concerns while the migration is unfinished.
This fix was written in a way that attempts to be as effective as
possible in reducing security risks without a risky and lenghty
refactor of the code that would delay the fix from being published.
Changelog-Fixed: Improved security around note validation
Closes: #1341
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Closes: #3187 Changelog-None Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
danieldaquino
force-pushed
the
nostrdb-update
branch
from
f05c855 to
fae061c
Compare
Collaborator
|
Rebased branch on top of the current |
Changelog-None Closes: #3190 Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This branch updates nostrdb to the latest version
Things done and left to do:
- [ ] Switch to local relay for querying and subscriptionssaving this for laterWhat's new in this version: