-
Notifications
You must be signed in to change notification settings - Fork 0
Integrate pre activity metadata and receive tags #219
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR integrates pre-activity metadata from Bitkit Core to automatically assign tags and addresses to activities. The implementation replaces manual tag application after sends with a pre-activity metadata system that creates metadata before activities are generated, enabling automatic tag assignment by Bitkit Core. The PR also refactors tag-related UI components into reusable views and fixes VSS backup to correctly handle activity tags and pre-activity metadata.
Key changes:
- Refactored tag-related UI into reusable components (
TagsListView,TagSelectionView,TagInputForm,PreviouslyUsedTagsView) - Implemented pre-activity metadata creation for send and receive flows
- Added efficient address lookup for received transactions using pre-activity metadata
- Updated VSS backup to properly backup/restore both activity tags and pre-activity metadata
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| AddTagSheet.swift | Refactored to use new reusable tag components |
| SendTagScreen.swift | Simplified by using shared tag input components |
| SendConfirmationView.swift | Creates pre-activity metadata instead of applying tags post-send |
| ReceiveTag.swift | Stores tags in pre-activity metadata for future activity creation |
| ReceiveSheet.swift | Initializes tag manager and creates pre-activity metadata on appear |
| ReceiveEdit.swift | Uses TagSelectionView component and manages pre-activity metadata tags |
| ActivityItemView.swift | Refactored to use TagsListView component |
| TagSettingsView.swift | Simplified using TagsListView component |
| TagsListView.swift | New reusable component for displaying tag lists |
| TagSelectionView.swift | Refactored to accept callbacks instead of navigation binding |
| TagInputForm.swift | New reusable component for tag text input |
| PreviouslyUsedTagsView.swift | New component for displaying previously used tags |
| WalletViewModel.swift | Added methods to retrieve payment IDs and persist pre-activity metadata |
| ActivityListViewModel.swift | Removed obsolete findActivityAndAddTags method |
| CoreService.swift | Added pre-activity metadata methods and improved address finding for received transactions |
| BackupService.swift | Fixed to backup/restore activity tags and pre-activity metadata correctly |
| BackupPayloads.swift | Updated backup structures to use PreActivityMetadata and ActivityTags |
| Package.resolved | Updated bitkit-core dependency revision |
|
@ben-kaufman Is this ready for review? I noticed review was requested early but then there was more commits, so now it's not certain 🤷🏻 :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
|
Next time it would be good to add suggestions of test flows |
|
Tests: Lightning
lightning_receive_tags.mp4
On-chain
|
jvsena42
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found just an issue in the tests
Yes please 🙏🏻 Also for docs' purposes and tests blueprints. |
|
Issue should now be fixed @jvsena42 Testing instructions are as you did + click in savings balance -> transfer to spending -> make sure the resulting activity is shown as transfer. |
|
Waiting E2E tests finish to check again |
|
@ben-kaufman could you confirm if this solves #197 ? If yes, please, add it to the description to the issue get closed automatically |
|
@jvsena42 yes, add to the description. |
For How To, see: Using keywords in issues and pull requests |
|
@ben-kaufman I guess the test tag |
|
Readded the test identifier |
3584d6d to
48ec99a
Compare
48ec99a to
8d4e284
Compare
You mean on the previous PR that was closed in favour of this?! |
Referring to the steps here #219 (comment) |
|
Testing... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lightning
- Create invoice -> add tags and description -> receive -> check if tags and description were added ✅
- scan invoice -> add tags -> send -> check tags ✅
On-chain
- Create BIP 21 invoice -> add tags -> receive -> activity details -> check tags ✅
- Scan BIP 21 invoice -> add tags -> send -> activity details -> check address, fee, federate and tags ✅
- Transfer to spending ✅
- Reset and restore ✅
OBS: On transfer to spending, the pending transfer activities are not being displayed. They are only displayed after confirmed. (can be fixed in #226 )
transfer.mp4
|
@ovitrif Merged because this branch was getting too complicated to maintain. If you find some issue, we can fix in other PR |
No worries, I tested myself and was about to approve, I was just writing my PR review comment 🙃 Had no requests for changes in THIS PR, but I randomly found another issue related to receive that needs fixing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests
- Receive Tags: all tests ok 🟢
- Transfer: test ok 🟢
- Send Tags: all tests ok 🟢
While testing receive tags, In found an issue not added in these changes, but related to the feature.
Receive > Edit > Tap Note textfield > Can't add tags anymore
Really need to show QR code and then edit again.
| let version: Int | ||
| let createdAt: UInt64 | ||
| let tagMetadata: [ActivityTagsMetadata] | ||
| let tagMetadata: [PreActivityMetadata] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we name it preActivityMetadata? I can do the same in Android ;)
Or even activityMetadata, since we won't likely have a different activity metadata soon 🤷🏻
Added issue for it: |
This PR integrates pre-activity metadata from Bitkit Core.
Fixes #197
The pre-activity metadata is created before the send/ receive activities are created, and allow Bitkit Core to automatically add tags to an activity once created (for sent txs, the previous logic to apply tags was removed). Also for sent txs, it let's Bitkit Core auto fill the address for the activity, and for receive txs it helps in a more efficient lookup for the address the activity belongs to.
This PR also extracts certain UI components related to tags and reuses them across the app.
It also fixes the VSS backup to correctly backup and restore activities tags and pre-activity metadata.
Screen.Recording.2025-11-13.at.7.31.57.AM.mov