Skip to content
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

p2p: Validate incoming Farcaster Messages #19

Open
haardikk21 opened this issue Feb 21, 2024 · 2 comments
Open

p2p: Validate incoming Farcaster Messages #19

haardikk21 opened this issue Feb 21, 2024 · 2 comments
Assignees

Comments

@haardikk21
Copy link
Contributor

For the Farcaster Message type (protobuf) that we receive over the gossip network, we need to add validation. This issue is primarily concerned with verifying the body of the message - a separate issue will be created for verifying the signature on the message.

There are a few different key types of messages as in this struct:

pub enum Body {
        #[prost(message, tag = "5")]
        CastAddBody(super::CastAddBody),
        #[prost(message, tag = "6")]
        CastRemoveBody(super::CastRemoveBody),
        #[prost(message, tag = "7")]
        ReactionBody(super::ReactionBody),
        #[prost(message, tag = "9")]
        VerificationAddAddressBody(super::VerificationAddAddressBody),
        #[prost(message, tag = "10")]
        VerificationRemoveBody(super::VerificationRemoveBody),
        /// SignerAddBody signer_add_body = 11; // Deprecated
        #[prost(message, tag = "12")]
        UserDataBody(super::UserDataBody),
        /// SignerRemoveBody signer_remove_body = 13; // Deprecated
        #[prost(message, tag = "14")]
        LinkBody(super::LinkBody),
        #[prost(message, tag = "15")]
        UsernameProofBody(super::UserNameProof),
        #[prost(message, tag = "16")]
        FrameActionBody(super::FrameActionBody),
    }

The validation logic for all of them is the same, with a couple of exceptions:

  • For UserDataBody, there are multiple sub-messages that are possible. In the case of it being a UserDataType::Username type - where the FC username (fname) is being updated, we need some additional validation.
  • For UsernameProofBody, we have some extra validation
  • For VerificationAddAddress, we need to throw if a Solana address is being added - since that is not yet enabled.

Specifically, now:

Common Flow

  1. Ensure that message_data is present and not missing
  2. Ensure the message is from the correct network (mainnet v. testnet - FC network not OP)
  3. Ensure that the user sending the message has a custody address linked to their FC account - we should have this in our DB from syncing onchain events
  4. Ensure that the signer for this message is one associated with the above user - we should have associated signers for a user in our DB from syncing onchain events

for fname update messages

Check that the fname being set for this user is actually owned by this user.

This can be done either by looking at UsernameProof events from onchain events, or in the case an ENS name is being set, by doing a reverse resolution on L1 for the ENS name and it should resolve to the custody address or a linked verified address.

for usernameproof messages

If an ENS domain is being submitted for the username proof, ensure through reverse-resolution on L1 that is actually owned either by the custody address or a linked verified address.

for VerificationAddAddress messages

If protocol for this is SOLANA, throw an error. The protobuf schemas have this but is not yet supported.


Relevant hubble code: https://github.com/farcasterxyz/hub-monorepo/blob/1f97d0c249f79980f3580677d27adf194ab260fb/apps/hubble/src/storage/engine/index.ts#L919

In Teleport: lib/hub/src/validation.rs

@avichalp
Copy link
Collaborator

Thanks for articulating this issue.

Solana verification is now enabled on Hubble, so Teleport should support it now.

I noticed that Hubble's validation logic split into two modules. One part is in the Storage Engine, as the original issue mentions. The other part is in the validations package in the core. The first part is tightly coupled with the DB, as validations happen when storing or retrieving the data. Later part is essentially pure utility functions in a library. I think this kind of separation also makes sense in Teleport.

I can already start tackling this issue now. It seems like this problem is wide enough that we can create a few more sub-issues. I will scope those out as I work on this.

@haardikk21
Copy link
Contributor Author

Sounds good, assigned

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants