farcasterxyz / hub-monorepo

Implementation of the Farcaster Hub specification and supporting libraries for building applications on Farcaster
https://www.thehubble.xyz
MIT License
708 stars 404 forks source link

bug: a fid cannot recast the same cast multiple times #144

Closed varunsrin closed 2 years ago

varunsrin commented 2 years ago

What is the bug? The Farcaster protocol specifies that a fid cannot have multiple recasts of the same cast. Recasts should be LWW by timestamp and a future recast should simple overwrite an older one.

How can it be reproduced? Per discussion in #143 it is currently implemented so that a user can recast the same cast infinite times.

Additional context

One approach is to preserve the RW-LWW semantics make the CastSet check if a previous CastRecast exists with the same targetCastUri. If it does, the older CastRecast is replaced by the newer one in the add-set. If the older CastRecast was removed with a CastRemove after which a newer CastRecast was observed, both the CastRemove and CastRecast are preserved. The benefits are that the privacy of recasts is preserved on delete (similar to casts) but the data size is not as optimal as it can be when a delete and recast are involved. The set is also more complicated to reason about since there are now multiple add messages that map to a single remove type, and it is not clear if this pattern will scale to all future cast types.

Another approach is to make Recasts behave like Actions (Follows, Likes) with LWW-RW semantics. We would introduce a new set (CastRecastSet) which has an add message (CastRecast) and a remove message (CastRecastRemove). Unlike CastRemove (CastShortRemove) the remove message has the full information of the recast and not just its hash. The benefits of this approach are that we would keep storage minimal since we can ensure that only one add or remove message per target URI is ever stored in this set, and it behaves very similarly to other sets. The downside is that it splits casts into multiple sets, and it is unclear if this is desirable or if future extensions to casts will each have their own sets.

Alternatively, the status quo would be to amend the protocol to allow multiple recasts and have them behave exactly like Casts.

varunsrin commented 2 years ago

After further discussion with @danromero offline, we agree that it's desirable to implement Recasts in a way that makes them behave similarly to Actions (the middle option above).

@pfletcherhill we should discuss the tradeoffs of implementing this, i think it's a medium size change but one we should get right before shipping v2.

pfletcherhill commented 2 years ago
  1. I'm still not entirely sure why enforcing one recast per cast is important at the protocol level. If the main reason is to limit spam, I think this is just one of many vectors for potential spam and we should address those more holistically (rate limits, temporary block lists, etc)
  2. How do we think about the equivalent of quote retweets? If we have separate sets for casts and recasts, which set would we extend to support that feature? (This question is relevant in our current state too with CastShort and CastRecast message types)
  3. Is there any world in which we would want to make "recast" just another reaction type? Right now the reaction set only allows one reaction (of any type) per cast for an fid, and we'll be updating that to be one reaction per cast per reaction type. I assume the idea was to separate them so it's easier for clients to make feeds of cats and recasts, but just want to raise that as another option. (Follow set is obviously different because the target is a user)
pfletcherhill commented 2 years ago

Re (1), I only just saw the comment here about recasts that answers my question

varunsrin commented 2 years ago

fixed by adding a reaction set where the recast is a type of reaction toggle