feat: Add ContactInfoContent signing + config toggle for strict validation + StrictNoSign #1545

CassOnMars · 2023-10-24T23:09:18Z

Motivation

StrictSign support on hubs is using javascript methods for signature creation and validation, which are both very slow compared to our rust implementation and make up a large portion of CPU time. To support use of StrictNoSign, we need signatures added to ContactInfoContent to verify provenance, and also need to allow network config based toggling of the features.

Change Summary

This PR updates ContactInfoContent messages to include signature payloads following the message signing convention used for other Farcaster messages, and utilizes native methods. It also adds configuration support both at the hub options and network configuration level for contactinfo strict validation and StrictNoSign.

Merge Checklist

Choose all relevant options below by adding an x now or at any time before submitting for review

PR title adheres to the conventional commits standard
PR has a changeset
PR has been tagged with a change label(s) (i.e. documentation, feature, bugfix, or chore)
PR includes documentation if necessary.
All commits have been signed

Additional Context

If this is a relatively large or complex change, provide more details here that will help reviewers

PR-Codex overview

This PR focuses on adding support for contact info content signing and introducing a new strictNoSign option.

Detailed summary

Adds support for contact info content signing and strictNoSign option.
Updates several files related to contact info content and signature handling.
Includes changes to network configuration and validation methods.
Adds a new native function for signing message hashes.
Updates tests for native signing and verification methods.
Makes changes to the libp2p gossipNode class for message handling.
Includes changes to the libp2p node worker and sync engine.

The following files were skipped due to too many changes: packages/core/src/validations.ts, apps/hubble/src/network/utils/networkConfig.test.ts, packages/core/src/protobufs/generated/gossip.ts, apps/hubble/src/hubble.ts, packages/hub-web/src/generated/rpc.ts

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

…igning

changeset-bot · 2023-10-24T23:09:23Z

🦋 Changeset detected

Latest commit: 8ea555c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
@farcaster/hub-web	Minor
@farcaster/core	Minor
@farcaster/hubble	Minor
@farcaster/hub-nodejs	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel · 2023-10-24T23:09:24Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
hub-monorepo	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 26, 2023 10:20pm

codecov · 2023-10-24T23:33:21Z

Codecov Report

Attention: 51 lines in your changes are missing coverage. Please review.

Files	Coverage Δ
apps/hubble/src/network/p2p/gossipNode.ts	`84.02% <100.00%> (+0.59%)`	⬆️
apps/hubble/src/network/p2p/gossipNodeWorker.ts	`23.73% <100.00%> (+0.24%)`	⬆️
apps/hubble/src/network/sync/syncEngine.ts	`76.19% <100.00%> (-0.15%)`	⬇️
apps/hubble/src/network/utils/networkConfig.ts	`43.33% <100.00%> (+1.95%)`	⬆️
apps/hubble/src/rustfunctions.ts	`100.00% <100.00%> (ø)`
.../hubble/src/storage/jobs/updateNetworkConfigJob.ts	`19.23% <0.00%> (-3.50%)`	⬇️
packages/core/src/validations.ts	`82.92% <12.50%> (-4.45%)`	⬇️
apps/hubble/src/hubble.ts	`53.36% <57.62%> (+0.40%)`	⬆️

... and 2 files with indirect coverage changes

📢 Thoughts on this report? Let us know!.

apps/hubble/src/hubble.ts

sanjayprabhu · 2023-10-24T23:27:18Z

apps/hubble/src/network/utils/networkConfig.test.ts

