Add consensus level decentralized post pinning mechanism to core. #50
Conversation
Bring up to date with main.
|
Is this a global feature or will each node be able to have their own pinned posts? I see this is written to Core so I assume it's global. |
This feature allows users to pin posts to their own profile and storing the the fact that the post is pinned will live on the blockchain. So this is a "global" or what I generally think of as a "protocol-level" feature. This is distinct from posts that are pinned to the global feed. Those will only ever be at the node-level. Each node operator will have the ability to pin posts to their global feed. Hope that answers your question. |
| IsPinned bool | ||
|
|
||
| // Indicator if a post is pinned to the global feed. | ||
| IsGlobalPinned bool |
There was a problem hiding this comment.
Hmm.. I don't think we want anyone to have the ability to pin to the global feed do we?
There was a problem hiding this comment.
This is just the admin ability to pin to global feed that already exists. It was renamed in this PR.
| isPinned := false | ||
| if _, hasPinned := extraData[IsPinnedPostKey]; hasPinned && blockHeight > PinnedPostTrackingUpdateBlockHeight { | ||
| isPinned = true | ||
| delete(extraData, IsPinnedPostKey) |
There was a problem hiding this comment.
Why do we modify the extraData here?
There was a problem hiding this comment.
Similar to why we delete the reclout keys from the extra data map - it saves space in the DB for each PostEntry as we don't need the keys in the PostEntry's PostExtraData map since we explicitly extract it to the IsPinned attribute.
| @@ -4341,6 +4435,17 @@ func (bav *UtxoView) _connectSubmitPost( | |||
| } | |||
| } | |||
|
|
|||
| // We set/delete the pin entry mappings depending on the previous and current state of the post. | |||
| if hidingPostEntry { | |||
| @@ -276,6 +276,21 @@ type RecloutEntry struct { | |||
| isDeleted bool | |||
| } | |||
|
|
|||
| // PinEntry stores the timestamp and state of a PostEntry. | |||
| // It's used in the UtxoView to keep track of pinned posts not yet in the database. | |||
| type PinEntry struct { | |||
There was a problem hiding this comment.
High-level thoughts after reading through.
It would be good to understand what the value of a PinEntry is. My view on pinned posts is we'd implement them the exact same way as we implement IsHidden on posts, which is brain-dead simple and doesn't even require any change to the disconnect logic.
Then, to get an index of the pinned posts, we can modify PutPostEntryMappings and DeletePostEntryMappings in db_utils.go to simply update a separate little index of <PublicKey, Timestamp> for the pinned posts. We could even include another field in addition to IsPinned on the PostEntry, which is PinTimestamp, which can allow the user to order the pinned posts on their profile by pinning them in a different order. This would also require no changes to the disconnect logic I think.
I may be missing the reason why PinEntry is required, and if that's the case please let me know!
There was a problem hiding this comment.
I think the reason @bluepartyhat added the PinEntry due to the complications that would arise in DBPutPostEntryMappingsWithTxn. Unlike isHidden, isPinned results in different indexing. So when we flush to the DB, we need to know the previous pin status of the PostEntry (which is not retained on the PostEntry) so we can delete it in DBDeletePostEntryMappingsWithTxn and can then be correctly re-added in DBPutPostEntryMappingsWithTxn. I think adding the complication there would probably be preferable to adding the PinEntry, but let's chat about this since the indexing is odd.
No description provided.