Skip to content

Feat/nostr sync#13306

Closed
krzyczak wants to merge 2 commits intoTeamNewPipe:refactorfrom
krzyczak:feat/nostr-sync
Closed

Feat/nostr sync#13306
krzyczak wants to merge 2 commits intoTeamNewPipe:refactorfrom
krzyczak:feat/nostr-sync

Conversation

@krzyczak
Copy link
Copy Markdown

@krzyczak krzyczak commented Mar 4, 2026 •
edited
Loading

What is it?

  • Bugfix (user facing)
  • Feature (user facing) ⚠️ Your PR must target the refactor branch
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

Implemented private sync using Nostr protocol. At the moment the syncable features are

  • watch history (including the
  • subscriptions

Related to #5325

This is achieved using:

  • Logging in to Nostr can be done using a signer app (NIP-55, like Primal or Amber)
  • Also an account can be created in-app (Then "Show Identity" button displays QR codes for npub and nsec, so the account can be used in other apps)
  • The data is stored using kind:30078 from NIP-78, and ut uses NIP-44 encryption so everything stays private
  • There is a set of default relays, but custom relays can be added
    More details are in the NOSTR_SYNC.md

Before/After Screenshots/Screen Record

  • Before:
  • After:
screenshot_01_drawer_nostr_sync_entry_small screenshot_02_nostr_sync_main_small screenshot_03_sign_in_dialog_small screenshot_04_sign_in_advanced_small

Fixes the following issue(s)

  • Fixes #

Relies on the following changes

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR. You can find more info and a video demonstration on this wiki page.

Due diligence

AI NOTE

I do not know Kotlin well enough to be able to code in it such a big feature, so I did not adhere to the AI usage policy.

However I was missing the sync feature a lot. Nostr was my choice because of its decentralized nature and self-hosting possibility.

@github-actions github-actions Bot added the size/giant PRs with more than 750 changed lines label Mar 4, 2026
@krzyczak krzyczak changed the base branch from dev to refactor March 4, 2026 21:22
krzyczak added 2 commits March 4, 2026 21:36
@krzyczak
feat: implement private sync over Nostr protocol
e461bc9
@krzyczak
fix: prevent fullscreen rotation crash from detached fragment prefere…
1fc68dd 
…nce callback

Problem
When entering fullscreen (rotation or fullscreen button), the app could crash with:
java.lang.IllegalStateException: Fragment CommentsFragment ... not attached to a context
at BaseListFragment.onSharedPreferenceChanged(BaseListFragment.java:477).

Root cause
BaseListFragment was calling getString(R.string.list_view_mode_key) inside onSharedPreferenceChanged(). During configuration change, SharedPreferences listeners can still be invoked while a fragment is already detached. At that moment, getString() requires an attached context and throws.

Why this surfaced with Nostr sync
Nostr sync updates SharedPreferences frequently (for relay status and sync bookkeeping), which increases preference change callbacks during lifecycle transitions and made this latent fragment bug reproducible while rotating or fullscreening.

Fix
- Added a cached field listViewModeKey in BaseListFragment.
- Initialized listViewModeKey once in onCreate() while the fragment is attached.
- Replaced getString(...) call in onSharedPreferenceChanged() with a null-safe comparison against the cached key.

Result
The callback no longer touches fragment context when detached, so fullscreen and rotation no longer crash through this code path. Functional behavior is unchanged: LIST_MODE_UPDATE_FLAG is still set only for list_view_mode_key changes.
@absurdlylongusername
Copy link
Copy Markdown
Member

absurdlylongusername commented Mar 5, 2026

Given your comment about AI usage, I presume you don't understand how this actually works.

If you do, please provide an in-depth explanation.

PR descriptions should be within the actual PR description, and not in a separate markdown file; that reduces cohesion and makes it inconvenient in having to go read a file somewhere else.

I don't know what Nostr is or how it works, and if you consider this a big feature it would be useful to have a proper explanation that explains architectural design decisions, implementation functionality and etc.

As it is, the NOSTR_SYNC is not understandable at all without extra research to make sense of it; that should not be the case.
As long as I know how NewPipe works, I should be able to understand what this PR is and how it works just by reading the PR description, and then I would clone the PR and map the description to the code 1 to 1 and then everything will become clear.

Right now, the code is the primary source of information for this PR, and that read me doesn't serve much use.

All that being said, you did not follow contribution guidelines anyway as you have left no indication anywhere of your intent to implement this feature, nor did you discuss this with the team.

Large features such as this require discussion so we can validate feasibility of solutions. Without this, such a feature will never be merged, and this step cannot be skipped because it will just end up being done at PR time (as it is now).

Therefore it would have been better to discssued with the team beforehand.

It's not worth our time to code review all of this out of the blue unprovoked.

So, for these reasons I'll be closing this PR. If you're able to fix the problems I mentioned then it can be reopened.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

size/giant PRs with more than 750 changed lines

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@krzyczak @absurdlylongusername