@@ -113,9 +123,11 @@ describe("networkConfig", () => {
      keyRegistryAddress: undefined,
      idRegistryAddress: undefined,
      allowlistedImmunePeers: undefined,
+      strictContactInfoValidation: undefined,
+      strictNoSign: undefined,


Maybe time to extract this to a common function. Always annoying to have to modify all these test just to add an extra param.

sanjayprabhu · 2023-10-24T23:28:46Z

apps/hubble/src/hubble.ts

@@ -710,6 +734,8 @@ export class Hub implements HubInterface {
      this.deniedPeerIds = deniedPeerIds;

      this.allowlistedImmunePeers = allowlistedImmunePeers;
+      this.strictContactInfoValidation = !!strictContactInfoValidation;
+      this.strictNoSign = !!strictNoSign;


We'd need to restart the GossipNode node if this changed right?

correct, will follow up

sanjayprabhu · 2023-10-24T23:30:01Z

apps/hubble/src/hubble.ts

+      appVersion: content.appVersion,
+      timestamp: content.timestamp,
+      dataBytes: content.dataBytes,
+    });


ContactInfoContent.decode(ContactInfoContent.encode(content).finish()) is more future proofs for when new fields get added. Maybe also worth pulling the dataBytes check out here for clarity.

Because we don't have a envelope to this, we have to explicitly extract the values that would exist pre-signed, but agreed we need a way to future proof this better.

We could introduce a body field just like Message and duplicate the fields on there since we're changing it anyway. We can deprecate the outer fields in 1.8

sounds good, happy to proceed with this

apps/hubble/src/addon/src/lib.rs

sanjayprabhu

Worth testing on testnet or locally with two hub instances.

sanjayprabhu · 2023-10-25T23:09:22Z

apps/hubble/src/storage/jobs/updateNetworkConfigJob.ts

+    if (shouldRestart) {
+      log.info({}, "Network config restart signal");
+      await this._hub.stop();
+      await this._hub.start();


Could we just restart the gossipNode instead of the whole hub?

There are calls being made to the worker which could behave unpredictably if we only restarted the gossipNode

sanjayprabhu · 2023-10-25T23:12:23Z

apps/hubble/src/storage/jobs/updateNetworkConfigJob.ts

    if (shouldExit) {
      log.error({}, "Network config exit signal");
      process.kill(process.pid, "SIGQUIT");
    }

+    if (shouldRestart) {
+      log.info({}, "Network config restart signal");
+      await this._hub.stop();


GossipNode.stop calls terminate on the worker.

hub-monorepo/apps/hubble/src/network/p2p/gossipNode.ts

Line 292 in b47c65b

await this._nodeWorker?.terminate();

I think we'll need to modify start to re-create the worker. Otherwise it won't start back up.

good call, lmk if this approach handles this better

CassOnMars added 3 commits October 23, 2023 23:47

first pass, working native signing + tests

ca291b3

WAR-1064 – add configuration support and validation of contact info s…

3a58d93

…igning

remove signing info from message signature validation

3f949a6

add changeset

9f673a1

CassOnMars added the t-feat Add a new feature or protocol improvement label Oct 24, 2023

vercel bot deployed to Preview October 24, 2023 23:10 View deployment

misref

10ca432

vercel bot deployed to Preview October 24, 2023 23:29 View deployment

sanjayprabhu reviewed Oct 24, 2023

View reviewed changes

pr feedback

9f522e9

vercel bot deployed to Preview October 25, 2023 01:53 View deployment

Support clean restarts on specific changes to network config

39a40cb

vercel bot deployed to Preview October 25, 2023 18:42 View deployment

Merge branch 'main' into cassie-heart/WAR-1064

24c169d

vercel bot deployed to Preview October 25, 2023 18:48 View deployment

sanjayprabhu reviewed Oct 25, 2023

View reviewed changes

don't terminate gossip node unless we are following default termination

23e315d

vercel bot deployed to Preview October 26, 2023 17:15 View deployment

delay invocation if gossip node worker isn't started

e1bf57c

vercel bot deployed to Preview October 26, 2023 22:13 View deployment

sanjayprabhu approved these changes Oct 26, 2023

View reviewed changes

reduce delay to 1s, shouldn't need to exceed

8ea555c

vercel bot deployed to Preview October 26, 2023 22:20 View deployment

CassOnMars merged commit 3313c23 into main Oct 26, 2023
7 of 8 checks passed

CassOnMars deleted the cassie-heart/WAR-1064 branch October 26, 2023 22:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add ContactInfoContent signing + config toggle for strict validation + StrictNoSign #1545

feat: Add ContactInfoContent signing + config toggle for strict validation + StrictNoSign #1545

CassOnMars commented Oct 24, 2023 •

edited by pr-codex bot

Loading

changeset-bot bot commented Oct 24, 2023 •

edited

Loading

vercel bot commented Oct 24, 2023 •

edited

Loading

codecov bot commented Oct 24, 2023 •

edited

Loading

sanjayprabhu Oct 24, 2023

sanjayprabhu Oct 24, 2023

CassOnMars Oct 25, 2023

sanjayprabhu Oct 24, 2023

CassOnMars Oct 25, 2023

sanjayprabhu Oct 25, 2023

CassOnMars Oct 25, 2023

sanjayprabhu left a comment

sanjayprabhu Oct 25, 2023

CassOnMars Oct 26, 2023

sanjayprabhu Oct 25, 2023

CassOnMars Oct 26, 2023

CassOnMars Oct 26, 2023

feat: Add ContactInfoContent signing + config toggle for strict validation + StrictNoSign #1545

feat: Add ContactInfoContent signing + config toggle for strict validation + StrictNoSign #1545

Conversation

CassOnMars commented Oct 24, 2023 • edited by pr-codex bot Loading

Motivation

Change Summary

Merge Checklist

Additional Context

PR-Codex overview

Detailed summary

changeset-bot bot commented Oct 24, 2023 • edited Loading

🦋 Changeset detected

vercel bot commented Oct 24, 2023 • edited Loading

codecov bot commented Oct 24, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanjayprabhu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CassOnMars commented Oct 24, 2023 •

edited by pr-codex bot

Loading

changeset-bot bot commented Oct 24, 2023 •

edited

Loading

vercel bot commented Oct 24, 2023 •

edited

Loading

codecov bot commented Oct 24, 2023 •

edited

Loading