fix: Prevent issues with fnames being associated with multiple fids #442
Conversation
|
@FrederikBolding is attempting to deploy a commit to the farcaster Team on Vercel. A member of the Team first needs to authorize it. |
FrederikBolding
changed the title
There was a problem hiding this comment.
This is a tricky change, we should be careful about merging this. @aditiharini is working on some logic to do protocol version upgrades. We might want to wait until we have that in place first.
| // If a username is being transferred, remove the existing secondary key. | ||
| if let Some(existing_fid) = existing_fid { | ||
| let secondary_key = make_fname_username_proof_by_fid_key(existing_fid); | ||
| txn.delete(secondary_key); |
There was a problem hiding this comment.
Hmm, have to consider how to handle existing bad data. This doesn't break consensus since it's just an index change. We might need a migration to fix, but we don't have any migration harness code. Will ticket this, let me know if you're interested in taking that on.
There was a problem hiding this comment.
Feels like it's probably something to deal with outside of this PR
| .route_fid(fname_transfer.from_fid, self.read_node_mempool.num_shards); | ||
|
|
||
| if mirror_shard_id != shard_id { | ||
| self.insert_into_shard(mirror_shard_id, message, source) |
There was a problem hiding this comment.
hmm, this technically affects consensus. But we might be ok if nodes upgrade at different times since this logic is only hit in the proposer? Other nodes will behave the same regardless of version, as long as the transfer is in the transactions list.
There was a problem hiding this comment.
Not sure it will affect consensus in practice since it only changes where the message is stored tbh.
Fixes two problems, when an fname was transferred the secondary index was not correctly deleted causing outdated info to still be returned by the API. In addition, fname transfers are now mirrored to both the sender and received shard to prevent outdated info in one of the shards during a transfer.
Fixes #438