Thanks for your interest in improving the Farcaster Hub!
No contribution is too small and we welcome to your help. There's always something to work on, no matter how experienced you are. If you're looking for ideas, start with the good first issue or help wanted sections in the issues. You can help make Farcaster better by:
- Opening issues or adding details to existing issues
- Fixing bugs in the code
- Making tests or the ci builds faster
- Improving the documentation
- Keeping packages up-to-date
- Proposing and implementing new features
Before you get down to coding, take a minute to consider this:
- If your proposal modifies the farcaster protocol, open an issue there first.
- If your proposal is a non-trivial change, consider opening an issue first to get buy-in.
- If your issue is a small bugfix or improvement, you can simply make the changes and open the PR.
First, ensure that the following are installed globally on your machine:
- Node.js 18+
- Yarn
- Flatbuffers (
brew install flatbuffers
on OSX)
Then, from the root folder run:
yarn install
to install dependenciesyarn build
to ensure that the test suite runs correctlyyarn test
to ensure that the test suite runs correctly
All commits need to be signed with a GPG key. This adds a second factor of authentication that proves that it came from you, and not someone who managed to compromise your GitHub account. You can enable signing by following these steps:
-
Generate GPG Keys and upload them to your Github account, GPG Suite is recommended for OSX
-
Use
gpg-agent
to remember your password locally
vi ~/.gnupg/gpg-agent.conf
default-cache-ttl 100000000
max-cache-ttl 100000000
-
Configure Git to use your keys when signing.
-
Configure Git to always sign commits by running
git config --global commit.gpgsign true
-
Commit all changes with your usual git commands and you should see a
Verified
badge near your commits
The repository is a monorepo with a primary application in the /app/
folder that imports several packages /packages/
. It is composed of yarn workspaces and uses TurboRepo as its build system.
You can run commands like yarn test
and yarn build
which TurboRepo will automatically parallelize and execute across all workspaces. To execute the application, you'll need to navigate into the app folder and follow the instructions there. The TurboRepo documentation covers other important topics like:
TurboRepo uses a local cache which can be disabled by adding the --force
option to yarn commmands. Remote caching is not enabled since the performance gains at our scale are not worth the cost of introducing subtle caching bugs.
When proposing a change, make sure that you've followed all of these steps before you ask for a review.
All changes that involve features or bugfixes should be accompanied by tests, and remember that:
- Unit tests should live side-by-side with code as
foo.test.ts
- Tests that span multiple files should live in
src/test/
under the appropriate subfolder - Tests should use factories instead of stubs wherever possible.
- Critical code paths should have 100% test coverage, which you can check in the Coveralls CI.
All changes should have supporting documentation that makes reviewing and understand the code easy. You should:
- Update high-level changes in the contract docs.
- Always use TSDoc style comments for functions, variables, constants, events and params.
- Prefer single-line comments
/** The comment */
when the TSDoc comment fits on a single line. - Always use regular comments
//
for inline commentary on code. - Comments explaining the 'why' when code is not obvious.
- Do not comment obvious changes (e.g.
starts the db
before the linedb.start()
) - Add a
Safety: ..
comment explaining every use ofas
. - Prefer active, present-tense doing form (
Gets the connection
) over other forms (Connection is obtained
,Get the connection
,We get the connection
,will get a connection
)
Errors are not handled using throw
and try / catch
as is common with Javascript programs. This pattern makes it hard for people to reason about whether methods are safe which leads to incomplete test coverage, unexpected errors and less safety. Instead we use a more functional approach to dealing with errors. See this issue for the rationale behind this approach.
All errors must be constructed using the HubError
class which extends extends Error. It is stricter than error and requires a Hub Error Code (e.g. unavailable.database_error
) and some additional context. Codes are used a replacement for error subclassing since they can be easily serialized over network calls. Codes also have multiple levels (e.g. database_error
is a type of unavailable
) which help with making decisions about error handling.
Functions that can fail should always return HubResult
which is a type that can either be the desired value or an error. Callers of the function should inspect the value and handle the success and failure case explicitly. The HubResult is an alias over neverthrow's Result. If you have never used a language where this is common (like Rust) you may want to start with the API docs. This pattern ensures that:
- Readers can immediately tell whether a function is safe or unsafe
- Readers know the type of error that may get thrown
- Authors can never accidentally ignore an error.
We also enforce the following rules during code reviews:
Always return HubResult<T>
instead of throwing if the function can fail
// incorrect usage
const parseMessage = (message: string): Uint8Array => {
if (message == '') throw new HubError(...);
return message;
};
// correct usage
const parseMessage = (message: string): HubResult<Uint8Array> => {
if (message == '') return new HubError(...)
return ok(message);
};
Always wrap external calls with Result.fromThrowable
or ResultAsync.fromPromise
and wrap external an Error
into a HubError
.
// incorrect usage
const parseMessage = (message: string): string => {
try {
return JSON.parse(message);
} catch (err) {
return err as Error;
}
};
// correct usage: wrap the external call for safety
const parseMessage = (message: string): HubResult<string> => {
return Result.fromThrowable(
() => JSON.parse(message),
(err) => new HubError("bad_request.parse_failure", err as Error)
)();
};
// correct usage: build a convenience method so you can call it easily
const safeJsonStringify = Result.fromThrowable(
JSON.stringify,
() => new HubError("bad_request", "json stringify failure")
);
const result = safeJsonStringify(json);
Prefer result.match
to handle HubResult since it is explicit about how all branches are handled
const result = computationThatMightFail().match(
(str) => str.toUpperCase(),
(error) => err(error)
);
Only use isErr()
in cases where you want to short-circuit early on failure and refactoring is unwieldy or not performant
public something(): HubResult<number> {
const result = computationThatMightFail();
if (result.isErr()) return err(new HubError('unavailable', 'down'));
// do a lot of things that would be unweidly to put in a match
// ...
// ...
return ok(200);
}
Use _unsafeUnwrap()
and _unsafeUnwrapErr()
in tests to assert results
// when expecting an error
const error = foo()._unsafeUnwrapErr();
expect(error.errCode).toEqual("bad_request");
expect(error.message).toMatch("invalid AddressInfo family");
Prefer combine
and combineWithAllErrors
when operating on multiple results
const results = await Promise.all(things.map((thing) => foo(thing)));
// 1. Only fail if all failed
const combinedResults = Result.combineWithAllErrors(results) as Result<
void[],
HubError[]
>;
if (combinedResults.isErr() && combinedResults.error.length == things.length) {
return err(
new HubError("unavailable", "could not connect to any bootstrap nodes")
);
}
// 2. Fail if at least one failed
const combinedResults = Result.combine(results);
if (combinedResults.isErr()) {
return err(
new HubError("unavailable", "could not connect to any bootstrap nodes")
);
}
All submissions must be opened as a Pull Request and reviewed and approved by a project member. The CI build process will ensure that all tests pass and that all linters have been run correctly. In addition, you should ensure that:
- The PR titles must follow the Conventional Commits specification
- Commit titles should follow the Conventional Commits specification
As an example, a good PR title would look like this:
fix(signers): validate signatures correctly
While a good commit message might look like this:
fix(signers): validate signatures correctly
Called Signer.verify with the correct parameter to ensure that older signature
types would not pass verification in our Signer Sets