This commit fixes a crash that occurred when clicking "follow all"
during onboarding.
This fix works by making `Contacts` and `PostBox` isolated into a
specific Swift Actor, and updating direct and indirect usages
accordingly.
Changelog-Fixed: Fixed a crash that occurred when clicking "follow all" during onboarding.
Closes: https://github.com/damus-io/damus/issues/3422
Co-authored-by: alltheseas <64376233+alltheseas@users.noreply.github.com>
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
This adds a sync mechanism in Ndb.swift to coordinate certain usage of
nostrdb.c calls and the need to close nostrdb due to app lifecycle
requirements. Furthermore, it fixes the order of operations when
re-opening NostrDB, to avoid race conditions where a query uses an older
Ndb generation.
This sync mechanism allows multiple queries to happen simultaneously
(from the Swift-side), while preventing ndb from simultaneously closing
during such usages. It also does that while keeping the Ndb interface
sync and nonisolated, which keeps the API easy to use from
Swift/SwiftUI and allows for parallel operations to occur.
If Swift Actors were to be used (e.g. creating an NdbActor), the Ndb.swift
interface would change in such a way that it would propagate the need for
several changes throughout the codebase, including loading logic in
some ViewModels. Furthermore, it would likely decrease performance by
forcing Ndb.swift operations to run sequentially when they could run in
parallel.
Changelog-Fixed: Fixed crashes that happened when the app went into background mode
Closes: https://github.com/damus-io/damus/issues/3245
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
This commit redesigns the Ndb.swift interface with a focus on build-time
safety against crashes.
It removes the external usage of NdbTxn and SafeNdbTxn, restricting it
to be used only in NostrDB internal code.
This prevents dangerous and crash prone usages throughout the app, such
as holding transactions in a variable in an async function (which can
cause thread-based reference counting to incorrectly deinit inherited
transactions in use by separate callers), as well as holding unsafe
unowned values longer than the lifetime of their corresponding
transactions.
Closes: https://github.com/damus-io/damus/issues/3364
Changelog-Fixed: Fixed several crashes throughout the app
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
This commit implements support for nicely formatting reply push
notifications.
Testing
-------
PASS
Device: iPhone 15 simulator
notepush: 11568aa6285142e4c19bb0da30977957a92b7d9b
Damus: This commit
Settings: Local push notification setup
Steps:
1. Create a post from account 1
2. On account 2, make a reply to that post
3. Ensure we get a push notification with:
- A title formatted as "<ACCOUNT_2_NAME> replied to your note"
- A body with the contents of that reply
4. Click on that push notification. Ensure you are taken to the reply
5. Now make a post from account 2 and mention account 1 in it
6. Ensure push notification says that account 2 mentioned account 1 (i.e. does not talk about a reply)
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Closes: https://github.com/damus-io/damus/issues/2403
Daniel D’Aquino (7):
Add convenience functions
Simplify SelectableText state management
Add support for rendering highlights with comments
Add support for adding comments when creating a highlight
Add highlighter extension
Fix highlight tag ambiguity with specifiers
Improve handling of NostrDB when switching apps
William Casarin (4):
lmdb: patch semaphore names to use group container prefix
notifications: add extended virtual addressing entitlement
highlighter: add extended virtual addressing entitlement
It looks like our push notification service was missing the extended
virtual memory entitlement. This is required to open nostrdb databases.
Signed-off-by: William Casarin <jb55@jb55.com>
This commit fixes an issue where DM contents would not be displayed on a
push notification, by giving the notification extension access to the
keychain group which contains the user's private key
Testing
--------
PASS
Device: iPhone 13 mini
iOS: 17.6.1
Damus: This commit
Setup:
- Make sure that device is setup with push notifications
- DM notifications enabled
- Device registered with push notification server
Steps:
1. Send a DM push notification to yourself
2. Ensure DM contents can be decrypted on the push notification body
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Closes: https://github.com/damus-io/damus/issues/2388
This allows the user to switch between local and push notifications
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Reviewed-by: William Casarin <jb55@jb55.com>
We do not have the ability to suppress push notifications yet (we are
waiting to receive the entitlement from Apple)
In the meantime, attempt to fallback gracefully where possible
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Reviewed-by: William Casarin <jb55@jb55.com>
The user keypair is necessary to determine whether or not a given event
is muted or not. Previously, the user keypair had to be passed on each
function call as an optional parameter, and word filtering would not
work unless the caller remembered to add the keypair parameter.
All usages of MutelistManager functions indicate that the desired base
keypair is always the same as the DamusState's keypair that owns the
MutelistManager. Therefore, it is simpler and less error prone to simply
pass the keypair to MutelistManager during its initialization.
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Signed-off-by: William Casarin <jb55@jb55.com>
This patch is slightly large (I think still within the guidelines tho) ,
but also pretty straightforward. I thought for a while about how I could
split this up in a straightforward way, and I couldn’t come up with
anything without breaking intermediate builds.
- Deleted a lot of old/unnecessary code (ie. the Collection extension
for MuteItem, since we’re now using the MutelistManager sets)
- Changed damus_state.contacts to damus_state.mutelist_manager for all
mute list manager work
- Updated MutelistView to take advantage of new code (this is probably
the largest change in this patch)
Lighting-Address: fishcharlie@strike.me
Signed-off-by: Charlie Fish <contact@charlie.fish>
Reviewed-by: William Casarin <jb55@jb55.com>
Link: 20240210163650.42884-4-contact@charlie.fish
Signed-off-by: William Casarin <jb55@jb55.com>
Since there may be situations where we close and re-open the database,
we need to make sure transactions fail when the database is not open.
Make NdbTxn an init?() constructor and check for ndb.closed. If it's
closed, then fail transaction construction.
This fixes crashes during high database activity when switching from
background to foreground and vice-versa.
Fixes: da2bdad18d ("nostrdb: close database when backgrounded")
Notification settings test
--------------------------
PASS
Device: iPhone 15 Pro simulator
iOS: 17.0.1
Damus: This commit
Setup:
- Two phones running Damus on different accounts
- Local relay with strfry-push-notify test setup
- Apple push notification test tool
Coverage:
1. Mention notifications
2. DM notifications
3. Reaction notifications
4. Repost notifications
Steps for each notification type:
1. Trigger a push notification (Resend push notification from test tool)
2. Ensure that the notification is received on the other device
3. Turn off notifications for that type on settings
4. Trigger a push notification (Resend push notification from test tool)
5. Ensure that the notification is not received on the other device
Result: PASS (notifications are received when enabled and not received when disabled)
Closes: https://github.com/damus-io/damus/issues/1764
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Signed-off-by: William Casarin <jb55@jb55.com>
The code paths for generating zap notifications were very different from
the paths used by most other notifications. In this commit, I include
the logic and data structures necessary for formatting zap notifications
in the same fashion as local notifications.
A good amount of refactoring and moving functions/structures around was
necessary to reuse zap local notification logic. I also attempted to
make the notification generation process more consistent between zaps
and other notifications, without changing too much of existing logic to
avoid even more regression risk.
General push notifications + local notifications test
-----------------------------------------------------
PASS
Device: iPhone 15 Pro simulator
iOS: 17.0.1
Damus: This commit
Setup:
- Two phones running Damus on different accounts
- Local relay with strfry-push-notify test setup
- Apple push notification test tool
Coverage:
1. Mention notifications
2. DM notifications
3. Reaction notifications
4. Repost notifications
Steps for each notification type:
1. Trigger a notification (local and then push)
2. Ensure that the notification is received on the other device
3. Ensure that the notification is formatted correctly
4. Ensure that DMs are decrypted correctly
5. Ensure that profile names are unfurled correctly
6. Click on the notification and ensure that the app opens to the correct screen
Result: PASS (all notifications received and formatted correctly)
Notes:
- For some reason my relay is not receiving zap events, so I could not
test zap notifications yet.
- Reply notifications do not seem to be implemented yet
- These apply to the tests below as well
Changelog-Added: Zap notification support for push notifications
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Signed-off-by: William Casarin <jb55@jb55.com>
Testing
-------
Conditional pass
Device: iPhone 15 Pro simulator
iOS: 17.0.1
Damus: This commit
Coverage:
1. Mention notification works (local and push). PASS
2. Thread replies do not appear (but upon code inspection it seems like
it was not supported before). PASS?
3. DM notification works with decryption (local and push). PASS
4. Zaps not yet implemented. Coming later.
Closes: https://github.com/damus-io/damus/issues/1702
Closes: https://github.com/damus-io/damus/issues/1703
Changelog-Changed: Improve push notification support to match local notification support
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Signed-off-by: William Casarin <jb55@jb55.com>
This commit adds support for the unfurling of author profile names on remote push notifications
It also makes the following changes:
- Notification extension now uses NdbNote
- Some of the logic between push notifications and local notifications was unified
Testing
-------
Device: iPhone 15 Pro simulator
iOS: 17.0.1
Damus: This commit
Coverage:
1. Basic smoke tests on the app by browsing different notes and different tabs
2. Sent test push notifications for mentions and DMs to check the unfurling of profile names
3. Ran unit tests
Closes: https://github.com/damus-io/damus/issues/1703
Changelog-Added: Unfurl profile name on remote push notifications
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Signed-off-by: William Casarin <jb55@jb55.com>
This change was made so that NostrDB data can be accessed from different build targets such as the notification service extension.
Upon initialization of NostrDB, it will check both DB file locations (the old documents directory, and the new shared app group container). If it sees the DB is present on the old location, and not on the new location, it will move the files to the new location. In any other condition it will keep the files intact to prevent data loss.
In order to avoid any conflicts between the damusApp's Ndb instance and the extension's Ndb instance when writing or moving the file, a new parameter called "owns_db_file" was added, and set to "false" for the extension. This ensures that the extension will not attempt to move DB files or create a new DB file on its own. Only the main app can move or create the DB file.
Testing
-------
PASS
Device: iPhone 15 Pro simulator
iOS: 17.0.1
Damus: This commit
Steps:
1. Run with the debugger attached to the extension target.
2. Using Apple's push notification testing dashboard, send a test push notification with a real payload (that includes the nostr event under `nostr_event`. Payload generated by strfry-push-notify).
3. Watch logs. It should show a message like "Got push notification from <DISPLAY_NAME>", where `DISPLAY_NAME` is the correct profile name of the user who generated the event. PASS
Regression testing
------------------
Device: iPhone 13 Mini (Real device)
iOS: 17.1.1
Damus: This commit
Other preconditions:
- Damus is at 1.6 (29) at the start of the test
- NostrDB filled with real data on the old location
Steps:
1. Flash (upgrade) the new Damus version (this commit) (This will be the first time upgrading, shared file container is empty)
2. Try to use the app normally. Scroll and navigate to several locations. Interact with some notes. App should be stable, work, and appear to have profile names already (i.e. It shouldn't start with a bunch of npubs in the place of profile names on known contacts). PASS
3. Downgrade back to the App store version (v1.6 (29))
4. Try to use the app normally. Scroll and navigate, interact, etc. App should work and be stable, but profile name cache is expected to be lost (i.e. shows npubs for a bit until profile is reloaded into NostrDB). PASS
5. Upgrade app again to the version in this commit.
6. Repeat step 2. Everything should work as normal and all profiles should be preloaded from the start. PASS
Closes: https://github.com/damus-io/damus/issues/1744
I added support for the experimental push notifications feature. There are many improvements to be made, so this feature is currently opt-in only. If the user does not opt-in, their device tokens will not be sent out and thus they will receive no push notifications.
We should perform more testing on real-life staging environments before fully releasing this feature.
Testing
-------
Testing was done gradually during development.
Device: iOS simulators
iOS: 17
Damus version: A few different but recent prototypes
Rough coverage:
1. Checked that no device tokens are sent out when setting is off
2. Checked that I can successfully receive device tokens when feature is ON and set to localhost.
3. Checked sending test push notifications of types "note" (kind: 1), reaction (kind: 7) and DMs (kind 4) works and shows a generic but reasonable push notification message
4. Checked that clicking on the notifications above take the user to the correct screen
Closes: https://github.com/damus-io/damus/issues/67
Changelog-Added: Add experimental push notification support
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Signed-off-by: William Casarin <jb55@jb55.com>
